Skip to content

Commit

Permalink
Merge pull request #87890 from raulsntos/dotnet/generics
Browse files Browse the repository at this point in the history
Improve handling of generic C# types
  • Loading branch information
akien-mga committed Feb 12, 2024
2 parents 27575c4 + 5815d1c commit efcb23f
Show file tree
Hide file tree
Showing 10 changed files with 419 additions and 138 deletions.
96 changes: 30 additions & 66 deletions modules/mono/csharp_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CSharpScript> 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 {
Expand Down Expand Up @@ -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<CSharpScript> get_base_script(const CSharpScript *p_script) const {
Ref<CSharpScript> 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() {
Expand Down Expand Up @@ -937,7 +898,7 @@ void CSharpLanguage::reload_assemblies(bool p_soft_reload) {
obj->set_script(Ref<RefCounted>()); // 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();
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -2334,9 +2296,7 @@ void CSharpScript::reload_registered_script(Ref<CSharpScript> p_script) {

// Extract information about the script using the mono class.
void CSharpScript::update_script_class_info(Ref<CSharpScript> p_script) {
bool tool = false;
bool global_class = false;
bool abstract_class = false;
TypeInfo type_info;

// TODO: Use GDExtension godot_dictionary
Array methods_array;
Expand All @@ -2346,18 +2306,12 @@ void CSharpScript::update_script_class_info(Ref<CSharpScript> p_script) {
Dictionary signals_dict;
signals_dict.~Dictionary();

String class_name;
String icon_path;
Ref<CSharpScript> 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;
Expand Down Expand Up @@ -2436,7 +2390,7 @@ void CSharpScript::update_script_class_info(Ref<CSharpScript> 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
Expand All @@ -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 {
Expand All @@ -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<RefCounted> ref;
Expand Down Expand Up @@ -2772,11 +2728,11 @@ bool CSharpScript::inherits_script(const Ref<Script> &p_script) const {
}

Ref<Script> CSharpScript::get_base_script() const {
return base_script.is_valid() && !base_script->get_path().is_empty() ? base_script : nullptr;
return base_script;
}

StringName CSharpScript::get_global_name() const {
return global_class ? StringName(class_name) : StringName();
return type_info.is_global_class ? StringName(type_info.class_name) : StringName();
}

void CSharpScript::get_script_property_list(List<PropertyInfo> *r_list) const {
Expand Down Expand Up @@ -2833,7 +2789,7 @@ Error CSharpScript::load_source_code(const String &p_path) {
}

void CSharpScript::_clear() {
tool = false;
type_info = TypeInfo();
valid = false;
reload_invalidated = true;
}
Expand Down Expand Up @@ -2881,17 +2837,25 @@ Ref<Resource> ResourceFormatLoaderCSharpScript::load(const String &p_path, const

// TODO ignore anything inside bin/ and obj/ in tools builds?

String real_path = p_path;
if (p_path.begins_with("csharp://")) {
// This is a virtual path used by generic types, extract the real path.
real_path = "res://" + p_path.trim_prefix("csharp://");
real_path = real_path.substr(0, real_path.rfind(":"));
}

Ref<CSharpScript> scr;

if (GDMonoCache::godot_api_cache_updated) {
GDMonoCache::managed_callbacks.ScriptManagerBridge_GetOrCreateScriptBridgeForPath(&p_path, &scr);
ERR_FAIL_NULL_V_MSG(scr, Ref<Resource>(), "Could not create C# script '" + real_path + "'.");
} else {
scr = Ref<CSharpScript>(memnew(CSharpScript));
}

#if defined(DEBUG_ENABLED) || defined(TOOLS_ENABLED)
Error err = scr->load_source_code(p_path);
ERR_FAIL_COND_V_MSG(err != OK, Ref<Resource>(), "Cannot load C# script file '" + p_path + "'.");
Error err = scr->load_source_code(real_path);
ERR_FAIL_COND_V_MSG(err != OK, Ref<Resource>(), "Cannot load C# script file '" + real_path + "'.");
#endif

// Only one instance of a C# script is allowed to exist.
Expand Down
93 changes: 84 additions & 9 deletions modules/mono/csharp_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,88 @@ class CSharpScript : public Script {

friend class CSharpInstance;
friend class CSharpLanguage;
friend struct CSharpScriptDepSort;

bool tool = false;
bool global_class = false;
bool abstract_class = false;
public:
struct TypeInfo {
/**
* Name of the C# class.
*/
String class_name;

/**
* Path to the icon that will be used for this class by the editor.
*/
String icon_path;

/**
* Script is marked as tool and runs in the editor.
*/
bool is_tool = false;

/**
* Script is marked as global class and will be registered in the editor.
* Registered classes can be created using certain editor dialogs and
* can be referenced by name from other languages that support the feature.
*/
bool is_global_class = false;

/**
* Script is declared abstract.
*/
bool is_abstract = false;

/**
* The C# type that corresponds to this script is a constructed generic type.
* E.g.: `Dictionary<int, string>`
*/
bool is_constructed_generic_type = false;

/**
* The C# type that corresponds to this script is a generic type definition.
* E.g.: `Dictionary<,>`
*/
bool is_generic_type_definition = false;

/**
* The C# type that corresponds to this script contains generic type parameters,
* regardless of whether the type parameters are bound or not.
*/
bool is_generic() const {
return is_constructed_generic_type || is_generic_type_definition;
}

/**
* Check if the script can be instantiated.
* C# types can't be instantiated if they are abstract or contain generic
* type parameters, but a CSharpScript is still created for them.
*/
bool can_instantiate() const {
return !is_abstract && !is_generic_type_definition;
}
};

private:
/**
* Contains the C# type information for this script.
*/
TypeInfo type_info;

/**
* Scripts are valid when the corresponding C# class is found and used
* to extract the script info using the [update_script_class_info] method.
*/
bool valid = false;
/**
* Scripts extract info from the C# class in the reload methods but,
* if the reload is not invalidated, then the current extracted info
* is still valid and there's no need to reload again.
*/
bool reload_invalidated = false;

/**
* Base script that this script derives from, or null if it derives from a
* native Godot class.
*/
Ref<CSharpScript> base_script;

HashSet<Object *> instances;
Expand All @@ -88,9 +162,10 @@ class CSharpScript : public Script {
HashSet<ObjectID> pending_replace_placeholders;
#endif

/**
* Script source code.
*/
String source;
String class_name;
String icon_path;

SelfList<CSharpScript> script_list = this;

Expand Down Expand Up @@ -167,7 +242,7 @@ class CSharpScript : public Script {
return docs;
}
virtual String get_class_icon_path() const override {
return icon_path;
return type_info.icon_path;
}
#endif // TOOLS_ENABLED

Expand All @@ -185,13 +260,13 @@ class CSharpScript : public Script {
void get_members(HashSet<StringName> *p_members) override;

bool is_tool() const override {
return tool;
return type_info.is_tool;
}
bool is_valid() const override {
return valid;
}
bool is_abstract() const override {
return abstract_class;
return type_info.is_abstract;
}

bool inherits_script(const Ref<Script> &p_script) const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ public void Execute(GeneratorExecutionContext context)
)
.Where(x =>
// Ignore classes whose name is not the same as the file name
Path.GetFileNameWithoutExtension(x.cds.SyntaxTree.FilePath) == x.symbol.Name &&
// Ignore generic classes
!x.symbol.IsGenericType)
Path.GetFileNameWithoutExtension(x.cds.SyntaxTree.FilePath) == x.symbol.Name)
.GroupBy(x => x.symbol)
.ToDictionary(g => g.Key, g => g.Select(x => x.cds));

Expand Down Expand Up @@ -160,6 +158,8 @@ private static void AddScriptTypesAssemblyAttr(GeneratorExecutionContext context
first = false;
sourceBuilder.Append("typeof(");
sourceBuilder.Append(qualifiedName);
if (godotClass.Key.IsGenericType)
sourceBuilder.Append($"<{new string(',', godotClass.Key.TypeParameters.Count() - 1)}>");
sourceBuilder.Append(")");
}

Expand Down
Loading

0 comments on commit efcb23f

Please sign in to comment.