Fix PartialReflect::apply for maps, remove get_at/_mut from Map trait (#19802)
# Objective - Fixes https://github.com/bevyengine/bevy/issues/14328 - `DynamicMap::drain` was broken (indices weren't cleared, causing a panic when reading later) - `PartialReflect::apply` was broken for maps and sets, because they don't remove entries from the `self` map that aren't in the applied map. - I discovered this bug when implementing MapEntities on a Component containing a `HashMap<Entity, _>`. Because `apply` is used to reapply the changes to the reflected map, the map ended up littered with a ton of outdated entries. ## Solution - Remove the separate `Vec` in `DynamicMap` and use the `HashTable` directly, like it is in `DynamicSet`. - Replace `MapIter` by `Box<dyn Iterator>` (like for `DynamicSet`), and `Map::get_at` and `Map::get_at_mut` which are now unused. - Now assume `DynamicMap` types are unordered and adjust documentation accordingly. - Fix documentation of `DynamicSet` (ordered -> unordered) - Added `Map::retain` and `Set::retain`, and use them to remove excess entries in `PartialReflect::apply` implementations. ## Testing - Added `map::tests::apply` and `set::tests::apply` to validate `<DynamicMap as PartialReflect>::apply` and `<DynamicSet as PartialReflect>::apply`
This commit is contained in:
Alix Bott
GitHub
parent
97dcb279fb
commit
dd4479ed30
@ -2,7 +2,7 @@ use crate::{
|
||||
error::ReflectCloneError,
|
||||
generics::{Generics, TypeParamInfo},
|
||||
kind::{ReflectKind, ReflectMut, ReflectOwned, ReflectRef},
|
||||
map::{map_apply, map_partial_eq, map_try_apply, Map, MapInfo, MapIter},
|
||||
map::{map_apply, map_partial_eq, map_try_apply, Map, MapInfo},
|
||||
prelude::*,
|
||||
reflect::{impl_full_reflect, ApplyError},
|
||||
type_info::{MaybeTyped, TypeInfo, Typed},
|
||||
@ -31,27 +31,15 @@ where
|
||||
.map(|value| value as &mut dyn PartialReflect)
|
||||
}
|
||||
|
||||
fn get_at(&self, index: usize) -> Option<(&dyn PartialReflect, &dyn PartialReflect)> {
|
||||
self.iter()
|
||||
.nth(index)
|
||||
.map(|(key, value)| (key as &dyn PartialReflect, value as &dyn PartialReflect))
|
||||
}
|
||||
|
||||
fn get_at_mut(
|
||||
&mut self,
|
||||
index: usize,
|
||||
) -> Option<(&dyn PartialReflect, &mut dyn PartialReflect)> {
|
||||
self.iter_mut()
|
||||
.nth(index)
|
||||
.map(|(key, value)| (key as &dyn PartialReflect, value as &mut dyn PartialReflect))
|
||||
}
|
||||
|
||||
fn len(&self) -> usize {
|
||||
Self::len(self)
|
||||
}
|
||||
|
||||
fn iter(&self) -> MapIter {
|
||||
MapIter::new(self)
|
||||
fn iter(&self) -> Box<dyn Iterator<Item = (&dyn PartialReflect, &dyn PartialReflect)> + '_> {
|
||||
Box::new(
|
||||
self.iter()
|
||||
.map(|(k, v)| (k as &dyn PartialReflect, v as &dyn PartialReflect)),
|
||||
)
|
||||
}
|
||||
|
||||
fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
|
||||
@ -68,6 +56,10 @@ where
|
||||
result
|
||||
}
|
||||
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn PartialReflect, &mut dyn PartialReflect) -> bool) {
|
||||
self.retain(move |k, v| f(k, v));
|
||||
}
|
||||
|
||||
fn insert_boxed(
|
||||
&mut self,
|
||||
key: Box<dyn PartialReflect>,
|
||||
|
||||
@ -19,27 +19,12 @@ macro_rules! impl_reflect_for_hashmap {
|
||||
.map(|value| value as &mut dyn $crate::reflect::PartialReflect)
|
||||
}
|
||||
|
||||
fn get_at(&self, index: usize) -> Option<(&dyn $crate::reflect::PartialReflect, &dyn $crate::reflect::PartialReflect)> {
|
||||
self.iter()
|
||||
.nth(index)
|
||||
.map(|(key, value)| (key as &dyn $crate::reflect::PartialReflect, value as &dyn $crate::reflect::PartialReflect))
|
||||
}
|
||||
|
||||
fn get_at_mut(
|
||||
&mut self,
|
||||
index: usize,
|
||||
) -> Option<(&dyn $crate::reflect::PartialReflect, &mut dyn $crate::reflect::PartialReflect)> {
|
||||
self.iter_mut().nth(index).map(|(key, value)| {
|
||||
(key as &dyn $crate::reflect::PartialReflect, value as &mut dyn $crate::reflect::PartialReflect)
|
||||
})
|
||||
}
|
||||
|
||||
fn len(&self) -> usize {
|
||||
Self::len(self)
|
||||
}
|
||||
|
||||
fn iter(&self) -> $crate::map::MapIter {
|
||||
$crate::map::MapIter::new(self)
|
||||
fn iter(&self) -> bevy_platform::prelude::Box<dyn Iterator<Item = (&dyn $crate::reflect::PartialReflect, &dyn $crate::reflect::PartialReflect)> + '_> {
|
||||
bevy_platform::prelude::Box::new(self.iter().map(|(k, v)| (k as &dyn $crate::reflect::PartialReflect, v as &dyn $crate::reflect::PartialReflect)))
|
||||
}
|
||||
|
||||
fn drain(&mut self) -> bevy_platform::prelude::Vec<(bevy_platform::prelude::Box<dyn $crate::reflect::PartialReflect>, bevy_platform::prelude::Box<dyn $crate::reflect::PartialReflect>)> {
|
||||
@ -53,6 +38,10 @@ macro_rules! impl_reflect_for_hashmap {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn $crate::reflect::PartialReflect, &mut dyn $crate::reflect::PartialReflect) -> bool) {
|
||||
self.retain(move |key, value| f(key, value));
|
||||
}
|
||||
|
||||
fn to_dynamic_map(&self) -> $crate::map::DynamicMap {
|
||||
let mut dynamic_map = $crate::map::DynamicMap::default();
|
||||
dynamic_map.set_represented_type($crate::reflect::PartialReflect::get_represented_type_info(self));
|
||||
|
||||
@ -28,6 +28,10 @@ macro_rules! impl_reflect_for_hashset {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn $crate::reflect::PartialReflect) -> bool) {
|
||||
self.retain(move |value| f(value));
|
||||
}
|
||||
|
||||
fn insert_boxed(&mut self, value: bevy_platform::prelude::Box<dyn $crate::reflect::PartialReflect>) -> bool {
|
||||
let value = V::take_from_reflect(value).unwrap_or_else(|value| {
|
||||
panic!(
|
||||
|
||||
@ -1583,7 +1583,6 @@ mod tests {
|
||||
foo.apply(&foo_patch);
|
||||
|
||||
let mut hash_map = <HashMap<_, _>>::default();
|
||||
hash_map.insert(1, 1);
|
||||
hash_map.insert(2, 3);
|
||||
hash_map.insert(3, 4);
|
||||
|
||||
|
||||
@ -56,15 +56,6 @@ pub trait Map: PartialReflect {
|
||||
/// If no value is associated with `key`, returns `None`.
|
||||
fn get_mut(&mut self, key: &dyn PartialReflect) -> Option<&mut dyn PartialReflect>;
|
||||
|
||||
/// Returns the key-value pair at `index` by reference, or `None` if out of bounds.
|
||||
fn get_at(&self, index: usize) -> Option<(&dyn PartialReflect, &dyn PartialReflect)>;
|
||||
|
||||
/// Returns the key-value pair at `index` by reference where the value is a mutable reference, or `None` if out of bounds.
|
||||
fn get_at_mut(
|
||||
&mut self,
|
||||
index: usize,
|
||||
) -> Option<(&dyn PartialReflect, &mut dyn PartialReflect)>;
|
||||
|
||||
/// Returns the number of elements in the map.
|
||||
fn len(&self) -> usize;
|
||||
|
||||
@ -74,13 +65,18 @@ pub trait Map: PartialReflect {
|
||||
}
|
||||
|
||||
/// Returns an iterator over the key-value pairs of the map.
|
||||
fn iter(&self) -> MapIter;
|
||||
fn iter(&self) -> Box<dyn Iterator<Item = (&dyn PartialReflect, &dyn PartialReflect)> + '_>;
|
||||
|
||||
/// Drain the key-value pairs of this map to get a vector of owned values.
|
||||
///
|
||||
/// After calling this function, `self` will be empty.
|
||||
fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)>;
|
||||
|
||||
/// Retain only the elements specified by the predicate.
|
||||
///
|
||||
/// In other words, remove all pairs `(k, v)` such that `f(&k, &mut v)` returns `false`.
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn PartialReflect, &mut dyn PartialReflect) -> bool);
|
||||
|
||||
/// Creates a new [`DynamicMap`] from this map.
|
||||
fn to_dynamic_map(&self) -> DynamicMap {
|
||||
let mut map = DynamicMap::default();
|
||||
@ -218,12 +214,11 @@ macro_rules! hash_error {
|
||||
}}
|
||||
}
|
||||
|
||||
/// An ordered mapping between reflected values.
|
||||
/// An unordered mapping between reflected values.
|
||||
#[derive(Default)]
|
||||
pub struct DynamicMap {
|
||||
represented_type: Option<&'static TypeInfo>,
|
||||
values: Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)>,
|
||||
indices: HashTable<usize>,
|
||||
hash_table: HashTable<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)>,
|
||||
}
|
||||
|
||||
impl DynamicMap {
|
||||
@ -254,13 +249,12 @@ impl DynamicMap {
|
||||
value.reflect_hash().expect(&hash_error!(value))
|
||||
}
|
||||
|
||||
fn internal_eq<'a>(
|
||||
value: &'a dyn PartialReflect,
|
||||
values: &'a [(Box<dyn PartialReflect>, Box<dyn PartialReflect>)],
|
||||
) -> impl FnMut(&usize) -> bool + 'a {
|
||||
|&index| {
|
||||
value
|
||||
.reflect_partial_eq(&*values[index].0)
|
||||
fn internal_eq(
|
||||
key: &dyn PartialReflect,
|
||||
) -> impl FnMut(&(Box<dyn PartialReflect>, Box<dyn PartialReflect>)) -> bool + '_ {
|
||||
|(other, _)| {
|
||||
key
|
||||
.reflect_partial_eq(&**other)
|
||||
.expect("underlying type does not reflect `PartialEq` and hence doesn't support equality checks")
|
||||
}
|
||||
}
|
||||
@ -268,46 +262,33 @@ impl DynamicMap {
|
||||
|
||||
impl Map for DynamicMap {
|
||||
fn get(&self, key: &dyn PartialReflect) -> Option<&dyn PartialReflect> {
|
||||
let hash = Self::internal_hash(key);
|
||||
let eq = Self::internal_eq(key, &self.values);
|
||||
self.indices
|
||||
.find(hash, eq)
|
||||
.map(|&index| &*self.values[index].1)
|
||||
self.hash_table
|
||||
.find(Self::internal_hash(key), Self::internal_eq(key))
|
||||
.map(|(_, value)| &**value)
|
||||
}
|
||||
|
||||
fn get_mut(&mut self, key: &dyn PartialReflect) -> Option<&mut dyn PartialReflect> {
|
||||
let hash = Self::internal_hash(key);
|
||||
let eq = Self::internal_eq(key, &self.values);
|
||||
self.indices
|
||||
.find(hash, eq)
|
||||
.map(|&index| &mut *self.values[index].1)
|
||||
}
|
||||
|
||||
fn get_at(&self, index: usize) -> Option<(&dyn PartialReflect, &dyn PartialReflect)> {
|
||||
self.values
|
||||
.get(index)
|
||||
.map(|(key, value)| (&**key, &**value))
|
||||
}
|
||||
|
||||
fn get_at_mut(
|
||||
&mut self,
|
||||
index: usize,
|
||||
) -> Option<(&dyn PartialReflect, &mut dyn PartialReflect)> {
|
||||
self.values
|
||||
.get_mut(index)
|
||||
.map(|(key, value)| (&**key, &mut **value))
|
||||
self.hash_table
|
||||
.find_mut(Self::internal_hash(key), Self::internal_eq(key))
|
||||
.map(|(_, value)| &mut **value)
|
||||
}
|
||||
|
||||
fn len(&self) -> usize {
|
||||
self.values.len()
|
||||
self.hash_table.len()
|
||||
}
|
||||
|
||||
fn iter(&self) -> MapIter {
|
||||
MapIter::new(self)
|
||||
fn iter(&self) -> Box<dyn Iterator<Item = (&dyn PartialReflect, &dyn PartialReflect)> + '_> {
|
||||
let iter = self.hash_table.iter().map(|(k, v)| (&**k, &**v));
|
||||
Box::new(iter)
|
||||
}
|
||||
|
||||
fn drain(&mut self) -> Vec<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> {
|
||||
self.values.drain(..).collect()
|
||||
self.hash_table.drain().collect()
|
||||
}
|
||||
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn PartialReflect, &mut dyn PartialReflect) -> bool) {
|
||||
self.hash_table
|
||||
.retain(move |(key, value)| f(&**key, &mut **value));
|
||||
}
|
||||
|
||||
fn insert_boxed(
|
||||
@ -322,20 +303,15 @@ impl Map for DynamicMap {
|
||||
);
|
||||
|
||||
let hash = Self::internal_hash(&*key);
|
||||
let eq = Self::internal_eq(&*key, &self.values);
|
||||
match self.indices.find(hash, eq) {
|
||||
Some(&index) => {
|
||||
let (key_ref, value_ref) = &mut self.values[index];
|
||||
*key_ref = key;
|
||||
let old_value = core::mem::replace(value_ref, value);
|
||||
Some(old_value)
|
||||
}
|
||||
let eq = Self::internal_eq(&*key);
|
||||
match self.hash_table.find_mut(hash, eq) {
|
||||
Some((_, old)) => Some(core::mem::replace(old, value)),
|
||||
None => {
|
||||
let index = self.values.len();
|
||||
self.values.push((key, value));
|
||||
self.indices.insert_unique(hash, index, |&index| {
|
||||
Self::internal_hash(&*self.values[index].0)
|
||||
});
|
||||
self.hash_table.insert_unique(
|
||||
Self::internal_hash(key.as_ref()),
|
||||
(key, value),
|
||||
|(key, _)| Self::internal_hash(&**key),
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
@ -343,26 +319,10 @@ impl Map for DynamicMap {
|
||||
|
||||
fn remove(&mut self, key: &dyn PartialReflect) -> Option<Box<dyn PartialReflect>> {
|
||||
let hash = Self::internal_hash(key);
|
||||
let eq = Self::internal_eq(key, &self.values);
|
||||
match self.indices.find_entry(hash, eq) {
|
||||
let eq = Self::internal_eq(key);
|
||||
match self.hash_table.find_entry(hash, eq) {
|
||||
Ok(entry) => {
|
||||
let (index, _) = entry.remove();
|
||||
let (_, old_value) = self.values.swap_remove(index);
|
||||
|
||||
// The `swap_remove` might have moved the last element of `values`
|
||||
// to `index`, so we might need to fix up its index in `indices`.
|
||||
// If the removed element was also the last element there's nothing to
|
||||
// fixup and this will return `None`, otherwise it returns the key
|
||||
// whose index needs to be fixed up.
|
||||
if let Some((moved_key, _)) = self.values.get(index) {
|
||||
let hash = Self::internal_hash(&**moved_key);
|
||||
let moved_index = self
|
||||
.indices
|
||||
.find_mut(hash, |&moved_index| moved_index == self.values.len())
|
||||
.expect("key inserted in a `DynamicMap` is no longer present, this means its reflected `Hash` might be incorrect");
|
||||
*moved_index = index;
|
||||
}
|
||||
|
||||
let ((_, old_value), _) = entry.remove();
|
||||
Some(old_value)
|
||||
}
|
||||
Err(_) => None,
|
||||
@ -451,35 +411,6 @@ impl Debug for DynamicMap {
|
||||
}
|
||||
}
|
||||
|
||||
/// An iterator over the key-value pairs of a [`Map`].
|
||||
pub struct MapIter<'a> {
|
||||
map: &'a dyn Map,
|
||||
index: usize,
|
||||
}
|
||||
|
||||
impl MapIter<'_> {
|
||||
/// Creates a new [`MapIter`].
|
||||
#[inline]
|
||||
pub const fn new(map: &dyn Map) -> MapIter {
|
||||
MapIter { map, index: 0 }
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> Iterator for MapIter<'a> {
|
||||
type Item = (&'a dyn PartialReflect, &'a dyn PartialReflect);
|
||||
|
||||
fn next(&mut self) -> Option<Self::Item> {
|
||||
let value = self.map.get_at(self.index);
|
||||
self.index += value.is_some() as usize;
|
||||
value
|
||||
}
|
||||
|
||||
fn size_hint(&self) -> (usize, Option<usize>) {
|
||||
let size = self.map.len();
|
||||
(size, Some(size))
|
||||
}
|
||||
}
|
||||
|
||||
impl FromIterator<(Box<dyn PartialReflect>, Box<dyn PartialReflect>)> for DynamicMap {
|
||||
fn from_iter<I: IntoIterator<Item = (Box<dyn PartialReflect>, Box<dyn PartialReflect>)>>(
|
||||
items: I,
|
||||
@ -504,24 +435,30 @@ impl<K: Reflect, V: Reflect> FromIterator<(K, V)> for DynamicMap {
|
||||
|
||||
impl IntoIterator for DynamicMap {
|
||||
type Item = (Box<dyn PartialReflect>, Box<dyn PartialReflect>);
|
||||
type IntoIter = alloc::vec::IntoIter<Self::Item>;
|
||||
type IntoIter = bevy_platform::collections::hash_table::IntoIter<Self::Item>;
|
||||
|
||||
fn into_iter(self) -> Self::IntoIter {
|
||||
self.values.into_iter()
|
||||
self.hash_table.into_iter()
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> IntoIterator for &'a DynamicMap {
|
||||
type Item = (&'a dyn PartialReflect, &'a dyn PartialReflect);
|
||||
type IntoIter = MapIter<'a>;
|
||||
type IntoIter = core::iter::Map<
|
||||
bevy_platform::collections::hash_table::Iter<
|
||||
'a,
|
||||
(Box<dyn PartialReflect>, Box<dyn PartialReflect>),
|
||||
>,
|
||||
fn(&'a (Box<dyn PartialReflect>, Box<dyn PartialReflect>)) -> Self::Item,
|
||||
>;
|
||||
|
||||
fn into_iter(self) -> Self::IntoIter {
|
||||
self.iter()
|
||||
self.hash_table
|
||||
.iter()
|
||||
.map(|(k, v)| (k.as_ref(), v.as_ref()))
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> ExactSizeIterator for MapIter<'a> {}
|
||||
|
||||
/// Compares a [`Map`] with a [`PartialReflect`] value.
|
||||
///
|
||||
/// Returns true if and only if all of the following are true:
|
||||
@ -584,6 +521,7 @@ pub fn map_debug(dyn_map: &dyn Map, f: &mut Formatter<'_>) -> core::fmt::Result
|
||||
/// Applies the elements of reflected map `b` to the corresponding elements of map `a`.
|
||||
///
|
||||
/// If a key from `b` does not exist in `a`, the value is cloned and inserted.
|
||||
/// If a key from `a` does not exist in `b`, the value is removed.
|
||||
///
|
||||
/// # Panics
|
||||
///
|
||||
@ -599,6 +537,7 @@ pub fn map_apply<M: Map>(a: &mut M, b: &dyn PartialReflect) {
|
||||
/// and returns a Result.
|
||||
///
|
||||
/// If a key from `b` does not exist in `a`, the value is cloned and inserted.
|
||||
/// If a key from `a` does not exist in `b`, the value is removed.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
@ -615,117 +554,17 @@ pub fn map_try_apply<M: Map>(a: &mut M, b: &dyn PartialReflect) -> Result<(), Ap
|
||||
a.insert_boxed(key.to_dynamic(), b_value.to_dynamic());
|
||||
}
|
||||
}
|
||||
a.retain(&mut |key, _| map_value.get(key).is_some());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
|
||||
use crate::PartialReflect;
|
||||
|
||||
use super::{DynamicMap, Map};
|
||||
use alloc::{
|
||||
borrow::ToOwned,
|
||||
string::{String, ToString},
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn test_into_iter() {
|
||||
let expected = ["foo", "bar", "baz"];
|
||||
|
||||
let mut map = DynamicMap::default();
|
||||
map.insert(0usize, expected[0].to_string());
|
||||
map.insert(1usize, expected[1].to_string());
|
||||
map.insert(2usize, expected[2].to_string());
|
||||
|
||||
for (index, item) in map.into_iter().enumerate() {
|
||||
let key = item
|
||||
.0
|
||||
.try_take::<usize>()
|
||||
.expect("couldn't downcast to usize");
|
||||
let value = item
|
||||
.1
|
||||
.try_take::<String>()
|
||||
.expect("couldn't downcast to String");
|
||||
assert_eq!(index, key);
|
||||
assert_eq!(expected[index], value);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_map_get_at() {
|
||||
let values = ["first", "second", "third"];
|
||||
let mut map = DynamicMap::default();
|
||||
map.insert(0usize, values[0].to_string());
|
||||
map.insert(1usize, values[1].to_string());
|
||||
map.insert(1usize, values[2].to_string());
|
||||
|
||||
let (key_r, value_r) = map.get_at(1).expect("Item wasn't found");
|
||||
let value = value_r
|
||||
.try_downcast_ref::<String>()
|
||||
.expect("Couldn't downcast to String");
|
||||
let key = key_r
|
||||
.try_downcast_ref::<usize>()
|
||||
.expect("Couldn't downcast to usize");
|
||||
assert_eq!(key, &1usize);
|
||||
assert_eq!(value, &values[2].to_owned());
|
||||
|
||||
assert!(map.get_at(2).is_none());
|
||||
map.remove(&1usize);
|
||||
assert!(map.get_at(1).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_map_get_at_mut() {
|
||||
let values = ["first", "second", "third"];
|
||||
let mut map = DynamicMap::default();
|
||||
map.insert(0usize, values[0].to_string());
|
||||
map.insert(1usize, values[1].to_string());
|
||||
map.insert(1usize, values[2].to_string());
|
||||
|
||||
let (key_r, value_r) = map.get_at_mut(1).expect("Item wasn't found");
|
||||
let value = value_r
|
||||
.try_downcast_mut::<String>()
|
||||
.expect("Couldn't downcast to String");
|
||||
let key = key_r
|
||||
.try_downcast_ref::<usize>()
|
||||
.expect("Couldn't downcast to usize");
|
||||
assert_eq!(key, &1usize);
|
||||
assert_eq!(value, &mut values[2].to_owned());
|
||||
|
||||
value.clone_from(&values[0].to_owned());
|
||||
|
||||
assert_eq!(
|
||||
map.get(&1usize)
|
||||
.expect("Item wasn't found")
|
||||
.try_downcast_ref::<String>()
|
||||
.expect("Couldn't downcast to String"),
|
||||
&values[0].to_owned()
|
||||
);
|
||||
|
||||
assert!(map.get_at(2).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn next_index_increment() {
|
||||
let values = ["first", "last"];
|
||||
let mut map = DynamicMap::default();
|
||||
map.insert(0usize, values[0]);
|
||||
map.insert(1usize, values[1]);
|
||||
|
||||
let mut iter = map.iter();
|
||||
let size = iter.len();
|
||||
|
||||
for _ in 0..2 {
|
||||
let prev_index = iter.index;
|
||||
assert!(iter.next().is_some());
|
||||
assert_eq!(prev_index, iter.index - 1);
|
||||
}
|
||||
|
||||
// When None we should no longer increase index
|
||||
for _ in 0..2 {
|
||||
assert!(iter.next().is_none());
|
||||
assert_eq!(size, iter.index);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove() {
|
||||
@ -743,4 +582,21 @@ mod tests {
|
||||
assert!(map.remove(&1).is_none());
|
||||
assert!(map.get(&1).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply() {
|
||||
let mut map_a = DynamicMap::default();
|
||||
map_a.insert(0, 0);
|
||||
map_a.insert(1, 1);
|
||||
|
||||
let mut map_b = DynamicMap::default();
|
||||
map_b.insert(10, 10);
|
||||
map_b.insert(1, 5);
|
||||
|
||||
map_a.apply(&map_b);
|
||||
|
||||
assert!(map_a.get(&0).is_none());
|
||||
assert_eq!(map_a.get(&1).unwrap().try_downcast_ref(), Some(&5));
|
||||
assert_eq!(map_a.get(&10).unwrap().try_downcast_ref(), Some(&10));
|
||||
}
|
||||
}
|
||||
|
||||
@ -167,10 +167,10 @@ where
|
||||
/// and excess elements in `value` are appended to `self`.
|
||||
/// - If `Self` 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 `self` are inserted.
|
||||
/// Keys which are not present in `self` are inserted, and keys from `self` which are not present in `value` are removed.
|
||||
/// - If `Self` is a [`Set`], then each element of `value` is applied to the corresponding
|
||||
/// element of `Self`. If an element of `value` does not exist in `Self` then it is
|
||||
/// cloned and inserted.
|
||||
/// cloned and inserted. If an element from `self` is not present in `value` then it is removed.
|
||||
/// - If `Self` is none of these, then `value` is downcast to `Self`, cloned, and
|
||||
/// assigned to `self`.
|
||||
///
|
||||
|
||||
@ -67,6 +67,11 @@ pub trait Set: PartialReflect {
|
||||
/// After calling this function, `self` will be empty.
|
||||
fn drain(&mut self) -> Vec<Box<dyn PartialReflect>>;
|
||||
|
||||
/// Retain only the elements specified by the predicate.
|
||||
///
|
||||
/// In other words, remove all elements `e` for which `f(&e)` returns `false`.
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn PartialReflect) -> bool);
|
||||
|
||||
/// Creates a new [`DynamicSet`] from this set.
|
||||
fn to_dynamic_set(&self) -> DynamicSet {
|
||||
let mut set = DynamicSet::default();
|
||||
@ -139,7 +144,7 @@ impl SetInfo {
|
||||
impl_generic_info_methods!(generics);
|
||||
}
|
||||
|
||||
/// An ordered set of reflected values.
|
||||
/// An unordered set of reflected values.
|
||||
#[derive(Default)]
|
||||
pub struct DynamicSet {
|
||||
represented_type: Option<&'static TypeInfo>,
|
||||
@ -205,6 +210,10 @@ impl Set for DynamicSet {
|
||||
self.hash_table.drain().collect::<Vec<_>>()
|
||||
}
|
||||
|
||||
fn retain(&mut self, f: &mut dyn FnMut(&dyn PartialReflect) -> bool) {
|
||||
self.hash_table.retain(move |value| f(&**value));
|
||||
}
|
||||
|
||||
fn insert_boxed(&mut self, value: Box<dyn PartialReflect>) -> bool {
|
||||
assert_eq!(
|
||||
value.reflect_partial_eq(&*value),
|
||||
@ -441,27 +450,23 @@ pub fn set_debug(dyn_set: &dyn Set, f: &mut Formatter<'_>) -> core::fmt::Result
|
||||
/// Applies the elements of reflected set `b` to the corresponding elements of set `a`.
|
||||
///
|
||||
/// If a value from `b` does not exist in `a`, the value is cloned and inserted.
|
||||
/// If a value from `a` does not exist in `b`, the value is removed.
|
||||
///
|
||||
/// # Panics
|
||||
///
|
||||
/// This function panics if `b` is not a reflected set.
|
||||
#[inline]
|
||||
pub fn set_apply<M: Set>(a: &mut M, b: &dyn PartialReflect) {
|
||||
if let ReflectRef::Set(set_value) = b.reflect_ref() {
|
||||
for b_value in set_value.iter() {
|
||||
if a.get(b_value).is_none() {
|
||||
a.insert_boxed(b_value.to_dynamic());
|
||||
}
|
||||
}
|
||||
} else {
|
||||
panic!("Attempted to apply a non-set type to a set type.");
|
||||
if let Err(err) = set_try_apply(a, b) {
|
||||
panic!("{err}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Tries to apply the elements of reflected set `b` to the corresponding elements of set `a`
|
||||
/// and returns a Result.
|
||||
///
|
||||
/// If a key from `b` does not exist in `a`, the value is cloned and inserted.
|
||||
/// If a value from `b` does not exist in `a`, the value is cloned and inserted.
|
||||
/// If a value from `a` does not exist in `b`, the value is removed.
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
@ -476,12 +481,15 @@ pub fn set_try_apply<S: Set>(a: &mut S, b: &dyn PartialReflect) -> Result<(), Ap
|
||||
a.insert_boxed(b_value.to_dynamic());
|
||||
}
|
||||
}
|
||||
a.retain(&mut |value| set_value.get(value).is_some());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::{PartialReflect, Set};
|
||||
|
||||
use super::DynamicSet;
|
||||
use alloc::string::{String, ToString};
|
||||
|
||||
@ -505,4 +513,21 @@ mod tests {
|
||||
assert_eq!(expected[index], value);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply() {
|
||||
let mut map_a = DynamicSet::default();
|
||||
map_a.insert(0);
|
||||
map_a.insert(1);
|
||||
|
||||
let mut map_b = DynamicSet::default();
|
||||
map_b.insert(1);
|
||||
map_b.insert(2);
|
||||
|
||||
map_a.apply(&map_b);
|
||||
|
||||
assert!(map_a.get(&0).is_none());
|
||||
assert_eq!(map_a.get(&1).unwrap().try_downcast_ref(), Some(&1));
|
||||
assert_eq!(map_a.get(&2).unwrap().try_downcast_ref(), Some(&2));
|
||||
}
|
||||
}
|
||||
|
||||
11
release-content/migration-guides/map_set_apply.md
Normal file
11
release-content/migration-guides/map_set_apply.md
Normal file
@ -0,0 +1,11 @@
|
||||
---
|
||||
title: `DynamicMap` is now unordered, `Map::get_at` and `Map::get_at_mut` are now removed, and `apply` removes excess entries from reflected maps.
|
||||
pull_requests: [19802]
|
||||
---
|
||||
|
||||
`DynamicMap` is now unordered, and the `Map` trait no longer assumes implementors to be ordered. If you previously relied on them being ordered, you should now store a list of keys (`Vec<Box<dyn PartialReflect>>`) separately.
|
||||
|
||||
`Map::get_at` and `Map::get_at_mut` are now removed. You should no longer use `usize` to index into the map, and instead use `&dyn PartialReflect` with `Map::get` and `Map::get_mut`.
|
||||
|
||||
`PartialReflect::apply(self, other)` for maps now removes excess entries (entries present in `self` which are not present in `other`).
|
||||
If you need those entries to be preserved, you will need to re-insert them manually.
|
||||
Loading…
Reference in New Issue
Block a user