bevy_reflect: support map insertion

crates/bevy_reflect/src/impls/std.rs

@@ -1,8 +1,8 @@
 use crate as bevy_reflect;
 use crate::{
-    map_partial_eq, serde::Serializable, DynamicMap, FromReflect, FromType, GetTypeRegistration,
-    List, ListIter, Map, MapIter, Reflect, ReflectDeserialize, ReflectMut, ReflectRef,
-    TypeRegistration,
+    map_apply, map_partial_eq, serde::Serializable, DynamicMap, FromReflect, FromType,
+    GetTypeRegistration, List, ListIter, Map, MapIter, Reflect, ReflectDeserialize, ReflectMut,
+    ReflectRef, TypeRegistration,
 };
 
 use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value};
@@ -166,7 +166,7 @@ impl<T: FromReflect> FromReflect for Vec<T> {
     }
 }
 
-impl<K: Reflect + Eq + Hash, V: Reflect> Map for HashMap<K, V> {
+impl<K: FromReflect + Eq + Hash, V: FromReflect> Map for HashMap<K, V> {
     fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
         key.downcast_ref::<K>()
             .and_then(|key| HashMap::get(self, key))
@@ -204,10 +204,35 @@ impl<K: Reflect + Eq + Hash, V: Reflect> Map for HashMap<K, V> {
         }
         dynamic_map
     }
+
+    fn insert_boxed(
+        &mut self,
+        key: Box<dyn Reflect>,
+        value: Box<dyn Reflect>,
+    ) -> Option<Box<dyn Reflect>> {
+        let key = key.take::<K>().unwrap_or_else(|key| {
+            K::from_reflect(&*key).unwrap_or_else(|| {
+                panic!(
+                    "Attempted to insert invalid key of type {}.",
+                    key.type_name()
+                )
+            })
+        });
+        let value = value.take::<V>().unwrap_or_else(|value| {
+            V::from_reflect(&*value).unwrap_or_else(|| {
+                panic!(
+                    "Attempted to push invalid value of type {}.",
+                    value.type_name()
+                )
+            })
+        });
+        self.insert(key, value)
+            .map(|old_value| Box::new(old_value) as Box<dyn Reflect>)
+    }
 }
 
 // SAFE: any and any_mut both return self
-unsafe impl<K: Reflect + Eq + Hash, V: Reflect> Reflect for HashMap<K, V> {
+unsafe impl<K: FromReflect + Eq + Hash, V: FromReflect> Reflect for HashMap<K, V> {
     fn type_name(&self) -> &str {
         std::any::type_name::<Self>()
     }
@@ -221,15 +246,7 @@ unsafe impl<K: Reflect + Eq + Hash, V: Reflect> Reflect for HashMap<K, V> {
     }
 
     fn apply(&mut self, value: &dyn Reflect) {
-        if let ReflectRef::Map(map_value) = value.reflect_ref() {
-            for (key, value) in map_value.iter() {
-                if let Some(v) = Map::get_mut(self, key) {
-                    v.apply(value)
-                }
-            }
-        } else {
-            panic!("Attempted to apply a non-map type to a map type.");
-        }
+        map_apply(self, value)
     }
 
     fn set(&mut self, value: Box<dyn Reflect>) -> Result<(), Box<dyn Reflect>> {
@@ -264,8 +281,8 @@ unsafe impl<K: Reflect + Eq + Hash, V: Reflect> Reflect for HashMap<K, V> {
 
 impl<K, V> GetTypeRegistration for HashMap<K, V>
 where
-    K: Reflect + Clone + Eq + Hash + for<'de> Deserialize<'de>,
-    V: Reflect + Clone + for<'de> Deserialize<'de>,
+    K: FromReflect + Clone + Eq + Hash + for<'de> Deserialize<'de>,
+    V: FromReflect + Clone + for<'de> Deserialize<'de>,
 {
     fn get_type_registration() -> TypeRegistration {
         let mut registration = TypeRegistration::of::<HashMap<K, V>>();

crates/bevy_reflect/src/lib.rs

@@ -247,6 +247,7 @@ mod tests {
 
         let mut map = DynamicMap::default();
         map.insert(2usize, 3i8);
+        map.insert(3usize, 4i8);
         foo_patch.insert("d", map);
 
         let mut bar_patch = DynamicStruct::default();
@@ -285,6 +286,7 @@ mod tests {
         let mut hash_map = HashMap::default();
         hash_map.insert(1, 1);
         hash_map.insert(2, 3);
+        hash_map.insert(3, 4);
 
         let mut hash_map_baz = HashMap::default();
         hash_map_baz.insert(1, Bar { x: 7 });
@@ -306,6 +308,7 @@ mod tests {
 
         let mut hash_map = HashMap::default();
         hash_map.insert(2, 3);
+        hash_map.insert(3, 4);
 
         let expected_new_foo = Foo {
             a: 2,

crates/bevy_reflect/src/map.rs

@@ -41,6 +41,16 @@ pub trait Map: Reflect {
 
     /// Clones the map, producing a [`DynamicMap`].
     fn clone_dynamic(&self) -> DynamicMap;
+
+    /// Inserts a key-value pair into the map.
+    ///
+    /// If the map did not have this key present, `None` is returned.
+    /// If the map did have this key present, the value is updated, and the old value is returned.
+    fn insert_boxed(
+        &mut self,
+        key: Box<dyn Reflect>,
+        value: Box<dyn Reflect>,
+    ) -> Option<Box<dyn Reflect>>;
 }
 
 const HASH_ERROR: &str = "the given key does not support hashing";
@@ -74,19 +84,6 @@ impl DynamicMap {
     pub fn insert<K: Reflect, V: Reflect>(&mut self, key: K, value: V) {
         self.insert_boxed(Box::new(key), Box::new(value));
     }
-
-    /// Inserts a key-value pair of [`Reflect`] values into the map.
-    pub fn insert_boxed(&mut self, key: Box<dyn Reflect>, value: Box<dyn Reflect>) {
-        match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) {
-            Entry::Occupied(entry) => {
-                self.values[*entry.get()] = (key, value);
-            }
-            Entry::Vacant(entry) => {
-                entry.insert(self.values.len());
-                self.values.push((key, value));
-            }
-        }
-    }
 }
 
 impl Map for DynamicMap {
@@ -131,6 +128,25 @@ impl Map for DynamicMap {
             .get(index)
             .map(|(key, value)| (&**key, &**value))
     }
+
+    fn insert_boxed(
+        &mut self,
+        key: Box<dyn Reflect>,
+        mut value: Box<dyn Reflect>,
+    ) -> Option<Box<dyn Reflect>> {
+        match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) {
+            Entry::Occupied(entry) => {
+                let (_old_key, old_value) = self.values.get_mut(*entry.get()).unwrap();
+                std::mem::swap(old_value, &mut value);
+                Some(value)
+            }
+            Entry::Vacant(entry) => {
+                entry.insert(self.values.len());
+                self.values.push((key, value));
+                None
+            }
+        }
+    }
 }
 
 // SAFE: any and any_mut both return self
@@ -148,15 +164,7 @@ unsafe impl Reflect for DynamicMap {
     }
 
     fn apply(&mut self, value: &dyn Reflect) {
-        if let ReflectRef::Map(map_value) = value.reflect_ref() {
-            for (key, value) in map_value.iter() {
-                if let Some(v) = self.get_mut(key) {
-                    v.apply(value)
-                }
-            }
-        } else {
-            panic!("Attempted to apply a non-map type to a map type.");
-        }
+        map_apply(self, value);
     }
 
     fn set(&mut self, value: Box<dyn Reflect>) -> Result<(), Box<dyn Reflect>> {
@@ -243,3 +251,25 @@ pub fn map_partial_eq<M: Map>(a: &M, b: &dyn Reflect) -> Option<bool> {
 
     Some(true)
 }
+
+/// Applies the elements of `b` to the corresponding elements of `a`.
+///
+/// If a key from `b` does not exist in `a`, the value is cloned and inserted.
+///
+/// # Panics
+///
+/// This function panics if `b` is not a map.
+#[inline]
+pub fn map_apply<M: Map>(a: &mut M, b: &dyn Reflect) {
+    if let ReflectRef::Map(map_value) = b.reflect_ref() {
+        for (key, b_value) in map_value.iter() {
+            if let Some(a_value) = a.get_mut(key) {
+                a_value.apply(b_value);
+            } else {
+                a.insert_boxed(key.clone_value(), b_value.clone_value());
+            }
+        }
+    } else {
+        panic!("Attempted to apply a non-map type to a map type.");
+    }
+}

crates/bevy_reflect/src/reflect.rs

@@ -72,17 +72,18 @@ pub unsafe trait Reflect: Any + Send + Sync {
     ///   and excess elements in `value` are appended to `self`.
     /// - If `T` is a [`Map`], then for each key in `value`, the associated
     ///   value is applied to the value associated with the same key in `self`.
-    ///   Keys which are not present in both maps are ignored.
+    ///   Keys which are not present in `self` are inserted.
     /// - If `T` is none of these, then `value` is downcast to `T`, cloned, and
     ///   assigned to `self`.
     ///
     /// Note that `Reflect` must be implemented manually for [`List`]s and
     /// [`Map`]s in order to achieve the correct semantics, as derived
     /// implementations will have the semantics for [`Struct`], [`TupleStruct`]
-    /// or none of the above depending on the kind of type. For lists, use the
-    /// [`list_apply`] helper function when implementing this method.
+    /// or none of the above depending on the kind of type. For lists and maps, use the
+    /// [`list_apply`] and [`map_apply`] helper functions when implementing this method.
     ///
     /// [`list_apply`]: crate::list_apply
+    /// [`map_apply`]: crate::map_apply
     ///
     /// # Panics
     ///

crates/bevy_reflect/src/serde/de.rs

@@ -1,6 +1,6 @@
 use crate::{
     serde::type_fields, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, DynamicTupleStruct,
-    Reflect, ReflectDeserialize, TypeRegistry,
+    Map, Reflect, ReflectDeserialize, TypeRegistry,
 };
 use erased_serde::Deserializer;
 use serde::de::{self, DeserializeSeed, MapAccess, SeqAccess, Visitor};