Skip to content

Conversation

@PapyChacal
Copy link
Contributor

This PR offers an implementation for binding inherited methods, using ClassDB::bind_inherited_method<DerivedClass>(...).
It can avoid a lot of boilerplate code in some extensions, where the registered class has to redefine every bound method.
I wasn't sure if this would be helpful on Godot's side or even if it doesn't already work. So tell me if it would be better done there first.

The main obstacle was TYPED_METHOD_BIND in include/godot_cpp/core/method_bind.hpp: it is necessary to call the method on a derived instance to offset this correctly. (Without doing so, the extension compiled but the calls went nuts because this was wrong)

Thus I also offer to get completely rid of TYPED_METHOD_BIND, as in always using typed method bindings. It was introduced in #649 because it was mandatory for Windows builds, but I don't see where not using it would be required: please enlighten me if there is an example.
On top of that, it makes the file much more readable!

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 15, 2023

Thanks!

However, I don't entirely understand why this is necessary? You shouldn't need to bind methods from parent classes again. By virtue of being a child class, the parent methods should all already be bound.

Regarding TYPED_METHOD_BIND: I believe that was added specifically to allow building on MSVC, but (I think) it requires generating more code (ie. the template is instantiated more times for more types). Assuming I have that right, the non-TYPED_METHOD_BIND version allows us to generate smaller binaries when building with GCC or LLVM. So, while all compilers should work fine with the TYPED_METHOD_BIND version, ideally it'd be nice to maintain both.

@Calinou Calinou added enhancement This is an enhancement on the current functionality topic:gdextension This relates to the new Godot 4 extension implementation labels Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This is an enhancement on the current functionality topic:gdextension This relates to the new Godot 4 extension implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants