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

Make extension instances create the corresponding godot object in their constructor #663

Merged

Conversation

groud
Copy link
Member

@groud groud commented Nov 30, 2021

This requires godotengine/godot#55470 to work.

First, the Godot object creation is the first step executed when creating an extension instance. This is done by making the default constructor call the parent one in the initialization list. The topmost Wrapped constructor thus call the Godot object creation as soon as possible (using classdb_construct_object).

As we want to keep the possibility to create a default Constructor in a custom class (those using the GDCLASS macro), I could not pass the final p_godot_class parameter to the Wrapped(const char *p_godot_class) constructor with the actual class name. Instead, the name of the first class built-in to Godot is passed. This is thus enough to construct the Godot object, but not to finalize the initialization correctly.

As a consequence, i used the same trick as we have in Godot, by modifying the memnew macro so that it sets the object instance after the object is created. The memnew calls a _postinitialize function, that registers the instance to the object using godot::internal::gdn_interface->object_set_instance(_owner, _get_class(), this);. There, the _get_class function is available, which allow finally setting up the object correctly.
Note: this cannot be done in the Wrapped constructor as, at that point, the virtual functions are not initialized yet. So calling _get_class() in the Wrapped constructor would not compile.

Finally, to avoid users creating object without using memnew, I made operator= private. The same trick is used in godot so that should not be a problem.

@groud groud added bug This has been identified as a bug enhancement This is an enhancement on the current functionality labels Nov 30, 2021
@groud groud added this to the 4.0 milestone Nov 30, 2021
@groud groud requested a review from BastiaanOlij November 30, 2021 14:08
@groud groud force-pushed the move_godot_object_init_to_extension branch 2 times, most recently from 1c12299 to 405d5aa Compare December 1, 2021 16:16
@groud groud marked this pull request as ready for review December 1, 2021 16:18
@groud groud force-pushed the move_godot_object_init_to_extension branch from 7cf0264 to 3fcb8a4 Compare December 3, 2021 14:38
@akien-mga akien-mga merged commit f58a2f2 into godotengine:master Dec 3, 2021
@akien-mga
Copy link
Member

Thanks!

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 enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants