GDExtension: Add method to set the class icon

[raulsntos]

Allows GDExtensions to set the icon of a class programmatically, which is useful for extensions that don't use a .gdextension file.

• Matching PR in godot-cpp repo: Add a method to set the class icon godot-cpp#1666
• Example project for testing: https://github.com/raulsntos/threen/tree/set_class_icon

[dsnopek]

Thanks!

Having a way to set the icon via code is a much requested feature from GDExtension bindings authors, so I'm in support of the feature in general. However, I'm not sure this is the right API.

On the godot-cpp side, we'd want to emulate the workflow that Godot module developers use, so pointing the build-system at a directory and having godot-cpp automatically register the icons it finds there. (Or, if we wanted to add a new workflow, we'd want for it to be available to both GDExtensions and modules, but I think the existing workflow of the icons getting picked up automatically is pretty good.)

And then for other language bindings, we'd want them to be able to either (1) implement a similar automatic workflow, or (2) allow them to include setting the icon as part of their usual registration process (for example, if a Python binding used class decorators to register classes, the icon path could be an argument to the decorator)

I think the function added to the GDExtension interface here could be used. For godot-cpp, we could have the build-system generate a HashMap with the paths to the class icons, and then automatically call the new function right after each class is registered. However, having a new function would seem to imply that the icon can be changed at any time - do we want to support that? If not, maybe this would be better as part of class registration, ie add a path to the GDExtensionClassCreationInfo4 struct?

[raulsntos]

Thanks for the review. My intention is to use this on the C# bindings which, similar to how it works today, would allow you to specify the icon path with an attribute:

[Icon("res://MyNode.svg")]
public class MyNode : Node { }

The necessary calls to the GDExtension API to register the icon are generated by the C# source generators at build time. I have no intention of changing the icon at runtime. This likely works similar to the Python example you mentioned.

[...] maybe this would be better as part of class registration, ie add a path to the GDExtensionClassCreationInfo4 struct?

That works for me too. The current implementation is just the first thing that came to mind, but I'm totally fine with changing the implementation to whatever the team prefers.

It may be a bit more complicated though. In C# we have a configuration method where we bind all the methods/properties/signals (similar to bind_methods in godot-cpp) and I intend to use this method to set the icon too. This method needs to be called after classdb_register_extension-class4 though, otherwise I think binding the methods/properties/signals will fail.

[dsnopek]

It may be a bit more complicated though. In C# we have a configuration method where we bind all the methods/properties/signals (similar to bind_methods in godot-cpp) and I intend to use this method to set the icon too. This method needs to be called after classdb_register_extension-class4 though, otherwise I think binding the methods/properties/signals will fail.

Hm. There is no way to stash the path to the icon somewhere and use it as part of the class registration?

For better or worse, binding methods/properties/signals does work at any time. You really shouldn't do it - it's intended that those are bound shortly after registering the class - but it works. :-)

So, I think if we do implement setting the icon as a function to be called like binding a method/property/signal, it should also work at any time. We really have no way of restricting developers to calling those functions only during a _bind_methods()-type function anyway, since that's really just a convention used by the bindings, it's not part of the GDExtension interface.

(We could have maybe constructed the GDExtension interface such that we'd pass a callback to Godot that would call a _bind_methods()-type function, and that way we could limit certain operations to occurring during that callback - but we didn't. I suppose we could maybe mark the classes that are from a GDExtension that's still in the process of being loaded as "mutable", allow binding methods/properties/signals only on those, and then once the GDExtension is finished loading, clear the mutable flag on its classes? That may be a good idea, just to protect people from doing weird things :-))

[raulsntos]

Hm. There is no way to stash the path to the icon somewhere and use it as part of the class registration?

Yes, I think I'll end up making our "_bind_methods"-like function not really register anything and just stash the icon/methods/properties/signals somewhere, then we should be able to register everything in whatever order we want at a later point.

I suppose we could maybe mark the classes that are from a GDExtension that's still in the process of being loaded as "mutable", allow binding methods/properties/signals only on those, and then once the GDExtension is finished loading, clear the mutable flag on its classes? That may be a good idea, just to protect people from doing weird things :-))

That sounds good to me, and as I said I think I'll end up implementing something like that on the C# side. If GDExtension does this then it'd be less work for me on the bindings side, but I can make it work either way.

[dsnopek]

Thanks, this is looking good to me!

@raulsntos Let me know if you plan to update the godot-cpp PR, or if you'd like me to take that over?

[raulsntos]

Let me know if you plan to update the godot-cpp PR, or if you'd like me to take that over?

Sure. I updated the gdextension_interface.h and I'm passing nullptr as the icon path in class_db.hpp. I'm not sure if you wanted to expose a way to set the icon path in godot-cpp, since you can already do that from the .gdextension file. Feel free to suggest any further changes you want me to make in that PR, or feel free to take over the PR as well if you prefer.

[dsnopek]

I'm not sure if you wanted to expose a way to set the icon path in godot-cpp, since you can already do that from the .gdextension file. Feel free to suggest any further changes you want me to make in that PR, or feel free to take over the PR as well if you prefer.

I'd like to add something to the build system that will automatically pull in icons from a directory, allowing the same workflow as with Godot modules. I'd be happy to take over the godot-cpp changes and add that in :-)

[akien-mga]

Thanks!