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

Calling connect within extension class ctor causes set_instance_bindings error #1312

Closed
Naros opened this issue Nov 19, 2023 · 7 comments
Closed
Labels

Comments

@Naros
Copy link
Contributor

Naros commented Nov 19, 2023

Godot version

4.2

godot-cpp version

4.2

System information

Windows 11

Issue description

When calling the connect method from within a GDE class ctor, the following error is thrown:

ERROR: Condition "_instance_bindings != nullptr && _instance_bindings[0].binding != nullptr" is true.
   at: Object::set_instance_binding (core\object\object.cpp:1824)

Steps to reproduce

Create a simple GDE class and register signal listener callback in the ctor:

class MyClass : public LineEdit {
 ...
}

MyClass::MyClass() {
  connect("text_submitted", callable_mp(this, &MyClass::_on_text_submitted));
}
...

When the class is created, the error will be thrown.

Minimal reproduction project

N/A

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 20, 2023

Thanks!

I think this is a variation on issue #1039

Basically, at present, you can't pass this into any Godot APIs during the constructor, because the object isn't finished being setup at that point.

@AThousandShips
Copy link
Member

Can you try using NOTIFICATION_POSTINITIALIZE instead as a workaround?

@Naros
Copy link
Contributor Author

Naros commented Nov 22, 2023

Can you try using NOTIFICATION_POSTINITIALIZE instead as a workaround?

No, because unfortunately NOTIFICATION_POSTINITIALIZE is not fired like it is in GDScript, see #1269 ; however, right now I am using NOTIFICATION_ENTER_TREE or NOTIFICATION_READY at the moment.

@bearman92
Copy link
Contributor

bearman92 commented Nov 22, 2023

I found a workaround that works for me

I added _init() method to Wrapped

class Wrapped() {
...
	virtual void _init() { } // <- here
	void _notification(int p_what) {}
...
}

And call at end of the Wrapped::_postinitialize

void Wrapped::_postinitialize() {
	const StringName *extension_class = _get_extension_class_name();
	if (extension_class) {
		godot::internal::gdextension_interface_object_set_instance(_owner, reinterpret_cast<GDExtensionConstStringNamePtr>(extension_class), this);
	}
	godot::internal::gdextension_interface_object_set_instance_binding(_owner, godot::internal::token, this, _get_bindings_callbacks());
	_init(); // <- here
}

And I just use _init instead of constructors.

@AThousandShips
Copy link
Member

Generally many signals are best to connect on entering the tree, partially due to threading, see:

@dsnopek
Copy link
Collaborator

dsnopek commented Dec 6, 2023

Can you try using NOTIFICATION_POSTINITIALIZE instead as a workaround?

No, because unfortunately NOTIFICATION_POSTINITIALIZE is not fired like it is in GDScript, see #1269 ; however, right now I am using NOTIFICATION_ENTER_TREE or NOTIFICATION_READY at the moment.

PR #1321 was recently merged to fix NOTIFICATION_POSTINITIALIZE. Perhaps that is now another way to work around this issue?

@Naros
Copy link
Contributor Author

Naros commented Jan 12, 2024

Thanks, based on the feedback, I am going to close this as it seems the agreed approach is to use a later phase during object construction than the constructor to perform signal connections.

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

No branches or pull requests

4 participants