From 38ee8bfcf724de3294146d04c6c83f2c06bc198b Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 9 Sep 2021 20:40:40 -0300 Subject: [PATCH] Change constructor/destructor management of extension classes This makes sure custom constructors are always called on extension classes. However, note that constructors should not take any parameters, since Godot doesn't support that. Parameters are ignore in memnew macro. Use memnew(MyClass()) instead of memnew(MyClass) since it now needs a value instead of a class name. This macro calls MyClass::_new() (define in GDCLASS macro) which ultimately calls Godot to create the object, ensuring that both the Godot and the extension instances are created. Non Godot classes (that don't derive godot::Object) are constructed as usual an can have parameters. memdelete is also changed for the same reason, as it needs to destroy the Godot object as well, and that automatically frees the bound extension instance. --- binding_generator.py | 19 +- include/godot_cpp/classes/ref.hpp | 2 +- include/godot_cpp/classes/wrapped.hpp | 263 ++++++++++++++----------- include/godot_cpp/core/memory.hpp | 22 ++- include/godot_cpp/core/method_bind.hpp | 2 +- 5 files changed, 179 insertions(+), 129 deletions(-) diff --git a/binding_generator.py b/binding_generator.py index 5d9f5247..b66a9561 100644 --- a/binding_generator.py +++ b/binding_generator.py @@ -889,6 +889,10 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us result.append("public:") + # Constructor override, since parent Wrapped has protected constructor. + result.append(f"\t{class_name}() = default;") + result.append("") + if "enums" in class_api: for enum_api in class_api["enums"]: result.append(f'\tenum {enum_api["name"]} {{') @@ -963,10 +967,6 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us "\tT *get_node(const NodePath &p_path) const { return Object::cast_to(get_node_internal(p_path)); }" ) - # Constructor. - result.append("") - result.append(f"\t{class_name}();") - result.append("") result.append("};") result.append("") @@ -1104,17 +1104,6 @@ def generate_engine_class_source(class_api, used_classes, fully_used_classes, us result.append(method_signature) result.append("") - # Constructor. - result.append(f"{class_name}::{class_name}() : {inherits}(godot::internal::empty_constructor()) {{") - result.append( - f'\tstatic GDNativeClassConstructor constructor = internal::interface->classdb_get_constructor("{class_name}");' - ) - result.append("\t_owner = (GodotObject *)constructor();") - result.append( - f"\tinternal::interface->object_set_instance_binding((GDNativeObjectPtr)_owner, internal::token, this, &{class_name}::___binding_callbacks);" - ) - result.append("}") - result.append("") result.append("} // namespace godot ") diff --git a/include/godot_cpp/classes/ref.hpp b/include/godot_cpp/classes/ref.hpp index 2140ef35..2ca5f835 100644 --- a/include/godot_cpp/classes/ref.hpp +++ b/include/godot_cpp/classes/ref.hpp @@ -199,7 +199,7 @@ public: } void instantiate() { - ref(memnew(T)); + ref(memnew(T())); } Ref() {} diff --git a/include/godot_cpp/classes/wrapped.hpp b/include/godot_cpp/classes/wrapped.hpp index 8402e9c7..2a0422ae 100644 --- a/include/godot_cpp/classes/wrapped.hpp +++ b/include/godot_cpp/classes/wrapped.hpp @@ -33,10 +33,8 @@ #include +#include namespace godot { -namespace internal { -struct empty_constructor {}; -} // namespace internal typedef void GodotObject; @@ -50,124 +48,169 @@ class Wrapped { protected: Wrapped() = default; - Wrapped(internal::empty_constructor empty) {} public: // Must be public but you should not touch this. GodotObject *_owner = nullptr; + + static Wrapped *_new() { + return nullptr; + } }; +namespace internal { + +template +struct Creator { + static T *_new() { return nullptr; } +}; + +template +struct Creator>::type> { + static T *_new() { return T::_new(); } +}; + +// template +// struct Creator { +// }; + +// template +// struct Creator, bool>> { +// static T *_new() { return T::_new(); } +// }; + +}; // namespace internal + } // namespace godot -#define GDCLASS(m_class, m_inherits) \ -private: \ - friend class ClassDB; \ - \ - using SelfType = m_class; \ - \ -protected: \ - static void (*_get_bind_methods())() { \ - return &m_class::_bind_methods; \ - } \ - \ - m_class(godot::GodotObject *owner) : m_inherits(godot::internal::empty_constructor()) { \ - _owner = owner; \ - } \ - \ - m_class(godot::internal::empty_constructor empty) : m_inherits(empty) {} \ - \ - template \ - static void register_virtuals() { \ - m_inherits::register_virtuals(); \ - } \ - \ -public: \ - static void initialize_class() { \ - static bool initialized = false; \ - if (initialized) { \ - return; \ - } \ - m_inherits::initialize_class(); \ - if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { \ - _bind_methods(); \ - m_inherits::register_virtuals(); \ - } \ - initialized = true; \ - } \ - \ - static const char *get_class_static() { \ - return #m_class; \ - } \ - \ - static const char *get_parent_class_static() { \ - return #m_inherits; \ - } \ - \ - static GDExtensionClassInstancePtr create(void *data) { \ - return (GDExtensionClassInstancePtr)godot::Memory::alloc_static(sizeof(m_class)); \ - } \ - \ - static void free(void *data, GDExtensionClassInstancePtr ptr) { \ - godot::memdelete(reinterpret_cast(ptr)); \ - } \ - \ - static void set_object_instance(GDExtensionClassInstancePtr p_instance, GDNativeObjectPtr p_object_instance) { \ - memnew_placement((void *)p_instance, m_class((godot::GodotObject *)p_object_instance)); \ - } \ - \ - static void *___binding_create_callback(void *p_token, void *p_instance) { \ - return memnew(m_class((godot::GodotObject *)p_instance)); \ - } \ - static void ___binding_free_callback(void *p_token, void *p_instance, void *p_binding) { \ - memdelete((m_class *)p_binding); \ - } \ - static GDNativeBool ___binding_reference_callback(void *p_token, void *p_instance, GDNativeBool p_reference) { \ - return true; \ - } \ - static constexpr GDNativeInstanceBindingCallbacks ___binding_callbacks = { \ - ___binding_create_callback, \ - ___binding_free_callback, \ - ___binding_reference_callback, \ - }; \ - \ +#ifdef DEBUG_ENABLED +#define CHECK_CLASS_CONSTRUCTOR(m_constructor, m_class) \ + if (unlikely(!m_constructor)) { \ + ERR_PRINT_ONCE("Constructor for class " #m_class "not found. Likely wasn't registered in ClassDB."); \ + return nullptr; \ + } else \ + ((void)0) +#else +#define CHECK_CLASS_CONSTRUCTOR(m_constructor, m_class) +#endif + +#define GDCLASS(m_class, m_inherits) \ +private: \ + friend class ClassDB; \ + \ + using SelfType = m_class; \ + \ +protected: \ + static void (*_get_bind_methods())() { \ + return &m_class::_bind_methods; \ + } \ + \ + template \ + static void register_virtuals() { \ + m_inherits::register_virtuals(); \ + } \ + \ +public: \ + static void initialize_class() { \ + static bool initialized = false; \ + if (initialized) { \ + return; \ + } \ + m_inherits::initialize_class(); \ + if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { \ + _bind_methods(); \ + m_inherits::register_virtuals(); \ + } \ + initialized = true; \ + } \ + \ + static const char *get_class_static() { \ + return #m_class; \ + } \ + \ + static const char *get_parent_class_static() { \ + return #m_inherits; \ + } \ + \ + static GDExtensionClassInstancePtr create(void *data) { \ + return reinterpret_cast(new ("") m_class); \ + } \ + \ + static void free(void *data, GDExtensionClassInstancePtr ptr) { \ + Memory::free_static(reinterpret_cast(ptr)); \ + } \ + \ + static void set_object_instance(GDExtensionClassInstancePtr p_instance, GDNativeObjectPtr p_object_instance) { \ + reinterpret_cast(p_instance)->_owner = reinterpret_cast(p_object_instance); \ + } \ + \ + static void *___binding_create_callback(void *p_token, void *p_instance) { \ + m_class *result = new ("") m_class; \ + result->_owner = reinterpret_cast(p_instance); \ + return result; \ + } \ + static void ___binding_free_callback(void *p_token, void *p_instance, void *p_binding) { \ + Memory::free_static(reinterpret_cast(p_binding)); \ + } \ + static GDNativeBool ___binding_reference_callback(void *p_token, void *p_instance, GDNativeBool p_reference) { \ + return true; \ + } \ + static constexpr GDNativeInstanceBindingCallbacks ___binding_callbacks = { \ + ___binding_create_callback, \ + ___binding_free_callback, \ + ___binding_reference_callback, \ + }; \ + \ + static m_class *_new() { \ + static GDNativeClassConstructor ___constructor = godot::internal::interface->classdb_get_constructor(#m_class); \ + CHECK_CLASS_CONSTRUCTOR(___constructor, m_class); \ + GDNativeObjectPtr obj = ___constructor(); \ + return reinterpret_cast(godot::internal::interface->object_get_instance_binding(obj, godot::internal::token, &m_class::___binding_callbacks)); \ + } \ + \ private: // Don't use this for your classes, use GDCLASS() instead. -#define GDNATIVE_CLASS(m_class, m_inherits) \ -protected: \ - static void (*_get_bind_methods())() { \ - return nullptr; \ - } \ - m_class(godot::internal::empty_constructor empty) : m_inherits(empty) {} \ - \ -public: \ - static void initialize_class() {} \ - \ - static const char *get_class_static() { \ - return #m_class; \ - } \ - \ - static const char *get_parent_class_static() { \ - return #m_inherits; \ - } \ - \ - static void *___binding_create_callback(void *p_token, void *p_instance) { \ - m_class *obj = memnew(m_class(godot::internal::empty_constructor())); \ - obj->_owner = (godot::GodotObject *)p_instance; \ - return obj; \ - } \ - static void ___binding_free_callback(void *p_token, void *p_instance, void *p_binding) { \ - memdelete((m_class *)p_binding); \ - } \ - static GDNativeBool ___binding_reference_callback(void *p_token, void *p_instance, GDNativeBool p_reference) { \ - return true; \ - } \ - static constexpr GDNativeInstanceBindingCallbacks ___binding_callbacks = { \ - ___binding_create_callback, \ - ___binding_free_callback, \ - ___binding_reference_callback, \ - }; \ - \ +#define GDNATIVE_CLASS(m_class, m_inherits) \ +protected: \ + static void (*_get_bind_methods())() { \ + return nullptr; \ + } \ + \ +public: \ + static void initialize_class() {} \ + \ + static const char *get_class_static() { \ + return #m_class; \ + } \ + \ + static const char *get_parent_class_static() { \ + return #m_inherits; \ + } \ + \ + static void *___binding_create_callback(void *p_token, void *p_instance) { \ + m_class *obj = new ("") m_class; \ + obj->_owner = (godot::GodotObject *)p_instance; \ + return obj; \ + } \ + static void ___binding_free_callback(void *p_token, void *p_instance, void *p_binding) { \ + Memory::free_static(reinterpret_cast(p_binding)); \ + } \ + static GDNativeBool ___binding_reference_callback(void *p_token, void *p_instance, GDNativeBool p_reference) { \ + return true; \ + } \ + static constexpr GDNativeInstanceBindingCallbacks ___binding_callbacks = { \ + ___binding_create_callback, \ + ___binding_free_callback, \ + ___binding_reference_callback, \ + }; \ + static m_class *_new() { \ + static GDNativeClassConstructor ___constructor = godot::internal::interface->classdb_get_constructor(#m_class); \ + CHECK_CLASS_CONSTRUCTOR(___constructor, m_class); \ + GDNativeObjectPtr obj = ___constructor(); \ + return reinterpret_cast(godot::internal::interface->object_get_instance_binding(obj, godot::internal::token, &m_class::___binding_callbacks)); \ + } \ + \ private: #endif // ! GODOT_CPP_WRAPPED_HPP diff --git a/include/godot_cpp/core/memory.hpp b/include/godot_cpp/core/memory.hpp index d95ca403..c493e892 100644 --- a/include/godot_cpp/core/memory.hpp +++ b/include/godot_cpp/core/memory.hpp @@ -36,6 +36,9 @@ #include #include +#include + +#include void *operator new(size_t p_size, const char *p_description); ///< operator new that takes a description and uses MemoryStaticPool void *operator new(size_t p_size, void *p_pointer, size_t check, const char *p_description); ///< operator new that takes a description and uses a pointer to the preallocated memory @@ -54,6 +57,8 @@ void operator delete(void *p_mem, void *p_pointer, size_t check, const char *p_d namespace godot { +class Wrapped; + class Memory { Memory(); @@ -63,11 +68,19 @@ public: static void free_static(void *p_ptr); }; -#define memnew(m_v) (new ("") m_v) +#define memnew(m_v) \ + ([&]() { \ + if constexpr (std::is_base_of::value) { \ + return godot::internal::Creator::_new(); \ + } else { \ + return new ("") m_v; \ + } \ + }()) + #define memnew_placement(m_placement, m_class) (new (m_placement, sizeof(m_class), "") m_class) template -void memdelete(T *p_class) { +void memdelete(T *p_class, typename std::enable_if>::type * = 0) { if (!__has_trivial_destructor(T)) { p_class->~T(); } @@ -75,6 +88,11 @@ void memdelete(T *p_class) { Memory::free_static(p_class); } +template , bool> = true> +void memdelete(T *p_class) { + godot::internal::interface->object_destroy(p_class->_owner); +} + #define memnew_arr(m_class, m_count) memnew_arr_template(m_count) template diff --git a/include/godot_cpp/core/method_bind.hpp b/include/godot_cpp/core/method_bind.hpp index 2c3e7b30..176a53c8 100644 --- a/include/godot_cpp/core/method_bind.hpp +++ b/include/godot_cpp/core/method_bind.hpp @@ -194,7 +194,7 @@ public: template MethodBind *create_vararg_method_bind(Variant (T::*p_method)(const Variant **, GDNativeInt, GDNativeCallError &), const MethodInfo &p_info, bool p_return_nil_is_variant) { - MethodBindVarArg *a = memnew((MethodBindVarArg)); + MethodBindVarArg *a = memnew(MethodBindVarArg()); a->set_method(p_method); a->set_method_info(p_info, p_return_nil_is_variant); a->set_instance_class(T::get_class_static());