Make drain take a mutable borrow instead of Box<Self> for reflected Map, List, and Set. (#15406)

# Objective

Fixes #15185.

# Solution

Change `drain` to take a `&mut self` for most reflected types.

Some notable exceptions to this change are `Array` and `Tuple`. These
types don't make sense with `drain` taking a mutable borrow since they
can't get "smaller". Also `BTreeMap` doesn't have a `drain` function, so
we have to pop elements off one at a time.

## Testing

- The existing tests are sufficient.

---

## Migration Guide

- `reflect::Map`, `reflect::List`, and `reflect::Set` all now take a
`&mut self` instead of a `Box<Self>`. Callers of these traits should add
`&mut` before their boxes, and implementers of these traits should
update to match.

crates/bevy_reflect/src/impls/smallvec.rs

@@ -71,8 +71,8 @@ where
         ListIter::new(self)
     }
 
-    fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-        self.into_iter()
+        self.drain(..)
             .map(|value| Box::new(value) as Box<dyn PartialReflect>)
             .collect()
     }

crates/bevy_reflect/src/impls/std.rs

@@ -442,8 +442,8 @@ macro_rules! impl_reflect_for_veclike {
             }
 
             #[inline]
-            fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+            fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-                self.into_iter()
+                self.drain(..)
                     .map(|value| Box::new(value) as Box<dyn PartialReflect>)
                     .collect()
             }
@@ -619,8 +619,8 @@ macro_rules! impl_reflect_for_hashmap {
                 MapIter::new(self)
             }
 
-            fn drain(self: Box<Self>) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
+            fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
-                self.into_iter()
+                self.drain()
                     .map(|(key, value)| {
                         (
                             Box::new(key) as Box<dyn PartialReflect>,
@@ -856,8 +856,8 @@ macro_rules! impl_reflect_for_hashset {
                 Box::new(iter)
             }
 
-            fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+            fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-                self.into_iter()
+                self.drain()
                     .map(|value| Box::new(value) as Box<dyn PartialReflect>)
                     .collect()
             }
@@ -1092,15 +1092,18 @@ where
         MapIter::new(self)
     }
 
-    fn drain(self: Box<Self>) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
+    fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
-        self.into_iter()
+        // BTreeMap doesn't have a `drain` function. See
-            .map(|(key, value)| {
+        // https://github.com/rust-lang/rust/issues/81074. So we have to fake one by popping
-                (
+        // elements off one at a time.
-                    Box::new(key) as Box<dyn PartialReflect>,
+        let mut result = Vec::with_capacity(self.len());
-                    Box::new(value) as Box<dyn PartialReflect>,
+        while let Some((k, v)) = self.pop_first() {
-                )
+            result.push((
-            })
+                Box::new(k) as Box<dyn PartialReflect>,
-            .collect()
+                Box::new(v) as Box<dyn PartialReflect>,
+            ));
+        }
+        result
     }
 
     fn clone_dynamic(&self) -> DynamicMap {
@@ -1686,12 +1689,10 @@ impl<T: FromReflect + MaybeTyped + Clone + TypePath + GetTypeRegistration> List
         ListIter::new(self)
     }
 
-    fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-        // into_owned() is not unnecessary here because it avoids cloning whenever you have a Cow::Owned already
+        self.to_mut()
-        #[allow(clippy::unnecessary_to_owned)]
+            .drain(..)
-        self.into_owned()
+            .map(|value| Box::new(value) as Box<dyn PartialReflect>)
-            .into_iter()
-            .map(|value| value.clone_value())
             .collect()
     }
 }

crates/bevy_reflect/src/lib.rs

@@ -1460,7 +1460,7 @@ mod tests {
         assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
         assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());
 
-        let list_value: Box<dyn List> = Box::new(vec![123_i32, 321_i32]);
+        let mut list_value: Box<dyn List> = Box::new(vec![123_i32, 321_i32]);
         let fields = list_value.drain();
         assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
         assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());
@@ -1470,7 +1470,7 @@ mod tests {
         assert!(fields[0].reflect_partial_eq(&123_i32).unwrap_or_default());
         assert!(fields[1].reflect_partial_eq(&321_i32).unwrap_or_default());
 
-        let map_value: Box<dyn Map> = Box::new(HashMap::from([(123_i32, 321_i32)]));
+        let mut map_value: Box<dyn Map> = Box::new(HashMap::from([(123_i32, 321_i32)]));
         let fields = map_value.drain();
         assert!(fields[0].0.reflect_partial_eq(&123_i32).unwrap_or_default());
         assert!(fields[0].1.reflect_partial_eq(&321_i32).unwrap_or_default());

crates/bevy_reflect/src/list.rs

@@ -96,7 +96,10 @@ pub trait List: PartialReflect {
     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 PartialReflect>>;
+    ///
+    /// After calling this function, `self` will be empty. The order of items in the returned
+    /// [`Vec`] will match the order of items in `self`.
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>>;
 
     /// Clones the list, producing a [`DynamicList`].
     fn clone_dynamic(&self) -> DynamicList {
@@ -229,8 +232,8 @@ impl List for DynamicList {
         ListIter::new(self)
     }
 
-    fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-        self.values
+        self.values.drain(..).collect()
     }
 
     fn clone_dynamic(&self) -> DynamicList {

crates/bevy_reflect/src/map.rs

@@ -75,7 +75,9 @@ pub trait Map: PartialReflect {
     fn iter(&self) -> MapIter;
 
     /// Drain the key-value pairs of this map to get a vector of owned values.
-    fn drain(self: Box<Self>) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)>;
+    ///
+    /// After calling this function, `self` will be empty.
+    fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)>;
 
     /// Clones the map, producing a [`DynamicMap`].
     fn clone_dynamic(&self) -> DynamicMap;
@@ -286,8 +288,8 @@ impl Map for DynamicMap {
         MapIter::new(self)
     }
 
-    fn drain(self: Box<Self>) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
+    fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
-        self.values
+        self.values.drain(..).collect()
     }
 
     fn clone_dynamic(&self) -> DynamicMap {

crates/bevy_reflect/src/set.rs

@@ -61,7 +61,9 @@ pub trait Set: PartialReflect {
     fn iter(&self) -> Box<dyn Iterator<Item = &dyn PartialReflect> + '_>;
 
     /// Drain the values of this set to get a vector of owned values.
-    fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>>;
+    ///
+    /// After calling this function, `self` will be empty.
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>>;
 
     /// Clones the set, producing a [`DynamicSet`].
     fn clone_dynamic(&self) -> DynamicSet;
@@ -187,8 +189,8 @@ impl Set for DynamicSet {
         Box::new(iter)
     }
 
-    fn drain(self: Box<Self>) -> Vec<Box<dyn PartialReflect>> {
+    fn drain(&mut self) -> Vec<Box<dyn PartialReflect>> {
-        self.hash_table.into_iter().collect::<Vec<_>>()
+        self.hash_table.drain().collect::<Vec<_>>()
     }
 
     fn clone_dynamic(&self) -> DynamicSet {