Array<T> + AnyArray cast API consistency

Conversion methods between `Array<T>` and `AnyArray` values now follow the precedent of
`Gd::try_cast()` + `Gd::upcast()`, meaning:
* Upcast: `upcast_any_array`
* Downcast: `cast_array<T>`, `cast_var_array`

Also returns `self` instead of `ConvertError` on failure. This may need to be revisited
in the future, it's also possible (but more expensive?) to return a pair of both.

Author: Bromeon

godot-core/src/builtin/collections/any_array.rs

@@ -13,7 +13,7 @@ use sys::{ffi_methods, GodotFfi};
 use crate::builtin::iter::ArrayFunctionalOps;
 use crate::builtin::*;
 use crate::meta;
-use crate::meta::error::{ArrayMismatch, ConvertError, ErrorKind};
+use crate::meta::error::ConvertError;
 use crate::meta::{
     ArrayElement, ElementType, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot,
 };
@@ -38,15 +38,8 @@ use crate::meta::{
 ///
 /// `AnyArray` does not provide any operations where data flows _in_ to the array, such as `push()` or `insert()`.
 ///
-/// # Conversions
-/// Engine APIs accept `&AnyArray` parameters. Due to [deref coercion], you can pass any `&Array<T>` implicitly, even for `T=Variant`.
-///
-/// If you need to upcast `Array<T>` _values_ to `AnyArray`, use [`Array::into_any()`].
-///
-/// [deref coercion]: struct.Array.html#deref-methods-AnyArray
-///
-/// You can explicitly downcast an `AnyArray` using [`try_into_typed()`][Self::try_into_typed] and
-/// [`try_into_untyped()`][Self::try_into_untyped].
+/// ## Conversions
+/// See the [corresponding section in `Array`](struct.Array.html#conversions-between-arrays).
 #[derive(PartialEq, PartialOrd)]
 #[repr(transparent)] // Guarantees same layout as VarArray, enabling Deref from Array<T>.
 pub struct AnyArray {
@@ -215,7 +208,7 @@ impl AnyArray {
     /// To create a deep copy, use [`duplicate_deep()`][Self::duplicate_deep] instead.
     /// To create a new reference to the same array data, use [`clone()`][Clone::clone].
     pub fn duplicate_shallow(&self) -> AnyArray {
-        self.array.duplicate_shallow().into_any()
+        self.array.duplicate_shallow().upcast_any_array()
     }
 
     /// Returns a deep copy, duplicating nested `Array`/`Dictionary` elements but keeping `Object` elements shared.
@@ -225,7 +218,7 @@ impl AnyArray {
     /// To create a shallow copy, use [`duplicate_shallow()`][Self::duplicate_shallow] instead.
     /// To create a new reference to the same array data, use [`clone()`][Clone::clone].
     pub fn duplicate_deep(&self) -> Self {
-        self.array.duplicate_deep().into_any()
+        self.array.duplicate_deep().upcast_any_array()
     }
 
     /// Returns a sub-range as a new array.
@@ -238,7 +231,7 @@ impl AnyArray {
     #[doc(alias = "slice")]
     pub fn subarray_shallow(&self, range: impl meta::SignedRange, step: Option<i32>) -> Self {
         let sliced = self.array.subarray_shallow(range, step);
-        sliced.into_any()
+        sliced.upcast_any_array()
     }
 
     /// Returns a sub-range as a new `Array`.
@@ -251,7 +244,7 @@ impl AnyArray {
     #[doc(alias = "slice")]
     pub fn subarray_deep(&self, range: impl meta::SignedRange, step: Option<i32>) -> Self {
         let sliced = self.array.subarray_deep(range, step);
-        sliced.into_any()
+        sliced.upcast_any_array()
     }
 
     /// Returns an non-exclusive iterator over the elements of the `Array`.
@@ -407,9 +400,15 @@ impl AnyArray {
 
     /// Converts to `Array<T>` if the runtime type matches.
     ///
-    /// Returns `Err` if the array's dynamic type differs from `T`. Check [`element_type()`][Self::element_type]
+    /// If `T=Variant`, this will attempt to "downcast" to an untyped array, identical to [`try_cast_var_array()`][Self::try_cast_var_array].
+    ///
+    /// Returns `Err(self)` if the array's dynamic type differs from `T`. Check [`element_type()`][Self::element_type]
     /// before calling to determine what type the array actually holds.
-    pub fn try_into_typed<T: ArrayElement>(self) -> Result<Array<T>, ConvertError> {
+    ///
+    /// Consumes `self`, to avoid incrementing reference-count. Use `clone()` if you need to keep the original. Using `self` also has the nice
+    /// side effect that this method cannot be called on concrete `Array<T>` types, as `Deref` only operates on references, not values.
+    // Naming: not `try_into_typed` because T can be Variant.
+    pub fn try_cast_array<T: ArrayElement>(self) -> Result<Array<T>, Self> {
         let from_type = self.array.element_type();
         let to_type = ElementType::of::<T>();
 
@@ -418,19 +417,22 @@ impl AnyArray {
             let array = unsafe { self.array.assume_type::<T>() };
             Ok(array)
         } else {
-            let mismatch = ArrayMismatch {
-                expected: to_type,
-                actual: from_type,
-            };
-            Err(ConvertError::with_kind(ErrorKind::FromOutArray(mismatch)))
+            // If we add ConvertError here:
+            // let mismatch = ArrayMismatch { expected: to_type, actual: from_type };
+            // Err(ConvertError::with_kind(ErrorKind::FromAnyArray(mismatch)))
+
+            Err(self)
         }
     }
 
-    /// Converts to `VarArray` if the array is untyped.
+    /// Converts to an untyped `VarArray` if the array is untyped.
+    ///
+    /// This is a shorthand for [`try_cast_array::<Variant>()`][Self::try_cast_array].
     ///
-    /// This is a shorthand for [`try_into_typed::<Variant>()`][Self::try_into_typed].
-    pub fn try_into_untyped(self) -> Result<VarArray, ConvertError> {
-        self.try_into_typed::<Variant>()
+    /// Consumes `self`, to avoid incrementing reference-count. Use `clone()` if you need to keep the original. Using `self` also has the nice
+    /// side effect that this method cannot be called on concrete `Array<T>` types, as `Deref` only operates on references, not values.
+    pub fn try_cast_var_array(self) -> Result<VarArray, Self> {
+        self.try_cast_array::<Variant>()
     }
 
     // If we add direct-conversion methods that panic, we can use meta::element_godot_type_name::<T>() to mention type in case of mismatch.

godot-core/src/builtin/collections/array.rs

@@ -33,7 +33,6 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// Check out the [book](https://godot-rust.github.io/book/godot-api/builtins.html#arrays-and-dictionaries) for a tutorial on arrays.
 ///
 /// # Reference semantics
-///
 /// Like in GDScript, `Array` acts as a reference type: multiple `Array` instances may
 /// refer to the same underlying array, and changes to one are visible in the other.
 ///
@@ -42,7 +41,6 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// or [`duplicate_deep()`][Self::duplicate_deep].
 ///
 /// # Element type
-///
 /// Godot's `Array` builtin can be either typed or untyped. In Rust, this maps to three representations:
 ///
 /// - Untyped array: `VariantArray`, a type alias for `Array<Variant>`.  \
@@ -55,14 +53,29 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 ///   Represents either typed or untyped arrays. This is different from `Array<Variant>`, because the latter allows you to insert
 ///   `Variant` objects. However, for an array that has dynamic type `i64`, it is not allowed to insert any variants that are not `i64`.
 ///
-/// Godot APIs reflect this. You can `Deref`-coerce from any `Array<T>` to `AnyArray` (e.g. when passing arguments), and explicitly
-/// convert the other way, using [`AnyArray::try_into_typed()`] and [`AnyArray::try_into_untyped()`].
-///
 /// If you plan to use any integer or float types apart from `i64` and `f64`, read
 /// [this documentation](../meta/trait.ArrayElement.html#integer-and-float-types).
 ///
-/// ## Typed array example
+/// ## Conversions between arrays
+/// The following table gives an overview of useful conversions.
+///
+/// | From               | To               | Conversion method                                        |
+/// |--------------------|------------------|----------------------------------------------------------|
+/// | `AnyArray`         | `Array<T>`       | [`AnyArray::try_cast_array::<T>`]                        |
+/// | `AnyArray`         | `Array<Variant>` | [`AnyArray::try_cast_var_array`]                         |
+/// | `&Array<T>`        | `&AnyArray`      | Implicit via [deref coercion]; often used in class APIs. |
+/// | `Array<T>`         | `AnyArray`       | [`Array<T>::upcast_any_array`]                           |
+/// | `Array<T>`         | `PackedArray<T>` | [`Array<T>::to_packed_array`]                            |
+/// | `PackedArray<T>`   | `Array<T>`       | [`PackedArray<T>::to_typed_array`]                       |
+/// | `PackedArray<T>`   | `Array<Variant>` | [`PackedArray<T>::to_var_array`]                         |
 ///
+/// Note that it's **not** possible to upcast `Array<Gd<Derived>>` to `Array<Gd<Base>>`. `Array<T>` is not covariant over the `T` parameter,
+/// otherwise it would be possible to insert `Base` pointers that aren't actually `Derived`.
+// Note: the above could theoretically be implemented by making AnyArray<T> generic and covariant over T.
+///
+/// [deref coercion]: struct.Array.html#deref-methods-AnyArray
+///
+/// ## Typed array example
 /// ```no_run
 /// # use godot::prelude::*;
 /// // Create typed Array<i64> and add values.
@@ -95,7 +108,6 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// ```
 ///
 /// ## Untyped array example
-///
 /// ```no_run
 /// # use godot::prelude::*;
 /// // VarArray allows dynamic element types.
@@ -116,7 +128,6 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// ```
 ///
 /// # Working with signed ranges and steps
-///
 /// For negative indices, use [`wrapped()`](crate::meta::wrapped).
 ///
 /// ```no_run
@@ -145,14 +156,12 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 /// ```
 ///
 /// # Thread safety
-///
 /// Usage is safe if the `Array` is used on a single thread only. Concurrent reads on
 /// different threads are also safe, but any writes must be externally synchronized. The Rust
 /// compiler will enforce this as long as you use only Rust threads, but it cannot protect against
 /// concurrent modification on other threads (e.g. created through GDScript).
 ///
 /// # Element type safety
-///
 /// We provide a richer set of element types than Godot, for convenience and stronger invariants in your _Rust_ code.
 /// This, however, means that the Godot representation of such arrays is not capable of incorporating the additional "Rust-side" information.
 /// This can lead to situations where GDScript code or the editor UI can insert values that do not fulfill the Rust-side invariants.
@@ -168,11 +177,9 @@ use crate::registry::property::{BuiltinExport, Export, Var};
 ///   Godot doesn't know Rust traits and will only see the `T` part.
 ///
 /// # Differences from GDScript
-///
 /// Unlike GDScript, all indices and sizes are unsigned, so negative indices are not supported.
 ///
 /// # Godot docs
-///
 /// [`Array[T]` (stable)](https://docs.godotengine.org/en/stable/classes/class_array.html)
 pub struct Array<T: ArrayElement> {
     // Safety Invariant: The type of all values in `opaque` matches the type `T`.
@@ -761,7 +768,7 @@ impl<T: ArrayElement> Array<T> {
     ///
     /// Typically, you can use deref coercion to convert `&Array<T>` to `&AnyArray`. This method is useful if you need `AnyArray` by value.
     /// It consumes `self` to avoid incrementing the reference count; use `clone()` if you use the original array further.
-    pub fn into_any(self) -> AnyArray {
+    pub fn upcast_any_array(self) -> AnyArray {
         AnyArray::from_typed_or_untyped(self)
     }
 
@@ -890,6 +897,7 @@ impl<T: ArrayElement> Array<T> {
     /// Note also that any `GodotType` can be written to a `Variant` array.
     ///
     /// In the current implementation, both cases will produce a panic rather than undefined behavior, but this should not be relied upon.
+    #[cfg(safeguards_strict)]
     unsafe fn assume_type_ref<U: ArrayElement>(&self) -> &Array<U> {
         // The memory layout of `Array<T>` does not depend on `T`.
         std::mem::transmute::<&Array<T>, &Array<U>>(self)
@@ -1088,12 +1096,6 @@ impl<T: ArrayElement> Array<T> {
     }
 }
 
-#[test]
-fn correct_variant_t() {
-    assert!(Array::<Variant>::has_variant_t());
-    assert!(!Array::<i64>::has_variant_t());
-}
-
 impl VarArray {
     /// # Safety
     /// - Variant must have type `VariantType::ARRAY`.
@@ -1371,6 +1373,9 @@ impl<T: ArrayElement> GodotFfiVariant for Array<T> {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Conversion traits
 
+// From impls converting values (like [T; N] or Vec<T>) are explicitly not supported, because there's no perf benefit in submitting ownership.
+// This is different for PackedArray, which can move values. See also related: https://github.com/godot-rust/gdext/pull/1286.
+
 /// Creates a `Array` from the given Rust array.
 impl<T: ArrayElement + ToGodot, const N: usize> From<&[T; N]> for Array<T> {
     fn from(arr: &[T; N]) -> Self {
@@ -1444,6 +1449,7 @@ impl<T: ArrayElement + FromGodot> From<&Array<T>> for Vec<T> {
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
+// Iterators
 
 /// An iterator over typed elements of an [`Array`].
 pub struct Iter<'a, T: ArrayElement> {
@@ -1515,6 +1521,7 @@ impl<T: ArrayElement> PartialOrd for Array<T> {
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
+// Expression macros
 
 /// Constructs [`Array`] literals, similar to Rust's standard `vec!` macro.
 ///
@@ -1634,6 +1641,7 @@ macro_rules! vslice {
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
+// Serde support
 
 #[cfg(feature = "serde")]
 mod serialize {
@@ -1704,3 +1712,36 @@ mod serialize {
         }
     }
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Tests
+
+/// Verifies that `AnyArray::try_cast*()` cannot be called on concrete arrays.
+///
+/// > error[E0507]: cannot move out of dereference of `godot::prelude::Array<i64>`
+///
+/// ```compile_fail
+/// use godot::prelude::*;
+/// let a: Array<i64> = array![1, 2, 3];
+/// a.try_cast_var_array();
+/// ```
+///
+/// ```compile_fail
+/// use godot::prelude::*;
+/// let a: VarArray = varray![1, 2, 3];
+/// a.try_cast_array::<i64>();
+/// ```
+///
+/// This works:
+/// ```no_run
+/// use godot::prelude::*;
+/// let a: VarArray = varray![1, 2, 3];
+/// a.upcast_any_array().try_cast_array::<i64>();
+/// ```
+fn __cannot_downcast_from_concrete() {}
+
+#[test]
+fn correct_variant_t() {
+    assert!(Array::<Variant>::has_variant_t());
+    assert!(!Array::<i64>::has_variant_t());
+}

godot-core/src/builtin/collections/packed_array.rs

@@ -87,6 +87,9 @@ pub type PackedColorArray = PackedArray<Color>;
 /// but any writes must be externally synchronized. The Rust compiler will enforce this as
 /// long as you use only Rust threads, but it cannot protect against concurrent modification
 /// on other threads (e.g. created through GDScript).
+///
+/// # Element type and conversions
+/// See the [corresponding section in `Array`](struct.Array.html#conversions-between-arrays).
 pub struct PackedArray<T: PackedArrayElement> {
     // All packed arrays have same memory layout.
     opaque: sys::types::OpaquePackedByteArray,

godot-core/src/meta/error/convert_error.rs

@@ -36,6 +36,7 @@ impl ConvertError {
     }
 
     /// Create a new custom error for a conversion, without associated value.
+    #[allow(dead_code)] // Needed a few times already, stays to prevent churn on refactorings.
     pub(crate) fn with_kind(kind: ErrorKind) -> Self {
         Self { kind, value: None }
     }
@@ -162,7 +163,7 @@ pub(crate) enum ErrorKind {
     FromGodot(FromGodotError),
     FromFfi(FromFfiError),
     FromVariant(FromVariantError),
-    FromOutArray(ArrayMismatch),
+    // FromAnyArray(ArrayMismatch), -- needed if AnyArray downcasts return ConvertError one day.
     Custom(Option<Cause>),
 }
 
@@ -172,7 +173,6 @@ impl fmt::Display for ErrorKind {
             Self::FromGodot(from_godot) => write!(f, "{from_godot}"),
             Self::FromVariant(from_variant) => write!(f, "{from_variant}"),
             Self::FromFfi(from_ffi) => write!(f, "{from_ffi}"),
-            Self::FromOutArray(array_mismatch) => write!(f, "{array_mismatch}"),
             Self::Custom(cause) => match cause {
                 Some(c) => write!(f, "{c}"),
                 None => write!(f, "custom error"),

godot-core/src/obj/traits.rs

@@ -773,7 +773,7 @@ pub mod cap {
     use super::*;
     use crate::builtin::{StringName, Variant};
     use crate::meta::PropertyInfo;
-    use crate::obj::{Base, Bounds, Gd};
+    use crate::obj::{Base, Gd};
     use crate::storage::{IntoVirtualMethodReceiver, VirtualMethodReceiver};
 
     /// Trait for all classes that are default-constructible from the Godot engine.