Sync C# Array with Core

[raulsntos]

Summary

• Add AddRange method.
  • There's precedent in .NET for a method named AddRange (e.g.: List<T>).
  • Serves as the equivalent of GDScript's append_array. Added to Core in Add append_array() method to Array class #43398.
  • Uses similar naming to Add which is the equivalent of GDScript's append.
  • Should offer better performance than adding items in a for loop because the implementation checks if the IEnumerable is a Godot array and uses a single interop call; otherwise, falls back to iterating the collection.
• Add Fill method.
  • Added to Core in Add fill method to Arrays and PackedArrays #46476.
  • Should offer better performance than setting the items from C# as that would require multiple interop calls.
• Add Max and Min methods.
  • Added to Core in adc0188.
  • Should offer better performance than iterating the array from C# or using LINQ as that would require marshaling every item.
• Add PickRandom method.
  • Added to Core in Add ability to pick random value from array #67444.
  • Users may want to consider using System.Random instead.
• Add Reverse method.
  • There's precedent in .NET for a method named Reverse that reverses in place (e.g.: List<T>).
  • Also implmented in Core and exposed to scripting.
  • Should offer better performance than reversing the array from C# or using LINQ as that would require marshaling every item.
• Add RecursiveEqual method.
  • Serves as the equivalent of GDScript's == and != operators which compare by value. These operators still compare by reference in .NET since that's likely the users expectation as all .NET collections work like this.
  • Should offer better performance than comparing iterating and comparing the array items from C# or using LINQ as that would require marshaling every item.
  • Intentionally avoiding the name SequenceEqual to avoid confusing users with the behavior differences with Enumerable.SequenceEqual (see Sync C# Array with Core #71786 (comment)).
• Add Slice and GetSliceRange methods.
  • There's precedent in .NET for a method named Slice that returns a value of the same type (e.g.: List<T>, Span, ImmutableArray, ...).
  • Implements the Slice method with the needed signature to get implicit Range support.
  • Should offer better performance than creating a new array and copying the items from C# or using LINQ as that would require marshaling every item.
  • Also implements a GetSliceRange which follows more closely the behavior of GDScript's slice method and allows to specify a step and deep parameters.
    • I explicitly chose a different name since Slice methods in .NET usually use start and length as parameter, but Godot's slice method uses start and end which has a different meaning and could be confusing as overloads.
• Add Sort method.
  • There's precedent in .NET for a method named Sort that sorts in place (e.g.: List<T>).
  • Also implmented in Core and exposed to scripting.
  • Should offer better performance than sorting the array from C# or using LINQ as that would require marshaling every item.
• Add IndexOf method overload that takes index parameter.
  • There's precedent in .NET for a method named IndexOf with the same parameters (e.g.: List<T>).
  • GDScript's find method also allows for an optional index parameter.
• Add LastIndexOf method.
  • There's precedent in .NET for a method named LastIndexOf with the same overloads (e.g.: List<T>).
  • Serves as the equivalent of GDScript's rfind.
  • Matches the naming of IndexOf which we use as the equivalent of GDScript's find.
• Add BinarySearch method.
  • There's precedent in .NET for a method named BinarySearch (e.g.: List<T>).
  • Added to Core in Add bsearch and bsearch_custom to Array #12590.
  • Should offer better performance than implementing it manually in C# as that would require marshaling every item.
• Add/update documentation.

Methods not added

• All the methods that take Callable (usually their name ends with _custom, e.g.: sort_custom, all, any).
  • Since these methods would take a Callable created in C# (likely a C# lambda), it would end up marshalling every item to call the C# Callable for each item, thus there does not seem to be any performance gain compared to just using LINQ which C# developers would already be familiar with, so I see no benefit.
• All "type"-related methods (get_typed_builtin, get_typed_class_name, get_typed_script, is_typed, typed_assign, set_typed) since they feel very out of place in .NET.
  • They seem like they were made to be used in a dynamic language like GDScript.
  • Some of their behavior can probably be replicated in .NET by using the typing system, pattern matching and the constructors and explicit conversions implemented.
• hash.
  • We may want to consider implementing GetHashCode instead.
  • Users can probably use GD.Hash as well, so maybe we don't want to implement it in the Array class.
• bsearch.
  • I think this functionality is too specific and should probably be added as an extension rather than in the Array class, if at all. (I ended up implementing it as BinarySearch).
• count because its name conflicts with the Count property, we would have to think of a different name, it also doesn't seem as useful to me as the other methods.
  • As a workaround users today can use LINQ (array.Count(x => x == value)), although the performance won't be great because it would have to marshal every item.
  • If this method turns out to be very popular we can always add it in the future without breaking compatibility so I don't see a strong reason to add it now.
• Push and pop methods (push_front, push_back, pop_front, pop_back, pop_at).
  • Users can already use the existing Add, Insert, Remove and RemoveAt so there doesn't seem to be much benefit in adding these methods.
  • They feel out of place in a .NET collection that implements IList<T> since those methods sound like they'd fit better in a Queue or Stack collection.
• Comparison operators (<, <=, >, >=) because they don't seem particularly useful or intuitive.
  • We can always add them in the future without breaking compatibility so I don't see a strong reason to add them now.

[raulsntos]

Not sure if resizing the array instead of growing as the items are added makes that much of a difference and is worth the complexity.

[RedworkDE]

About bsearch: BinarySearch is a static (not extension) method on System.Array; instance method on System.Collections.Generic.List<T>; and an extension method on System.Span<T> (basically everything is an extension method on spans) so the BCL is not exactly sure what that method is supposed to be. Since I don't think there are currently any extension methods for Godot.Collections.Array I would probably just add it as an instance method as well.

[raulsntos]

I ended up adding bsearch with the name BinarySearch as an instance method to follow List<T>.

[raulsntos]

I removed the changes about readonly-ness since I'm not convinced it was the proper way to do it and don't want to back ourselves into a corner by implementing something we can't break compat with later. I'll extract it to a different PR (see #71986) so it can be merged separately and doesn't hold back this PR.

[RedworkDE]

Suggested change

	/// all nested arrays and dictionaries are duplicated and will not be shared with
	/// the original array. If <see langword="false"/>, a shallow copy is made and
	/// references to the original nested arrays and dictionaries are kept, so that
	/// modifying a sub-array or dictionary in the copy will also impact those
	/// referenced in the source array. Note that any <see cref="Object"/> derived
	/// elements will be shallow copied regardless of the <paramref name="deep"/>
	/// setting.
	/// all nested arrays and dictionaries are recursively duplicated and will not be shared with
	/// the original array. If <see langword="false"/>, a shallow copy is made and
	/// references to the original nested arrays and dictionaries are kept, so that
	/// modifying a sub-array or dictionary in the copy will also impact those
	/// referenced in the source array. Note that any <see cref="Object"/> derived
	/// elements will not be copied regardless of the <paramref name="deep"/>
	/// setting.

Should probably be changed in the classref as well. At least the bit about objects being shallow copied is very miss understandable / being even more explicit then my suggestion may be even better.

[neikeq]

Does our version of binary search return different values (to match Godot) than that of some core C# classes? If that's the case, to avoid confusion, I think it would be better to name our version BSearch (after Godot's bsearch) rather than BinarySearch (used by those core C# classes).

[raulsntos]

The current implementation of Godot.Collections.Array.BinarySearch in this PR follows the .NET implementation (e.g.: System.Array.BinarySearch) which returns a bitwise complement of the result if the searched item was NOT in the array. This is why I kept the BinarySearch name; otherwise, I would agree that naming it differently would be better.

I also don't think it's useful to provide a BSearch implementation that follows Godot's bsearch having BinarySearch which provides more information, if the result is negative doing a bitwise complement should give you the same result than Godot's bsearch, as far as I understand it.

[akien-mga]

Thanks!