Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Implement len and is_empty for EventReaders #2969

Closed
wants to merge 6 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 88 additions & 2 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ impl<T> Default for ManualEventReader<T> {
}
}

impl<T> ManualEventReader<T> {
#[allow(clippy::len_without_is_empty)] // Check fails since the is_empty implementation has a signature other than `(&self) -> bool`
impl<T: Resource> ManualEventReader<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does T need to be a Resource?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it calls events.event_reader_len() which is only implemented for Events<T: Resource>. This seems reasonable to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although in practice, events dont need to be resources themselves. This could get more confusing if we decide to require Resource derives / enable more configuration options. Removing that bound in favor of something like (Send + Sync) is worth considering (in a separate issue / pr).

/// See [`EventReader::iter`]
pub fn iter<'a>(&'a mut self, events: &'a Events<T>) -> impl DoubleEndedIterator<Item = &'a T> {
internal_event_reader(&mut self.last_event_count, events).map(|(e, _)| e)
Expand All @@ -205,6 +206,16 @@ impl<T> ManualEventReader<T> {
) -> impl DoubleEndedIterator<Item = (&'a T, EventId<T>)> {
internal_event_reader(&mut self.last_event_count, events)
}

/// See [`EventReader::len`]
pub fn len(&self, events: &Events<T>) -> usize {
events.event_reader_len(self.last_event_count)
}

/// See [`EventReader::is_empty`]
pub fn is_empty(&self, events: &Events<T>) -> bool {
self.len(events) == 0
}
}

/// Like [`iter_with_id`](EventReader::iter_with_id) except not emitting any traces for read
Expand Down Expand Up @@ -253,6 +264,16 @@ impl<'w, 's, T: Resource> EventReader<'w, 's, T> {
(event, id)
})
}

/// Determines the number of events available to be read from this [`EventReader`] without consuming any.
pub fn len(&self) -> usize {
self.events.event_reader_len(self.last_event_count.0)
}

/// Determines if are any events available to be read without consuming any.
pub fn is_empty(&self) -> bool {
self.len() == 0
}
}

impl<T: Resource> Events<T> {
Expand Down Expand Up @@ -355,7 +376,7 @@ impl<T: Resource> Events<T> {

/// Iterates over events that happened since the last "update" call.
/// WARNING: You probably don't want to use this call. In most cases you should use an
/// `EventReader`. You should only use this if you know you only need to consume events
/// [`EventReader`]. You should only use this if you know you only need to consume events
/// 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.
Expand All @@ -365,6 +386,29 @@ impl<T: Resource> Events<T> {
State::B => self.events_b.iter().map(map_instance_event),
}
}

/// Determines how many events are in the reader after the given `last_event_count` parameter
fn event_reader_len(&self, last_event_count: usize) -> usize {
let a_count = if last_event_count <= self.a_start_event_count {
self.events_a.len()
} else {
self.events_a
.len()
.checked_sub(last_event_count - self.a_start_event_count)
.unwrap_or_default()
};

let b_count = if last_event_count <= self.b_start_event_count {
self.events_b.len()
} else {
self.events_b
.len()
.checked_sub(last_event_count - self.b_start_event_count)
.unwrap_or_default()
};

a_count + b_count
}
}

impl<T> std::iter::Extend<T> for Events<T> {
Expand Down Expand Up @@ -566,4 +610,46 @@ mod tests {
events.update();
assert!(events.is_empty());
}

#[test]
fn test_event_reader_len_empty() {
let events = Events::<TestEvent>::default();
assert_eq!(events.get_reader().len(&events), 0);
assert!(events.get_reader().is_empty(&events));
}

#[test]
fn test_event_reader_len_filled() {
let mut events = Events::<TestEvent>::default();
events.send(TestEvent { i: 0 });
assert_eq!(events.get_reader().len(&events), 1);
assert!(!events.get_reader().is_empty(&events));
}

#[test]
fn test_event_reader_len_current() {
let mut events = Events::<TestEvent>::default();
events.send(TestEvent { i: 0 });
let reader = events.get_reader_current();
assert!(reader.is_empty(&events));
events.send(TestEvent { i: 0 });
assert_eq!(reader.len(&events), 1);
assert!(!reader.is_empty(&events));
}

#[test]
fn test_event_reader_len_update() {
let mut events = Events::<TestEvent>::default();
events.send(TestEvent { i: 0 });
events.send(TestEvent { i: 0 });
let reader = events.get_reader();
assert_eq!(reader.len(&events), 2);
events.update();
events.send(TestEvent { i: 0 });
assert_eq!(reader.len(&events), 3);
events.update();
assert_eq!(reader.len(&events), 1);
events.update();
assert!(reader.is_empty(&events));
}
}