From 32023a5f6a15fa0d219e6df71b3e7f559b1ae31d Mon Sep 17 00:00:00 2001
From: ira <JustTheCoolDude@gmail.com>
Date: Sun, 5 Feb 2023 15:18:19 +0000
Subject: [PATCH] Remove broken `DoubleEndedIterator` impls on event iterators
 (#7469)

The `DoubleEndedIterator` impls produce incorrect results on subsequent calls to `iter()` if the iterator is only partially consumed.

The following code shows what happens
```rust
fn next_back_is_bad() {
    let mut events = Events::<TestEvent>::default();
    events.send(TestEvent { i: 0 });
    events.send(TestEvent { i: 1 });
    events.send(TestEvent { i: 2 });
    let mut reader = events.get_reader();
    let mut iter = reader.iter(&events);
    assert_eq!(iter.next_back(), Some(&TestEvent { i: 2 }));
    assert_eq!(iter.next(), Some(&TestEvent { i: 0 }));

    let mut iter = reader.iter(&events);
    // `i: 2` event is returned twice! The `i: 1` event is missed.
    assert_eq!(iter.next(), Some(&TestEvent { i: 2 }));
    assert_eq!(iter.next(), None);
}
```

I don't think this can be fixed without adding some very convoluted bookkeeping.

## Migration Guide
`ManualEventIterator` and `ManualEventIteratorWithId` are no longer `DoubleEndedIterator`s.



Co-authored-by: devil-ira <justthecooldude@gmail.com>
---
 crates/bevy_ecs/src/event.rs     | 31 ++++---------------------------
 crates/bevy_ecs/src/world/mod.rs |  7 ++-----
 crates/bevy_ui/src/flex/mod.rs   |  3 ++-
 3 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs
index 3c0b278e00..8ef402fd09 100644
--- a/crates/bevy_ecs/src/event.rs
+++ b/crates/bevy_ecs/src/event.rs
@@ -390,12 +390,6 @@ impl<'a, E: Event> ExactSizeIterator for ManualEventIterator<'a, E> {
     }
 }
 
-impl<'a, E: Event> DoubleEndedIterator for ManualEventIterator<'a, E> {
-    fn next_back(&mut self) -> Option<Self::Item> {
-        self.iter.next_back().map(|(event, _)| event)
-    }
-}
-
 #[derive(Debug)]
 pub struct ManualEventIteratorWithId<'a, E: Event> {
     reader: &'a mut ManualEventReader<E>,
@@ -457,23 +451,6 @@ impl<'a, E: Event> Iterator for ManualEventIteratorWithId<'a, E> {
     }
 }
 
-impl<'a, E: Event> DoubleEndedIterator for ManualEventIteratorWithId<'a, E> {
-    fn next_back(&mut self) -> Option<Self::Item> {
-        match self
-            .chain
-            .next_back()
-            .map(|instance| (&instance.event, instance.event_id))
-        {
-            Some(item) => {
-                event_trace(item.1);
-                self.unread -= 1;
-                Some(item)
-            }
-            None => None,
-        }
-    }
-}
-
 impl<'a, E: Event> ExactSizeIterator for ManualEventIteratorWithId<'a, E> {
     fn len(&self) -> usize {
         self.unread
@@ -577,9 +554,7 @@ impl<E: Event> Events<E> {
     /// between the last `update()` call and your call to `iter_current_update_events`.
     /// If events happen outside that window, they will not be handled. For example, any events that
     /// happen after this call and before the next `update()` call will be dropped.
-    pub fn iter_current_update_events(
-        &self,
-    ) -> impl DoubleEndedIterator<Item = &E> + ExactSizeIterator<Item = &E> {
+    pub fn iter_current_update_events(&self) -> impl ExactSizeIterator<Item = &E> {
         self.events_b.iter().map(|i| &i.event)
     }
 
@@ -837,8 +812,10 @@ mod tests {
         assert_eq!(iter.len(), 3);
         iter.next();
         assert_eq!(iter.len(), 2);
-        iter.next_back();
+        iter.next();
         assert_eq!(iter.len(), 1);
+        iter.next();
+        assert_eq!(iter.len(), 0);
     }
 
     #[test]
diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs
index 0001a754bb..981accba96 100644
--- a/crates/bevy_ecs/src/world/mod.rs
+++ b/crates/bevy_ecs/src/world/mod.rs
@@ -772,7 +772,7 @@ impl World {
 
     /// Returns an iterator of entities that had components of type `T` removed
     /// since the last call to [`World::clear_trackers`].
-    pub fn removed<T: Component>(&self) -> impl DoubleEndedIterator<Item = Entity> + '_ {
+    pub fn removed<T: Component>(&self) -> impl Iterator<Item = Entity> + '_ {
         self.components
             .get_id(TypeId::of::<T>())
             .map(|component_id| self.removed_with_id(component_id))
@@ -782,10 +782,7 @@ impl World {
 
     /// Returns an iterator of entities that had components with the given `component_id` removed
     /// since the last call to [`World::clear_trackers`].
-    pub fn removed_with_id(
-        &self,
-        component_id: ComponentId,
-    ) -> impl DoubleEndedIterator<Item = Entity> + '_ {
+    pub fn removed_with_id(&self, component_id: ComponentId) -> impl Iterator<Item = Entity> + '_ {
         self.removed_components
             .get(component_id)
             .map(|removed| removed.iter_current_update_events().cloned())
diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs
index 37dca527b7..b353371154 100644
--- a/crates/bevy_ui/src/flex/mod.rs
+++ b/crates/bevy_ui/src/flex/mod.rs
@@ -270,7 +270,8 @@ pub fn flex_node_system(
         }
     }
 
-    if scale_factor_events.iter().next_back().is_some() || ui_scale.is_changed() {
+    if !scale_factor_events.is_empty() || ui_scale.is_changed() {
+        scale_factor_events.clear();
         update_changed(&mut flex_surface, scale_factor, full_node_query);
     } else {
         update_changed(&mut flex_surface, scale_factor, node_query);