Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual function binding #1199

Closed
Kehom opened this issue Aug 4, 2023 · 3 comments · Fixed by #1377
Closed

Virtual function binding #1199

Kehom opened this issue Aug 4, 2023 · 3 comments · Fixed by #1377

Comments

@Kehom
Copy link
Contributor

Kehom commented Aug 4, 2023

Godot version

4.1 stable

godot-cpp version

4.1 stable

System information

Windows 10 and Linux Mint

Issue description

When binding a virtual function in a custom class, things become rather finicky. For the purposes of explanation I will exemplify things using Resource as base class, but this behavior occurs with any other base.

Suppose I have this:

class CustomResource : public Resource {
....
   static void _bind_methods() {
      BIND_VIRTUAL_METHOD(CustomResource, perform_task);
   }
...
   virtual void perform_task() { some_default_behavior... }
};

When in GDScript, if I attempt to directly call that virtual function with some_instance.perform_task() I will get an error telling that the function does not exist. Yet, If I do some_instance.call("perform_task") then the function will be called and the default behavior will occur. I can then create a derived class in GDScript and override perform_task() without any problem. In that case either .perform_task() or call("perform_task") will work in GDScript.

Now, from the C++ code:

1 - If I directly call ->perform_task(), then the default behavior will occur. If I create a derived class in C++ and override that virtual function, then I will get the overridden code. However if I override that function in a GDScript class then I will still get the C++ default behavior.
2 - If I call the function with ->call("perform_task") I will be able to override that function from GDScript but not in a C++ class.

The workaround that I'm currently using is something like this:

class CustomResource : public Resource {
...
   static void _bind_methods() {
      BIND_VIRTUAL_METHOD(CustomResource, _perform_task);
      ClassDB::bind_method(D_METHOD("perform_task"), &CustomResource::perform_task);
   }
...
   virtual void _perform_task() { some_default_behavior... }
   void perform_task() { call("_perform_task"); }
};

This does present the intuitive .perform_task() way of calling the function from GDScript and ->perform-task() from C++. However there are problems here.

1 - Using call() does not work in a const function. Well, not without some "hacky" code (casting the this into a non cost version).

2 - In a C++ derived class I need to do this:

class SpecialCustomResource : public CustomResource {
...
   static void _bind_methods() {
      ClassDB::bind_method(D_METHOD("_perform_task"), &SpecialCustomResource::_perform_task);
   }
...
   virtual void _perform_task() override { some_other_behavior... }
};

If I don't bind the overridden function as a normal one then the call() will not find the desired function. Another thing that occurs when the function is not registered is that if you are expecting a return value nullptr will be the result.

Is the described intended behavior?

Steps to reproduce

The entire explanation is, in a way, steps to reproduce the problem.

Minimal reproduction project

I don't have one in hand. If really absolutely necessary I can work on one

@dsnopek
Copy link
Collaborator

dsnopek commented Aug 7, 2023

This seems similar to these earlier issues:

I think the problem may be that virtual methods are meant to be implemented in godot-cpp the same way as in Godot modules, which means using the GDVIRTUAL*() and GDVIRTUAL_CALL() macros, but those haven't actually been implemented in godot-cpp yet.

I'll double check with some of the more senior members of the team.

@Zylann
Copy link
Collaborator

Zylann commented Aug 7, 2023

Correct me if I'm wrong but I keep seeing confusion about what constitutes actual virtual method bindings in GodotCpp, because while there is godot::ClassDB::bind_virtual_method (which is wrapped by BIND_VIRTUAL_METHOD), it seems to have been thought solely for the internal bindings of core classes and not for creating actual custom virtual functions (which should rather need GDVIRTUAL and GDVIRTUAL_BIND). Therefore attempts to use it as if it was the latter results in mixed up behaviors.

@Kehom
Copy link
Contributor Author

Kehom commented Aug 7, 2023

I think the problem may be that virtual methods are meant to be implemented in godot-cpp the same way as in Godot modules, which means using the GDVIRTUAL*() and GDVIRTUAL_CALL() macros, but those haven't actually been implemented in godot-cpp yet.

I'll double check with some of the more senior members of the team.

Thank you. I did stumble before into #1072 but not #910. The first issue seem like the solution, while the second issue matches more closely my description (so sorry for somewhat creating a repeated issue).

Correct me if I'm wrong but I keep seeing confusion about what constitutes actual virtual method bindings in GodotCpp, because while there is godot::ClassDB::bind_virtual_method (which is wrapped by BIND_VIRTUAL_METHOD), it seems to have been thought solely for the internal bindings of core classes and not for creating actual custom virtual functions (which should rather need GDVIRTUAL and GDVIRTUAL_BIND). Therefore attempts to use it as if it was the latter results in mixed up behaviors.

This basically confirms what @dsnopek said. But then, ClassDB::bind_virtual_method currently being the only way for a GDExtension to tell Godot "hey, this is a virtual function" I think it's only natural that people will indeed attempt to use (and be confused with) like in the second use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants