From 5f6082a96b4a7b3f91f2b7d8d6dbc9b0a60d8650 Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Sat, 20 May 2023 13:28:40 +0200 Subject: [PATCH] C#: Generate and use compat methods - Implements `ClassDB::get_method_list_with_compatibility` to retrieve all methods from a class including compat methods. - C# bindings generator now also generates compat methods. - All generated C# methods now use `ClassDB::get_method_with_compatibility`. --- core/object/class_db.cpp | 64 +++++++++- core/object/class_db.h | 1 + modules/mono/editor/bindings_generator.cpp | 110 ++++++++++++++++-- modules/mono/editor/bindings_generator.h | 14 +++ .../GodotSharp/Core/GodotObject.base.cs | 12 ++ .../Core/NativeInterop/NativeFuncs.cs | 3 + modules/mono/glue/runtime_interop.cpp | 5 + 7 files changed, 197 insertions(+), 12 deletions(-) diff --git a/core/object/class_db.cpp b/core/object/class_db.cpp index c8c50fb957b..42c8c32ca56 100644 --- a/core/object/class_db.cpp +++ b/core/object/class_db.cpp @@ -463,7 +463,6 @@ void ClassDB::get_method_list(const StringName &p_class, List *p_met } #ifdef DEBUG_METHODS_ENABLED - for (const MethodInfo &E : type->virtual_methods) { p_methods->push_back(E); } @@ -478,15 +477,12 @@ void ClassDB::get_method_list(const StringName &p_class, List *p_met p_methods->push_back(minfo); } - #else - for (KeyValue &E : type->method_map) { MethodBind *m = E.value; MethodInfo minfo = info_from_bind(m); p_methods->push_back(minfo); } - #endif if (p_no_inheritance) { @@ -497,6 +493,66 @@ void ClassDB::get_method_list(const StringName &p_class, List *p_met } } +void ClassDB::get_method_list_with_compatibility(const StringName &p_class, List> *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 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 pair(minfo, method->get_hash()); + p_methods->push_back(pair); + } +#else + for (KeyValue &E : type->method_map) { + MethodBind *method = E.value; + MethodInfo minfo = info_from_bind(method); + + Pair pair(minfo, method->get_hash()); + p_methods->push_back(pair); + } +#endif + + for (const KeyValue> &E : type->method_map_compatibility) { + LocalVector compat = E.value; + for (MethodBind *method : compat) { + MethodInfo minfo = info_from_bind(method); + + Pair 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; diff --git a/core/object/class_db.h b/core/object/class_db.h index 3aae3b452e6..d5ccf4f883b 100644 --- a/core/object/class_db.h +++ b/core/object/class_db.h @@ -380,6 +380,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 *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> *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); diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 342c94cbd90..1b5880c4cbf 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/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" @@ -1354,6 +1355,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"); @@ -1827,7 +1829,13 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str } output << "\n" << INDENT1 "{\n"; + HashMap 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 "/// \n" << INDENT2 "/// Cached name for the '" << imethod.cname << "' method.\n" << INDENT2 "/// \n" @@ -2080,7 +2088,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 { @@ -2168,8 +2176,8 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf p_output << "GodotObject."; } - p_output << ICALL_CLASSDB_GET_METHOD "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName." - << p_imethod.proxy_name + p_output << ICALL_CLASSDB_GET_METHOD_WITH_COMPATIBILITY "(" BINDINGS_NATIVE_NAME_FIELD ", MethodName." + << p_imethod.proxy_name << ", " << itos(p_imethod.hash) << "ul" << ");\n"; } @@ -2208,6 +2216,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 "); @@ -2924,6 +2936,12 @@ bool method_has_ptr_parameter(MethodInfo p_method_info) { return false; } +struct SortMethodWithHashes { + _FORCE_INLINE_ bool operator()(const Pair &p_a, const Pair &p_b) const { + return p_a.first < p_b.first; + } +}; + bool BindingsGenerator::_populate_object_type_interfaces() { obj_types.clear(); @@ -3051,11 +3069,15 @@ bool BindingsGenerator::_populate_object_type_interfaces() { List virtual_method_list; ClassDB::get_virtual_methods(type_cname, &virtual_method_list, true); - List method_list; - ClassDB::get_method_list(type_cname, &method_list, true); - method_list.sort(); + List> method_list_with_hashes; + ClassDB::get_method_list_with_compatibility(type_cname, &method_list_with_hashes, true); + method_list_with_hashes.sort_custom_inplace(); + + List compat_methods; + for (const Pair &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()) { @@ -3077,6 +3099,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; @@ -3089,7 +3112,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(); @@ -3210,6 +3243,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) { @@ -3224,6 +3265,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 &signal_map = class_info->signal_map; @@ -4007,6 +4057,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' 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 diff --git a/modules/mono/editor/bindings_generator.h b/modules/mono/editor/bindings_generator.h index d4c7a59e74d..55c36f8f996 100644 --- a/modules/mono/editor/bindings_generator.h +++ b/modules/mono/editor/bindings_generator.h @@ -130,6 +130,11 @@ class BindingsGenerator { */ String proxy_name; + /** + * Hash of the ClassDB method + */ + uint64_t hash = 0; + /** * [TypeInterface::name] of the return type */ @@ -165,6 +170,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 arguments; const DocData::MethodDoc *method_doc = nullptr; @@ -783,6 +794,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); diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs index c6337e56efd..43598ca84d1 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs +++ b/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 ClassDB_get_constructor(StringName type) { // for some reason the '??' operator doesn't support 'delegate*' diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs index d42ee156577..d181bf2c0fe 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs +++ b/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 godotsharp_get_class_constructor( in godot_string_name p_classname); diff --git a/modules/mono/glue/runtime_interop.cpp b/modules/mono/glue/runtime_interop.cpp index 24a9d4030ae..3518507f8c7 100644 --- a/modules/mono/glue/runtime_interop.cpp +++ b/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,