Merge pull request #855 from Zylann/fix_issue854_virtual_methods

Fix deriving a custom class with virtual methods

binding_generator.py

@@ -1177,17 +1177,22 @@ def generate_engine_class_header(class_api, used_classes, fully_used_classes, us
             result.append(method_signature + ";")
 
     result.append("protected:")
-    result.append("\ttemplate <class T>")
+    # 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 <class T, class B>")
     result.append("\tstatic void register_virtuals() {")
     if class_name != "Object":
-        result.append(f"\t\t{inherits}::register_virtuals<T>();")
+        result.append(f"\t\t{inherits}::register_virtuals<T, B>();")
         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<decltype(&{class_name}::{method_name}),decltype(&T::{method_name})>) {{"
+                    # 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<decltype(&B::{method_name}),decltype(&T::{method_name})>) {{"
                 )
                 result.append(f"\t\t\tBIND_VIRTUAL_METHOD(T, {method_name});")
                 result.append("\t\t}")

include/godot_cpp/classes/wrapped.hpp

@@ -145,9 +145,9 @@ protected:
 		return (::godot::String(::godot::Wrapped::*)()) & m_class::_to_string;                                                                           \
 	}                                                                                                                                                    \
                                                                                                                                                          \
-	template <class T>                                                                                                                                   \
+	template <class T, class B>                                                                                                                          \
 	static void register_virtuals() {                                                                                                                    \
-		m_inherits::register_virtuals<T>();                                                                                                              \
+		m_inherits::register_virtuals<T, B>();                                                                                                           \
 	}                                                                                                                                                    \
                                                                                                                                                          \
 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_class>();                                                                                                    \
+			m_inherits::register_virtuals<m_class, m_inherits>();                                                                                        \
 		}                                                                                                                                                \
 		initialized = true;                                                                                                                              \
 	}                                                                                                                                                    \

include/godot_cpp/core/class_db.hpp

@@ -89,10 +89,12 @@ public:
 		std::unordered_map<std::string, GDNativeExtensionClassCallVirtual> virtual_methods;
 		std::set<std::string> property_names;
 		std::set<std::string> 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<std::string, ClassInfo> 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<std::string, ClassInfo>::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 = {

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<std::string, ClassInfo>::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;
+
+	// Find method in current class, or any of its parent classes (Godot classes not included)
+	while (type != nullptr) {
+		std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::const_iterator method_it = type->virtual_methods.find(p_name);
 
-	std::unordered_map<std::string, GDNativeExtensionClassCallVirtual>::iterator method_it = type.virtual_methods.find(p_name);
+		if (method_it != type->virtual_methods.end()) {
+			return method_it->second;
+		}
 
-	if (method_it == type.virtual_methods.end()) {
-		return nullptr;
+		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) {