From f2bb90607341a9d6d8b21842d200e29c8aa23518 Mon Sep 17 00:00:00 2001 From: Aceeri Date: Thu, 16 Feb 2023 12:51:32 -0800 Subject: [PATCH 1/4] Make RemovedComponents effectively mirror EventReaders api surface --- crates/bevy_ecs/src/removal_detection.rs | 86 +++++++++++++++++++++++- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index f9953ff77cf9e..15653f5890b3e 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -4,7 +4,7 @@ use crate::{ self as bevy_ecs, component::{Component, ComponentId, ComponentIdFor}, entity::Entity, - event::{Events, ManualEventIterator, ManualEventReader}, + event::{Events, ManualEventIterator, ManualEventIteratorWithId, ManualEventReader, EventId}, prelude::Local, storage::SparseSet, system::{ReadOnlySystemParam, SystemMeta, SystemParam}, @@ -96,6 +96,8 @@ impl RemovedComponentEvents { } /// A [`SystemParam`] that grants access to the entities that had their `T` [`Component`] removed. +/// +/// This acts effectively the same as an [`EventReader`]. /// /// Note that this does not allow you to see which data existed before removal. /// If you need this, you will need to track the component data value on your own, @@ -141,15 +143,93 @@ pub type RemovedIter<'a> = iter::Map< fn(RemovedComponentEntity) -> Entity, >; +/// Iterator over entities that had a specific component removed. +/// +/// See [`RemovedComponents`]. +pub type RemovedIterWithId<'a> = iter::Map>, +>, fn((&RemovedComponentEntity, EventId)) -> (Entity, EventId)>; + +fn map_id_events((entity, id): (&RemovedComponentEntity, EventId)) -> (Entity, EventId) { + (entity.clone().into(), id) +} + +// For all practical purposes, the api surface of `RemovedComponents` +// should be similar to `EventReader` to reduce confusion. impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> { - pub fn iter(&mut self) -> RemovedIter<'_> { + /// Fetch underlying [`ManualEventReader`]. + pub fn reader(&self) -> &ManualEventReader { + &*self.reader + } + + /// Fetch underlying [`ManualEventReader`] mutably. + pub fn reader_mut(&mut self) -> &mut ManualEventReader { + &mut *self.reader + } + + /// Fetch underlying [`Events`]. + pub fn events(&self) -> Option<&Events> { + self.event_sets.get(**self.component_id) + } + + /// Destructures to get a mutable reference to the `ManualEventReader` + /// and a reference to `Events`. + /// + /// This is necessary since Rust can't detect destructuring through methods and most + /// usecases of the reader uses the `Events` as well. + pub fn reader_mut_with_events( + &mut self, + ) -> Option<( + &mut RemovedComponentReader, + &Events, + )> { self.event_sets .get(**self.component_id) - .map(|events| self.reader.iter(events).cloned()) + .map(|events| (&mut *self.reader, events)) + } + + /// Iterates over the events this [`RemovedComponents`] has not seen yet. This updates the + /// [`RemovedComponents`]'s event counter, which means subsequent event reads will not include events + /// that happened before now. + pub fn iter(&mut self) -> RemovedIter<'_> { + self.reader_mut_with_events() + .map(|(reader, events)| reader.iter(events).cloned()) .into_iter() .flatten() .map(RemovedComponentEntity::into) } + + /// Like [`iter`](Self::iter), except also returning the [`EventId`] of the events. + pub fn iter_with_id(&mut self) -> RemovedIterWithId<'_> { + self.reader_mut_with_events() + .map(|(reader, events)| reader.iter_with_id(events)) + .into_iter() + .flatten() + .map(map_id_events) + } + + /// Determines the number of removal events available to be read from this [`RemovedComponents`] without consuming any. + pub fn len(&self) -> usize { + self.events() + .map(|events| self.reader.len(&events)) + .unwrap_or(0) + } + + /// Returns `true` if there are no events available to read. + pub fn is_empty(&self) -> bool { + self.events() + .map(|events| self.reader.is_empty(&events)) + .unwrap_or(true) + } + + /// Consumes all available events. + /// + /// This means these events will not appear in calls to [`RemovedComponents::iter()`] or + /// [`RemovedComponents::iter_with_id()`] and [`RemovedComponents::is_empty()`] will return `true`. + pub fn clear(&mut self) { + self.reader_mut_with_events() + .map(|(reader, events)| reader.clear(&events)); + } } impl<'a, 'w, 's: 'a, T> IntoIterator for &'a mut RemovedComponents<'w, 's, T> From 0c2f6156959ed75cc83cf0c4e4767cf5cf3bd009 Mon Sep 17 00:00:00 2001 From: Aceeri Date: Thu, 16 Feb 2023 13:00:59 -0800 Subject: [PATCH 2/4] formatting --- crates/bevy_ecs/src/removal_detection.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index 15653f5890b3e..5d63b70d7fab3 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -4,7 +4,7 @@ use crate::{ self as bevy_ecs, component::{Component, ComponentId, ComponentIdFor}, entity::Entity, - event::{Events, ManualEventIterator, ManualEventIteratorWithId, ManualEventReader, EventId}, + event::{EventId, Events, ManualEventIterator, ManualEventIteratorWithId, ManualEventReader}, prelude::Local, storage::SparseSet, system::{ReadOnlySystemParam, SystemMeta, SystemParam}, @@ -96,7 +96,7 @@ impl RemovedComponentEvents { } /// A [`SystemParam`] that grants access to the entities that had their `T` [`Component`] removed. -/// +/// /// This acts effectively the same as an [`EventReader`]. /// /// Note that this does not allow you to see which data existed before removal. @@ -146,11 +146,16 @@ pub type RemovedIter<'a> = iter::Map< /// Iterator over entities that had a specific component removed. /// /// See [`RemovedComponents`]. -pub type RemovedIterWithId<'a> = iter::Map>, ->, fn((&RemovedComponentEntity, EventId)) -> (Entity, EventId)>; +pub type RemovedIterWithId<'a> = iter::Map< + iter::Flatten>>, + fn( + (&RemovedComponentEntity, EventId), + ) -> (Entity, EventId), +>; -fn map_id_events((entity, id): (&RemovedComponentEntity, EventId)) -> (Entity, EventId) { +fn map_id_events( + (entity, id): (&RemovedComponentEntity, EventId), +) -> (Entity, EventId) { (entity.clone().into(), id) } @@ -174,7 +179,7 @@ impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> { /// Destructures to get a mutable reference to the `ManualEventReader` /// and a reference to `Events`. - /// + /// /// This is necessary since Rust can't detect destructuring through methods and most /// usecases of the reader uses the `Events` as well. pub fn reader_mut_with_events( From 0059a9b2b2675d01c5b19263e3e877ef7d2042e2 Mon Sep 17 00:00:00 2001 From: Aceeri Date: Thu, 16 Feb 2023 13:24:29 -0800 Subject: [PATCH 3/4] Fix up some clippy issues --- crates/bevy_ecs/src/removal_detection.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index 5d63b70d7fab3..1ebb013245186 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -164,12 +164,12 @@ fn map_id_events( impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> { /// Fetch underlying [`ManualEventReader`]. pub fn reader(&self) -> &ManualEventReader { - &*self.reader + &self.reader } /// Fetch underlying [`ManualEventReader`] mutably. pub fn reader_mut(&mut self) -> &mut ManualEventReader { - &mut *self.reader + &mut self.reader } /// Fetch underlying [`Events`]. @@ -216,14 +216,14 @@ impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> { /// Determines the number of removal events available to be read from this [`RemovedComponents`] without consuming any. pub fn len(&self) -> usize { self.events() - .map(|events| self.reader.len(&events)) + .map(|events| self.reader.len(events)) .unwrap_or(0) } /// Returns `true` if there are no events available to read. pub fn is_empty(&self) -> bool { self.events() - .map(|events| self.reader.is_empty(&events)) + .map(|events| self.reader.is_empty(events)) .unwrap_or(true) } @@ -232,8 +232,9 @@ impl<'w, 's, T: Component> RemovedComponents<'w, 's, T> { /// This means these events will not appear in calls to [`RemovedComponents::iter()`] or /// [`RemovedComponents::iter_with_id()`] and [`RemovedComponents::is_empty()`] will return `true`. pub fn clear(&mut self) { - self.reader_mut_with_events() - .map(|(reader, events)| reader.clear(&events)); + if let Some((reader, events)) = self.reader_mut_with_events() { + reader.clear(events); + } } } From 53c9661eb31808dc4191442c6ea75fd51690bc05 Mon Sep 17 00:00:00 2001 From: Aceeri Date: Thu, 16 Feb 2023 13:36:23 -0800 Subject: [PATCH 4/4] Fix path for event reader --- crates/bevy_ecs/src/removal_detection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs index 1ebb013245186..004f08ca347e7 100644 --- a/crates/bevy_ecs/src/removal_detection.rs +++ b/crates/bevy_ecs/src/removal_detection.rs @@ -97,7 +97,7 @@ impl RemovedComponentEvents { /// A [`SystemParam`] that grants access to the entities that had their `T` [`Component`] removed. /// -/// This acts effectively the same as an [`EventReader`]. +/// This acts effectively the same as an [`EventReader`](crate::event::EventReader). /// /// Note that this does not allow you to see which data existed before removal. /// If you need this, you will need to track the component data value on your own,