Browse Source

Merge pull request #79470 from raulsntos/dotnet/singleton-can-be-instances-too

C#: Generate instance types for singletons
Rémi Verschelde 2 years ago
parent
commit
8018b478ca

+ 1 - 1
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<String> ignored_types = { "PhysicsServer2DExtension", "PhysicsServer3DExtension" };
+const Vector<String> ignored_types = {};
 
 #ifdef TOOLS_ENABLED
 static bool _create_project_solution_if_needed() {

+ 94 - 37
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<String> ignored_types = { "PhysicsServer2DExtension", "PhysicsServer3DExtension" };
+const Vector<String> 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;
@@ -1504,37 +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
+	// Add native name static field and cached type.
 
-		output.append(MEMBER_BEGIN "private static GodotObject singleton;\n");
+	if (is_derived_type && !itype.is_singleton) {
+		output << MEMBER_BEGIN "private static readonly System.Type CachedType = typeof(" << itype.proxy_name << ");\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.append(MEMBER_BEGIN "private static readonly StringName " BINDINGS_NATIVE_NAME_FIELD " = \"");
+	output.append(itype.name);
+	output.append("\";\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 || 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";
+	}
+
+	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
 
@@ -1894,7 +1912,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 +2001,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 +2022,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 +2040,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 +2298,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 +2725,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<TypeReference> &p_generic_type_parameters) {
 	if (p_generic_type_parameters.is_empty()) {
 		return "";
@@ -2715,8 +2752,8 @@ const String BindingsGenerator::_get_generic_type_parameters(const TypeInterface
 	int i = 0;
 	String params = "<";
 	for (const TypeReference &param_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 + "'.");
@@ -2932,17 +2969,18 @@ 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";
 
 		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 +3384,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();
 	}
 
@@ -3962,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;
@@ -3981,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");
 

+ 12 - 0
modules/mono/editor/bindings_generator.h

@@ -227,8 +227,17 @@ 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;
 
+		/**
+		 * 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
@@ -614,8 +623,10 @@ class BindingsGenerator {
 	HashMap<const MethodInterface *, const InternalCall *> method_icalls_map;
 
 	HashMap<StringName, List<StringName>> blacklisted_methods;
+	HashSet<StringName> compat_singletons;
 
 	void _initialize_blacklisted_methods();
+	void _initialize_compat_singletons();
 
 	struct NameCache {
 		StringName type_void = StaticCString::create("void");
@@ -756,6 +767,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<TypeReference> &p_generic_type_parameters);
 

+ 7 - 2
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs

@@ -276,8 +276,13 @@ namespace Godot.Bridge
 
             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;