Browse Source

Merge pull request #98545 from juanjp600/dotnet-generic-collections-set-typed

Fix generic arrays and dictionaries in .NET not calling `set_typed`
Thaddeus Crews 8 months ago
parent
commit
b4bd38444b

+ 1 - 3
modules/mono/csharp_script.cpp

@@ -2344,9 +2344,7 @@ bool CSharpScript::can_instantiate() const {
 }
 
 StringName CSharpScript::get_instance_base_type() const {
-	StringName native_name;
-	GDMonoCache::managed_callbacks.ScriptManagerBridge_GetScriptNativeName(this, &native_name);
-	return native_name;
+	return type_info.native_base_name;
 }
 
 CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_is_ref_counted, Callable::CallError &r_error) {

+ 5 - 0
modules/mono/csharp_script.h

@@ -68,6 +68,11 @@ public:
 		 */
 		String class_name;
 
+		/**
+		 * Name of the native class this script derives from.
+		 */
+		StringName native_base_name;
+
 		/**
 		 * Path to the icon that will be used for this class by the editor.
 		 */

+ 19 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs

@@ -1060,6 +1060,22 @@ namespace Godot.Collections
         private static Array<T> FromVariantFunc(in godot_variant variant) =>
             VariantUtils.ConvertToArray<T>(variant);
 
+        private void SetTypedForUnderlyingArray()
+        {
+            Marshaling.GetTypedCollectionParameterInfo<T>(out var elemVariantType, out var elemClassName, out var elemScriptRef);
+
+            var self = (godot_array)NativeValue;
+
+            using (elemScriptRef)
+            {
+                NativeFuncs.godotsharp_array_set_typed(
+                    ref self,
+                    (uint)elemVariantType,
+                    elemClassName,
+                    elemScriptRef);
+            }
+        }
+
         static unsafe Array()
         {
             VariantUtils.GenericConversion<Array<T>>.ToVariantCb = &ToVariantFunc;
@@ -1083,6 +1099,7 @@ namespace Godot.Collections
         public Array()
         {
             _underlyingArray = new Array();
+            SetTypedForUnderlyingArray();
         }
 
         /// <summary>
@@ -1099,6 +1116,7 @@ namespace Godot.Collections
                 throw new ArgumentNullException(nameof(collection));
 
             _underlyingArray = new Array();
+            SetTypedForUnderlyingArray();
 
             foreach (T element in collection)
                 Add(element);
@@ -1118,6 +1136,7 @@ namespace Godot.Collections
                 throw new ArgumentNullException(nameof(array));
 
             _underlyingArray = new Array();
+            SetTypedForUnderlyingArray();
 
             foreach (T element in array)
                 Add(element);

+ 8 - 14
modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/ScriptManagerBridge.cs

@@ -182,18 +182,7 @@ namespace Godot.Bridge
                     return;
                 }
 
-                var native = GodotObject.InternalGetClassNativeBase(scriptType);
-
-                var field = native.GetField("NativeName", BindingFlags.DeclaredOnly | BindingFlags.Static |
-                                                           BindingFlags.Public | BindingFlags.NonPublic);
-
-                if (field == null)
-                {
-                    *outRes = default;
-                    return;
-                }
-
-                var nativeName = (StringName?)field.GetValue(null);
+                var nativeName = GodotObject.InternalGetClassNativeBaseName(scriptType);
 
                 if (nativeName == null)
                 {
@@ -658,10 +647,14 @@ namespace Godot.Bridge
 
         private static unsafe void GetScriptTypeInfo(Type scriptType, godot_csharp_type_info* outTypeInfo)
         {
-            Type native = GodotObject.InternalGetClassNativeBase(scriptType);
-
             godot_string className = Marshaling.ConvertStringToNative(ReflectionUtils.ConstructTypeName(scriptType));
 
+            StringName? nativeBase = GodotObject.InternalGetClassNativeBaseName(scriptType);
+
+            godot_string_name nativeBaseName = nativeBase != null
+                ? NativeFuncs.godotsharp_string_name_new_copy((godot_string_name)nativeBase.NativeValue)
+                : default;
+
             bool isTool = scriptType.IsDefined(typeof(ToolAttribute), inherit: false);
 
             // If the type is nested and the parent type is a tool script,
@@ -686,6 +679,7 @@ namespace Godot.Bridge
             godot_string iconPath = Marshaling.ConvertStringToNative(iconAttr?.Path);
 
             outTypeInfo->ClassName = className;
+            outTypeInfo->NativeBaseName = nativeBaseName;
             outTypeInfo->IconPath = iconPath;
             outTypeInfo->IsTool = isTool.ToGodotBool();
             outTypeInfo->IsGlobalClass = isGlobalClass.ToGodotBool();

+ 23 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs

@@ -496,6 +496,27 @@ namespace Godot.Collections
         private static Dictionary<TKey, TValue> FromVariantFunc(in godot_variant variant) =>
             VariantUtils.ConvertToDictionary<TKey, TValue>(variant);
 
+        private void SetTypedForUnderlyingDictionary()
+        {
+            Marshaling.GetTypedCollectionParameterInfo<TKey>(out var keyVariantType, out var keyClassName, out var keyScriptRef);
+            Marshaling.GetTypedCollectionParameterInfo<TValue>(out var valueVariantType, out var valueClassName, out var valueScriptRef);
+
+            var self = (godot_dictionary)NativeValue;
+
+            using (keyScriptRef)
+            using (valueScriptRef)
+            {
+                NativeFuncs.godotsharp_dictionary_set_typed(
+                    ref self,
+                    (uint)keyVariantType,
+                    keyClassName,
+                    keyScriptRef,
+                    (uint)valueVariantType,
+                    valueClassName,
+                    valueScriptRef);
+            }
+        }
+
         static unsafe Dictionary()
         {
             VariantUtils.GenericConversion<Dictionary<TKey, TValue>>.ToVariantCb = &ToVariantFunc;
@@ -519,6 +540,7 @@ namespace Godot.Collections
         public Dictionary()
         {
             _underlyingDict = new Dictionary();
+            SetTypedForUnderlyingDictionary();
         }
 
         /// <summary>
@@ -535,6 +557,7 @@ namespace Godot.Collections
                 throw new ArgumentNullException(nameof(dictionary));
 
             _underlyingDict = new Dictionary();
+            SetTypedForUnderlyingDictionary();
 
             foreach (KeyValuePair<TKey, TValue> entry in dictionary)
                 Add(entry.Key, entry.Value);

+ 49 - 5
modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
+using System.Reflection;
 using System.Runtime.InteropServices;
 using Godot.Bridge;
 using Godot.NativeInterop;
@@ -13,6 +15,8 @@ namespace Godot
         private bool _disposed;
         private static readonly Type _cachedType = typeof(GodotObject);
 
+        private static readonly Dictionary<Type, StringName?> _nativeNames = new Dictionary<Type, StringName?>();
+
         internal IntPtr NativePtr;
         private bool _memoryOwn;
 
@@ -191,16 +195,56 @@ namespace Godot
             return new SignalAwaiter(source, signal, this);
         }
 
+        internal static bool IsNativeClass(Type t)
+        {
+            if (ReferenceEquals(t.Assembly, typeof(GodotObject).Assembly))
+            {
+                return true;
+            }
+
+            if (ReflectionUtils.IsEditorHintCached)
+            {
+                return t.Assembly.GetName().Name == "GodotSharpEditor";
+            }
+
+            return false;
+        }
+
         internal static Type InternalGetClassNativeBase(Type t)
         {
-            var name = t.Assembly.GetName().Name;
+            while (!IsNativeClass(t))
+            {
+                Debug.Assert(t.BaseType is not null, "Script types must derive from a native Godot type.");
+
+                t = t.BaseType;
+            }
+
+            return t;
+        }
+
+        internal static StringName? InternalGetClassNativeBaseName(Type t)
+        {
+            if (_nativeNames.TryGetValue(t, out var name))
+            {
+                return name;
+            }
+
+            var baseType = InternalGetClassNativeBase(t);
+
+            if (_nativeNames.TryGetValue(baseType, out name))
+            {
+                return name;
+            }
+
+            var field = baseType.GetField("NativeName",
+                BindingFlags.DeclaredOnly | BindingFlags.Static |
+                BindingFlags.Public | BindingFlags.NonPublic);
 
-            if (name == "GodotSharp" || name == "GodotSharpEditor")
-                return t;
+            name = field?.GetValue(null) as StringName;
 
-            Debug.Assert(t.BaseType is not null, "Script types must derive from a native Godot type.");
+            _nativeNames[baseType] = name;
 
-            return InternalGetClassNativeBase(t.BaseType);
+            return name;
         }
 
         // ReSharper disable once VirtualMemberNeverOverridden.Global

+ 7 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs

@@ -109,6 +109,7 @@ namespace Godot.NativeInterop
     public ref struct godot_csharp_type_info
     {
         private godot_string _className;
+        private godot_string_name _nativeBaseName;
         private godot_string _iconPath;
         private godot_bool _isTool;
         private godot_bool _isGlobalClass;
@@ -122,6 +123,12 @@ namespace Godot.NativeInterop
             set => _className = value;
         }
 
+        public godot_string_name NativeBaseName
+        {
+            readonly get => _nativeBaseName;
+            set => _nativeBaseName = value;
+        }
+
         public godot_string IconPath
         {
             readonly get => _iconPath;

+ 38 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/Marshaling.cs

@@ -199,6 +199,44 @@ namespace Godot.NativeInterop
             return Variant.Type.Nil;
         }
 
+        internal static void GetTypedCollectionParameterInfo<T>(
+            out Variant.Type variantType,
+            out godot_string_name className,
+            out godot_ref script)
+        {
+            variantType = ConvertManagedTypeToVariantType(typeof(T), out _);
+
+            if (variantType != Variant.Type.Object)
+            {
+                className = default;
+                script = default;
+                return;
+            }
+
+            godot_ref scriptRef = default;
+
+            if (!GodotObject.IsNativeClass(typeof(T)))
+            {
+                unsafe
+                {
+                    Godot.Bridge.ScriptManagerBridge.GetOrLoadOrCreateScriptForType(typeof(T), &scriptRef);
+                }
+
+                // Don't call GodotObject.InternalGetClassNativeBaseName here!
+                // godot_dictionary_set_typed and godot_array_set_typed will call CSharpScript::get_instance_base_type
+                // when a script is passed, because this is better for performance than using reflection to find the
+                // native base type.
+                className = default;
+            }
+            else
+            {
+                StringName? nativeBaseName = GodotObject.InternalGetClassNativeBaseName(typeof(T));
+                className = nativeBaseName != null ? (godot_string_name)nativeBaseName.NativeValue : default;
+            }
+
+            script = scriptRef;
+        }
+
         // String
 
         public static unsafe godot_string ConvertStringToNative(string? p_mono_string)

+ 33 - 0
modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs

@@ -402,6 +402,14 @@ namespace Godot.NativeInterop
 
         public static partial void godotsharp_array_make_read_only(ref godot_array p_self);
 
+        public static partial void godotsharp_array_set_typed(
+            ref godot_array p_self,
+            uint p_elem_type,
+            in godot_string_name p_elem_class_name,
+            in godot_ref p_elem_script);
+
+        public static partial godot_bool godotsharp_array_is_typed(ref godot_array p_self);
+
         public static partial void godotsharp_array_max(ref godot_array p_self, out godot_variant r_value);
 
         public static partial void godotsharp_array_min(ref godot_array p_self, out godot_variant r_value);
@@ -463,6 +471,31 @@ namespace Godot.NativeInterop
 
         public static partial void godotsharp_dictionary_make_read_only(ref godot_dictionary p_self);
 
+        public static partial void godotsharp_dictionary_set_typed(
+            ref godot_dictionary p_self,
+            uint p_key_type,
+            in godot_string_name p_key_class_name,
+            in godot_ref p_key_script,
+            uint p_value_type,
+            in godot_string_name p_value_class_name,
+            in godot_ref p_value_script);
+
+        public static partial godot_bool godotsharp_dictionary_is_typed_key(ref godot_dictionary p_self);
+
+        public static partial godot_bool godotsharp_dictionary_is_typed_value(ref godot_dictionary p_self);
+
+        public static partial uint godotsharp_dictionary_get_typed_key_builtin(ref godot_dictionary p_self);
+
+        public static partial uint godotsharp_dictionary_get_typed_value_builtin(ref godot_dictionary p_self);
+
+        public static partial void godotsharp_dictionary_get_typed_key_class_name(ref godot_dictionary p_self, out godot_string_name r_dest);
+
+        public static partial void godotsharp_dictionary_get_typed_value_class_name(ref godot_dictionary p_self, out godot_string_name r_dest);
+
+        public static partial void godotsharp_dictionary_get_typed_key_script(ref godot_dictionary p_self, out godot_variant r_dest);
+
+        public static partial void godotsharp_dictionary_get_typed_value_script(ref godot_dictionary p_self, out godot_variant r_dest);
+
         public static partial void godotsharp_dictionary_to_string(ref godot_dictionary p_self, out godot_string r_str);
 
         // StringExtensions

+ 4 - 4
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs

@@ -12,12 +12,12 @@ internal class ReflectionUtils
 {
     private static readonly HashSet<Type>? _tupleTypeSet;
     private static readonly Dictionary<Type, string>? _builtinTypeNameDictionary;
-    private static readonly bool _isEditorHintCached;
+    internal static readonly bool IsEditorHintCached;
 
     static ReflectionUtils()
     {
-        _isEditorHintCached = Engine.IsEditorHint();
-        if (!_isEditorHintCached)
+        IsEditorHintCached = Engine.IsEditorHint();
+        if (!IsEditorHintCached)
         {
             return;
         }
@@ -66,7 +66,7 @@ internal class ReflectionUtils
 
     public static string ConstructTypeName(Type type)
     {
-        if (!_isEditorHintCached)
+        if (!IsEditorHintCached)
         {
             return type.Name;
         }

+ 73 - 0
modules/mono/glue/runtime_interop.cpp

@@ -1098,6 +1098,20 @@ void godotsharp_array_make_read_only(Array *p_self) {
 	p_self->make_read_only();
 }
 
+void godotsharp_array_set_typed(Array *p_self, uint32_t p_elem_type, const StringName *p_elem_class_name, const Ref<CSharpScript> *p_elem_script) {
+	Variant elem_script_variant;
+	StringName elem_class_name = *p_elem_class_name;
+	if (p_elem_script && p_elem_script->is_valid()) {
+		elem_script_variant = Variant(p_elem_script->ptr());
+		elem_class_name = p_elem_script->ptr()->get_instance_base_type();
+	}
+	p_self->set_typed(p_elem_type, elem_class_name, p_elem_script->ptr());
+}
+
+bool godotsharp_array_is_typed(const Array *p_self) {
+	return p_self->is_typed();
+}
+
 void godotsharp_array_max(const Array *p_self, Variant *r_value) {
 	*r_value = p_self->max();
 }
@@ -1207,6 +1221,54 @@ void godotsharp_dictionary_make_read_only(Dictionary *p_self) {
 	p_self->make_read_only();
 }
 
+void godotsharp_dictionary_set_typed(Dictionary *p_self, uint32_t p_key_type, const StringName *p_key_class_name, const Ref<CSharpScript> *p_key_script, uint32_t p_value_type, const StringName *p_value_class_name, const Ref<CSharpScript> *p_value_script) {
+	Variant key_script_variant;
+	StringName key_class_name = *p_key_class_name;
+	if (p_key_script && p_key_script->is_valid()) {
+		key_script_variant = Variant(p_key_script->ptr());
+		key_class_name = p_key_script->ptr()->get_instance_base_type();
+	}
+	Variant value_script_variant;
+	StringName value_class_name = *p_value_class_name;
+	if (p_value_script && p_value_script->is_valid()) {
+		value_script_variant = Variant(p_value_script->ptr());
+		value_class_name = p_value_script->ptr()->get_instance_base_type();
+	}
+	p_self->set_typed(p_key_type, key_class_name, p_key_script->ptr(), p_value_type, value_class_name, p_value_script->ptr());
+}
+
+bool godotsharp_dictionary_is_typed_key(const Dictionary *p_self) {
+	return p_self->is_typed_key();
+}
+
+bool godotsharp_dictionary_is_typed_value(const Dictionary *p_self) {
+	return p_self->is_typed_value();
+}
+
+uint32_t godotsharp_dictionary_get_typed_key_builtin(const Dictionary *p_self) {
+	return p_self->get_typed_key_builtin();
+}
+
+uint32_t godotsharp_dictionary_get_typed_value_builtin(const Dictionary *p_self) {
+	return p_self->get_typed_value_builtin();
+}
+
+void godotsharp_dictionary_get_typed_key_class_name(const Dictionary *p_self, StringName *r_dest) {
+	memnew_placement(r_dest, StringName(p_self->get_typed_key_class_name()));
+}
+
+void godotsharp_dictionary_get_typed_value_class_name(const Dictionary *p_self, StringName *r_dest) {
+	memnew_placement(r_dest, StringName(p_self->get_typed_value_class_name()));
+}
+
+void godotsharp_dictionary_get_typed_key_script(const Dictionary *p_self, Variant *r_dest) {
+	memnew_placement(r_dest, Variant(p_self->get_typed_key_script()));
+}
+
+void godotsharp_dictionary_get_typed_value_script(const Dictionary *p_self, Variant *r_dest) {
+	memnew_placement(r_dest, Variant(p_self->get_typed_value_script()));
+}
+
 void godotsharp_dictionary_to_string(const Dictionary *p_self, String *r_str) {
 	*r_str = Variant(*p_self).operator String();
 }
@@ -1585,6 +1647,8 @@ static const void *unmanaged_callbacks[]{
 	(void *)godotsharp_array_insert,
 	(void *)godotsharp_array_last_index_of,
 	(void *)godotsharp_array_make_read_only,
+	(void *)godotsharp_array_set_typed,
+	(void *)godotsharp_array_is_typed,
 	(void *)godotsharp_array_max,
 	(void *)godotsharp_array_min,
 	(void *)godotsharp_array_pick_random,
@@ -1610,6 +1674,15 @@ static const void *unmanaged_callbacks[]{
 	(void *)godotsharp_dictionary_recursive_equal,
 	(void *)godotsharp_dictionary_remove_key,
 	(void *)godotsharp_dictionary_make_read_only,
+	(void *)godotsharp_dictionary_set_typed,
+	(void *)godotsharp_dictionary_is_typed_key,
+	(void *)godotsharp_dictionary_is_typed_value,
+	(void *)godotsharp_dictionary_get_typed_key_builtin,
+	(void *)godotsharp_dictionary_get_typed_value_builtin,
+	(void *)godotsharp_dictionary_get_typed_key_class_name,
+	(void *)godotsharp_dictionary_get_typed_value_class_name,
+	(void *)godotsharp_dictionary_get_typed_key_script,
+	(void *)godotsharp_dictionary_get_typed_value_script,
 	(void *)godotsharp_dictionary_to_string,
 	(void *)godotsharp_string_simplify_path,
 	(void *)godotsharp_string_to_camel_case,