From 6b713b1682c453add138555092e3fa0a7ee4261c Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Fri, 14 Jul 2023 16:05:48 +0200 Subject: [PATCH 1/2] C#: Generate instance types for singletons --- modules/mono/csharp_script.cpp | 2 +- modules/mono/editor/bindings_generator.cpp | 83 ++++++++++++++----- modules/mono/editor/bindings_generator.h | 2 + .../Core/Bridge/ScriptManagerBridge.cs | 9 +- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index f592533a5ad7..2971706c75f7 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -72,7 +72,7 @@ // Types that will be skipped over (in favor of their base types) when setting up instance bindings. // This must be a superset of `ignored_types` in bindings_generator.cpp. -const Vector ignored_types = { "PhysicsServer2DExtension", "PhysicsServer3DExtension" }; +const Vector ignored_types = {}; #ifdef TOOLS_ENABLED static bool _create_project_solution_if_needed() { diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index bed93cd69e22..43b4962641cc 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -82,6 +82,7 @@ StringBuilder &operator<<(StringBuilder &r_sb, const char *p_cstring) { #define CS_STATIC_METHOD_GETINSTANCE "GetPtr" #define CS_METHOD_CALL "Call" #define CS_PROPERTY_SINGLETON "Singleton" +#define CS_SINGLETON_INSTANCE_SUFFIX "Instance" #define CS_METHOD_INVOKE_GODOT_CLASS_METHOD "InvokeGodotClassMethod" #define CS_METHOD_HAS_GODOT_CLASS_METHOD "HasGodotClassMethod" #define CS_METHOD_HAS_GODOT_CLASS_SIGNAL "HasGodotClassSignal" @@ -116,7 +117,7 @@ StringBuilder &operator<<(StringBuilder &r_sb, const char *p_cstring) { // Types that will be ignored by the generator and won't be available in C#. // This must be kept in sync with `ignored_types` in csharp_script.cpp -const Vector ignored_types = { "PhysicsServer2DExtension", "PhysicsServer3DExtension" }; +const Vector ignored_types = {}; void BindingsGenerator::TypeInterface::postsetup_enum_type(BindingsGenerator::TypeInterface &r_enum_itype) { // C interface for enums is the same as that of 'uint32_t'. Remember to apply @@ -653,6 +654,11 @@ void BindingsGenerator::_append_xml_constant(StringBuilder &p_xml_output, const _append_xml_undeclared(p_xml_output, p_link_target); } else { // Try to find the constant in the current class + if (p_target_itype->is_singleton_instance) { + // Constants and enums are declared in the static singleton class. + p_target_itype = &obj_types[p_target_itype->cname]; + } + const ConstantInterface *target_iconst = find_constant_by_name(p_target_cname, p_target_itype->constants); if (target_iconst) { @@ -1403,8 +1409,13 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str if (is_derived_type && !itype.is_singleton) { if (obj_types.has(itype.base_name)) { + TypeInterface base_type = obj_types[itype.base_name]; output.append(" : "); - output.append(obj_types[itype.base_name].proxy_name); + output.append(base_type.proxy_name); + if (base_type.is_singleton) { + // If the type is a singleton, use the instance type. + output.append(CS_SINGLETON_INSTANCE_SUFFIX); + } } else { ERR_PRINT("Base type '" + itype.base_name.operator String() + "' does not exist, for class '" + itype.name + "'."); return ERR_INVALID_DATA; @@ -1506,14 +1517,16 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str if (itype.is_singleton) { // Add the type name and the singleton pointer as static fields + StringName instance_name = itype.name + CS_SINGLETON_INSTANCE_SUFFIX; + String instance_type_name = obj_types.has(instance_name) + ? obj_types[instance_name].proxy_name + : "GodotObject"; - output.append(MEMBER_BEGIN "private static GodotObject singleton;\n"); + output.append(MEMBER_BEGIN "private static " + instance_type_name + " singleton;\n"); - output << MEMBER_BEGIN "public static GodotObject " CS_PROPERTY_SINGLETON "\n" INDENT1 "{\n" - << INDENT2 "get\n" INDENT2 "{\n" INDENT3 "if (singleton == null)\n" - << INDENT4 "singleton = " C_METHOD_ENGINE_GET_SINGLETON "(\"" - << itype.name - << "\");\n" INDENT3 "return singleton;\n" INDENT2 "}\n" INDENT1 "}\n"; + output << MEMBER_BEGIN "public static " + instance_type_name + " " CS_PROPERTY_SINGLETON " =>\n" + << INDENT2 "singleton \?\?= (" + instance_type_name + ")" + << C_METHOD_ENGINE_GET_SINGLETON "(\"" << itype.name << "\");\n"; output.append(MEMBER_BEGIN "private static readonly StringName " BINDINGS_NATIVE_NAME_FIELD " = \""); output.append(itype.name); @@ -1894,7 +1907,7 @@ Error BindingsGenerator::_generate_cs_property(const BindingsGenerator::TypeInte const TypeReference &proptype_name = getter ? getter->return_type : setter->arguments.back()->get().type; - const TypeInterface *prop_itype = _get_type_or_null(proptype_name); + const TypeInterface *prop_itype = _get_type_or_singleton_or_null(proptype_name); ERR_FAIL_NULL_V(prop_itype, ERR_BUG); // Property type not found ERR_FAIL_COND_V_MSG(prop_itype->is_singleton, ERR_BUG, @@ -1983,7 +1996,7 @@ Error BindingsGenerator::_generate_cs_property(const BindingsGenerator::TypeInte } Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterface &p_itype, const BindingsGenerator::MethodInterface &p_imethod, int &p_method_bind_count, StringBuilder &p_output) { - const TypeInterface *return_type = _get_type_or_null(p_imethod.return_type); + const TypeInterface *return_type = _get_type_or_singleton_or_null(p_imethod.return_type); ERR_FAIL_NULL_V(return_type, ERR_BUG); // Return type not found ERR_FAIL_COND_V_MSG(return_type->is_singleton, ERR_BUG, @@ -2004,12 +2017,17 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf String icall_params = method_bind_field; if (!p_imethod.is_static) { + String self_reference = "this"; + if (p_itype.is_singleton) { + self_reference = CS_PROPERTY_SINGLETON; + } + if (p_itype.cs_in.size()) { - cs_in_statements << sformat(p_itype.cs_in, p_itype.c_type, "this", + cs_in_statements << sformat(p_itype.cs_in, p_itype.c_type, self_reference, String(), String(), String(), INDENT2); } - icall_params += ", " + sformat(p_itype.cs_in_expr, "this"); + icall_params += ", " + sformat(p_itype.cs_in_expr, self_reference); } StringBuilder default_args_doc; @@ -2017,7 +2035,7 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf // Retrieve information from the arguments const ArgumentInterface &first = p_imethod.arguments.front()->get(); for (const ArgumentInterface &iarg : p_imethod.arguments) { - const TypeInterface *arg_type = _get_type_or_null(iarg.type); + const TypeInterface *arg_type = _get_type_or_singleton_or_null(iarg.type); ERR_FAIL_NULL_V(arg_type, ERR_BUG); // Argument type not found ERR_FAIL_COND_V_MSG(arg_type->is_singleton, ERR_BUG, @@ -2275,7 +2293,7 @@ Error BindingsGenerator::_generate_cs_signal(const BindingsGenerator::TypeInterf // Retrieve information from the arguments const ArgumentInterface &first = p_isignal.arguments.front()->get(); for (const ArgumentInterface &iarg : p_isignal.arguments) { - const TypeInterface *arg_type = _get_type_or_null(iarg.type); + const TypeInterface *arg_type = _get_type_or_singleton_or_null(iarg.type); ERR_FAIL_NULL_V(arg_type, ERR_BUG); // Argument type not found ERR_FAIL_COND_V_MSG(arg_type->is_singleton, ERR_BUG, @@ -2702,6 +2720,20 @@ const BindingsGenerator::TypeInterface *BindingsGenerator::_get_type_or_null(con return nullptr; } +const BindingsGenerator::TypeInterface *BindingsGenerator::_get_type_or_singleton_or_null(const TypeReference &p_typeref) { + const TypeInterface *itype = _get_type_or_null(p_typeref); + if (itype == nullptr) { + return nullptr; + } + + if (itype->is_singleton) { + StringName instance_type_name = itype->name + CS_SINGLETON_INSTANCE_SUFFIX; + itype = &obj_types.find(instance_type_name)->value; + } + + return itype; +} + const String BindingsGenerator::_get_generic_type_parameters(const TypeInterface &p_itype, const List &p_generic_type_parameters) { if (p_generic_type_parameters.is_empty()) { return ""; @@ -2715,8 +2747,8 @@ const String BindingsGenerator::_get_generic_type_parameters(const TypeInterface int i = 0; String params = "<"; for (const TypeReference ¶m_type : p_generic_type_parameters) { - const TypeInterface *param_itype = _get_type_or_null(param_type); - ERR_FAIL_NULL_V(param_itype, ""); + const TypeInterface *param_itype = _get_type_or_singleton_or_null(param_type); + ERR_FAIL_NULL_V(param_itype, ""); // Parameter type not found ERR_FAIL_COND_V_MSG(param_itype->is_singleton, "", "Generic type parameter is a singleton: '" + param_itype->name + "'."); @@ -2938,11 +2970,7 @@ bool BindingsGenerator::_populate_object_type_interfaces() { itype.cs_type = itype.proxy_name; - if (itype.is_singleton) { - itype.cs_in_expr = "GodotObject." CS_STATIC_METHOD_GETINSTANCE "(" CS_PROPERTY_SINGLETON ")"; - } else { - itype.cs_in_expr = "GodotObject." CS_STATIC_METHOD_GETINSTANCE "(%0)"; - } + itype.cs_in_expr = "GodotObject." CS_STATIC_METHOD_GETINSTANCE "(%0)"; itype.cs_out = "%5return (%2)%0(%1);"; @@ -3346,6 +3374,19 @@ bool BindingsGenerator::_populate_object_type_interfaces() { obj_types.insert(itype.cname, itype); + if (itype.is_singleton) { + // Add singleton instance type. + itype.proxy_name += CS_SINGLETON_INSTANCE_SUFFIX; + itype.is_singleton = false; + itype.is_singleton_instance = true; + + // Remove constants and enums, those will remain in the static class. + itype.constants.clear(); + itype.enums.clear(); + + obj_types.insert(itype.name + CS_SINGLETON_INSTANCE_SUFFIX, itype); + } + class_list.pop_front(); } diff --git a/modules/mono/editor/bindings_generator.h b/modules/mono/editor/bindings_generator.h index 38347a518152..80b0ed0b7df0 100644 --- a/modules/mono/editor/bindings_generator.h +++ b/modules/mono/editor/bindings_generator.h @@ -227,6 +227,7 @@ class BindingsGenerator { bool is_enum = false; bool is_object_type = false; bool is_singleton = false; + bool is_singleton_instance = false; bool is_ref_counted = false; /** @@ -756,6 +757,7 @@ class BindingsGenerator { Error _populate_method_icalls_table(const TypeInterface &p_itype); const TypeInterface *_get_type_or_null(const TypeReference &p_typeref); + const TypeInterface *_get_type_or_singleton_or_null(const TypeReference &p_typeref); const String _get_generic_type_parameters(const TypeInterface &p_itype, const List &p_generic_type_parameters); diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs index dfae85b667e7..0fe4bcdfce05 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs @@ -276,8 +276,13 @@ internal static void SetGodotObjectPtr(IntPtr gcHandlePtr, IntPtr newPtr) if (wrapperType != null && IsStatic(wrapperType)) { - // A static class means this is a Godot singleton class. If an instance is needed we use GodotObject. - return typeof(GodotObject); + // A static class means this is a Godot singleton class. Try to get the Instance proxy type. + wrapperType = TypeGetProxyClass($"{nativeTypeNameStr}Instance"); + if (wrapperType == null) + { + // Otherwise, fallback to GodotObject. + return typeof(GodotObject); + } } return wrapperType; From 23f7f24e8ab3673dd9967801bf4e4f4c82c54b8b Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Fri, 14 Jul 2023 20:22:04 +0200 Subject: [PATCH 2/2] C#: Add hard-coded singletons to avoid breaking compat Co-authored-by: Ignacio Etcheverry --- modules/mono/editor/bindings_generator.cpp | 56 ++++++++++++++-------- modules/mono/editor/bindings_generator.h | 10 ++++ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 43b4962641cc..9621bf5f103d 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -1515,39 +1515,44 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str "' for class '" + itype.name + "'."); } - if (itype.is_singleton) { - // Add the type name and the singleton pointer as static fields - StringName instance_name = itype.name + CS_SINGLETON_INSTANCE_SUFFIX; - String instance_type_name = obj_types.has(instance_name) - ? obj_types[instance_name].proxy_name - : "GodotObject"; + // Add native name static field and cached type. + + if (is_derived_type && !itype.is_singleton) { + output << MEMBER_BEGIN "private static readonly System.Type CachedType = typeof(" << itype.proxy_name << ");\n"; + } + + output.append(MEMBER_BEGIN "private static readonly StringName " BINDINGS_NATIVE_NAME_FIELD " = \""); + output.append(itype.name); + output.append("\";\n"); + + if (itype.is_singleton || itype.is_compat_singleton) { + // Add the Singleton static property. + + String instance_type_name; + + if (itype.is_singleton) { + StringName instance_name = itype.name + CS_SINGLETON_INSTANCE_SUFFIX; + instance_type_name = obj_types.has(instance_name) + ? obj_types[instance_name].proxy_name + : "GodotObject"; + } else { + instance_type_name = itype.proxy_name; + } output.append(MEMBER_BEGIN "private static " + instance_type_name + " singleton;\n"); output << MEMBER_BEGIN "public static " + instance_type_name + " " CS_PROPERTY_SINGLETON " =>\n" << INDENT2 "singleton \?\?= (" + instance_type_name + ")" << C_METHOD_ENGINE_GET_SINGLETON "(\"" << itype.name << "\");\n"; + } - output.append(MEMBER_BEGIN "private static readonly StringName " BINDINGS_NATIVE_NAME_FIELD " = \""); - output.append(itype.name); - output.append("\";\n"); - } else { + if (!itype.is_singleton) { // IMPORTANT: We also generate the static fields for GodotObject instead of declaring // them manually in the `GodotObject.base.cs` partial class declaration, because they're // required by other static fields in this generated partial class declaration. // Static fields are initialized in order of declaration, but when they're in different // partial class declarations then it becomes harder to tell (Rider warns about this). - // Add native name static field - - if (is_derived_type) { - output << MEMBER_BEGIN "private static readonly System.Type CachedType = typeof(" << itype.proxy_name << ");\n"; - } - - output.append(MEMBER_BEGIN "private static readonly StringName " BINDINGS_NATIVE_NAME_FIELD " = \""); - output.append(itype.name); - output.append("\";\n"); - if (itype.is_instantiable) { // Add native constructor static field @@ -2964,6 +2969,11 @@ bool BindingsGenerator::_populate_object_type_interfaces() { itype.is_ref_counted = ClassDB::is_parent_class(type_cname, name_cache.type_RefCounted); itype.memory_own = itype.is_ref_counted; + if (itype.is_singleton && compat_singletons.has(itype.cname)) { + itype.is_singleton = false; + itype.is_compat_singleton = true; + } + itype.c_out = "%5return "; itype.c_out += C_METHOD_UNMANAGED_GET_MANAGED; itype.c_out += itype.is_ref_counted ? "(%1.Reference);\n" : "(%1);\n"; @@ -4003,6 +4013,10 @@ void BindingsGenerator::_initialize_blacklisted_methods() { blacklisted_methods["Object"].push_back("_init"); // never called in C# (TODO: implement it) } +void BindingsGenerator::_initialize_compat_singletons() { + // No compat singletons yet. +} + void BindingsGenerator::_log(const char *p_format, ...) { if (log_print_enabled) { va_list list; @@ -4022,6 +4036,8 @@ void BindingsGenerator::_initialize() { _initialize_blacklisted_methods(); + _initialize_compat_singletons(); + bool obj_type_ok = _populate_object_type_interfaces(); ERR_FAIL_COND_MSG(!obj_type_ok, "Failed to generate object type interfaces"); diff --git a/modules/mono/editor/bindings_generator.h b/modules/mono/editor/bindings_generator.h index 80b0ed0b7df0..d4c7a59e74d1 100644 --- a/modules/mono/editor/bindings_generator.h +++ b/modules/mono/editor/bindings_generator.h @@ -230,6 +230,14 @@ class BindingsGenerator { bool is_singleton_instance = false; bool is_ref_counted = false; + /** + * Class is a singleton, but can't be declared as a static class as that would + * break backwards compatibility. As such, instead of going with a static class, + * we use the actual singleton pattern (private constructor with instance property), + * which doesn't break compatibility. + */ + bool is_compat_singleton = false; + /** * Determines whether the native return value of this type must be disposed * by the generated internal call (think of `godot_string`, whose destructor @@ -615,8 +623,10 @@ class BindingsGenerator { HashMap method_icalls_map; HashMap> blacklisted_methods; + HashSet compat_singletons; void _initialize_blacklisted_methods(); + void _initialize_compat_singletons(); struct NameCache { StringName type_void = StaticCString::create("void");