godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.69k stars 528 forks source link

Adding Simple Macros To Expose Attributes to Editor More Easily #1495

Open Anthony-J-G opened 3 months ago

Anthony-J-G commented 3 months ago

In my short experience of using godot-cpp and GDExtension, the pipeline has been quite smooth and easy to pick up from someone with a decent amount of C++ experience. However, there was one glaring piece that made small changes very painful and that was exposing class attributes from custom nodes to the editor.

Shown below is an example before and after of what the _bind_methods function looks like after the macros are applied to illustrate the difference they can make.

Before. image

After. image

With the addition of getting rid of a massive amount of getters and setters from both the source file and header file and putting them inside of macros, it creates a much simpler way to expose attributes.

Example Header File. image

Additionally, the header file added is very non-invasive and optional so those who wish to continue using the current way of exposing attributes can do so. Note: These changes could've probably been added into include/classes/wrapped.hpp along with the GDCLASS macro definition but with the goal of being as non-invasive as possible they were added to their own header inside of include/classes/bindings.hpp.

Love GDExtension and the work you guys have done so far and look forward to more!

AThousandShips commented 3 months ago

Please add some tests of this to make sure it works and maintain the working status, under the test directory

I'm not personally sure this kind of syntactic sugar is very useful, it only works with simple cases where the set/get is specific and named a particular way, and won't allow other patterns, which breaks the standard pattern for Boolean values in the code style for the engine (where is_ or has_ etc. is usually used)

I'd say the first part is far more useful but I'd say it should be a generic one allowing you to provide the get/set methods manually, with different versions with different details, so you can use default names if you like, and the c++ methods should be allowed to be different as well since you might want to make them private, and prefix with for example _

I think these should be highly configurable if they should be added at all, I don't think we should add macros just for very simple standard cases only

dsnopek commented 3 months ago

Thanks!

While this does seem like a DX improvement, I'm not sure about adding it to godot-cpp, when Godot modules won't have access to this. One of godot-cpp's design goals is trying to make its API as close as possible to Godot's internal APIs. We're not doing a perfect job of it, but we are progressively fixing the differences, and I'm not sure we should add a whole bunch of new differences.

Perhaps this can be proposed for inclusion in Godot itself as well?

Anthony-J-G commented 3 months ago

It could probably do well to exist inside of the engine once it is succinct and extendable enough, and allows for sufficient configuration as @AThousandShips was implying. The issue I'm seeing with that however is that it would probably end up breaking engine convention and result in a more drastic amount of changes there.

I don't know if anyone else has had the same problems that these macros try to address so perhaps it's not as applicable as I thought at first? If there are any other suggestions or ideas about how to decrease the verbosity of exposing attributes while keeping the configurability for more than just the standard case I'm totally down to make changes to comply with the design standards of godot-cpp. My original intent was based off of a mix of how the conventions worked and other codebases I've worked in where there were simple ways of exposing/binding to an external client.

Otherwise, I'm totally ok to close this and just stick to using it internally within my own modules. Would love to hear your thoughts either way!

dsnopek commented 3 months ago

Otherwise, I'm totally ok to close this and just stick to using it internally within my own modules

You could also make it as a separate header-only library in its own repo that could be used with godot-cpp? Then you're able to share it, but don't have to worry about conforming to the design goals or standards of godot-cpp or Godot