Merge pull request #80527 from raulsntos/dotnet/generate-compat-methods-from-classdb

C#: Generate and use compat methods

core/object/class_db.cpp

@@ -480,7 +480,6 @@ void ClassDB::get_method_list(const StringName &p_class, List<MethodInfo> *p_met
 		}
 
 #ifdef DEBUG_METHODS_ENABLED
-
 		for (const MethodInfo &E : type->virtual_methods) {
 			p_methods->push_back(E);
 		}
@@ -495,15 +494,12 @@ void ClassDB::get_method_list(const StringName &p_class, List<MethodInfo> *p_met
 
 			p_methods->push_back(minfo);
 		}
-
 #else
-
 		for (KeyValue<StringName, MethodBind *> &E : type->method_map) {
 			MethodBind *m = E.value;
 			MethodInfo minfo = info_from_bind(m);
 			p_methods->push_back(minfo);
 		}
-
 #endif
 
 		if (p_no_inheritance) {
@@ -514,6 +510,66 @@ void ClassDB::get_method_list(const StringName &p_class, List<MethodInfo> *p_met
 	}
 }
 
+void ClassDB::get_method_list_with_compatibility(const StringName &p_class, List<Pair<MethodInfo, uint32_t>> *p_methods, bool p_no_inheritance, bool p_exclude_from_properties) {
+	OBJTYPE_RLOCK;
+
+	ClassInfo *type = classes.getptr(p_class);
+
+	while (type) {
+		if (type->disabled) {
+			if (p_no_inheritance) {
+				break;
+			}
+
+			type = type->inherits_ptr;
+			continue;
+		}
+
+#ifdef DEBUG_METHODS_ENABLED
+		for (const MethodInfo &E : type->virtual_methods) {
+			Pair<MethodInfo, uint32_t> pair(E, 0);
+			p_methods->push_back(pair);
+		}
+
+		for (const StringName &E : type->method_order) {
+			if (p_exclude_from_properties && type->methods_in_properties.has(E)) {
+				continue;
+			}
+
+			MethodBind *method = type->method_map.get(E);
+			MethodInfo minfo = info_from_bind(method);
+
+			Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
+			p_methods->push_back(pair);
+		}
+#else
+		for (KeyValue<StringName, MethodBind *> &E : type->method_map) {
+			MethodBind *method = E.value;
+			MethodInfo minfo = info_from_bind(method);
+
+			Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
+			p_methods->push_back(pair);
+		}
+#endif
+
+		for (const KeyValue<StringName, LocalVector<MethodBind *, unsigned int, false, false>> &E : type->method_map_compatibility) {
+			LocalVector<MethodBind *> compat = E.value;
+			for (MethodBind *method : compat) {
+				MethodInfo minfo = info_from_bind(method);
+
+				Pair<MethodInfo, uint32_t> pair(minfo, method->get_hash());
+				p_methods->push_back(pair);
+			}
+		}
+
+		if (p_no_inheritance) {
+			break;
+		}
+
+		type = type->inherits_ptr;
+	}
+}
+
 bool ClassDB::get_method_info(const StringName &p_class, const StringName &p_method, MethodInfo *r_info, bool p_no_inheritance, bool p_exclude_from_properties) {
 	OBJTYPE_RLOCK;
 

core/object/class_db.h

@@ -399,6 +399,7 @@ public:
 	static void set_method_flags(const StringName &p_class, const StringName &p_method, int p_flags);
 
 	static void get_method_list(const StringName &p_class, List<MethodInfo> *p_methods, bool p_no_inheritance = false, bool p_exclude_from_properties = false);
+	static void get_method_list_with_compatibility(const StringName &p_class, List<Pair<MethodInfo, uint32_t>> *p_methods_with_hash, bool p_no_inheritance = false, bool p_exclude_from_properties = false);
 	static bool get_method_info(const StringName &p_class, const StringName &p_method, MethodInfo *r_info, bool p_no_inheritance = false, bool p_exclude_from_properties = false);
 	static MethodBind *get_method(const StringName &p_class, const StringName &p_name);
 	static MethodBind *get_method_with_compatibility(const StringName &p_class, const StringName &p_name, uint64_t p_hash, bool *r_method_exists = nullptr, bool *r_is_deprecated = nullptr);

modules/mono/editor/bindings_generator.cpp

@@ -94,6 +94,7 @@ StringBuilder &operator<<(StringBuilder &r_sb, const char *p_cstring) {
 
 #define ICALL_PREFIX "godot_icall_"
 #define ICALL_CLASSDB_GET_METHOD "ClassDB_get_method"
+#define ICALL_CLASSDB_GET_METHOD_WITH_COMPATIBILITY "ClassDB_get_method_with_compatibility"
 #define ICALL_CLASSDB_GET_CONSTRUCTOR "ClassDB_get_constructor"
 
 #define C_LOCAL_RET "ret"
@@ -1398,6 +1399,7 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
 	output.append("namespace " BINDINGS_NAMESPACE ";\n\n");
 
 	output.append("using System;\n"); // IntPtr
+	output.append("using System.ComponentModel;\n"); // EditorBrowsable
 	output.append("using System.Diagnostics;\n"); // DebuggerBrowsable
 	output.append("using Godot.NativeInterop;\n");
 
@@ -1865,7 +1867,13 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str
 	}
 	output << "\n"
 		   << INDENT1 "{\n";
+	HashMap<String, StringName> method_names;
 	for (const MethodInterface &imethod : itype.methods) {
+		if (method_names.has(imethod.proxy_name)) {
+			ERR_FAIL_COND_V_MSG(method_names[imethod.proxy_name] != imethod.cname, ERR_BUG, "Method name '" + imethod.proxy_name + "' already exists with a different value.");
+			continue;
+		}
+		method_names[imethod.proxy_name] = imethod.cname;
 		output << INDENT2 "/// <summary>\n"
 			   << INDENT2 "/// Cached name for the '" << imethod.cname << "' method.\n"
 			   << INDENT2 "/// </summary>\n"
@@ -2114,7 +2122,7 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 
 			arguments_sig += iarg.name;
 
-			if (iarg.default_argument.size()) {
+			if (!p_imethod.is_compat && iarg.default_argument.size()) {
 				if (iarg.def_param_mode != ArgumentInterface::CONSTANT) {
 					arguments_sig += " = null";
 				} else {
@@ -2202,8 +2210,8 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 				p_output << "GodotObject.";
 			}
 
-			p_output << ICALL_CLASSDB_GET_METHOD "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName."
+			p_output << ICALL_CLASSDB_GET_METHOD_WITH_COMPATIBILITY "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName."
-					 << p_imethod.proxy_name
+					 << p_imethod.proxy_name << ", " << itos(p_imethod.hash) << "ul"
 					 << ");\n";
 		}
 
@@ -2242,6 +2250,10 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf
 			p_output.append("\")]");
 		}
 
+		if (p_imethod.is_compat) {
+			p_output.append(MEMBER_BEGIN "[EditorBrowsable(EditorBrowsableState.Never)]");
+		}
+
 		p_output.append(MEMBER_BEGIN);
 		p_output.append(p_imethod.is_internal ? "internal " : "public ");
 
@@ -2958,6 +2970,12 @@ bool method_has_ptr_parameter(MethodInfo p_method_info) {
 	return false;
 }
 
+struct SortMethodWithHashes {
+	_FORCE_INLINE_ bool operator()(const Pair<MethodInfo, uint32_t> &p_a, const Pair<MethodInfo, uint32_t> &p_b) const {
+		return p_a.first < p_b.first;
+	}
+};
+
 bool BindingsGenerator::_populate_object_type_interfaces() {
 	obj_types.clear();
 
@@ -3085,11 +3103,15 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 		List<MethodInfo> virtual_method_list;
 		ClassDB::get_virtual_methods(type_cname, &virtual_method_list, true);
 
-		List<MethodInfo> method_list;
+		List<Pair<MethodInfo, uint32_t>> method_list_with_hashes;
-		ClassDB::get_method_list(type_cname, &method_list, true);
+		ClassDB::get_method_list_with_compatibility(type_cname, &method_list_with_hashes, true);
-		method_list.sort();
+		method_list_with_hashes.sort_custom_inplace<SortMethodWithHashes>();
+
+		List<MethodInterface> compat_methods;
+		for (const Pair<MethodInfo, uint32_t> &E : method_list_with_hashes) {
+			const MethodInfo &method_info = E.first;
+			const uint32_t hash = E.second;
 
-		for (const MethodInfo &method_info : method_list) {
 			int argc = method_info.arguments.size();
 
 			if (method_info.name.is_empty()) {
@@ -3111,6 +3133,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 			MethodInterface imethod;
 			imethod.name = method_info.name;
 			imethod.cname = cname;
+			imethod.hash = hash;
 
 			if (method_info.flags & METHOD_FLAG_STATIC) {
 				imethod.is_static = true;
@@ -3123,7 +3146,17 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 
 			PropertyInfo return_info = method_info.return_val;
 
-			MethodBind *m = imethod.is_virtual ? nullptr : ClassDB::get_method(type_cname, method_info.name);
+			MethodBind *m = nullptr;
+
+			if (!imethod.is_virtual) {
+				bool method_exists = false;
+				m = ClassDB::get_method_with_compatibility(type_cname, method_info.name, hash, &method_exists, &imethod.is_compat);
+
+				if (unlikely(!method_exists)) {
+					ERR_FAIL_COND_V_MSG(!virtual_method_list.find(method_info), false,
+							"Missing MethodBind for non-virtual method: '" + itype.name + "." + imethod.name + "'.");
+				}
+			}
 
 			imethod.is_vararg = m && m->is_vararg();
 
@@ -3244,6 +3277,14 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 			ERR_FAIL_COND_V_MSG(itype.find_property_by_name(imethod.cname), false,
 					"Method name conflicts with property: '" + itype.name + "." + imethod.name + "'.");
 
+			// Compat methods aren't added to the type yet, they need to be checked for conflicts
+			// after all the non-compat methods have been added. The compat methods are added in
+			// reverse so the most recently added ones take precedence over older compat methods.
+			if (imethod.is_compat) {
+				compat_methods.push_front(imethod);
+				continue;
+			}
+
 			// Methods starting with an underscore are ignored unless they're used as a property setter or getter
 			if (!imethod.is_virtual && imethod.name[0] == '_') {
 				for (const PropertyInterface &iprop : itype.properties) {
@@ -3258,6 +3299,15 @@ bool BindingsGenerator::_populate_object_type_interfaces() {
 			}
 		}
 
+		// Add compat methods that don't conflict with other methods in the type.
+		for (const MethodInterface &imethod : compat_methods) {
+			if (_method_has_conflicting_signature(imethod, itype)) {
+				WARN_PRINT("Method '" + imethod.name + "' conflicts with an already existing method in type '" + itype.name + "' and has been ignored.");
+				continue;
+			}
+			itype.methods.push_back(imethod);
+		}
+
 		// Populate signals
 
 		const HashMap<StringName, MethodInfo> &signal_map = class_info->signal_map;
@@ -4039,6 +4089,50 @@ void BindingsGenerator::_populate_global_constants() {
 	}
 }
 
+bool BindingsGenerator::_method_has_conflicting_signature(const MethodInterface &p_imethod, const TypeInterface &p_itype) {
+	// Compare p_imethod with all the methods already registered in p_itype.
+	for (const MethodInterface &method : p_itype.methods) {
+		if (method.proxy_name == p_imethod.proxy_name) {
+			if (_method_has_conflicting_signature(p_imethod, method)) {
+				return true;
+			}
+		}
+	}
+
+	return false;
+}
+
+bool BindingsGenerator::_method_has_conflicting_signature(const MethodInterface &p_imethod_left, const MethodInterface &p_imethod_right) {
+	// Check if a method already exists in p_itype with a method signature that would conflict with p_imethod.
+	// The return type is ignored because only changing the return type is not enough to avoid conflicts.
+	// The const keyword is also ignored since it doesn't generate different C# code.
+
+	if (p_imethod_left.arguments.size() != p_imethod_right.arguments.size()) {
+		// Different argument count, so no conflict.
+		return false;
+	}
+
+	for (int i = 0; i < p_imethod_left.arguments.size(); i++) {
+		const ArgumentInterface &iarg_left = p_imethod_left.arguments[i];
+		const ArgumentInterface &iarg_right = p_imethod_right.arguments[i];
+
+		if (iarg_left.type.cname != iarg_right.type.cname) {
+			// Different types for arguments in the same position, so no conflict.
+			return false;
+		}
+
+		if (iarg_left.def_param_mode != iarg_right.def_param_mode) {
+			// If the argument is a value type and nullable, it will be 'Nullable<T>' instead of 'T'
+			// and will not create a conflict.
+			if (iarg_left.def_param_mode == ArgumentInterface::NULLABLE_VAL || iarg_right.def_param_mode == ArgumentInterface::NULLABLE_VAL) {
+				return false;
+			}
+		}
+	}
+
+	return true;
+}
+
 void BindingsGenerator::_initialize_blacklisted_methods() {
 	blacklisted_methods["Object"].push_back("to_string"); // there is already ToString
 	blacklisted_methods["Object"].push_back("_to_string"); // override ToString instead

modules/mono/editor/bindings_generator.h

@@ -133,6 +133,11 @@ class BindingsGenerator {
 		 */
 		String proxy_name;
 
+		/**
+		 * Hash of the ClassDB method
+		 */
+		uint64_t hash = 0;
+
 		/**
 		 * [TypeInterface::name] of the return type
 		 */
@@ -168,6 +173,12 @@ class BindingsGenerator {
 		 */
 		bool is_internal = false;
 
+		/**
+		 * Determines if the method is a compatibility method added to avoid breaking binary compatibility.
+		 * These methods will be generated but hidden and are considered deprecated.
+		 */
+		bool is_compat = false;
+
 		List<ArgumentInterface> arguments;
 
 		const DocData::MethodDoc *method_doc = nullptr;
@@ -787,6 +798,9 @@ class BindingsGenerator {
 
 	void _populate_global_constants();
 
+	bool _method_has_conflicting_signature(const MethodInterface &p_imethod, const TypeInterface &p_itype);
+	bool _method_has_conflicting_signature(const MethodInterface &p_imethod_left, const MethodInterface &p_imethod_right);
+
 	Error _generate_cs_type(const TypeInterface &itype, const String &p_output_file);
 
 	Error _generate_cs_property(const TypeInterface &p_itype, const PropertyInterface &p_iprop, StringBuilder &p_output);

modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs

@@ -247,6 +247,18 @@ namespace Godot
             return methodBind;
         }
 
+        internal static IntPtr ClassDB_get_method_with_compatibility(StringName type, StringName method, ulong hash)
+        {
+            var typeSelf = (godot_string_name)type.NativeValue;
+            var methodSelf = (godot_string_name)method.NativeValue;
+            IntPtr methodBind = NativeFuncs.godotsharp_method_bind_get_method_with_compatibility(typeSelf, methodSelf, hash);
+
+            if (methodBind == IntPtr.Zero)
+                throw new NativeMethodBindNotFoundException(type + "." + method);
+
+            return methodBind;
+        }
+
         internal static unsafe delegate* unmanaged<IntPtr> ClassDB_get_constructor(StringName type)
         {
             // for some reason the '??' operator doesn't support 'delegate*'

modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs

@@ -42,6 +42,9 @@ namespace Godot.NativeInterop
         public static partial IntPtr godotsharp_method_bind_get_method(in godot_string_name p_classname,
             in godot_string_name p_methodname);
 
+        public static partial IntPtr godotsharp_method_bind_get_method_with_compatibility(
+            in godot_string_name p_classname, in godot_string_name p_methodname, ulong p_hash);
+
         public static partial delegate* unmanaged<IntPtr> godotsharp_get_class_constructor(
             in godot_string_name p_classname);
 

modules/mono/glue/runtime_interop.cpp

@@ -68,6 +68,10 @@ MethodBind *godotsharp_method_bind_get_method(const StringName *p_classname, con
 	return ClassDB::get_method(*p_classname, *p_methodname);
 }
 
+MethodBind *godotsharp_method_bind_get_method_with_compatibility(const StringName *p_classname, const StringName *p_methodname, uint64_t p_hash) {
+	return ClassDB::get_method_with_compatibility(*p_classname, *p_methodname, p_hash);
+}
+
 godotsharp_class_creation_func godotsharp_get_class_constructor(const StringName *p_classname) {
 	ClassDB::ClassInfo *class_info = ClassDB::classes.getptr(*p_classname);
 	if (class_info) {
@@ -1416,6 +1420,7 @@ void godotsharp_object_to_string(Object *p_ptr, godot_string *r_str) {
 static const void *unmanaged_callbacks[]{
 	(void *)godotsharp_dotnet_module_is_initialized,
 	(void *)godotsharp_method_bind_get_method,
+	(void *)godotsharp_method_bind_get_method_with_compatibility,
 	(void *)godotsharp_get_class_constructor,
 	(void *)godotsharp_engine_get_singleton,
 	(void *)godotsharp_stack_info_vector_resize,