Disambiguate functions issues: Ex + varying types

[ocornut]

I've initially seen this pattern in the ImGui_ValueXXX functions but thought it wouldn't be bad to remove those either way so brushed it aside. Then I noticed the same issue in other place:

CIMGUI_API ImU32 ImGui_GetColorU32Ex(ImGuiCol idx, float alpha_mul /* = 1.0f */);  // retrieve given style color with style alpha applied and optional extra alpha multiplier, packed as a 32-bit value suitable for ImDrawList
CIMGUI_API ImU32 ImGui_GetColorU32ImGuiCol(ImGuiCol idx);                          // Implied alpha_mul = 1.0f
CIMGUI_API ImU32 ImGui_GetColorU32(ImVec4 col);
CIMGUI_API ImU32 ImGui_GetColorU32ImU32(ImU32 col);

The bug is: ImGui_GetColorU32Ex() is the Ex of ImGui_GetColorU32ImGuiCol(), not the Ex of ImGui_GetColorU32.

Same here:

CIMGUI_API void ImGui_TreePushEx(const void* ptr_id /* = NULL */);                                                          // "
CIMGUI_API void ImGui_TreePushVoidPtr();                                                                                    // Implied ptr_id = NULL

• It did happen with ImGui_Value (currently removed, could bring back if this is fixed but that's not the priority).
• It doesn't happen with ImGui_BeginChildXXX functions.

Would running one modifier before the other fix it?

[ocornut]

The bug initially reported is indeed fixed.
Adding a few things which are technically different issues but on the same theme.

Looking at the current output, I found what I believe are three issues. Coincidentally the first one is on the same spot as above but it is a different issue.

(1)

• Here, we should prioritize the ImGuiCol overload as being the primary function

CIMGUI_API ImU32 ImGui_GetColorU32ImGuiColEx(ImGuiCol idx, float alpha_mul /* = 1.0f */);  // retrieve given style color with style alpha applied and optional extra alpha multiplier, packed as a 32-bit value suitable for ImDrawList
CIMGUI_API ImU32 ImGui_GetColorU32ImGuiCol(ImGuiCol idx);                                  // Implied alpha_mul = 1.0f
CIMGUI_API ImU32 ImGui_GetColorU32(ImVec4 col);                                            // retrieve given color with style alpha applied, packed as a 32-bit value suitable for ImDrawList
CIMGUI_API ImU32 ImGui_GetColorU32ImU32(ImU32 col);  

I believe the desired output would be:

CIMGUI_API ImU32 ImGui_GetColorU32Ex(ImGuiCol idx, float alpha_mul /* = 1.0f */);  // retrieve given style color with style alpha applied and optional extra alpha multiplier, packed as a 32-bit value suitable for ImDrawList
CIMGUI_API ImU32 ImGui_GetColorU32(ImGuiCol idx);                                  // Implied alpha_mul = 1.0f
CIMGUI_API ImU32 ImGui_GetColorU32ImVec4(ImVec4 col);                              // retrieve given color with style alpha applied, packed as a 32-bit value suitable for ImDrawList
CIMGUI_API ImU32 ImGui_GetColorU32ImU32(ImU32 col);  

(2)

• There are indeed two entry points in the C++ library, they are a little frivolous IHMO and here would make sense to collapse them.

Would turn:

CIMGUI_API void    ImGui_PushID(const char* str_id);                                   // push string into the ID stack (will hash string).
CIMGUI_API void    ImGui_PushIDStr(const char* str_id_begin, const char* str_id_end);  // push string into the ID stack (will hash string).
CIMGUI_API ImGuiID ImGui_GetID(const char* str_id);                                    // calculate unique ID (hash of whole ID stack + given parameter). e.g. if you want to query into ImGuiStorage yourself
CIMGUI_API ImGuiID ImGui_GetIDStr(const char* str_id_begin, const char* str_id_end);

into

CIMGUI_API void    ImGui_PushID(const char* str_id, const char* str_id_end /* = NULL */); // push string into the ID stack (will hash string).
CIMGUI_API ImGuiID ImGui_GetID(const char* str_id, const char* str_id_end /* = NULL */); // calculate unique ID (hash of whole ID stack + given parameter). e.g. if you want to query into ImGuiStorage yourself

(3)

• The Checkbox blocks uses IntPtr instead of UIntPtr. Given the nature of those functions I believe both version could be typed.

Would turn:

CIMGUI_API bool ImGui_CheckboxFlags(const char* label, int* flags, int flags_value);
CIMGUI_API bool ImGui_CheckboxFlagsIntPtr(const char* label, unsigned int* flags, unsigned int flags_value);

into:

CIMGUI_API bool ImGui_CheckboxFlagsIntPtr(const char* label, int* flags, int flags_value);
CIMGUI_API bool ImGui_CheckboxFlagsUIntPtr(const char* label, unsigned int* flags, unsigned int flags_value);

Note the function just below, rightfully using IntPtr:

CIMGUI_API bool ImGui_RadioButton(const char* label, bool active);                                            // use with e.g. if (RadioButton("one", my_value==1)) { my_value = 1; }
CIMGUI_API bool ImGui_RadioButtonIntPtr(const char* label, int* v, int v_button);                             // shortcut to handle the above pattern when value is an integer

[ShironekoBen]

So with regards to the above:

1. I've implemented that... I actually implemented a mechanism to allow prioritising certain types when disambiguating, but then realised it doesn't help for this case because the argument count is higher on the function we actually want to prioritise. I didn't feel it was worth adding even more complexity to allow overriding that as well, so instead I just went with some strategic renaming.

2. That's trivially easy to do, but I do have a question mark in my head about it... how often do people working in C use string_view style start/end notation?

My gut feel (which may be very wrong) is that the vast majority of PushID() calls are something like PushID("Blah"), and the annoyance factor of having to type PushID("Blah", nullptr) every time may well outweigh the benefit of not having to use PushIDStr() in the few cases where you're actually working with a bounded string. Any input from people who regularly use C would be very useful here!

3. Yeah, that feels like a good change to me - I've added a feature where the disambiguator can be told to "disambiguate everything" for certain functions, which neatly covers this use-case.

[ocornut]

Thanks for the update!

2. That's trivially easy to do, but I do have a question mark in my head about it... how often do people working in C use string_view style start/end notation?

You are completely right. Disregard for now. It should be separate function (the name is a bit odd but not too bad).

It however reminds me of some the discussions in #19, I'll open an issue about that (not urgent but worth remembering).