Merge #151

151: Rename `Array` -> `VariantArray`, `TypedArray` -> `Array` r=Bromeon a=Bromeon

Suggests that `Array<T>` is the default and preferred way of using arrays (with type-safe elements), while `VariantArray` is more a special case of untyped arrays.

We probably need to observe usage to see whether this makes sense. An alternative would be `VArray` instead of `VariantArray`, but that would require a frequent occurrence so that the shortened identifier pays off.

bors try

Co-authored-by: Jan Haller <[email protected]>

Author: bors[bot]

godot-codegen/src/class_generator.rs

@@ -431,7 +431,7 @@ fn make_enums(enums: &Option<Vec<Enum>>, _class_name: &TyName, _ctx: &Context) -
 fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStream {
     if class_name.godot_ty == "Array" {
         quote! {
-            pub fn from_outer_typed<T>(outer: &TypedArray<T>) -> Self
+            pub fn from_outer_typed<T>(outer: &Array<T>) -> Self
                 where T: crate::builtin::meta::VariantMetadata
             {
                 Self {

godot-codegen/src/util.rs

@@ -177,10 +177,11 @@ fn to_hardcoded_rust_type(ty: &str) -> Option<&str> {
         "int" => "i64",
         "float" => "f64",
         "String" => "GodotString",
+        "Array" => "VariantArray",
         //"enum::Error" => "GodotError",
         "enum::Variant.Type" => "VariantType",
-        "enum::Variant.Operator" => "VariantOperator", // currently not used, but future-proof
-        "enum::Vector3.Axis" => "Vector3Axis",         // TODO automate this
+        "enum::Variant.Operator" => "VariantOperator",
+        "enum::Vector3.Axis" => "Vector3Axis",
         _ => return None,
     };
     Some(result)
@@ -251,10 +252,10 @@ fn to_rust_type_uncached(ty: &str, ctx: &mut Context) -> RustTy {
 
         let rust_elem_ty = to_rust_type(elem_ty, ctx);
         return if ctx.is_builtin(elem_ty) {
-            RustTy::BuiltinArray(quote! { TypedArray<#rust_elem_ty> })
+            RustTy::BuiltinArray(quote! { Array<#rust_elem_ty> })
         } else {
             RustTy::EngineArray {
-                tokens: quote! { TypedArray<#rust_elem_ty> },
+                tokens: quote! { Array<#rust_elem_ty> },
                 elem_class: elem_ty.to_string(),
             }
         };

godot-core/src/builtin/array.rs

@@ -14,7 +14,7 @@ use std::ptr::addr_of_mut;
 use sys::types::OpaqueDictionary;
 use sys::{ffi_methods, interface_fn, GodotFfi};
 
-use super::Array;
+use super::VariantArray;
 
 /// Godot's `Dictionary` type.
 ///
@@ -23,7 +23,7 @@ use super::Array;
 ///
 /// # Thread safety
 ///
-/// The same principles apply as for [`Array`]. Consult its documentation for details.
+/// The same principles apply as for [`VariantArray`]. Consult its documentation for details.
 #[repr(C)]
 pub struct Dictionary {
     opaque: OpaqueDictionary,
@@ -131,7 +131,7 @@ impl Dictionary {
     /// Returns `true` if the dictionary contains all the given keys.
     ///
     /// _Godot equivalent: `has_all`_
-    pub fn contains_all_keys(&self, keys: Array) -> bool {
+    pub fn contains_all_keys(&self, keys: VariantArray) -> bool {
         self.as_inner().has_all(keys)
     }
 
@@ -143,14 +143,14 @@ impl Dictionary {
     /// Creates a new `Array` containing all the keys currently in the dictionary.
     ///
     /// _Godot equivalent: `keys`_
-    pub fn keys_array(&self) -> Array {
+    pub fn keys_array(&self) -> VariantArray {
         self.as_inner().keys()
     }
 
     /// Creates a new `Array` containing all the values currently in the dictionary.
     ///
     /// _Godot equivalent: `values`_
-    pub fn values_array(&self) -> Array {
+    pub fn values_array(&self) -> VariantArray {
         self.as_inner().values()
     }
 
@@ -464,7 +464,7 @@ impl<'a> Keys<'a> {
     }
 
     /// Returns an array of the keys
-    pub fn array(self) -> Array {
+    pub fn array(self) -> VariantArray {
         // Can only be called
         assert!(self.iter.is_first);
         self.iter.dictionary.keys_array()

godot-core/src/builtin/dictionary.rs

@@ -35,7 +35,7 @@
 // Re-export macros.
 pub use crate::{array, dict, varray};
 
-pub use array_inner::{Array, TypedArray};
+pub use array_inner::{Array, VariantArray};
 pub use basis::*;
 pub use color::*;
 pub use dictionary_inner::Dictionary;

godot-core/src/builtin/mod.rs

@@ -376,7 +376,7 @@ macro_rules! impl_packed_array {
             }
         }
 
-        impl_builtin_froms!($PackedArray; Array => $from_array);
+        impl_builtin_froms!($PackedArray; VariantArray => $from_array);
 
         impl fmt::Debug for $PackedArray {
             fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

godot-core/src/builtin/packed_array.rs

@@ -9,12 +9,12 @@ use godot::prelude::*;
 
 #[itest]
 fn array_default() {
-    assert_eq!(Array::default().len(), 0);
+    assert_eq!(VariantArray::default().len(), 0);
 }
 
 #[itest]
 fn array_new() {
-    assert_eq!(Array::new().len(), 0);
+    assert_eq!(VariantArray::new().len(), 0);
 }
 
 #[itest]
@@ -31,22 +31,22 @@ fn array_eq() {
 fn typed_array_from_to_variant() {
     let array = array![1, 2];
     let variant = array.to_variant();
-    let result = TypedArray::try_from_variant(&variant);
+    let result = Array::try_from_variant(&variant);
     assert_eq!(result, Ok(array));
 }
 
 #[itest]
 fn untyped_array_from_to_variant() {
     let array = varray![1, 2];
     let variant = array.to_variant();
-    let result = Array::try_from_variant(&variant);
+    let result = VariantArray::try_from_variant(&variant);
     assert_eq!(result, Ok(array));
 }
 
 #[itest]
 fn array_from_packed_array() {
     let packed_array = PackedInt32Array::from(&[42]);
-    let mut array = Array::from(&packed_array);
+    let mut array = VariantArray::from(&packed_array);
     // This tests that the resulting array doesn't secretly have a runtime type assigned to it,
     // which is not reflected in our static type. It would make sense if it did, but Godot decided
     // otherwise: we get an untyped array.
@@ -56,7 +56,7 @@ fn array_from_packed_array() {
 
 #[itest]
 fn array_from_iterator() {
-    let array = TypedArray::from_iter([1, 2]);
+    let array = Array::from_iter([1, 2]);
 
     assert_eq!(array.len(), 2);
     assert_eq!(array.get(0), 1);
@@ -65,7 +65,7 @@ fn array_from_iterator() {
 
 #[itest]
 fn array_from_slice() {
-    let array = TypedArray::from(&[1, 2]);
+    let array = Array::from(&[1, 2]);
 
     assert_eq!(array.len(), 2);
     assert_eq!(array.get(0), 1);
@@ -108,7 +108,7 @@ fn array_duplicate_shallow() {
     let subarray = array![2, 3];
     let array = varray![1, subarray];
     let duplicate = array.duplicate_shallow();
-    TypedArray::<i64>::try_from_variant(&duplicate.get(1))
+    Array::<i64>::try_from_variant(&duplicate.get(1))
         .unwrap()
         .set(0, 4);
     assert_eq!(subarray.get(0), 4);
@@ -119,7 +119,7 @@ fn array_duplicate_deep() {
     let subarray = array![2, 3];
     let array = varray![1, subarray];
     let duplicate = array.duplicate_deep();
-    TypedArray::<i64>::try_from_variant(&duplicate.get(1))
+    Array::<i64>::try_from_variant(&duplicate.get(1))
         .unwrap()
         .set(0, 4);
     assert_eq!(subarray.get(0), 2);
@@ -134,7 +134,7 @@ fn array_slice_shallow() {
     let subarray = array![2, 3];
     let array = varray![1, subarray];
     let slice = array.slice_shallow(1, 2, None);
-    TypedArray::<i64>::try_from_variant(&slice.get(0))
+    Array::<i64>::try_from_variant(&slice.get(0))
         .unwrap()
         .set(0, 4);
     assert_eq!(subarray.get(0), 4);
@@ -149,7 +149,7 @@ fn array_slice_deep() {
     let subarray = array![2, 3];
     let array = varray![1, subarray];
     let slice = array.slice_deep(1, 2, None);
-    TypedArray::<i64>::try_from_variant(&slice.get(0))
+    Array::<i64>::try_from_variant(&slice.get(0))
         .unwrap()
         .set(0, 4);
     assert_eq!(subarray.get(0), 2);
@@ -173,7 +173,7 @@ fn array_first_last() {
     assert_eq!(array.first(), Some(1));
     assert_eq!(array.last(), Some(2));
 
-    let empty_array = Array::new();
+    let empty_array = VariantArray::new();
 
     assert_eq!(empty_array.first(), None);
     assert_eq!(empty_array.last(), None);
@@ -220,15 +220,15 @@ fn array_min_max() {
     assert_eq!(uncomparable_array.min(), None);
     assert_eq!(uncomparable_array.max(), None);
 
-    let empty_array = Array::new();
+    let empty_array = VariantArray::new();
 
     assert_eq!(empty_array.min(), None);
     assert_eq!(empty_array.max(), None);
 }
 
 #[itest]
 fn array_pick_random() {
-    assert_eq!(Array::new().pick_random(), None);
+    assert_eq!(VariantArray::new().pick_random(), None);
     assert_eq!(array![1].pick_random(), Some(1));
 }
 
@@ -330,10 +330,7 @@ fn array_mixed_values() {
         PackedByteArray::try_from_variant(&array.get(2)).unwrap(),
         packed_array
     );
-    assert_eq!(
-        TypedArray::try_from_variant(&array.get(3)).unwrap(),
-        typed_array
-    );
+    assert_eq!(Array::try_from_variant(&array.get(3)).unwrap(), typed_array);
     assert_eq!(
         Gd::<Object>::try_from_variant(&array.get(4))
             .unwrap()
@@ -384,7 +381,7 @@ fn untyped_array_return_from_godot_func() {
     assert_eq!(result, varray![child, Variant::nil(), NodePath::default()]);
 }
 
-// TODO All API functions that take a `TypedArray` are even more obscure and not included in
+// TODO All API functions that take a `Array` are even more obscure and not included in
 // `SELECTED_CLASSES`. Decide if this test is worth having `Texture2DArray` and `Image` and their
 // ancestors in the list.
 #[itest]
@@ -431,22 +428,22 @@ struct ArrayTest;
 #[godot_api]
 impl ArrayTest {
     #[func]
-    fn pass_untyped_array(&self, array: Array) -> i64 {
+    fn pass_untyped_array(&self, array: VariantArray) -> i64 {
         array.len().try_into().unwrap()
     }
 
     #[func]
-    fn return_untyped_array(&self) -> Array {
+    fn return_untyped_array(&self) -> VariantArray {
         varray![42, "answer"]
     }
 
     #[func]
-    fn pass_typed_array(&self, array: TypedArray<i64>) -> i64 {
+    fn pass_typed_array(&self, array: Array<i64>) -> i64 {
         array.iter_shared().sum()
     }
 
     #[func]
-    fn return_typed_array(&self, n: i64) -> TypedArray<i64> {
+    fn return_typed_array(&self, n: i64) -> Array<i64> {
         (1..(n + 1)).collect()
     }
 }

itest/rust/src/array_test.rs

@@ -25,7 +25,7 @@ fn test_builtins_vector2() {
 
 #[itest]
 fn test_builtins_array() {
-    let array = Array::default();
+    let array = VariantArray::default();
     let mut inner: InnerArray = array.as_inner();
 
     let a = 7.to_variant();

itest/rust/src/builtin_test.rs

@@ -5,7 +5,7 @@
  */
 
 use godot::bind::{godot_api, GodotClass};
-use godot::builtin::{Array, ToVariant, Variant};
+use godot::builtin::{ToVariant, Variant, VariantArray};
 
 use crate::RustTestCase;
 use std::time::{Duration, Instant};
@@ -25,7 +25,7 @@ impl IntegrationTests {
     #[func]
     fn run_all_tests(
         &mut self,
-        gdscript_tests: Array,
+        gdscript_tests: VariantArray,
         gdscript_file_count: i64,
         allow_focus: bool,
     ) -> bool {
@@ -72,7 +72,7 @@ impl IntegrationTests {
         }
     }
 
-    fn run_gdscript_tests(&mut self, tests: Array) {
+    fn run_gdscript_tests(&mut self, tests: VariantArray) {
         let mut last_file = None;
         for test in tests.iter_shared() {
             let result = test.call("run", &[]);

itest/rust/src/runner.rs

@@ -10,7 +10,7 @@ use godot::builtin::{
 };
 use godot::engine::Node2D;
 use godot::obj::InstanceId;
-use godot::prelude::{Array, Basis, Dictionary, VariantConversionError};
+use godot::prelude::{Basis, Dictionary, VariantArray, VariantConversionError};
 use godot::sys::{GodotFfi, VariantOperator, VariantType};
 use std::cmp::Ordering;
 use std::fmt::{Debug, Display};
@@ -303,7 +303,7 @@ fn variant_conversion_fails() {
         Err(VariantConversionError)
     );
     assert_eq!(
-        Array::default().to_variant().try_to::<StringName>(),
+        VariantArray::default().to_variant().try_to::<StringName>(),
         Err(VariantConversionError)
     );
     //assert_eq!(
@@ -327,7 +327,10 @@ fn variant_type_correct() {
         StringName::from("string_name").to_variant().get_type(),
         VariantType::StringName
     );
-    assert_eq!(Array::default().to_variant().get_type(), VariantType::Array);
+    assert_eq!(
+        VariantArray::default().to_variant().get_type(),
+        VariantType::Array
+    );
     assert_eq!(
         Dictionary::default().to_variant().get_type(),
         VariantType::Dictionary