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.

Sign up for GitHub

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

Update for new NOTIFICATION_POSTINITIALIZE handling #1568

Merged
merged 1 commit into from
Oct 30, 2024
+90 −26
Merged

Update for new NOTIFICATION_POSTINITIALIZE handling #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 72 additions & 5 deletions gdextension/gdextension_interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ typedef void (*GDExtensionClassReference)(GDExtensionClassInstancePtr p_instance
typedef void (*GDExtensionClassUnreference)(GDExtensionClassInstancePtr p_instance);
typedef void (*GDExtensionClassCallVirtual)(GDExtensionClassInstancePtr p_instance, const GDExtensionConstTypePtr *p_args, GDExtensionTypePtr r_ret);
typedef GDExtensionObjectPtr (*GDExtensionClassCreateInstance)(void *p_class_userdata);
typedef GDExtensionObjectPtr (*GDExtensionClassCreateInstance2)(void *p_class_userdata, GDExtensionBool p_notify_postinitialize);
typedef void (*GDExtensionClassFreeInstance)(void *p_class_userdata, GDExtensionClassInstancePtr p_instance);
typedef GDExtensionClassInstancePtr (*GDExtensionClassRecreateInstance)(void *p_class_userdata, GDExtensionObjectPtr p_object);
typedef GDExtensionClassCallVirtual (*GDExtensionClassGetVirtual)(void *p_class_userdata, GDExtensionConstStringNamePtr p_name);
Expand All @@ -292,7 +293,7 @@ typedef struct {
GDExtensionClassGetVirtual get_virtual_func; // Queries a virtual function by name and returns a callback to invoke the requested virtual function.
GDExtensionClassGetRID get_rid_func;
void *class_userdata; // Per-class user data, later accessible in instance bindings.
} GDExtensionClassCreationInfo; // Deprecated. Use GDExtensionClassCreationInfo3 instead.
} GDExtensionClassCreationInfo; // Deprecated. Use GDExtensionClassCreationInfo4 instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt anyone will see a deprecation comment. Is there a reason we are avoiding [[deprecated("Use GDExtensionClassCreationInfo4 instead.")]]?

Copy link
Collaborator Author

@dsnopek dsnopek Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header is using a restricted subset of C: we want it to be possible to compile it with a C or C++ compiler, but it isn't meant to be "real code" because some bindings parse it, rather than compile it.

[[deprecated ...]] is a C++ thing, but even if it was C, I think we probably wouldn't use it anyway because it could create problems for bindings that manually parse the header.

FYI, I'd love to replace this with a JSON or XML definition that we generate the C code from, rather than the header being the definitive source.


typedef struct {
GDExtensionBool is_virtual;
Expand Down Expand Up @@ -325,7 +326,7 @@ typedef struct {
GDExtensionClassCallVirtualWithData call_virtual_with_data_func;
GDExtensionClassGetRID get_rid_func;
void *class_userdata; // Per-class user data, later accessible in instance bindings.
} GDExtensionClassCreationInfo2; // Deprecated. Use GDExtensionClassCreationInfo3 instead.
} GDExtensionClassCreationInfo2; // Deprecated. Use GDExtensionClassCreationInfo4 instead.

typedef struct {
GDExtensionBool is_virtual;
Expand Down Expand Up @@ -359,7 +360,40 @@ typedef struct {
GDExtensionClassCallVirtualWithData call_virtual_with_data_func;
GDExtensionClassGetRID get_rid_func;
void *class_userdata; // Per-class user data, later accessible in instance bindings.
} GDExtensionClassCreationInfo3;
} GDExtensionClassCreationInfo3; // Deprecated. Use GDExtensionClassCreationInfo4 instead.

typedef struct {
GDExtensionBool is_virtual;
GDExtensionBool is_abstract;
GDExtensionBool is_exposed;
GDExtensionBool is_runtime;
GDExtensionClassSet set_func;
GDExtensionClassGet get_func;
GDExtensionClassGetPropertyList get_property_list_func;
GDExtensionClassFreePropertyList2 free_property_list_func;
GDExtensionClassPropertyCanRevert property_can_revert_func;
GDExtensionClassPropertyGetRevert property_get_revert_func;
GDExtensionClassValidateProperty validate_property_func;
GDExtensionClassNotification2 notification_func;
GDExtensionClassToString to_string_func;
GDExtensionClassReference reference_func;
GDExtensionClassUnreference unreference_func;
GDExtensionClassCreateInstance2 create_instance_func; // (Default) constructor; mandatory. If the class is not instantiable, consider making it virtual or abstract.
GDExtensionClassFreeInstance free_instance_func; // Destructor; mandatory.
GDExtensionClassRecreateInstance recreate_instance_func;
// Queries a virtual function by name and returns a callback to invoke the requested virtual function.
GDExtensionClassGetVirtual get_virtual_func;
// Paired with `call_virtual_with_data_func`, this is an alternative to `get_virtual_func` for extensions that
// need or benefit from extra data when calling virtual functions.
// Returns user data that will be passed to `call_virtual_with_data_func`.
// Returning `NULL` from this function signals to Godot that the virtual function is not overridden.
// Data returned from this function should be managed by the extension and must be valid until the extension is deinitialized.
// You should supply either `get_virtual_func`, or `get_virtual_call_data_func` with `call_virtual_with_data_func`.
GDExtensionClassGetVirtualCallData get_virtual_call_data_func;
// Used to call virtual functions when `get_virtual_call_data_func` is not null.
GDExtensionClassCallVirtualWithData call_virtual_with_data_func;
void *class_userdata; // Per-class user data, later accessible in instance bindings.
} GDExtensionClassCreationInfo4;

typedef void *GDExtensionClassLibraryPtr;

Expand Down Expand Up @@ -2708,6 +2742,7 @@ typedef void *(*GDExtensionInterfaceCallableCustomGetUserData)(GDExtensionConstT
/**
* @name classdb_construct_object
* @since 4.1
* @deprecated in Godot 4.4. Use `classdb_construct_object2` instead.
*
* Constructs an Object of the requested class.
*
Expand All @@ -2719,6 +2754,22 @@ typedef void *(*GDExtensionInterfaceCallableCustomGetUserData)(GDExtensionConstT
*/
typedef GDExtensionObjectPtr (*GDExtensionInterfaceClassdbConstructObject)(GDExtensionConstStringNamePtr p_classname);

/**
* @name classdb_construct_object2
* @since 4.4
*
* Constructs an Object of the requested class.
*
* The passed class must be a built-in godot class, or an already-registered extension class. In both cases, object_set_instance() should be called to fully initialize the object.
*
* "NOTIFICATION_POSTINITIALIZE" must be sent after construction.
*
* @param p_classname A pointer to a StringName with the class name.
*
* @return A pointer to the newly created Object.
*/
typedef GDExtensionObjectPtr (*GDExtensionInterfaceClassdbConstructObject2)(GDExtensionConstStringNamePtr p_classname);

/**
* @name classdb_get_method_bind
* @since 4.1
Expand Down Expand Up @@ -2750,7 +2801,7 @@ typedef void *(*GDExtensionInterfaceClassdbGetClassTag)(GDExtensionConstStringNa
/**
* @name classdb_register_extension_class
* @since 4.1
* @deprecated in Godot 4.2. Use `classdb_register_extension_class3` instead.
* @deprecated in Godot 4.2. Use `classdb_register_extension_class4` instead.
*
* Registers an extension class in the ClassDB.
*
Expand All @@ -2766,7 +2817,7 @@ typedef void (*GDExtensionInterfaceClassdbRegisterExtensionClass)(GDExtensionCla
/**
* @name classdb_register_extension_class2
* @since 4.2
* @deprecated in Godot 4.3. Use `classdb_register_extension_class3` instead.
* @deprecated in Godot 4.3. Use `classdb_register_extension_class4` instead.
*
* Registers an extension class in the ClassDB.
*
Expand All @@ -2782,6 +2833,7 @@ typedef void (*GDExtensionInterfaceClassdbRegisterExtensionClass2)(GDExtensionCl
/**
* @name classdb_register_extension_class3
* @since 4.3
* @deprecated in Godot 4.4. Use `classdb_register_extension_class4` instead.
*
* Registers an extension class in the ClassDB.
*
Expand All @@ -2794,6 +2846,21 @@ typedef void (*GDExtensionInterfaceClassdbRegisterExtensionClass2)(GDExtensionCl
*/
typedef void (*GDExtensionInterfaceClassdbRegisterExtensionClass3)(GDExtensionClassLibraryPtr p_library, GDExtensionConstStringNamePtr p_class_name, GDExtensionConstStringNamePtr p_parent_class_name, const GDExtensionClassCreationInfo3 *p_extension_funcs);

/**
* @name classdb_register_extension_class4
* @since 4.4
*
* Registers an extension class in the ClassDB.
*
* Provided struct can be safely freed once the function returns.
*
* @param p_library A pointer the library received by the GDExtension's entry point function.
* @param p_class_name A pointer to a StringName with the class name.
* @param p_parent_class_name A pointer to a StringName with the parent class name.
* @param p_extension_funcs A pointer to a GDExtensionClassCreationInfo2 struct.
*/
typedef void (*GDExtensionInterfaceClassdbRegisterExtensionClass4)(GDExtensionClassLibraryPtr p_library, GDExtensionConstStringNamePtr p_class_name, GDExtensionConstStringNamePtr p_parent_class_name, const GDExtensionClassCreationInfo4 *p_extension_funcs);

/**
* @name classdb_register_extension_class_method
* @since 4.1
Expand Down
6 changes: 0 additions & 6 deletions include/godot_cpp/classes/wrapped.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ class Wrapped {
::godot::List<::godot::PropertyInfo> plist_owned;

void _postinitialize();
virtual void _notificationv(int32_t p_what, bool p_reversed = false) {}

Wrapped(const StringName p_godot_class);
Wrapped(GodotObject *p_godot_object);
Expand Down Expand Up @@ -408,11 +407,6 @@ public:
_gde_binding_reference_callback, \
}; \
\
protected: \
virtual void _notificationv(int32_t p_what, bool p_reversed = false) override { \
m_class::notification_bind(this, p_what, p_reversed); \
} \
\
private:

// Don't use this for your classes, use GDCLASS() instead.
Expand Down
13 changes: 8 additions & 5 deletions include/godot_cpp/core/class_db.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,13 @@ class ClassDB {
static void _register_class(bool p_virtual = false, bool p_exposed = true, bool p_runtime = false);

template <typename T>
static GDExtensionObjectPtr _create_instance_func(void *data) {
static GDExtensionObjectPtr _create_instance_func(void *data, GDExtensionBool p_notify_postinitialize) {
if constexpr (!std::is_abstract_v<T>) {
T *new_object = memnew(T);
Wrapped::_set_construct_info<T>();
Copy link
Contributor

@Ivorforce Ivorforce Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By replacing the memnew call with this, does the change imply that any Object created with memnew will not have postinitialize called? Should godot-cpp users be using ClassDB::instantiate (or just this function) instead henceforth? If so, we should update the docs accordingly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR shouldn't change anything about how developers use godot-cpp, this should be a purely internal change within godot-cpp.

If you look a couple lines down, it will call _postinitialize(), but only if p_notify_postinitialize is true. We're not using memnew() because that should always lead to NOTIFICATION_POSTINITIALIZE, but here we need to be able to control whether we're doing it or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh that makes sense. Thanks for explaining!

T *new_object = new ("", "") T;
if (p_notify_postinitialize) {
new_object->_postinitialize();
}
return new_object->_owner;
} else {
return nullptr;
Expand Down Expand Up @@ -241,7 +245,7 @@ void ClassDB::_register_class(bool p_virtual, bool p_exposed, bool p_runtime) {
class_register_order.push_back(cl.name);

// Register this class with Godot
GDExtensionClassCreationInfo3 class_info = {
GDExtensionClassCreationInfo4 class_info = {
p_virtual, // GDExtensionBool is_virtual;
is_abstract, // GDExtensionBool is_abstract;
p_exposed, // GDExtensionBool is_exposed;
Expand All @@ -263,11 +267,10 @@ void ClassDB::_register_class(bool p_virtual, bool p_exposed, bool p_runtime) {
&ClassDB::get_virtual_func, // GDExtensionClassGetVirtual get_virtual_func;
nullptr, // GDExtensionClassGetVirtualCallData get_virtual_call_data_func;
nullptr, // GDExtensionClassCallVirtualWithData call_virtual_func;
nullptr, // GDExtensionClassGetRID get_rid;
(void *)&T::get_class_static(), // void *class_userdata;
};

internal::gdextension_interface_classdb_register_extension_class3(internal::library, cl.name._native_ptr(), cl.parent_name._native_ptr(), &class_info);
internal::gdextension_interface_classdb_register_extension_class4(internal::library, cl.name._native_ptr(), cl.parent_name._native_ptr(), &class_info);

// call bind_methods etc. to register all members of the class
T::initialize_class();
Expand Down
4 changes: 2 additions & 2 deletions include/godot_cpp/godot.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ extern "C" GDExtensionInterfaceRefSetObject gdextension_interface_ref_set_object
extern "C" GDExtensionInterfaceScriptInstanceCreate3 gdextension_interface_script_instance_create3;
extern "C" GDExtensionInterfacePlaceHolderScriptInstanceCreate gdextension_interface_placeholder_script_instance_create;
extern "C" GDExtensionInterfacePlaceHolderScriptInstanceUpdate gdextension_interface_placeholder_script_instance_update;
extern "C" GDExtensionInterfaceClassdbConstructObject gdextension_interface_classdb_construct_object;
extern "C" GDExtensionInterfaceClassdbConstructObject2 gdextension_interface_classdb_construct_object2;
extern "C" GDExtensionInterfaceClassdbGetMethodBind gdextension_interface_classdb_get_method_bind;
extern "C" GDExtensionInterfaceClassdbGetClassTag gdextension_interface_classdb_get_class_tag;
extern "C" GDExtensionInterfaceClassdbRegisterExtensionClass3 gdextension_interface_classdb_register_extension_class3;
extern "C" GDExtensionInterfaceClassdbRegisterExtensionClass4 gdextension_interface_classdb_register_extension_class4;
extern "C" GDExtensionInterfaceClassdbRegisterExtensionClassMethod gdextension_interface_classdb_register_extension_class_method;
extern "C" GDExtensionInterfaceClassdbRegisterExtensionClassVirtualMethod gdextension_interface_classdb_register_extension_class_virtual_method;
extern "C" GDExtensionInterfaceClassdbRegisterExtensionClassIntegerConstant gdextension_interface_classdb_register_extension_class_integer_constant;
Expand Down
8 changes: 4 additions & 4 deletions src/classes/wrapped.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ void Wrapped::_postinitialize() {
Wrapped::_constructing_mutex.unlock();
#endif

// Only send NOTIFICATION_POSTINITIALIZE for extension classes.
if (_is_extension_class()) {
_notificationv(Object::NOTIFICATION_POSTINITIALIZE);
Object *obj = dynamic_cast<Object *>(this);
if (obj) {
obj->notification(Object::NOTIFICATION_POSTINITIALIZE);
}
}

Expand All @@ -74,7 +74,7 @@ Wrapped::Wrapped(const StringName p_godot_class) {
} else
#endif
{
_owner = godot::internal::gdextension_interface_classdb_construct_object(reinterpret_cast<GDExtensionConstStringNamePtr>(p_godot_class._native_ptr()));
_owner = godot::internal::gdextension_interface_classdb_construct_object2(reinterpret_cast<GDExtensionConstStringNamePtr>(p_godot_class._native_ptr()));
}

if (_constructing_extension_class_name) {
Expand Down
8 changes: 4 additions & 4 deletions src/godot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ GDExtensionInterfaceRefSetObject gdextension_interface_ref_set_object = nullptr;
GDExtensionInterfaceScriptInstanceCreate3 gdextension_interface_script_instance_create3 = nullptr;
GDExtensionInterfacePlaceHolderScriptInstanceCreate gdextension_interface_placeholder_script_instance_create = nullptr;
GDExtensionInterfacePlaceHolderScriptInstanceUpdate gdextension_interface_placeholder_script_instance_update = nullptr;
GDExtensionInterfaceClassdbConstructObject gdextension_interface_classdb_construct_object = nullptr;
GDExtensionInterfaceClassdbConstructObject2 gdextension_interface_classdb_construct_object2 = nullptr;
GDExtensionInterfaceClassdbGetMethodBind gdextension_interface_classdb_get_method_bind = nullptr;
GDExtensionInterfaceClassdbGetClassTag gdextension_interface_classdb_get_class_tag = nullptr;
GDExtensionInterfaceClassdbRegisterExtensionClass3 gdextension_interface_classdb_register_extension_class3 = nullptr;
GDExtensionInterfaceClassdbRegisterExtensionClass4 gdextension_interface_classdb_register_extension_class4 = nullptr;
GDExtensionInterfaceClassdbRegisterExtensionClassMethod gdextension_interface_classdb_register_extension_class_method = nullptr;
GDExtensionInterfaceClassdbRegisterExtensionClassVirtualMethod gdextension_interface_classdb_register_extension_class_virtual_method = nullptr;
GDExtensionInterfaceClassdbRegisterExtensionClassIntegerConstant gdextension_interface_classdb_register_extension_class_integer_constant = nullptr;
Expand Down Expand Up @@ -468,10 +468,10 @@ GDExtensionBool GDExtensionBinding::init(GDExtensionInterfaceGetProcAddress p_ge
LOAD_PROC_ADDRESS(script_instance_create3, GDExtensionInterfaceScriptInstanceCreate3);
LOAD_PROC_ADDRESS(placeholder_script_instance_create, GDExtensionInterfacePlaceHolderScriptInstanceCreate);
LOAD_PROC_ADDRESS(placeholder_script_instance_update, GDExtensionInterfacePlaceHolderScriptInstanceUpdate);
LOAD_PROC_ADDRESS(classdb_construct_object, GDExtensionInterfaceClassdbConstructObject);
LOAD_PROC_ADDRESS(classdb_construct_object2, GDExtensionInterfaceClassdbConstructObject2);
LOAD_PROC_ADDRESS(classdb_get_method_bind, GDExtensionInterfaceClassdbGetMethodBind);
LOAD_PROC_ADDRESS(classdb_get_class_tag, GDExtensionInterfaceClassdbGetClassTag);
LOAD_PROC_ADDRESS(classdb_register_extension_class3, GDExtensionInterfaceClassdbRegisterExtensionClass3);
LOAD_PROC_ADDRESS(classdb_register_extension_class4, GDExtensionInterfaceClassdbRegisterExtensionClass4);
LOAD_PROC_ADDRESS(classdb_register_extension_class_method, GDExtensionInterfaceClassdbRegisterExtensionClassMethod);
LOAD_PROC_ADDRESS(classdb_register_extension_class_virtual_method, GDExtensionInterfaceClassdbRegisterExtensionClassVirtualMethod);
LOAD_PROC_ADDRESS(classdb_register_extension_class_integer_constant, GDExtensionInterfaceClassdbRegisterExtensionClassIntegerConstant);
Expand Down
Loading