From 5de2c059d1e793f37ed4e128694c29a15eed313d Mon Sep 17 00:00:00 2001 From: Marc Gilleron Date: Tue, 20 Sep 2022 00:29:39 +0100 Subject: [PATCH] Fix deriving a custom class with virtual methods --- binding_generator.py | 11 ++++++++--- include/godot_cpp/classes/wrapped.hpp | 6 +++--- include/godot_cpp/core/class_db.hpp | 10 +++++++--- src/core/class_db.cpp | 19 ++++++++++++++----- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/binding_generator.py b/binding_generator.py index 316bd7ba..04842f1c 100644 --- a/binding_generator.py +++ b/binding_generator.py @@ -1071,17 +1071,22 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us result.append(method_signature + ";") result.append("protected:") - result.append("\ttemplate ") + # T is the custom class we want to register (from which the call initiates, going up the inheritance chain), + # B is its base class (can be a custom class too, that's why we pass it). + result.append("\ttemplate ") result.append("\tstatic void register_virtuals() {") if class_name != "Object": - result.append(f"\t\t{inherits}::register_virtuals();") + result.append(f"\t\t{inherits}::register_virtuals();") if "methods" in class_api: for method in class_api["methods"]: if not method["is_virtual"]: continue method_name = escape_identifier(method["name"]) result.append( - f"\t\tif constexpr (!std::is_same_v) {{" + # If the method is different from the base class, it means T overrides it, so it needs to be bound. + # Note that with an `if constexpr`, the code inside the `if` will not even be compiled if the + # condition returns false (in such cases it can't compile due to ambiguity). + f"\t\tif constexpr (!std::is_same_v) {{" ) result.append(f"\t\t\tBIND_VIRTUAL_METHOD(T, {method_name});") result.append("\t\t}") diff --git a/include/godot_cpp/classes/wrapped.hpp b/include/godot_cpp/classes/wrapped.hpp index 62ec0d86..94f9f97a 100644 --- a/include/godot_cpp/classes/wrapped.hpp +++ b/include/godot_cpp/classes/wrapped.hpp @@ -145,9 +145,9 @@ protected: return (::godot::String(::godot::Wrapped::*)()) & m_class::_to_string; \ } \ \ - template \ + template \ static void register_virtuals() { \ - m_inherits::register_virtuals(); \ + m_inherits::register_virtuals(); \ } \ \ public: \ @@ -159,7 +159,7 @@ public: m_inherits::initialize_class(); \ if (m_class::_get_bind_methods() != m_inherits::_get_bind_methods()) { \ _bind_methods(); \ - m_inherits::register_virtuals(); \ + m_inherits::register_virtuals(); \ } \ initialized = true; \ } \ diff --git a/include/godot_cpp/core/class_db.hpp b/include/godot_cpp/core/class_db.hpp index fd8be1ec..89b4286d 100644 --- a/include/godot_cpp/core/class_db.hpp +++ b/include/godot_cpp/core/class_db.hpp @@ -89,10 +89,12 @@ public: std::unordered_map virtual_methods; std::set property_names; std::set constant_names; + // Pointer to the parent custom class, if any. Will be null if the parent class is a Godot class. ClassInfo *parent_ptr = nullptr; }; private: + // This may only contain custom classes, not Godot classes static std::unordered_map classes; static MethodBind *bind_methodfi(uint32_t p_flags, MethodBind *p_bind, const MethodDefinition &method_name, const void **p_defs, int p_defcount); @@ -158,10 +160,12 @@ void ClassDB::register_class() { cl.name = T::get_class_static(); cl.parent_name = T::get_parent_class_static(); cl.level = current_level; - classes[cl.name] = cl; - if (classes.find(cl.parent_name) != classes.end()) { - cl.parent_ptr = &classes[cl.parent_name]; + std::unordered_map::iterator parent_it = classes.find(cl.parent_name); + if (parent_it != classes.end()) { + // Assign parent if it is also a custom class + cl.parent_ptr = &parent_it->second; } + classes[cl.name] = cl; // Register this class with Godot GDNativeExtensionClassCreationInfo class_info = { diff --git a/src/core/class_db.cpp b/src/core/class_db.cpp index 93938158..4ed73f3d 100644 --- a/src/core/class_db.cpp +++ b/src/core/class_db.cpp @@ -277,20 +277,29 @@ void ClassDB::bind_integer_constant(const char *p_class_name, const char *p_enum } GDNativeExtensionClassCallVirtual ClassDB::get_virtual_func(void *p_userdata, const char *p_name) { + // This is called by Godot the first time it calls a virtual function, and it caches the result, per object instance. + // Because of this, it can happen from different threads at once. + // It should be ok not using any mutex as long as we only READ data. + const char *class_name = (const char *)p_userdata; std::unordered_map::iterator type_it = classes.find(class_name); ERR_FAIL_COND_V_MSG(type_it == classes.end(), nullptr, "Class doesn't exist."); - ClassInfo &type = type_it->second; + const ClassInfo *type = &type_it->second; - std::unordered_map::iterator method_it = type.virtual_methods.find(p_name); + // Find method in current class, or any of its parent classes (Godot classes not included) + while (type != nullptr) { + std::unordered_map::const_iterator method_it = type->virtual_methods.find(p_name); - if (method_it == type.virtual_methods.end()) { - return nullptr; + if (method_it != type->virtual_methods.end()) { + return method_it->second; + } + + type = type->parent_ptr; } - return method_it->second; + return nullptr; } void ClassDB::bind_virtual_method(const char *p_class, const char *p_method, GDNativeExtensionClassCallVirtual p_call) {