bevy_reflect: Decouple List and Array traits (#7467)

# Objective

Resolves #7121

## Solution

Decouples `List` and `Array` by removing `Array` as a supertrait of `List`. Additionally, similar methods from `Array` have been added to `List` so that their usages can remain largely unchanged.

#### Possible Alternatives

##### `Sequence`

My guess for why we originally made `List` a subtrait of `Array` is that they share a lot of common operations. We could potentially move these overlapping methods to a `Sequence` (name taken from #7059) trait and make that a supertrait of both. This would allow functions to contain logic that simply operates on a sequence rather than "list vs array".

However, this means that we'd need to add methods for converting to a `dyn Sequence`. It also might be confusing since we wouldn't add a `ReflectRef::Sequence` or anything like that. Is such a trait worth adding (either in this PR or a followup one)? 

---

## Changelog

- Removed `Array` as supertrait of `List`
  - Added methods to `List` that were previously provided by `Array`

## Migration Guide

The `List` trait is no longer dependent on `Array`. Implementors of `List` can remove the `Array` impl and move its methods into the `List` impl (with only a couple tweaks).

```rust
// BEFORE
impl Array for Foo {
  fn get(&self, index: usize) -> Option<&dyn Reflect> {/* ... */}
  fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect> {/* ... */}
  fn len(&self) -> usize {/* ... */}
  fn is_empty(&self) -> bool {/* ... */}
  fn iter(&self) -> ArrayIter {/* ... */}
  fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {/* ... */}
  fn clone_dynamic(&self) -> DynamicArray {/* ... */}
}

impl List for Foo {
  fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {/* ... */}
  fn remove(&mut self, index: usize) -> Box<dyn Reflect> {/* ... */}
  fn push(&mut self, value: Box<dyn Reflect>) {/* ... */}
  fn pop(&mut self) -> Option<Box<dyn Reflect>> {/* ... */}
  fn clone_dynamic(&self) -> DynamicList {/* ... */}
}

// AFTER
impl List for Foo {
  fn get(&self, index: usize) -> Option<&dyn Reflect> {/* ... */}
  fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect> {/* ... */}
  fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {/* ... */}
  fn remove(&mut self, index: usize) -> Box<dyn Reflect> {/* ... */}
  fn push(&mut self, value: Box<dyn Reflect>) {/* ... */}
  fn pop(&mut self) -> Option<Box<dyn Reflect>> {/* ... */}
  fn len(&self) -> usize {/* ... */}
  fn is_empty(&self) -> bool {/* ... */}
  fn iter(&self) -> ListIter {/* ... */}
  fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {/* ... */}
  fn clone_dynamic(&self) -> DynamicList {/* ... */}
}
```

Some other small tweaks that will need to be made include:
- Use `ListIter` for `List::iter` instead of `ArrayIter` (the return type from `Array::iter`)
- Replace `array_hash` with `list_hash` in `Reflect::reflect_hash` for implementors of `List`

Author: MrGVSV

crates/bevy_reflect/src/array.rs

@@ -21,19 +21,25 @@ use std::{
 pub trait Array: Reflect {
     /// Returns a reference to the element at `index`, or `None` if out of bounds.
     fn get(&self, index: usize) -> Option<&dyn Reflect>;
+
     /// Returns a mutable reference to the element at `index`, or `None` if out of bounds.
     fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect>;
-    /// Returns the number of elements in the collection.
+
+    /// Returns the number of elements in the array.
     fn len(&self) -> usize;
+
     /// Returns `true` if the collection contains no elements.
     fn is_empty(&self) -> bool {
         self.len() == 0
     }
-    /// Returns an iterator over the collection.
+
+    /// Returns an iterator over the array.
     fn iter(&self) -> ArrayIter;
+
     /// Drain the elements of this array to get a vector of owned values.
     fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>>;
 
+    /// Clones the list, producing a [`DynamicArray`].
     fn clone_dynamic(&self) -> DynamicArray {
         DynamicArray {
             name: self.type_name().to_string(),

crates/bevy_reflect/src/impls/smallvec.rs

@@ -3,11 +3,11 @@ use std::any::Any;
 
 use crate::utility::GenericTypeInfoCell;
 use crate::{
-    Array, ArrayIter, FromReflect, FromType, GetTypeRegistration, List, ListInfo, Reflect,
-    ReflectFromPtr, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeRegistration, Typed,
+    FromReflect, FromType, GetTypeRegistration, List, ListInfo, ListIter, Reflect, ReflectFromPtr,
+    ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeRegistration, Typed,
 };
 
-impl<T: smallvec::Array + Send + Sync + 'static> Array for SmallVec<T>
+impl<T: smallvec::Array + Send + Sync + 'static> List for SmallVec<T>
 where
     T::Item: FromReflect,
 {
@@ -27,25 +27,6 @@ where
         }
     }
 
-    fn len(&self) -> usize {
-        <SmallVec<T>>::len(self)
-    }
-
-    fn iter(&self) -> ArrayIter {
-        ArrayIter::new(self)
-    }
-
-    fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
-        self.into_iter()
-            .map(|value| Box::new(value) as Box<dyn Reflect>)
-            .collect()
-    }
-}
-
-impl<T: smallvec::Array + Send + Sync + 'static> List for SmallVec<T>
-where
-    T::Item: FromReflect,
-{
     fn insert(&mut self, index: usize, value: Box<dyn Reflect>) {
         let value = value.take::<T::Item>().unwrap_or_else(|value| {
             <T as smallvec::Array>::Item::from_reflect(&*value).unwrap_or_else(|| {
@@ -77,6 +58,20 @@ where
     fn pop(&mut self) -> Option<Box<dyn Reflect>> {
         self.pop().map(|value| Box::new(value) as Box<dyn Reflect>)
     }
+
+    fn len(&self) -> usize {
+        <SmallVec<T>>::len(self)
+    }
+
+    fn iter(&self) -> ListIter {
+        ListIter::new(self)
+    }
+
+    fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
+        self.into_iter()
+            .map(|value| Box::new(value) as Box<dyn Reflect>)
+            .collect()
+    }
 }
 
 impl<T: smallvec::Array + Send + Sync + 'static> Reflect for SmallVec<T>
@@ -137,7 +132,7 @@ where
     }
 
     fn clone_value(&self) -> Box<dyn Reflect> {
-        Box::new(List::clone_dynamic(self))
+        Box::new(self.clone_dynamic())
     }
 
     fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {

crates/bevy_reflect/src/impls/std.rs

@@ -180,7 +180,7 @@ impl_from_reflect_value!(NonZeroI8);
 
 macro_rules! impl_reflect_for_veclike {
     ($ty:ty, $insert:expr, $remove:expr, $push:expr, $pop:expr, $sub:ty) => {
-        impl<T: FromReflect> Array for $ty {
+        impl<T: FromReflect> List for $ty {
             #[inline]
             fn get(&self, index: usize) -> Option<&dyn Reflect> {
                 <$sub>::get(self, index).map(|value| value as &dyn Reflect)
@@ -191,25 +191,6 @@ macro_rules! impl_reflect_for_veclike {
                 <$sub>::get_mut(self, index).map(|value| value as &mut dyn Reflect)
             }
 
-            #[inline]
-            fn len(&self) -> usize {
-                <$sub>::len(self)
-            }
-
-            #[inline]
-            fn iter(&self) -> ArrayIter {
-                ArrayIter::new(self)
-            }
-
-            #[inline]
-            fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
-                self.into_iter()
-                    .map(|value| Box::new(value) as Box<dyn Reflect>)
-                    .collect()
-            }
-        }
-
-        impl<T: FromReflect> List for $ty {
             fn insert(&mut self, index: usize, value: Box<dyn Reflect>) {
                 let value = value.take::<T>().unwrap_or_else(|value| {
                     T::from_reflect(&*value).unwrap_or_else(|| {
@@ -239,6 +220,23 @@ macro_rules! impl_reflect_for_veclike {
             fn pop(&mut self) -> Option<Box<dyn Reflect>> {
                 $pop(self).map(|value| Box::new(value) as Box<dyn Reflect>)
             }
+
+            #[inline]
+            fn len(&self) -> usize {
+                <$sub>::len(self)
+            }
+
+            #[inline]
+            fn iter(&self) -> $crate::ListIter {
+                $crate::ListIter::new(self)
+            }
+
+            #[inline]
+            fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
+                self.into_iter()
+                    .map(|value| Box::new(value) as Box<dyn Reflect>)
+                    .collect()
+            }
         }
 
         impl<T: FromReflect> Reflect for $ty {
@@ -296,11 +294,11 @@ macro_rules! impl_reflect_for_veclike {
             }
 
             fn clone_value(&self) -> Box<dyn Reflect> {
-                Box::new(List::clone_dynamic(self))
+                Box::new(self.clone_dynamic())
             }
 
             fn reflect_hash(&self) -> Option<u64> {
-                crate::array_hash(self)
+                crate::list_hash(self)
             }
 
             fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {

crates/bevy_reflect/src/lib.rs

@@ -394,7 +394,7 @@ mod tests {
         list.push(3isize);
         list.push(4isize);
         list.push(5isize);
-        foo_patch.insert("c", List::clone_dynamic(&list));
+        foo_patch.insert("c", list.clone_dynamic());
 
         let mut map = DynamicMap::default();
         map.insert(2usize, 3i8);
@@ -607,11 +607,11 @@ mod tests {
     #[test]
     fn dynamic_names() {
         let list = Vec::<usize>::new();
-        let dyn_list = List::clone_dynamic(&list);
+        let dyn_list = list.clone_dynamic();
         assert_eq!(dyn_list.type_name(), std::any::type_name::<Vec<usize>>());
 
         let array = [b'0'; 4];
-        let dyn_array = Array::clone_dynamic(&array);
+        let dyn_array = array.clone_dynamic();
         assert_eq!(dyn_array.type_name(), std::any::type_name::<[u8; 4]>());
 
         let map = HashMap::<usize, String>::default();

crates/bevy_reflect/src/list.rs

@@ -1,24 +1,32 @@
 use std::any::{Any, TypeId};
 use std::fmt::{Debug, Formatter};
+use std::hash::{Hash, Hasher};
 
 use crate::utility::NonGenericTypeInfoCell;
 use crate::{
-    Array, ArrayIter, DynamicArray, DynamicInfo, FromReflect, Reflect, ReflectMut, ReflectOwned,
-    ReflectRef, TypeInfo, Typed,
+    DynamicInfo, FromReflect, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed,
 };
 
 /// An ordered, mutable list of [Reflect] items. This corresponds to types like [`std::vec::Vec`].
 ///
-/// This is a sub-trait of [`Array`], however as it implements [insertion](List::insert) and [removal](List::remove),
-/// it's internal size may change.
+/// Unlike the [`Array`](crate::Array) trait, implementors of this type are not expected to
+/// maintain a constant length.
+/// Methods like [insertion](List::insert) and [removal](List::remove) explicitly allow for their
+/// internal size to change.
 ///
 /// This trait expects index 0 to contain the _front_ element.
 /// The _back_ element must refer to the element with the largest index.
 /// These two rules above should be upheld by manual implementors.
 ///
 /// [`push`](List::push) and [`pop`](List::pop) have default implementations,
 /// however it may be faster to implement them manually.
-pub trait List: Reflect + Array {
+pub trait List: Reflect {
+    /// Returns a reference to the element at `index`, or `None` if out of bounds.
+    fn get(&self, index: usize) -> Option<&dyn Reflect>;
+
+    /// Returns a mutable reference to the element at `index`, or `None` if out of bounds.
+    fn get_mut(&mut self, index: usize) -> Option<&mut dyn Reflect>;
+
     /// Inserts an element at position `index` within the list,
     /// shifting all elements after it towards the back of the list.
     ///
@@ -47,6 +55,20 @@ pub trait List: Reflect + Array {
         }
     }
 
+    /// Returns the number of elements in the list.
+    fn len(&self) -> usize;
+
+    /// Returns `true` if the collection contains no elements.
+    fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Returns an iterator over the list.
+    fn iter(&self) -> ListIter;
+
+    /// Drain the elements of this list to get a vector of owned values.
+    fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>>;
+
     /// Clones the list, producing a [`DynamicList`].
     fn clone_dynamic(&self) -> DynamicList {
         DynamicList {
@@ -162,7 +184,7 @@ impl DynamicList {
     }
 }
 
-impl Array for DynamicList {
+impl List for DynamicList {
     fn get(&self, index: usize) -> Option<&dyn Reflect> {
         self.values.get(index).map(|value| &**value)
     }
@@ -171,31 +193,6 @@ impl Array for DynamicList {
         self.values.get_mut(index).map(|value| &mut **value)
     }
 
-    fn len(&self) -> usize {
-        self.values.len()
-    }
-
-    fn iter(&self) -> ArrayIter {
-        ArrayIter::new(self)
-    }
-
-    fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
-        self.values
-    }
-
-    fn clone_dynamic(&self) -> DynamicArray {
-        DynamicArray {
-            name: self.name.clone(),
-            values: self
-                .values
-                .iter()
-                .map(|value| value.clone_value())
-                .collect(),
-        }
-    }
-}
-
-impl List for DynamicList {
     fn insert(&mut self, index: usize, element: Box<dyn Reflect>) {
         self.values.insert(index, element);
     }
@@ -212,6 +209,18 @@ impl List for DynamicList {
         self.values.pop()
     }
 
+    fn len(&self) -> usize {
+        self.values.len()
+    }
+
+    fn iter(&self) -> ListIter {
+        ListIter::new(self)
+    }
+
+    fn drain(self: Box<Self>) -> Vec<Box<dyn Reflect>> {
+        self.values
+    }
+
     fn clone_dynamic(&self) -> DynamicList {
         DynamicList {
             name: self.name.clone(),
@@ -292,12 +301,12 @@ impl Reflect for DynamicList {
 
     #[inline]
     fn clone_value(&self) -> Box<dyn Reflect> {
-        Box::new(List::clone_dynamic(self))
+        Box::new(self.clone_dynamic())
     }
 
     #[inline]
     fn reflect_hash(&self) -> Option<u64> {
-        crate::array_hash(self)
+        list_hash(self)
     }
 
     fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option<bool> {
@@ -333,6 +342,51 @@ impl IntoIterator for DynamicList {
     }
 }
 
+/// An iterator over an [`List`].
+pub struct ListIter<'a> {
+    list: &'a dyn List,
+    index: usize,
+}
+
+impl<'a> ListIter<'a> {
+    /// Creates a new [`ListIter`].
+    #[inline]
+    pub const fn new(list: &'a dyn List) -> ListIter {
+        ListIter { list, index: 0 }
+    }
+}
+
+impl<'a> Iterator for ListIter<'a> {
+    type Item = &'a dyn Reflect;
+
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        let value = self.list.get(self.index);
+        self.index += 1;
+        value
+    }
+
+    #[inline]
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        let size = self.list.len();
+        (size, Some(size))
+    }
+}
+
+impl<'a> ExactSizeIterator for ListIter<'a> {}
+
+/// Returns the `u64` hash of the given [list](List).
+#[inline]
+pub fn list_hash<L: List>(list: &L) -> Option<u64> {
+    let mut hasher = crate::ReflectHasher::default();
+    std::any::Any::type_id(list).hash(&mut hasher);
+    list.len().hash(&mut hasher);
+    for value in list.iter() {
+        hasher.write_u64(value.reflect_hash()?);
+    }
+    Some(hasher.finish())
+}
+
 /// Applies the elements of `b` to the corresponding elements of `a`.
 ///
 /// If the length of `b` is greater than that of `a`, the excess elements of `b`
@@ -350,7 +404,7 @@ pub fn list_apply<L: List>(a: &mut L, b: &dyn Reflect) {
                     v.apply(value);
                 }
             } else {
-                List::push(a, value.clone_value());
+                a.push(value.clone_value());
             }
         }
     } else {