Confusing child window with ### in name with top level window

[nem0]

I have a top level window named DockPanel###DockPanel, its child Shader Editor abc###Shader editor and other top level window Shader Editor abc###Shader editor. The way ImHash works causes imgui to confuse the child ShaderEditor with the top level Shader Editor

[ocornut]

I am not sure what solution you would suggest here?

Problem stem from the fact that BeginChild construct a string to pass to Begin which will hash it. If BeginChild applied both hashes in succession and provided an explicit ID to say, an hypothetical BeginEx() which would name both name and id, we'd have a fix. However I am not sure it is worth bothering so much and it would introduce some confusion and complication.

Since your ### string is an explicit string because you could including your own ID within it? It would be a trivial workaround.

[nem0]

There is no easy fix on imgui's side I can think of. I agree that this is probably not worth fixing. I am going to fix it on my side by using different string ids. It just cost me some time to find what's wrong and I wanted to save time for other people who encounter this bug. Maybe this problem can be mentioned in the documentation. Feel free to close this.

[nem0]

Just an idea, maybe we can put assert in ImHash when there is ## twice in the ID

[ocornut]

Note that you can also use BeginChild(ImU32 id) with our own id which would also ease fixing the problem on your side.

Just an idea, maybe we can put assert in ImHash when there is ## twice in the ID

We could add a strcmp() assert in the success path of FindWindowByName().

[nem0]

We could add a strcmp() assert in the success path of FindWindowByName().

That would be even better.

[ocornut]

We could add a strcmp() assert in the success path of FindWindowByName().

Ah no, that's actually stupid :( it would defeat the whole purpose of ### when changing the title of a window.

Just an idea, maybe we can put assert in ImHash when there is ## twice in the ID

That's also incorrect unfortunately. Here the problem isn't that you have two `###' but merely that the ID after the final ### of both window names are matching.

An unrelated assert that may have helped but completely by coincidence and not covering all cases, would be to check that e.g. the ImGuiWindowFlags_Child doesn't change.

Note that you can also call BeginChild with an ID in your use case.

Any other suggestion?

[nem0]

Note that you can also call BeginChild with an ID in your use case.

I am simply generating different string for the child https://github.com/nem0/LumixEngine/commit/3b9c08fa25712a8be5c0b92477a54d9bff7d413b

Is it useful in any way to have ### in children? Maybe we can assert when there is ### in the child.

[nem0]

Or ImFormatString(title, IM_ARRAYSIZE(title), "%s.%s", window->Name, str_id); in ImGui::BeginChild can take only part of str_id which is after ###

[ocornut]

Is it useful in any way to have ### in children? Maybe we can assert when there is ### in the child.

Never required, since child name are never printed out to the user you can always get a unique id with the base name without ###, but I don't see why it should be disallowed.

What sort of end-result did you get with the bug? Perhaps we could catch the error in a different spot. For example, for multiple begins in a same frame we could check that the parent is the same?

ImGui::BeginChild can take only part of str_id which is after ###

I don't understand, that would be no-op and equivalent to leaving things to way they are. Basically anything before the last ### will be ignored for the ID computation.

[nem0]

but I don't see why it should be disallowed

This bug would be avoided, since something like parent.xyz###child could not happen

What sort of end-result did you get with the bug?

It triggers IM_ASSERT(g.MovedWindow->RootWindow->MoveID == g.MovedWindowMoveId); in NewFrame()

ImGui::BeginChild can take only part of str_id which is after ###

Example: I have parent ImGui::Begin("DockPanel"), it has a child ImGui::BeginChild("def###ShaderEditor"), the full str_id of the child is "DockPanel.def###ShaderEditor" but hash is computed only from "ShaderEditor" part. If the full str_id is instead "DockPanel.ShaderEditor", the hash is computed from "DockPanel.ShaderEditor", which does not collide with the top level "ShaderEditor"

[ocornut]

(I tagged this issue with commit only because the topics are similar but the issue discussed here is still valid.)

[ocornut]

I've been looking at this problem today as part of another set of of issues.

Recap: ### is not meaningful in BeginChild() context. but ideally should not result in that issue.

Or ImFormatString(title, IM_ARRAYSIZE(title), "%s.%s", window->Name, str_id); in ImGui::BeginChild can take only part of str_id which is after ###

I'm tending toward this, aka more generally changing current law from:

GetID("Hello###World)" == ImHashData("###World") to GetID("Hello###World") == ImHashData("World")

Aka not including the ### in the hash. This has the benefit that it gives more flexibility in reconstructing strings.

Begin("Window1")
BeginChild("Child1###Hello")
--> child name "Window1/Child1###Hello" (omitting the fact we also include a 0x08X hash, but doesn't matter here)
Problem: We can't really transform it to "Window1/###Hello", still get same problem.
If we manually parse, we lose property that ImHash(window->Name) == window->ID, seems like a recipe for trouble.
If instead we remove the ### from the hash data, we can simply transform "Window1/Child1###Hello" to "Window1/Hello".

Nothing new here, those ideas transpired in your post above, I'm mostly recapping for myself :)

This seem decent but it also means we lose that "annotation" (the "Child1" part) from Debug Tools which may be undesirable, as I've seen people use "LocalizedName###SomeGUID".

[KristofMorva]

I'm tending toward this, aka more generally changing current law from:

GetID("Hello###World)" == ImHashData("###World") to GetID("Hello###World") == ImHashData("World")

Aka not including the ### in the hash. This has the benefit that it gives more flexibility in reconstructing strings.

Hey @ocornut, I also just encountered this issue - most of my windows use the "Hello###World" format (but not all), and in order to be able to call ImGui::ClearWindowSettings and similar methods that take a window name, it's not enough for me to have the ID of the window, because for labelled windows I have to pass "###World", while for windows where label == id I have to pass "World", which is pretty counter-intuitive and dirty. So +1 from my side for this proposed change :)

[ocornut]

Because for labelled windows I have to pass "###World", while for windows where label == id I have to pass "World", which is pretty counter-intuitive and dirty. So +1 from my side for this proposed change :)

I'm not sure I precisely understand your statement. You should be able to use "Hello###World" in the clear call, in both cases the same value passed to Begin().

[KristofMorva]

I'm not sure I precisely understand your statement. You should be able to use "Hello###World" in the clear call, in both cases the same value passed to Begin().

Sorry for the confusion. I mean that I do not have the whole "Hello###World" label stored (it's only constructed and passed for ImGui::Begin), I only know the window ID throughout the application, which is "World". So when I'm trying to call such methods, I have to construct the string "###World" for windows which do have custom label (e.g. "Hello"), but "World" for the ones that do not have a custom label, which is pretty unpleasant.

[ocornut]

I am also expecting that some usage pattern for renaming table columns while preserving columns data (#9108) may benefit from this change which I am currently testing.

[ocornut]

Sorry for the confusion. I mean that I do not have the whole "Hello###World" label stored (it's only constructed and passed for ImGui::Begin), I only know the window ID throughout the application, which is "World". So when I'm trying to call such methods, I have to construct the string "###World" for windows which do have custom label (e.g. "Hello"), but "World" for the ones that do not have a custom label, which is pretty unpleasant.

I guess you could construct it to be Begin("World###World") in the meanwhile.

The WIP change generally works but I'm worried about the accidental collision between ###Names and Names. Ironically it is exactly the purpose of this change.

In theory our own "###Menu_00" used by BeginMenu() in order to have e.g. "File###Menu_00" could use "#####Menu_00" to reduce conflicts with a possible user name of "Menu_00" but it would be unwieldy.

[ocornut]

(also note that an attempt to simplify child mangling #1698, change 8ee85137d which was quickly reverted afadf74a53 could probably be made to work if we made this change)

[ocornut]

I've changed the hashing to handle ### differently fc89c61 (+ missing comment dc48a7c), which would allow implementing a solution for this and generally simplify child mangling.