Skip to content

Commit

Permalink
Fix generic arrays and dictionaries not calling set_typed
Browse files Browse the repository at this point in the history
  • Loading branch information
juanjp600 committed Dec 8, 2024
1 parent aa8d9b8 commit 8ab27a7
Show file tree
Hide file tree
Showing 11 changed files with 260 additions and 26 deletions.
4 changes: 1 addition & 3 deletions modules/mono/csharp_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions modules/mono/csharp_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ class CSharpScript : public Script {
*/
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.
*/
Expand Down
19 changes: 19 additions & 0 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,22 @@ private static godot_variant ToVariantFunc(in Array<T> godotArray) =>
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;
Expand All @@ -1083,6 +1099,7 @@ internal ref godot_array.movable NativeValue
public Array()
{
_underlyingArray = new Array();
SetTypedForUnderlyingArray();
}

/// <summary>
Expand All @@ -1099,6 +1116,7 @@ public Array(IEnumerable<T> collection)
throw new ArgumentNullException(nameof(collection));

_underlyingArray = new Array();
SetTypedForUnderlyingArray();

foreach (T element in collection)
Add(element);
Expand All @@ -1118,6 +1136,7 @@ public Array(T[] array)
throw new ArgumentNullException(nameof(array));

_underlyingArray = new Array();
SetTypedForUnderlyingArray();

foreach (T element in array)
Add(element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,7 @@ internal static unsafe void GetScriptNativeName(IntPtr scriptPtr, godot_string_n
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)
{
Expand Down Expand Up @@ -658,10 +647,14 @@ internal static godot_bool TryReloadRegisteredScriptWithClass(IntPtr scriptPtr)

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,
Expand All @@ -686,6 +679,7 @@ private static unsafe void GetScriptTypeInfo(Type scriptType, godot_csharp_type_
godot_string iconPath = Marshaling.ConvertStringToNative(iconAttr?.Path);

outTypeInfo->ClassName = className;
outTypeInfo->NativeBaseName = nativeBaseName;
outTypeInfo->IconPath = iconPath;
outTypeInfo->IsTool = isTool.ToGodotBool();
outTypeInfo->IsGlobalClass = isGlobalClass.ToGodotBool();
Expand Down
23 changes: 23 additions & 0 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,27 @@ private static godot_variant ToVariantFunc(in Dictionary<TKey, TValue> godotDict
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;
Expand All @@ -519,6 +540,7 @@ internal ref godot_dictionary.movable NativeValue
public Dictionary()
{
_underlyingDict = new Dictionary();
SetTypedForUnderlyingDictionary();
}

/// <summary>
Expand All @@ -535,6 +557,7 @@ public Dictionary(IDictionary<TKey, TValue> dictionary)
throw new ArgumentNullException(nameof(dictionary));

_underlyingDict = new Dictionary();
SetTypedForUnderlyingDictionary();

foreach (KeyValuePair<TKey, TValue> entry in dictionary)
Add(entry.Key, entry.Value);
Expand Down
54 changes: 49 additions & 5 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/GodotObject.base.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -13,6 +15,8 @@ public partial class GodotObject : IDisposable
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;

Expand Down Expand Up @@ -191,16 +195,56 @@ public SignalAwaiter ToSignal(GodotObject source, StringName signal)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public Godot.Variant.Type Expected
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;
Expand All @@ -122,6 +123,12 @@ public godot_string ClassName
set => _className = value;
}

public godot_string_name NativeBaseName
{
readonly get => _nativeBaseName;
set => _nativeBaseName = value;
}

public godot_string IconPath
{
readonly get => _iconPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,44 @@ internal static Variant.Type ConvertManagedTypeToVariantType(Type type, out bool
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,14 @@ public static partial void

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);
Expand Down Expand Up @@ -463,6 +471,31 @@ public static partial godot_bool godotsharp_dictionary_remove_key(ref godot_dict

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -66,7 +66,7 @@ static ReflectionUtils()

public static string ConstructTypeName(Type type)
{
if (!_isEditorHintCached)
if (!IsEditorHintCached)
{
return type.Name;
}
Expand Down
Loading

0 comments on commit 8ab27a7

Please sign in to comment.