Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do childs have _<id hash> appended in their name? #1698

Closed
franciscod opened this issue Mar 26, 2018 · 7 comments
Closed

Why do childs have _<id hash> appended in their name? #1698

franciscod opened this issue Mar 26, 2018 · 7 comments
Labels
bug label/id and id stack implicit identifiers, pushid(), id stack

Comments

@franciscod
Copy link
Contributor

(not an issue, more of a question)

Just noticed named child windows are windows called "<parent_name>/<child_name>_<child_id>".

Why is the hexa representation of the id (window->GetID(str_id)) required at the child window name? Isn't parent_name/child_name enough? (Why?)

@franciscod
Copy link
Contributor Author

(I get it on the anonymous child case)

@franciscod franciscod changed the title [Question] Why childs have _DEADBEEF appended in the name? [Question] Why childs have _<id hash> appended in the name? Mar 26, 2018
@franciscod franciscod changed the title [Question] Why childs have _<id hash> appended in the name? [Question] Why do childs have _<id hash> appended in their name? Mar 26, 2018
@ocornut ocornut changed the title [Question] Why do childs have _<id hash> appended in their name? Why do childs have _<id hash> appended in their name? Apr 3, 2018
@ocornut
Copy link
Owner

ocornut commented Apr 3, 2018

Good question @franciscod, I looked into it and the commit history and couldn't find any valid reason for doing that, so I changed it. Thanks!

@ocornut ocornut closed this as completed Apr 3, 2018
@franciscod
Copy link
Contributor Author

Wheeeee, glad I asked it :D You're welcome!

@jadwallis
Copy link
Contributor

I think the ID hash might be necessary for child windows to respect the ID stack. Consider the following example:

for (int i = 0; i < 4; ++i)
{
	ImGui::PushID(i);
	ImGui::BeginChild("foo", ImVec2(100, 100), true);
	ImGui::Text("bar %d", i);
	ImGui::EndChild();
	ImGui::PopID();
}

I'd expect that to create four separate child windows, since although the child windows share a name they are each in their own ID scope. Instead, currently* the above code comes out like this:
image

If I revert 84fbc49 then it looks like I'd expect:
image

Is my expectation of how this code should behave wrong, or is the original code actually useful after all?

*I'm currently on revision d69b2a1 (v1.63 WIP from 1st August 2018), but I don't think this particular code has changed since then from a quick look.

@ocornut
Copy link
Owner

ocornut commented Sep 26, 2018

Thanks @jadwallis.
You are right and this is indeed a breaking change.
I think the change might be acceptable as is however, because you can always use BeginChild(GetID("blah")...); if you need the unique ID scope.

Additionally, there are two other documentation issues:

  • The change was documented as part of 1.53 where in reality it was part of 1.60 (it was a mistake when I added the line in the Changelog).
  • If it stays as such (give me some time to think about it and decide) it should be documented as a breaking change in 1.60.

@ocornut ocornut reopened this Sep 26, 2018
@ocornut ocornut added the bug label Sep 26, 2018
ocornut added a commit that referenced this issue Sep 26, 2018
…y not applying the ID stack to the provided string to uniquely identify the child window. This was undoing an intentional change introduced in 1.50 and broken in 1.60. (#1698, #894, #713) + reworked the Begin/BeginChild comments in imgui.h.
@ocornut
Copy link
Owner

ocornut commented Sep 26, 2018

@jadwallis I have now undoed the change done in 84fbc49, it was an error on my part. Thanks for pointing it out. The change was explicitly introduced in 1.50 (see #894, #713) and accidentally reverted following the discussion on this thread. It should now be fixed again. My bad!

@ocornut ocornut closed this as completed Sep 26, 2018
@jadwallis
Copy link
Contributor

Thanks @ocornut! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug label/id and id stack implicit identifiers, pushid(), id stack
Projects
None yet
Development

No branches or pull requests

3 participants