From 5815d1c8c821bab204caf46caf650a3cd009efa4 Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Mon, 5 Feb 2024 02:49:57 +0100 Subject: [PATCH] Improve handling of generic C# types - Create CSharpScript for generic C# types. - `ScriptPathAttributeGenerator` registers the path for the generic type definition. - `ScriptManagerBridge` lookup uses the generic type definition that was registered by the generator. - Constructed generic types use a virtual `csharp://` path so they can be registered in the map and loaded as if there was a different file for each constructed type, even though they all share the same real path. - This allows getting the base type for a C# type that derives from a generic type. - Shows base scripts in the _Add Node_ and _Create Resource_ dialogs even when they are generic types. - `get_global_class_name` implementation was moved to C# and now always returns the base type even if the script is not a global class (this behavior matches GDScript). - Create `CSharpScript::TypeInfo` struct to hold all the type information about the C# type that corresponds to the `CSharpScript`, and use it as the parameter in `UpdateScriptClassInfo` to avoid adding more parameters. --- modules/mono/csharp_script.cpp | 96 +++---- modules/mono/csharp_script.h | 93 ++++++- .../ScriptPathAttributeGenerator.cs | 6 +- .../GodotTools/Inspector/InspectorPlugin.cs | 15 +- .../Core/Bridge/ManagedCallbacks.cs | 4 +- .../Core/Bridge/ScriptManagerBridge.cs | 255 ++++++++++++++---- .../Core/Bridge/ScriptManagerBridge.types.cs | 28 +- .../Core/NativeInterop/InteropStructs.cs | 55 ++++ modules/mono/mono_gd/gd_mono_cache.cpp | 1 + modules/mono/mono_gd/gd_mono_cache.h | 4 +- 10 files changed, 419 insertions(+), 138 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 18724ff6e1b8..e08491729bfa 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -558,42 +558,9 @@ bool CSharpLanguage::handles_global_class_type(const String &p_type) const { } String CSharpLanguage::get_global_class_name(const String &p_path, String *r_base_type, String *r_icon_path) const { - Ref scr = ResourceLoader::load(p_path, get_type()); - // Always assign r_base_type and r_icon_path, even if the script - // is not a global one. In the case that it is not a global script, - // return an empty string AFTER assigning the return parameters. - // See GDScriptLanguage::get_global_class_name() in modules/gdscript/gdscript.cpp - - if (!scr.is_valid() || !scr->valid) { - // Invalid script. - return String(); - } - - if (r_icon_path) { - if (scr->icon_path.is_empty() || scr->icon_path.is_absolute_path()) { - *r_icon_path = scr->icon_path.simplify_path(); - } else if (scr->icon_path.is_relative_path()) { - *r_icon_path = p_path.get_base_dir().path_join(scr->icon_path).simplify_path(); - } - } - if (r_base_type) { - bool found_global_base_script = false; - const CSharpScript *top = scr->base_script.ptr(); - while (top != nullptr) { - if (top->global_class) { - *r_base_type = top->class_name; - found_global_base_script = true; - break; - } - - top = top->base_script.ptr(); - } - if (!found_global_base_script) { - *r_base_type = scr->get_instance_base_type(); - } - } - - return scr->global_class ? scr->class_name : String(); + String class_name; + GDMonoCache::managed_callbacks.ScriptManagerBridge_GetGlobalClassName(&p_path, r_base_type, r_icon_path, &class_name); + return class_name; } String CSharpLanguage::debug_get_error() const { @@ -697,25 +664,19 @@ struct CSharpScriptDepSort { // Shouldn't happen but just in case... return false; } - const CSharpScript *I = get_base_script(B.ptr()).ptr(); + const Script *I = B->get_base_script().ptr(); while (I) { if (I == A.ptr()) { // A is a base of B return true; } - I = get_base_script(I).ptr(); + I = I->get_base_script().ptr(); } // A isn't a base of B return false; } - - // Special fix for constructed generic types. - Ref get_base_script(const CSharpScript *p_script) const { - Ref base_script = p_script->base_script; - return base_script.is_valid() && !base_script->class_name.is_empty() ? base_script : nullptr; - } }; void CSharpLanguage::reload_all_scripts() { @@ -937,7 +898,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) { obj->set_script(Ref()); // Remove script and existing script instances (placeholder are not removed before domain reload) } - scr->was_tool_before_reload = scr->tool; + scr->was_tool_before_reload = scr->type_info.is_tool; scr->_clear(); } @@ -997,7 +958,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) { scr->exports_invalidated = true; #endif - if (!scr->get_path().is_empty()) { + if (!scr->get_path().is_empty() && !scr->get_path().begins_with("csharp://")) { scr->reload(p_soft_reload); if (!scr->valid) { @@ -1839,6 +1800,7 @@ bool CSharpInstance::_internal_new_managed() { ERR_FAIL_NULL_V(owner, false); ERR_FAIL_COND_V(script.is_null(), false); + ERR_FAIL_COND_V(!script->can_instantiate(), false); bool ok = GDMonoCache::managed_callbacks.ScriptManagerBridge_CreateManagedForGodotObjectScriptInstance( script.ptr(), owner, nullptr, 0); @@ -2161,7 +2123,7 @@ void GD_CLR_STDCALL CSharpScript::_add_property_info_list_callback(CSharpScript #ifdef TOOLS_ENABLED p_script->exported_members_cache.push_back(PropertyInfo( - Variant::NIL, *p_current_class_name, PROPERTY_HINT_NONE, + Variant::NIL, p_script->type_info.class_name, PROPERTY_HINT_NONE, p_script->get_path(), PROPERTY_USAGE_CATEGORY)); #endif @@ -2334,9 +2296,7 @@ void CSharpScript::reload_registered_script(Ref p_script) { // Extract information about the script using the mono class. void CSharpScript::update_script_class_info(Ref p_script) { - bool tool = false; - bool global_class = false; - bool abstract_class = false; + TypeInfo type_info; // TODO: Use GDExtension godot_dictionary Array methods_array; @@ -2346,18 +2306,12 @@ void CSharpScript::update_script_class_info(Ref p_script) { Dictionary signals_dict; signals_dict.~Dictionary(); - String class_name; - String icon_path; Ref base_script; GDMonoCache::managed_callbacks.ScriptManagerBridge_UpdateScriptClassInfo( - p_script.ptr(), &class_name, &tool, &global_class, &abstract_class, &icon_path, + p_script.ptr(), &type_info, &methods_array, &rpc_functions_dict, &signals_dict, &base_script); - p_script->class_name = class_name; - p_script->tool = tool; - p_script->global_class = global_class; - p_script->abstract_class = abstract_class; - p_script->icon_path = icon_path; + p_script->type_info = type_info; p_script->rpc_config.clear(); p_script->rpc_config = rpc_functions_dict; @@ -2436,7 +2390,7 @@ void CSharpScript::update_script_class_info(Ref p_script) { bool CSharpScript::can_instantiate() const { #ifdef TOOLS_ENABLED - bool extra_cond = tool || ScriptServer::is_scripting_enabled(); + bool extra_cond = type_info.is_tool || ScriptServer::is_scripting_enabled(); #else bool extra_cond = true; #endif @@ -2445,10 +2399,10 @@ bool CSharpScript::can_instantiate() const { // For tool scripts, this will never fire if the class is not found. That's because we // don't know if it's a tool script if we can't find the class to access the attributes. if (extra_cond && !valid) { - ERR_FAIL_V_MSG(false, "Cannot instance script because the associated class could not be found. Script: '" + get_path() + "'. Make sure the script exists and contains a class definition with a name that matches the filename of the script exactly (it's case-sensitive)."); + ERR_FAIL_V_MSG(false, "Cannot instantiate C# script because the associated class could not be found. Script: '" + get_path() + "'. Make sure the script exists and contains a class definition with a name that matches the filename of the script exactly (it's case-sensitive)."); } - return valid && !abstract_class && extra_cond; + return valid && type_info.can_instantiate() && extra_cond; } StringName CSharpScript::get_instance_base_type() const { @@ -2458,6 +2412,8 @@ StringName CSharpScript::get_instance_base_type() const { } CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_is_ref_counted, Callable::CallError &r_error) { + ERR_FAIL_COND_V_MSG(!type_info.can_instantiate(), nullptr, "Cannot instantiate C# script. Script: '" + get_path() + "'."); + /* STEP 1, CREATE */ Ref ref; @@ -2772,11 +2728,11 @@ bool CSharpScript::inherits_script(const Ref