From a1d026be0eaa2b6cd570bf4198c19d5e3579d79b Mon Sep 17 00:00:00 2001 From: radiish Date: Sat, 12 Nov 2022 19:03:20 +0000 Subject: [PATCH 1/4] added `Map::remove` --- crates/bevy_reflect/src/impls/std.rs | 6 ++++++ crates/bevy_reflect/src/map.rs | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 8eeac36461817..a53b4d12f79af 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -394,6 +394,12 @@ impl Map for HashMap { self.insert(key, value) .map(|old_value| Box::new(old_value) as Box) } + + fn remove(&mut self, key: &dyn Reflect) -> Option> { + key.downcast_ref::() + .and_then(|key| self.remove(key)) + .map(|value| Box::new(value) as Box) + } } impl Reflect for HashMap { diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index 9618b5cd2dd44..bf509afa12223 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -57,6 +57,12 @@ pub trait Map: Reflect { key: Box, value: Box, ) -> Option>; + + /// Removes a key from the map. + /// + /// If the map did not have this key present, `None` is returned. + /// If the map did have this key present, the removed value is returned. + fn remove(&mut self, key: &dyn Reflect) -> Option>; } /// A container for compile-time map info. @@ -246,6 +252,18 @@ impl Map for DynamicMap { } } + fn remove(&mut self, key: &dyn Reflect) -> Option> { + match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) { + Entry::Occupied(entry) => { + let index = *entry.get(); + entry.remove(); + let (_key, value) = self.values.remove(index); + Some(value) + }, + Entry::Vacant(_) => None, + } + } + fn drain(self: Box) -> Vec<(Box, Box)> { self.values } From b4a37edd932e1db40884e9a8c65b45c85f8b771f Mon Sep 17 00:00:00 2001 From: radiish Date: Sat, 12 Nov 2022 19:04:30 +0000 Subject: [PATCH 2/4] doc nit --- crates/bevy_reflect/src/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index bf509afa12223..f27eec732a4bc 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -58,7 +58,7 @@ pub trait Map: Reflect { value: Box, ) -> Option>; - /// Removes a key from the map. + /// Removes an entry from the map. /// /// If the map did not have this key present, `None` is returned. /// If the map did have this key present, the removed value is returned. From 088a8b9a9ae5df6acb6695e3089506ac460e4be7 Mon Sep 17 00:00:00 2001 From: radiish Date: Sat, 12 Nov 2022 19:25:51 +0000 Subject: [PATCH 3/4] format --- crates/bevy_reflect/src/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index f27eec732a4bc..092f54bcabeca 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -259,7 +259,7 @@ impl Map for DynamicMap { entry.remove(); let (_key, value) = self.values.remove(index); Some(value) - }, + } Entry::Vacant(_) => None, } } From 5f8c21948f3ef0a75a438b7fb1476a30cb8b32ef Mon Sep 17 00:00:00 2001 From: radiish Date: Sat, 12 Nov 2022 19:41:44 +0000 Subject: [PATCH 4/4] use from_reflect for `DynamicMap` and simplify implementation --- crates/bevy_reflect/src/impls/std.rs | 5 +++++ crates/bevy_reflect/src/map.rs | 14 +++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index a53b4d12f79af..ce56cef43f219 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -396,7 +396,12 @@ impl Map for HashMap { } fn remove(&mut self, key: &dyn Reflect) -> Option> { + let mut from_reflect = None; key.downcast_ref::() + .or_else(|| { + from_reflect = K::from_reflect(key); + from_reflect.as_ref() + }) .and_then(|key| self.remove(key)) .map(|value| Box::new(value) as Box) } diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index 092f54bcabeca..f5e4a8695f2a2 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -253,15 +253,11 @@ impl Map for DynamicMap { } fn remove(&mut self, key: &dyn Reflect) -> Option> { - match self.indices.entry(key.reflect_hash().expect(HASH_ERROR)) { - Entry::Occupied(entry) => { - let index = *entry.get(); - entry.remove(); - let (_key, value) = self.values.remove(index); - Some(value) - } - Entry::Vacant(_) => None, - } + let index = self + .indices + .remove(&key.reflect_hash().expect(HASH_ERROR))?; + let (_key, value) = self.values.remove(index); + Some(value) } fn drain(self: Box) -> Vec<(Box, Box)> {