From 1aa54141e649adc9cca60eea6ffe780692acbcdf Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Tue, 24 Jan 2023 17:53:51 +0100 Subject: [PATCH] C#: Implement readonly-ness in Array and Dictionary - Expose `IsReadOnly` and add `MakeReadOnly` method. --- .../glue/GodotSharp/GodotSharp/Core/Array.cs | 127 +++++++++++++++++- .../GodotSharp/GodotSharp/Core/Dictionary.cs | 112 ++++++++++++++- .../Core/NativeInterop/InteropStructs.cs | 59 ++++++-- .../Core/NativeInterop/NativeFuncs.cs | 4 + modules/mono/glue/runtime_interop.cpp | 10 ++ 5 files changed, 297 insertions(+), 15 deletions(-) diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs index f4646d1d3c7..a61c5403b90 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs @@ -189,10 +189,15 @@ namespace Godot.Collections /// /// Resizes this to the given size. /// + /// + /// The array is read-only. + /// /// The new size of the array. /// if successful, or an error code. public Error Resize(int newSize) { + ThrowIfReadOnly(); + var self = (godot_array)NativeValue; return NativeFuncs.godotsharp_array_resize(ref self, newSize); } @@ -200,8 +205,13 @@ namespace Godot.Collections /// /// Shuffles the contents of this into a random order. /// + /// + /// The array is read-only. + /// public void Shuffle() { + ThrowIfReadOnly(); + var self = (godot_array)NativeValue; NativeFuncs.godotsharp_array_shuffle(ref self); } @@ -240,6 +250,9 @@ namespace Godot.Collections /// /// Returns the item at the given . /// + /// + /// The property is assigned and the array is read-only. + /// /// The item at the given . public unsafe Variant this[int index] { @@ -250,8 +263,11 @@ namespace Godot.Collections } set { + ThrowIfReadOnly(); + if (index < 0 || index >= Count) throw new ArgumentOutOfRangeException(nameof(index)); + var self = (godot_array)NativeValue; godot_variant* ptrw = NativeFuncs.godotsharp_array_ptrw(ref self); godot_variant* itemPtr = &ptrw[index]; @@ -264,9 +280,14 @@ namespace Godot.Collections /// Adds an item to the end of this . /// This is the same as append or push_back in GDScript. /// + /// + /// The array is read-only. + /// /// The item to add. public void Add(Variant item) { + ThrowIfReadOnly(); + godot_variant variantValue = (godot_variant)item.NativeVar; var self = (godot_array)NativeValue; _ = NativeFuncs.godotsharp_array_add(ref self, variantValue); @@ -282,6 +303,9 @@ namespace Godot.Collections /// /// Erases all items from this . /// + /// + /// The array is read-only. + /// public void Clear() => Resize(0); /// @@ -303,10 +327,15 @@ namespace Godot.Collections /// or the position at the end of the array. /// Existing items will be moved to the right. /// + /// + /// The array is read-only. + /// /// The index to insert at. /// The item to insert. public void Insert(int index, Variant item) { + ThrowIfReadOnly(); + if (index < 0 || index > Count) throw new ArgumentOutOfRangeException(nameof(index)); @@ -319,9 +348,14 @@ namespace Godot.Collections /// Removes the first occurrence of the specified /// from this . /// + /// + /// The array is read-only. + /// /// The value to remove. public bool Remove(Variant item) { + ThrowIfReadOnly(); + int index = IndexOf(item); if (index >= 0) { @@ -335,9 +369,14 @@ namespace Godot.Collections /// /// Removes an element from this by index. /// + /// + /// The array is read-only. + /// /// The index of the element to remove. public void RemoveAt(int index) { + ThrowIfReadOnly(); + if (index < 0 || index > Count) throw new ArgumentOutOfRangeException(nameof(index)); @@ -358,7 +397,28 @@ namespace Godot.Collections object ICollection.SyncRoot => false; - bool ICollection.IsReadOnly => false; + /// + /// Returns if the array is read-only. + /// See . + /// + public bool IsReadOnly => NativeValue.DangerousSelfRef.IsReadOnly; + + /// + /// Makes the read-only, i.e. disabled modying of the + /// array's elements. Does not apply to nested content, e.g. content of + /// nested arrays. + /// + public void MakeReadOnly() + { + if (IsReadOnly) + { + // Avoid interop call when the array is already read-only. + return; + } + + var self = (godot_array)NativeValue; + NativeFuncs.godotsharp_array_make_read_only(ref self); + } /// /// Copies the elements of this to the given @@ -472,6 +532,14 @@ namespace Godot.Collections { elem = NativeValue.DangerousSelfRef.Elements[index]; } + + private void ThrowIfReadOnly() + { + if (IsReadOnly) + { + throw new InvalidOperationException("Array instance is read-only."); + } + } } internal interface IGenericGodotArray @@ -592,6 +660,9 @@ namespace Godot.Collections /// /// Resizes this to the given size. /// + /// + /// The array is read-only. + /// /// The new size of the array. /// if successful, or an error code. public Error Resize(int newSize) @@ -602,6 +673,9 @@ namespace Godot.Collections /// /// Shuffles the contents of this into a random order. /// + /// + /// The array is read-only. + /// public void Shuffle() { _underlyingArray.Shuffle(); @@ -634,6 +708,9 @@ namespace Godot.Collections /// /// Returns the value at the given . /// + /// + /// The property is assigned and the array is read-only. + /// /// The value at the given . public unsafe T this[int index] { @@ -644,8 +721,11 @@ namespace Godot.Collections } set { + ThrowIfReadOnly(); + if (index < 0 || index >= Count) throw new ArgumentOutOfRangeException(nameof(index)); + var self = (godot_array)_underlyingArray.NativeValue; godot_variant* ptrw = NativeFuncs.godotsharp_array_ptrw(ref self); godot_variant* itemPtr = &ptrw[index]; @@ -673,10 +753,15 @@ namespace Godot.Collections /// or the position at the end of the array. /// Existing items will be moved to the right. /// + /// + /// The array is read-only. + /// /// The index to insert at. /// The item to insert. public void Insert(int index, T item) { + ThrowIfReadOnly(); + if (index < 0 || index > Count) throw new ArgumentOutOfRangeException(nameof(index)); @@ -688,6 +773,9 @@ namespace Godot.Collections /// /// Removes an element from this by index. /// + /// + /// The array is read-only. + /// /// The index of the element to remove. public void RemoveAt(int index) { @@ -703,16 +791,35 @@ namespace Godot.Collections /// The number of elements. public int Count => _underlyingArray.Count; - bool ICollection.IsReadOnly => false; + /// + /// Returns if the array is read-only. + /// See . + /// + public bool IsReadOnly => _underlyingArray.IsReadOnly; + + /// + /// Makes the read-only, i.e. disabled modying of the + /// array's elements. Does not apply to nested content, e.g. content of + /// nested arrays. + /// + public void MakeReadOnly() + { + _underlyingArray.MakeReadOnly(); + } /// /// Adds an item to the end of this . /// This is the same as append or push_back in GDScript. /// + /// + /// The array is read-only. + /// /// The item to add. /// The new size after adding the item. public void Add(T item) { + ThrowIfReadOnly(); + using var variantValue = VariantUtils.CreateFrom(item); var self = (godot_array)_underlyingArray.NativeValue; _ = NativeFuncs.godotsharp_array_add(ref self, variantValue); @@ -721,6 +828,9 @@ namespace Godot.Collections /// /// Erases all items from this . /// + /// + /// The array is read-only. + /// public void Clear() { _underlyingArray.Clear(); @@ -769,10 +879,15 @@ namespace Godot.Collections /// Removes the first occurrence of the specified value /// from this . /// + /// + /// The array is read-only. + /// /// The value to remove. /// A indicating success or failure. public bool Remove(T item) { + ThrowIfReadOnly(); + int index = IndexOf(item); if (index >= 0) { @@ -812,5 +927,13 @@ namespace Godot.Collections [MethodImpl(MethodImplOptions.AggressiveInlining)] public static explicit operator Array(Variant from) => from.AsGodotArray(); + + private void ThrowIfReadOnly() + { + if (IsReadOnly) + { + throw new InvalidOperationException("Array instance is read-only."); + } + } } } diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs index fa304c1b946..923b2adafd9 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Dictionary.cs @@ -93,10 +93,15 @@ namespace Godot.Collections /// By default, duplicate keys are not copied over, unless /// is . /// + /// + /// The dictionary is read-only. + /// /// Dictionary to copy entries from. /// If duplicate keys should be copied over as well. public void Merge(Dictionary dictionary, bool overwrite = false) { + ThrowIfReadOnly(); + var self = (godot_dictionary)NativeValue; var other = (godot_dictionary)dictionary.NativeValue; NativeFuncs.godotsharp_dictionary_merge(ref self, in other, overwrite.ToGodotBool()); @@ -175,8 +180,12 @@ namespace Godot.Collections /// /// Returns the value at the given . /// + /// + /// The property is assigned and the dictionary is read-only. + /// /// - /// An entry for does not exist in the dictionary. + /// The property is retrieved and an entry for + /// does not exist in the dictionary. /// /// The value at the given . public Variant this[Variant key] @@ -197,6 +206,8 @@ namespace Godot.Collections } set { + ThrowIfReadOnly(); + var self = (godot_dictionary)NativeValue; NativeFuncs.godotsharp_dictionary_set_value(ref self, (godot_variant)key.NativeVar, (godot_variant)value.NativeVar); @@ -207,6 +218,9 @@ namespace Godot.Collections /// Adds an value at key /// to this . /// + /// + /// The dictionary is read-only. + /// /// /// An entry for already exists in the dictionary. /// @@ -214,6 +228,8 @@ namespace Godot.Collections /// The value to add. public void Add(Variant key, Variant value) { + ThrowIfReadOnly(); + var variantKey = (godot_variant)key.NativeVar; var self = (godot_dictionary)NativeValue; @@ -230,8 +246,13 @@ namespace Godot.Collections /// /// Clears the dictionary, removing all entries from it. /// + /// + /// The dictionary is read-only. + /// public void Clear() { + ThrowIfReadOnly(); + var self = (godot_dictionary)NativeValue; NativeFuncs.godotsharp_dictionary_clear(ref self); } @@ -267,15 +288,22 @@ namespace Godot.Collections /// /// Removes an element from this by key. /// + /// + /// The dictionary is read-only. + /// /// The key of the element to remove. public bool Remove(Variant key) { + ThrowIfReadOnly(); + var self = (godot_dictionary)NativeValue; return NativeFuncs.godotsharp_dictionary_remove_key(ref self, (godot_variant)key.NativeVar).ToBool(); } bool ICollection>.Remove(KeyValuePair item) { + ThrowIfReadOnly(); + godot_variant variantKey = (godot_variant)item.Key.NativeVar; var self = (godot_dictionary)NativeValue; bool found = NativeFuncs.godotsharp_dictionary_try_get_value(ref self, @@ -311,7 +339,28 @@ namespace Godot.Collections } } - bool ICollection>.IsReadOnly => false; + /// + /// Returns if the dictionary is read-only. + /// See . + /// + public bool IsReadOnly => NativeValue.DangerousSelfRef.IsReadOnly; + + /// + /// Makes the read-only, i.e. disabled modying of the + /// dictionary's elements. Does not apply to nested content, e.g. content of + /// nested dictionaries. + /// + public void MakeReadOnly() + { + if (IsReadOnly) + { + // Avoid interop call when the dictionary is already read-only. + return; + } + + var self = (godot_dictionary)NativeValue; + NativeFuncs.godotsharp_dictionary_make_read_only(ref self); + } /// /// Gets the value for the given in the dictionary. @@ -397,6 +446,14 @@ namespace Godot.Collections using (str) return Marshaling.ConvertStringToManaged(str); } + + private void ThrowIfReadOnly() + { + if (IsReadOnly) + { + throw new InvalidOperationException("Dictionary instance is read-only."); + } + } } internal interface IGenericGodotDictionary @@ -508,6 +565,9 @@ namespace Godot.Collections /// By default, duplicate keys are not copied over, unless /// is . /// + /// + /// The dictionary is read-only. + /// /// Dictionary to copy entries from. /// If duplicate keys should be copied over as well. public void Merge(Dictionary dictionary, bool overwrite = false) @@ -536,6 +596,13 @@ namespace Godot.Collections /// /// Returns the value at the given . /// + /// + /// The property is assigned and the dictionary is read-only. + /// + /// + /// The property is retrieved and an entry for + /// does not exist in the dictionary. + /// /// The value at the given . public TValue this[TKey key] { @@ -557,6 +624,8 @@ namespace Godot.Collections } set { + ThrowIfReadOnly(); + using var variantKey = VariantUtils.CreateFrom(key); using var variantValue = VariantUtils.CreateFrom(value); var self = (godot_dictionary)_underlyingDict.NativeValue; @@ -616,10 +685,15 @@ namespace Godot.Collections /// Adds an object at key /// to this . /// + /// + /// The dictionary is read-only. + /// /// The key at which to add the object. /// The object to add. public void Add(TKey key, TValue value) { + ThrowIfReadOnly(); + using var variantKey = VariantUtils.CreateFrom(key); var self = (godot_dictionary)_underlyingDict.NativeValue; @@ -645,9 +719,14 @@ namespace Godot.Collections /// /// Removes an element from this by key. /// + /// + /// The dictionary is read-only. + /// /// The key of the element to remove. public bool Remove(TKey key) { + ThrowIfReadOnly(); + using var variantKey = VariantUtils.CreateFrom(key); var self = (godot_dictionary)_underlyingDict.NativeValue; return NativeFuncs.godotsharp_dictionary_remove_key(ref self, variantKey).ToBool(); @@ -683,7 +762,21 @@ namespace Godot.Collections /// The number of elements. public int Count => _underlyingDict.Count; - bool ICollection>.IsReadOnly => false; + /// + /// Returns if the dictionary is read-only. + /// See . + /// + public bool IsReadOnly => _underlyingDict.IsReadOnly; + + /// + /// Makes the read-only, i.e. disabled + /// modying of the dictionary's elements. Does not apply to nested content, + /// e.g. content of nested dictionaries. + /// + public void MakeReadOnly() + { + _underlyingDict.MakeReadOnly(); + } void ICollection>.Add(KeyValuePair item) => Add(item.Key, item.Value); @@ -691,6 +784,9 @@ namespace Godot.Collections /// /// Clears the dictionary, removing all entries from it. /// + /// + /// The dictionary is read-only. + /// public void Clear() => _underlyingDict.Clear(); bool ICollection>.Contains(KeyValuePair item) @@ -740,6 +836,8 @@ namespace Godot.Collections bool ICollection>.Remove(KeyValuePair item) { + ThrowIfReadOnly(); + using var variantKey = VariantUtils.CreateFrom(item.Key); var self = (godot_dictionary)_underlyingDict.NativeValue; bool found = NativeFuncs.godotsharp_dictionary_try_get_value(ref self, @@ -789,5 +887,13 @@ namespace Godot.Collections [MethodImpl(MethodImplOptions.AggressiveInlining)] public static explicit operator Dictionary(Variant from) => from.AsGodotDictionary(); + + private void ThrowIfReadOnly() + { + if (IsReadOnly) + { + throw new InvalidOperationException("Dictionary instance is read-only."); + } + } } } diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs index c295e500dee..43e7c7eb9ac 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs @@ -697,6 +697,9 @@ namespace Godot.NativeInterop private uint _safeRefCount; public VariantVector _arrayVector; + + private unsafe godot_variant* _readOnly; + // There are more fields here, but we don't care as we never store this in C# public readonly int Size @@ -704,6 +707,12 @@ namespace Godot.NativeInterop [MethodImpl(MethodImplOptions.AggressiveInlining)] get => _arrayVector.Size; } + + public readonly unsafe bool IsReadOnly + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => _readOnly != null; + } } [StructLayout(LayoutKind.Sequential)] @@ -737,6 +746,12 @@ namespace Godot.NativeInterop get => _p != null ? _p->Size : 0; } + public readonly unsafe bool IsReadOnly + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => _p != null && _p->IsReadOnly; + } + public unsafe void Dispose() { if (_p == null) @@ -766,35 +781,59 @@ namespace Godot.NativeInterop // A correctly constructed value needs to call the native default constructor to allocate `_p`. // Don't pass a C# default constructed `godot_dictionary` to native code, unless it's going to // be re-assigned a new value (the copy constructor checks if `_p` is null so that's fine). - [StructLayout(LayoutKind.Sequential)] + [StructLayout(LayoutKind.Explicit)] // ReSharper disable once InconsistentNaming public ref struct godot_dictionary { [MethodImpl(MethodImplOptions.AggressiveInlining)] internal readonly unsafe godot_dictionary* GetUnsafeAddress() - => (godot_dictionary*)Unsafe.AsPointer(ref Unsafe.AsRef(in _p)); + => (godot_dictionary*)Unsafe.AsPointer(ref Unsafe.AsRef(in _getUnsafeAddressHelper)); - private IntPtr _p; + [FieldOffset(0)] private byte _getUnsafeAddressHelper; - public readonly bool IsAllocated + [FieldOffset(0)] private unsafe DictionaryPrivate* _p; + + [StructLayout(LayoutKind.Sequential)] + private struct DictionaryPrivate { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get => _p != IntPtr.Zero; + private uint _safeRefCount; + + private unsafe godot_variant* _readOnly; + + // There are more fields here, but we don't care as we never store this in C# + + public readonly unsafe bool IsReadOnly + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => _readOnly != null; + } } - public void Dispose() + public readonly unsafe bool IsAllocated { - if (_p == IntPtr.Zero) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => _p != null; + } + + public readonly unsafe bool IsReadOnly + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => _p != null && _p->IsReadOnly; + } + + public unsafe void Dispose() + { + if (_p == null) return; NativeFuncs.godotsharp_dictionary_destroy(ref this); - _p = IntPtr.Zero; + _p = null; } [StructLayout(LayoutKind.Sequential)] // ReSharper disable once InconsistentNaming internal struct movable { - private IntPtr _p; + private unsafe DictionaryPrivate* _p; public static unsafe explicit operator movable(in godot_dictionary value) => *(movable*)CustomUnsafe.AsPointer(ref CustomUnsafe.AsRef(value)); diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs index d116284e1c1..1e23689c954 100644 --- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs +++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs @@ -376,6 +376,8 @@ namespace Godot.NativeInterop public static partial Error godotsharp_array_resize(ref godot_array p_self, int p_new_size); + public static partial void godotsharp_array_make_read_only(ref godot_array p_self); + public static partial void godotsharp_array_shuffle(ref godot_array p_self); public static partial void godotsharp_array_to_string(ref godot_array p_self, out godot_string r_str); @@ -416,6 +418,8 @@ namespace Godot.NativeInterop public static partial godot_bool godotsharp_dictionary_remove_key(ref godot_dictionary p_self, in godot_variant p_key); + public static partial void godotsharp_dictionary_make_read_only(ref godot_dictionary p_self); + public static partial void godotsharp_dictionary_to_string(ref godot_dictionary p_self, out godot_string r_str); // StringExtensions diff --git a/modules/mono/glue/runtime_interop.cpp b/modules/mono/glue/runtime_interop.cpp index e7f50d2caae..d17fe3e75ff 100644 --- a/modules/mono/glue/runtime_interop.cpp +++ b/modules/mono/glue/runtime_interop.cpp @@ -1012,6 +1012,10 @@ int32_t godotsharp_array_resize(Array *p_self, int32_t p_new_size) { return (int32_t)p_self->resize(p_new_size); } +void godotsharp_array_make_read_only(Array *p_self) { + p_self->make_read_only(); +} + void godotsharp_array_shuffle(Array *p_self) { p_self->shuffle(); } @@ -1081,6 +1085,10 @@ bool godotsharp_dictionary_remove_key(Dictionary *p_self, const Variant *p_key) return p_self->erase(*p_key); } +void godotsharp_dictionary_make_read_only(Dictionary *p_self) { + p_self->make_read_only(); +} + void godotsharp_dictionary_to_string(const Dictionary *p_self, String *r_str) { *r_str = Variant(*p_self).operator String(); } @@ -1439,6 +1447,7 @@ static const void *unmanaged_callbacks[]{ (void *)godotsharp_array_insert, (void *)godotsharp_array_remove_at, (void *)godotsharp_array_resize, + (void *)godotsharp_array_make_read_only, (void *)godotsharp_array_shuffle, (void *)godotsharp_array_to_string, (void *)godotsharp_dictionary_try_get_value, @@ -1454,6 +1463,7 @@ static const void *unmanaged_callbacks[]{ (void *)godotsharp_dictionary_merge, (void *)godotsharp_dictionary_recursive_equal, (void *)godotsharp_dictionary_remove_key, + (void *)godotsharp_dictionary_make_read_only, (void *)godotsharp_dictionary_to_string, (void *)godotsharp_string_simplify_path, (void *)godotsharp_string_to_camel_case,