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

Deriving from a custom class that implements Godot virtual methods fails to compile #854

Closed
Zylann opened this issue Sep 19, 2022 · 2 comments · Fixed by #855
Closed

Deriving from a custom class that implements Godot virtual methods fails to compile #854

Zylann opened this issue Sep 19, 2022 · 2 comments · Fixed by #855
Labels
bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation

Comments

@Zylann
Copy link
Collaborator

Zylann commented Sep 19, 2022

With GodotCpp master branch.

Simple example:

class TestNode : public godot::Node {
	GDCLASS(TestNode, godot::Node)
public:
	void _ready() override;
private:
	static void _bind_methods();
};

class DerivedNode : public TestNode {
	GDCLASS(DerivedNode, TestNode)
private:
	static void _bind_methods();
};

A setup like this one produces the following compiling errors:

Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2672: 'call_with_ptr_args': no matching overloaded function found
TESTS\Godot4\Extension\addons\extension_test\cpp\src\test_node.h(9): note: see reference to function template instantiation 'void godot::Node::register_virtuals<T>(void)' being compiled
        with
        [
            T=extension_test::DerivedNode
        ]
TESTS\Godot4\Extension\addons\extension_test\cpp\src\derived_node.h(9): note: see reference to function template instantiation 'void extension_test::TestNode::register_virtuals<extension_test::DerivedNode>(void)' being compiled
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(207): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': could not deduce template argument for 'R (__cdecl T::* )(P...) const' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(207): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(202): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': could not deduce template argument for 'R (__cdecl T::* )(P...)' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(202): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(197): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': could not deduce template argument for 'void (__cdecl T::* )(P...) const' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(197): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(192): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': could not deduce template argument for 'void (__cdecl T::* )(P...)' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(192): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2672: 'call_with_ptr_args': no matching overloaded function found
TESTS\Godot4\Extension\addons\extension_test\cpp\src\test_node.h(9): note: see reference to function template instantiation 'void godot::Node::register_virtuals<T>(void)' being compiled
        with
        [
            T=extension_test::DerivedNode
        ]
TESTS\Godot4\Extension\addons\extension_test\cpp\src\derived_node.h(9): note: see reference to function template instantiation 'void extension_test::TestNode::register_virtuals<extension_test::DerivedNode>(void)' being compiled
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(207): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': could not deduce template argument for 'R (__cdecl T::* )(P...) const' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(207): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(202): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,R (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': could not deduce template argument for 'R (__cdecl T::* )(P...)' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(202): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(197): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...) const,const GDNativeTypePtr *,void *)': could not deduce template argument for 'void (__cdecl T::* )(P...) const' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(197): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2782: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': template parameter 'T' is ambiguous
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(192): note: see declaration of 'godot::call_with_ptr_args'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: could be 'extension_test::TestNode'
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): note: or       'T'
        with
        [
            T=extension_test::DerivedNode
        ]
Engine\godot_cpp_fork\gen\include\godot_cpp/classes/node.hpp(254): error C2784: 'void godot::call_with_ptr_args(T *,void (__cdecl T::* )(P...),const GDNativeTypePtr *,void *)': could not deduce template argument for 'void (__cdecl T::* )(P...)' from 'void (__cdecl extension_test::TestNode::* )(void)'
Engine\godot_cpp_fork\include\godot_cpp/core/binder_common.hpp(192): note: see declaration of 'godot::call_with_ptr_args'
scons: *** [src\derived_node.windows.debug.x86_64.obj] Error 2
scons: *** [src\register_types.windows.debug.x86_64.obj] Error 2

The only workaround I could find so far was to implement those virtual methods again in all derived classes, making them call the parent method. This is inconvenient and becomes a nightmare in cases like this https://github.com/Zylann/godot_voxel/blob/gdextension/editor/vox/vox_scene_importer.h

MRP:
extension_test_issue854_virtual_methods.zip
Note: change the path to GodotCpp in SConstruct.


It seems to happen because GDCLASS internally defines register_virtuals(), which calls BaseClass::register_virtuals<CurrentClass>(). And in there, it does this:

		// Checks if `T` overrides `_ready`. If so, bind it.
		if constexpr (!std::is_same_v<decltype(&Node::_ready), decltype(&T::_ready)>) {
			BIND_VIRTUAL_METHOD(T, _ready);
		}

When T is the derived class, it does not define _ready. Only its base class does. But for some reason, that makes BIND_VIRTUAL_METHOD fail to compile.

I thought that's because the if checks the wrong condition. Indeed, when T derives from a custom class, Node::_ready is not the same function as T::_ready. But T does not actually define _ready, its custom base class does. So we should not try to bind the method again if it's the same as the base class.
So I tried this:

		if constexpr (!std::is_same_v<decltype(&B::_ready), decltype(&T::_ready)>) {
			BIND_VIRTUAL_METHOD(T, _ready);
		}

Where B is the actual base class, so a more informed check can be done.
Unfortunately, the fact there is an if does not prevent the code inside from being compiled. The error remains exactly the same: it can't compile the inside of the if.

Here with the macro expanded:

		if constexpr (!std::is_same_v<decltype(&B::_ready), decltype(&T::_ready)>) {
			// BIND_VIRTUAL_METHOD(T, _ready);
			auto ___call_ready = [](GDNativeObjectPtr p_instance, const GDNativeTypePtr *p_args, GDNativeTypePtr p_ret) -> void {
				call_with_ptr_args(reinterpret_cast<T *>(p_instance), &T::_ready, p_args, p_ret);
			};
			godot::ClassDB::bind_virtual_method(T::get_class_static(), "_ready", ___call_ready);
		}

So what I dont understand is, why all the methods I don't override are compiling just fine, while they should be in the same situation?
With if constexpr, if the condition doesn't pass, the code inside isn't even compiled. But here it looks like it tries to compile it anyways.

Funny enough, if I replace the condition with

		if constexpr (false) {

It compiles fine. When I try !std::is_same_v<decltype(&B::_ready), decltype(&T::_ready)>, I tested and expected it to be false too for the derived class, but somehow discarding isn't working.
I even tried somthing similar on https://godbolt.org/ and it worked...
image

@Calinou Calinou added bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation labels Sep 19, 2022
@Zylann
Copy link
Collaborator Author

Zylann commented Sep 19, 2022

Ok I was actually on the right track, something was wrong in the way I passed the B template argument. The project compiles now.

So this is a simplified example of the trick:

// In the real code, this is `Node`
struct A {
	virtual void _ready() {
		std::cout << "A::foo()" << std::endl;
	}
};

struct B : public A {
	void _ready() override {
		std::cout << "A::foo()" << std::endl;
	}
};

struct C : public B {
};

template <typename T, typename TBase>
void register_virtuals() {
    if constexpr(!std::is_same_v<decltype(&T::_ready), decltype(&TBase::_ready)>) {
        // The inside is not even compiled if the condition is false
        auto call__ready = [](void *p_instance, const void*p_args, void *p_ret) -> void {
            // I didn't even define `call_with_ptr_args` btw
            call_with_ptr_args(reinterpret_cast<T *>(p_instance), &T::_ready, p_args, p_ret);
        };
        // [...] bind_virtual_method(call__ready)...
    }
}

register_virtuals<C, B>();

The idea is to check if the class defines a _ready method that is different from the base class. If it is, it will be bound.
The old code was always comparing with the Godot class, so in this situation it was still thinking the method was different, when it wasn't. And it failed to compile because then it couldn't figure out which version of the method to use (because there was no direct definition, I think).

@Zylann
Copy link
Collaborator Author

Zylann commented Mar 4, 2023

Quick note because errors aren't obvious: if you write GDCLASS(A, B) and B is not a direct base of A, you get the same errors. B must be a direct base, which is not warned about in modules, but results in errors in GodotCpp (just very numerous/confusing ones).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug topic:gdextension This relates to the new Godot 4 extension implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants