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

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

Closed
Kajinor opened this issue Nov 27, 2022 · 7 comments
Closed

Comments

@Kajinor
Copy link

Kajinor commented Nov 27, 2022

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 Kajinor changed the title get_class() returns wrong name if node created in C++/GDExtension get_class() returns wrong name, breaks signal connections, C++/GDExtension Nov 27, 2022
@adamscott
Copy link
Member

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.

@Kajinor
Copy link
Author

Kajinor commented Nov 28, 2022

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

@Chaosus Chaosus added this to the 4.0 milestone Nov 28, 2022
@bsil78
Copy link

bsil78 commented Dec 10, 2022

Seems related to #61396 and #21789

@Kajinor
Copy link
Author

Kajinor commented Dec 10, 2022

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.

@dashdotdashdot
Copy link

dashdotdashdot commented Sep 23, 2023

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
Copy link

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.

@dsnopek
Copy link
Contributor

dsnopek commented Jun 14, 2024

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

@akien-mga akien-mga removed this from the 4.3 milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants