diff --git a/benches/benches/bevy_ecs/run_criteria.rs b/benches/benches/bevy_ecs/run_criteria.rs index 7d842d01ea9599..26a9ef1984e72c 100644 --- a/benches/benches/bevy_ecs/run_criteria.rs +++ b/benches/benches/bevy_ecs/run_criteria.rs @@ -2,7 +2,7 @@ use bevy_ecs::{ component::Component, prelude::{ParallelSystemDescriptorCoercion, Res, RunCriteriaDescriptorCoercion}, schedule::{ShouldRun, Stage, SystemStage}, - system::{IntoSystem, Query}, + system::Query, world::World, }; use criterion::{criterion_group, criterion_main, Criterion}; diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs index 0a4a459073a5dd..00e0c6b74607ed 100644 --- a/crates/bevy_ecs/src/event.rs +++ b/crates/bevy_ecs/src/event.rs @@ -3,6 +3,7 @@ use crate as bevy_ecs; use crate::system::{Local, Res, ResMut, SystemParam}; use bevy_utils::tracing::trace; +use std::ops::{Deref, DerefMut}; use std::{ fmt::{self}, hash::Hash, @@ -56,12 +57,6 @@ struct EventInstance { pub event: E, } -#[derive(Debug)] -enum State { - A, - B, -} - /// An event collection that represents the events that occurred within the last two /// [`Events::update`] calls. /// Events can be written to using an [`EventWriter`] @@ -135,42 +130,92 @@ enum State { /// #[derive(Debug)] pub struct Events { - events_a: Vec>, - events_b: Vec>, - a_start_event_count: usize, - b_start_event_count: usize, + /// Holds the oldest still active events. + /// Note that a.start_event_count + a.len() should always === events_b.start_event_count. + events_a: EventSequence, + /// Holds the newer events. + events_b: EventSequence, event_count: usize, - state: State, } +// Derived Default impl would incorrectly require E: Default impl Default for Events { fn default() -> Self { - Events { - a_start_event_count: 0, - b_start_event_count: 0, - event_count: 0, - events_a: Vec::new(), - events_b: Vec::new(), - state: State::A, + Self { + events_a: Default::default(), + events_b: Default::default(), + event_count: Default::default(), } } } -fn map_instance_event_with_id(event_instance: &EventInstance) -> (&E, EventId) { - (&event_instance.event, event_instance.event_id) +#[derive(Debug)] +struct EventSequence { + events: Vec>, + start_event_count: usize, +} + +// Derived Default impl would incorrectly require E: Default +impl Default for EventSequence { + fn default() -> Self { + Self { + events: Default::default(), + start_event_count: Default::default(), + } + } } -fn map_instance_event(event_instance: &EventInstance) -> &E { - &event_instance.event +impl Deref for EventSequence { + type Target = Vec>; + + fn deref(&self) -> &Self::Target { + &self.events + } +} + +impl DerefMut for EventSequence { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.events + } } /// Reads events of type `T` in order and tracks which events have already been read. #[derive(SystemParam)] pub struct EventReader<'w, 's, E: Event> { - last_event_count: Local<'s, (usize, PhantomData)>, + reader: Local<'s, ManualEventReader>, events: Res<'w, Events>, } +impl<'w, 's, E: Event> EventReader<'w, 's, E> { + /// Iterates over the events this [`EventReader`] has not seen yet. This updates the + /// [`EventReader`]'s event counter, which means subsequent event reads will not include events + /// that happened before now. + pub fn iter(&mut self) -> impl DoubleEndedIterator + ExactSizeIterator { + self.iter_with_id().map(|(event, _id)| event) + } + + /// Like [`iter`](Self::iter), except also returning the [`EventId`] of the events. + pub fn iter_with_id( + &mut self, + ) -> impl DoubleEndedIterator)> + ExactSizeIterator)> + { + self.reader.iter_with_id(&self.events).map(|r @ (_, id)| { + trace!("EventReader::iter() -> {}", id); + r + }) + } + + /// Determines the number of events available to be read from this [`EventReader`] without consuming any. + pub fn len(&self) -> usize { + self.reader.len(&self.events) + } + + /// Determines if are any events available to be read without consuming any. + pub fn is_empty(&self) -> bool { + self.len() == 0 + } +} + /// Sends events of type `T`. #[derive(SystemParam)] pub struct EventWriter<'w, 's, E: Event> { @@ -199,6 +244,7 @@ impl<'w, 's, E: Event> EventWriter<'w, 's, E> { } } +#[derive(Debug)] pub struct ManualEventReader { last_event_count: usize, _marker: PhantomData, @@ -220,7 +266,7 @@ impl ManualEventReader { &'a mut self, events: &'a Events, ) -> impl DoubleEndedIterator + ExactSizeIterator { - internal_event_reader(&mut self.last_event_count, events).map(|(e, _)| e) + self.iter_with_id(events).map(|(e, _)| e) } /// See [`EventReader::iter_with_id`] @@ -229,12 +275,34 @@ impl ManualEventReader { events: &'a Events, ) -> impl DoubleEndedIterator)> + ExactSizeIterator)> { - internal_event_reader(&mut self.last_event_count, events) + // if the reader has seen some of the events in a buffer, find the proper index offset. + // otherwise read all events in the buffer + let a_index = (self.last_event_count).saturating_sub(events.events_a.start_event_count); + let b_index = (self.last_event_count).saturating_sub(events.events_b.start_event_count); + let a = events.events_a.get(a_index..).unwrap_or_default(); + let b = events.events_b.get(b_index..).unwrap_or_default(); + let unread_count = a.len() + b.len(); + // Ensure `len` is implemented correctly + debug_assert_eq!(unread_count, self.len(events)); + self.last_event_count = events.event_count - unread_count; + // Iterate the oldest first, then the newer events + let iterator = a.iter().chain(b.iter()); + iterator + .map(|e| (&e.event, e.event_id)) + .with_exact_size(unread_count) + .inspect(move |(_, id)| self.last_event_count = (id.id + 1).max(self.last_event_count)) } /// See [`EventReader::len`] pub fn len(&self, events: &Events) -> usize { - internal_event_reader(&mut self.last_event_count.clone(), events).len() + // The number of events in this reader is the difference between the most recent event + // and the last event seen by it. This will be at most the number of events contained + // with the events (any others have already been dropped) + // TODO: Warn when there are dropped events, or return e.g. a `Result` + events + .event_count + .saturating_sub(self.last_event_count) + .min(events.len()) } /// See [`EventReader::is_empty`] @@ -243,39 +311,6 @@ impl ManualEventReader { } } -/// Like [`iter_with_id`](EventReader::iter_with_id) except not emitting any traces for read -/// messages. -fn internal_event_reader<'a, E: Event>( - last_event_count: &'a mut usize, - events: &'a Events, -) -> impl DoubleEndedIterator)> + ExactSizeIterator)> -{ - // if the reader has seen some of the events in a buffer, find the proper index offset. - // otherwise read all events in the buffer - let a_index = if *last_event_count > events.a_start_event_count { - *last_event_count - events.a_start_event_count - } else { - 0 - }; - let b_index = if *last_event_count > events.b_start_event_count { - *last_event_count - events.b_start_event_count - } else { - 0 - }; - let a = events.events_a.get(a_index..).unwrap_or_default(); - let b = events.events_b.get(b_index..).unwrap_or_default(); - let unread_count = a.len() + b.len(); - *last_event_count = events.event_count - unread_count; - let iterator = match events.state { - State::A => b.iter().chain(a.iter()), - State::B => a.iter().chain(b.iter()), - }; - iterator - .map(map_instance_event_with_id) - .with_exact_size(unread_count) - .inspect(move |(_, id)| *last_event_count = (id.id + 1).max(*last_event_count)) -} - trait IteratorExt { fn with_exact_size(self, len: usize) -> ExactSize where @@ -330,36 +365,6 @@ impl ExactSizeIterator for ExactSize { } } -impl<'w, 's, E: Event> EventReader<'w, 's, E> { - /// Iterates over the events this [`EventReader`] has not seen yet. This updates the - /// [`EventReader`]'s event counter, which means subsequent event reads will not include events - /// that happened before now. - pub fn iter(&mut self) -> impl DoubleEndedIterator + ExactSizeIterator { - self.iter_with_id().map(|(event, _id)| event) - } - - /// Like [`iter`](Self::iter), except also returning the [`EventId`] of the events. - pub fn iter_with_id( - &mut self, - ) -> impl DoubleEndedIterator)> + ExactSizeIterator)> - { - internal_event_reader(&mut self.last_event_count.0, &self.events).map(|(event, id)| { - trace!("EventReader::iter() -> {}", id); - (event, id) - }) - } - - /// Determines the number of events available to be read from this [`EventReader`] without consuming any. - pub fn len(&self) -> usize { - internal_event_reader(&mut self.last_event_count.0.clone(), &self.events).len() - } - - /// Determines if are any events available to be read without consuming any. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } -} - impl Events { /// "Sends" an `event` by writing it to the current event buffer. [`EventReader`]s can then read /// the event. @@ -372,11 +377,7 @@ impl Events { let event_instance = EventInstance { event_id, event }; - match self.state { - State::A => self.events_a.push(event_instance), - State::B => self.events_b.push(event_instance), - } - + self.events_b.push(event_instance); self.event_count += 1; } @@ -390,10 +391,7 @@ impl Events { /// Gets a new [`ManualEventReader`]. This will include all events already in the event buffers. pub fn get_reader(&self) -> ManualEventReader { - ManualEventReader { - last_event_count: 0, - _marker: PhantomData, - } + ManualEventReader::default() } /// Gets a new [`ManualEventReader`]. This will ignore all events already in the event buffers. @@ -401,25 +399,20 @@ impl Events { pub fn get_reader_current(&self) -> ManualEventReader { ManualEventReader { last_event_count: self.event_count, - _marker: PhantomData, + ..Default::default() } } /// Swaps the event buffers and clears the oldest event buffer. In general, this should be /// called once per frame/update. pub fn update(&mut self) { - match self.state { - State::A => { - self.events_b.clear(); - self.state = State::B; - self.b_start_event_count = self.event_count; - } - State::B => { - self.events_a.clear(); - self.state = State::A; - self.a_start_event_count = self.event_count; - } - } + std::mem::swap(&mut self.events_a, &mut self.events_b); + self.events_b.clear(); + self.events_b.start_event_count = self.event_count; + debug_assert_eq!( + self.events_a.start_event_count + self.events_a.len(), + self.events_b.start_event_count + ); } /// A system that calls [`Events::update`] once per frame. @@ -429,8 +422,8 @@ impl Events { #[inline] fn reset_start_event_count(&mut self) { - self.a_start_event_count = self.event_count; - self.b_start_event_count = self.event_count; + self.events_a.start_event_count = self.event_count; + self.events_b.start_event_count = self.event_count; } /// Removes all events. @@ -441,29 +434,26 @@ impl Events { self.events_b.clear(); } + #[inline] + pub fn len(&self) -> usize { + self.events_a.len() + self.events_b.len() + } + /// Returns true if there are no events in this collection. #[inline] pub fn is_empty(&self) -> bool { - self.events_a.is_empty() && self.events_b.is_empty() + self.len() == 0 } /// Creates a draining iterator that removes all events. pub fn drain(&mut self) -> impl Iterator + '_ { self.reset_start_event_count(); - let map = |i: EventInstance| i.event; - match self.state { - State::A => self - .events_b - .drain(..) - .map(map) - .chain(self.events_a.drain(..).map(map)), - State::B => self - .events_a - .drain(..) - .map(map) - .chain(self.events_b.drain(..).map(map)), - } + // Drain the oldest events first, then the newest + self.events_a + .drain(..) + .chain(self.events_b.drain(..)) + .map(|i| i.event) } /// Iterates over events that happened since the last "update" call. @@ -475,10 +465,7 @@ impl Events { pub fn iter_current_update_events( &self, ) -> impl DoubleEndedIterator + ExactSizeIterator { - match self.state { - State::A => self.events_a.iter().map(map_instance_event), - State::B => self.events_b.iter().map(map_instance_event), - } + self.events_b.iter().map(|i| &i.event) } } @@ -497,10 +484,7 @@ impl std::iter::Extend for Events { EventInstance { event_id, event } }); - match self.state { - State::A => self.events_a.extend(events), - State::B => self.events_b.extend(events), - } + self.events_b.extend(events); trace!( "Events::extend() -> ids: ({}..{})", @@ -719,6 +703,8 @@ mod tests { let mut events = Events::::default(); events.send(TestEvent { i: 0 }); let reader = events.get_reader_current(); + dbg!(&reader); + dbg!(&events); assert!(reader.is_empty(&events)); events.send(TestEvent { i: 0 }); assert_eq!(reader.len(&events), 1);