From c5702b9b5d51dd66652edc9ff2dba138e9df89e1 Mon Sep 17 00:00:00 2001
From: Aceeri <conmcclusk@gmail.com>
Date: Fri, 17 Feb 2023 00:01:13 +0000
Subject: [PATCH] Make RemovedComponents mirror EventReaders api surface
 (#7713)

# Objective
- RemovedComponents is just a thin wrapper around Events/ManualEventReader which is the same as an EventReader, so most usecases that of an EventReader will probably be useful for RemovedComponents too.

I was thinking of making a trait for this but I don't think it is worth the overhead currently.

## Solution
- Mirror the api surface of EventReader
---
 crates/bevy_ecs/src/removal_detection.rs | 92 +++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/crates/bevy_ecs/src/removal_detection.rs b/crates/bevy_ecs/src/removal_detection.rs
index f9953ff77c..004f08ca34 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::{EventId, Events, ManualEventIterator, ManualEventIteratorWithId, ManualEventReader},
     prelude::Local,
     storage::SparseSet,
     system::{ReadOnlySystemParam, SystemMeta, SystemParam},
@@ -97,6 +97,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`](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,
 /// using a regularly scheduled system that requests `Query<(Entity, &T), Changed<T>>`
@@ -141,15 +143,99 @@ 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<
+    iter::Flatten<option::IntoIter<ManualEventIteratorWithId<'a, RemovedComponentEntity>>>,
+    fn(
+        (&RemovedComponentEntity, EventId<RemovedComponentEntity>),
+    ) -> (Entity, EventId<RemovedComponentEntity>),
+>;
+
+fn map_id_events(
+    (entity, id): (&RemovedComponentEntity, EventId<RemovedComponentEntity>),
+) -> (Entity, EventId<RemovedComponentEntity>) {
+    (entity.clone().into(), id)
+}
+
+// For all practical purposes, the api surface of `RemovedComponents<T>`
+// should be similar to `EventReader<T>` 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<RemovedComponentEntity> {
+        &self.reader
+    }
+
+    /// Fetch underlying [`ManualEventReader`] mutably.
+    pub fn reader_mut(&mut self) -> &mut ManualEventReader<RemovedComponentEntity> {
+        &mut self.reader
+    }
+
+    /// Fetch underlying [`Events`].
+    pub fn events(&self) -> Option<&Events<RemovedComponentEntity>> {
+        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<T>,
+        &Events<RemovedComponentEntity>,
+    )> {
         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) {
+        if let Some((reader, events)) = self.reader_mut_with_events() {
+            reader.clear(events);
+        }
+    }
 }
 
 impl<'a, 'w, 's: 'a, T> IntoIterator for &'a mut RemovedComponents<'w, 's, T>