godot icon indicating copy to clipboard operation
godot copied to clipboard

get_class() returns wrong name, breaks signal connections, C++/GDExtension

Open Kajinor opened this issue 2 years ago • 2 comments

Godot version

4.0.beta6

System information

macOS Monterey, Apple M1

Issue description

If this is not a bug, I am sorry but there is not much documentation yet. If you create a class in C++ using GDExtension like this:

class MyNode : public Node3D 
{
    GDCLASS(MyNode, Node3D)
protected:
    static void _bind_methods() { ..... }
...............
};

Next you register the class and stuff. If you then create a new node of this type (MyNode) using the Godot Editor, then everything runs fine and get_class() returns "MyNode". But if you instance that node via C++ like this:

auto node = new MyNode();
UtilitiyFunctions::print(node->get_class());

It will print out "Node3D" and this seems to be a problem if you connect to signals via Object::connect because it seems that it uses get_class() or get_class_name() to get the list of methods and it won't find your method because it looks into the wrong class.

So this is either bug or I instantiated it the wrong way. I saw that Object and other builtin classes have a constructor taking the class name and if I pass the correct class name in there manually like this, it works:

auto node = new MyNode("MyNode");

(MyNode will pass down the class name.)

But I guess that is not how it is intended to be. Especially because it will crash if you pass down a class name but the node has been created via Editor. Which is why I couldn't automatically pass down "MyNode" via default constructor. I saw there is also get_class() but it is not virtual so I cannot override it.

Shouldn't that all be handled by the GDCLASS macro?

Steps to reproduce

Create a class in C++ using GDExtension like this:

class MyNode : public Node3D 
{
    GDCLASS(MyNode, Node3D)
protected:
    static void _bind_methods() { ..... }
...............
};

If you instance that node via C++ like this:

auto node = new MyNode();
UtilitiyFunctions::print(node->get_class());

It will print out "Node3D" instead of "MyNode".

Minimal reproduction project

To run this, you have to compile it so make sure to copy/clone the godot-cpp repo into the root folder. I couldn't include as file size it too big then. There is a SConstruct file which will build godot-cpp together with the extension then. So just move into the root folder and hit:

scons

To compile it. If you then open the godot project (inside project folder) and run the empty scene, on the output console you will see that the class name from the node added via Editor is "MyNode" and "Node" for the same one but created via C++. gdextension_bug.zip

Kajinor avatar Nov 27 '22 22:11 Kajinor

This seems to be a godotengine/godot-cpp issue.

And please join a MRP, especially since there seem to be a lot of steps to take to compile the issue.

adamscott avatar Nov 28 '22 00:11 adamscott

Ok, maybe this should be moved to godot-cpp then. Also I attached an MRP.

Kajinor avatar Nov 28 '22 13:11 Kajinor

Seems related to #61396 and #21789

bsil78 avatar Dec 10 '22 09:12 bsil78

I think in Godot 3.x and probably also in Godot 4.0, get_class only returns the class name of the built-in class and no class named defined via "class_name" in GDScript. I personally don't mind, that's not the main issue, the main issue is with signals/receivers not working in C++ it seems.

In Godot 4.0 with C++ for custom nodes, get_class returns the actual class name which is defined via GDCLASS macro and registered via ClassDB::register_class. But only IF the node has been created in the Editor and is loaded with the scene. If you instantiate the node at runtime with C++ code, then the class name is the one of the closest parent built-in class. That is different behavior for the same thing so a bug I guess.

Main Issue

The problem with this is that emitting of signals as well as receiving of signals doesn't work with custom nodes created at runtime in C++ which is... a huge issue I think. The reason seems to be that the class name is used to search through the signals/receivers list so it will not find signals and receivers in custom nodes created at runtime due to the wrong class name.

Workaround

My workaround is that I created hidden instances of my custom nodes in the Editor and then use duplicate() at runtime from C++ to create an actual instance....still not how it should be I think.

So it would be cool if that could be fixed soon.

Kajinor avatar Dec 10 '22 16:12 Kajinor

I ran into this issue in 4.1.1-stable when it caused deferred calls (via call_deferred) to break with

_call_function: Error calling deferred method: '<Godot parent class>::<method name>': Method not found.

In addition to replicating the findings above, I found that any GDExtension-registered classes instantiated at runtime in C# via ClassDB.Instantiate("MyGDExtensionClass") would behave as expected, with deferred calls working and get_class() returning MyGDExtensionClass as though they had been created in the editor.

Instantiating GDExtension classes in GDScript at runtime also work as expected; something like

var instance = MyGDExtensionClass.new();
print(instance.get_class());

outputs MyGDExtensionClass.

So it seems that it is indeed only in C++ that the runtime-created GDExtension instances end up broken in the way this issue describes.

dashdotdashdot avatar Sep 23 '23 19:09 dashdotdashdot

As far as I can tell, this has the same root cause as godotengine/godot-cpp#1057 and specifically affects all objects instantiated in C++ from a user-defined GDExtension class when they are instantiated with an automatic storage duration (colloquially "on the stack") or any non-memnew-created dynamic storage duration (colloquially "on the heap"). Only when objects of these classes are instantiated using memnew will their get_class() methods, deferred calls, signals, etc. work correctly.

Although definitely an unexpected little trap for the unwary, it's unclear whether this is presently considered a bug. Were this to be fixed or changed, it would probably require changes in both this project and in godot-cpp, hence this note and issue link.

dashdotdashdot avatar Oct 02 '23 07:10 dashdotdashdot

Although definitely an unexpected little trap for the unwary, it's unclear whether this is presently considered a bug.

This isn't a bug - with godot-cpp you have to use memnew(). Since there is a docs issue already, I'm going to close this one

dsnopek avatar Jun 14 '24 13:06 dsnopek