dearimgui / dear_bindings

C header (and language binding metadata) generator for Dear ImGui
MIT License
221 stars 12 forks source link

Suggestions #1

Closed ocornut closed 2 years ago

ocornut commented 3 years ago

Following on https://github.com/ocornut/imgui_private/issues/11 (unfortunately github has "transfer issue" but doesn't work yet toward the org "dearimgui")

Thank you Ben for the great work! I can confirm that with small edit of the paths in main.py it works:

e.g.

    convert_header(r"..\imgui\imgui.h",
                   r"output\cimgui",
                   r"templates/cimgui-header.cpp")
## python main.py
Parsing ..\imgui\imgui.h
Storing unmodified DOM
Applying modifiers
Writing output
Done

Output is already REALLY good and much better than old cimgui.

I've also added a .json metadata generator that emits information about all the generated code for the benefit of binding to other languages... I've taken some guesses about what counts as "useful information" and formatting but haven't actually tried writing anything to consume this data yet, so please let me know if you think I've missed something/got it all horribly wrong/etc.

Possibly cimgui.h/.cpp should be generated entirely from cimgui.json ? That would constitute a proof that all the information are there, as well as constitute a "project template" for using that json for custom needs.

Assuming it actually works in real use case (haven't tried yet!) I think this may be close to be usable.

I had a go at improving the name disambiguation code and have something that I'm moderately happy with now - it tries to find the smallest disambiguation possible now so we get things like:

CIMGUI_API bool ImGui_Combo(const char* label, int* current_item, const char* items_separated_by_zeros);
CIMGUI_API bool ImGui_ComboCallback(const char* label, int* current_item, bool (*items_getter)(void* data, int idx, const char** out_text), void* data, int items_count);

...instead of the hilarious 64-character string of nonsense it tended to emit before. Possibly more contentiously, I had a go at adding default argument "helpers" - i.e. version of functions that elide defaulted arguments. After playing with this a bit I actually felt like it worked best when the "helpers" are the default, so you get functions like this:

CIMGUI_API bool ImGui_BeginCombo(const char* label, const char* preview_value); // Implied flags = 0
CIMGUI_API bool ImGui_BeginComboEx(const char* label, const char* preview_value, ImGuiComboFlags flags /* = 0 */);

...the logic being that this gives an experience close to C++, where if you type "ImGui_BeginCombo(" you get the version with only the required arguments, and if you want to specify everything you need ImGui_BeginComboEx() instead.

I picked "Ex" as the suffix for that after considering a bunch of alternatives ("WithArgs", and even at one point just "_" as a kind of minimilist "doesn't get in the way when reading code" option), mainly because it's short and also there's a bit of prior history in that the Win32 API has a reasonable number of places where there are "basic" versions of function calls and then "Ex" versions with more arguments (for slightly different reasons, admittedly, but still).

The downside of this is that there are already a couple of places that use "Ex" in the code, though, so I'm not 100% sure this is the right avenue to go down. Other suggestions gratefully accepted!

I may need to get myself to use it to give better feedback. Maybe @thomcc or @mellinoe would have feedback on that? I think I've noticed the Rust version providing alternative buttons of that kind, too.

Some suggestions. They are mostly polish aimed at getting a version out.

Features

(EDIT extracted part of the message into separate issues)

ShironekoBen commented 2 years ago

Sorry for the massive delay in getting back onto this - various things conspired to get in the way, but I've finally found time to take a look at some of the issues.

I've pushed a bunch of changes that hopefully add some of the requested features/fixes. imgui_internal.h isn't supported yet both most of the other stuff is.

ocornut commented 2 years ago

Thanks, latest version is becoming really great! Closing this "general" issue now, will open one to mark "support for imgui_internal.h" specifically.