From 3c689b9ca8b5c1568300f78525348eecfcdf8e97 Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Fri, 17 Nov 2023 04:20:43 +1100 Subject: [PATCH] Update `Event` send methods to return `EventId` (#10551) # Objective - Fixes #10532 ## Solution I've updated the various `Event` send methods to return the sent `EventId`(s). Since these methods previously returned nothing, and this information is cheap to copy, there should be minimal negative consequences to providing this additional information. In the case of `send_batch`, an iterator is returned built from `Range` and `Map`, which only consumes 16 bytes on the stack with no heap allocations for all batch sizes. As such, the cost of this information is negligible. These changes are reflected for `EventWriter` and `World`. For `World`, the return types are optional to account for the possible lack of an `Events` resource. Again, these methods previously returned no information, so its inclusion should only be a benefit. ## Usage Now when sending events, the IDs of those events is available for immediate use: ```rust // Example of a request-response system where the requester can track handled requests. /// A system which can make and track requests fn requester( mut requests: EventWriter, mut handled: EventReader, mut pending: Local>>, ) { // Check status of previous requests for Handled(id) in handled.read() { pending.remove(&id); } if !pending.is_empty() { error!("Not all my requests were handled on the previous frame!"); pending.clear(); } // Send a new request and remember its ID for later let request_id = requests.send(Request::MyRequest { /* ... */ }); pending.insert(request_id); } /// A system which handles requests fn responder( mut requests: EventReader, mut handled: EventWriter, ) { for (request, id) in requests.read_with_id() { if handle(request).is_ok() { handled.send(Handled(id)); } } } ``` In the above example, a `requester` system can send request events, and keep track of which ones are currently pending by `EventId`. Then, a `responder` system can act on that event, providing the ID as a reference that the `requester` can use. Before this PR, it was not trivial for a system sending events to keep track of events by ID. This is unfortunate, since for a system reading events, it is trivial to access the ID of a event. --- ## Changelog - Updated `Events`: - Added `send_batch` - Modified `send` to return the sent `EventId` - Modified `send_default` to return the sent `EventId` - Updated `EventWriter` - Modified `send_batch` to return all sent `EventId`s - Modified `send` to return the sent `EventId` - Modified `send_default` to return the sent `EventId` - Updated `World` - Modified `send_event` to return the sent `EventId` if sent, otherwise `None`. - Modified `send_event_default` to return the sent `EventId` if sent, otherwise `None`. - Modified `send_event_batch` to return all sent `EventId`s if sent, otherwise `None`. - Added unit test `test_send_events_ids` to ensure returned `EventId`s match the sent `Event`s - Updated uses of modified methods. ## Migration Guide ### `send` / `send_default` / `send_batch` For the following methods: - `Events::send` - `Events::send_default` - `Events::send_batch` - `EventWriter::send` - `EventWriter::send_default` - `EventWriter::send_batch` - `World::send_event` - `World::send_event_default` - `World::send_event_batch` Ensure calls to these methods either handle the returned value, or suppress the result with `;`. ```rust // Now fails to compile due to mismatched return type fn send_my_event(mut events: EventWriter) { events.send_default() } // Fix fn send_my_event(mut events: EventWriter) { events.send_default(); } ``` This will most likely be noticed within `match` statements: ```rust // Before match is_pressed { true => events.send(PlayerAction::Fire), // ^--^ No longer returns () false => {} } // After match is_pressed { true => { events.send(PlayerAction::Fire); }, false => {} } ``` --------- Co-authored-by: Alice Cecile Co-authored-by: Nicola Papale --- crates/bevy_ecs/src/event.rs | 113 ++++++++++++++++++++++++-- crates/bevy_ecs/src/world/mod.rs | 36 +++++--- crates/bevy_gilrs/src/gilrs_system.rs | 7 +- crates/bevy_input/src/gamepad.rs | 8 +- crates/bevy_winit/src/lib.rs | 26 +++--- examples/games/game_menu.rs | 4 +- 6 files changed, 157 insertions(+), 37 deletions(-) diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs index 781bbde448..222c6101ce 100644 --- a/crates/bevy_ecs/src/event.rs +++ b/crates/bevy_ecs/src/event.rs @@ -191,7 +191,8 @@ impl Events { /// "Sends" an `event` by writing it to the current event buffer. [`EventReader`]s can then read /// the event. - pub fn send(&mut self, event: E) { + /// This method returns the [ID](`EventId`) of the sent `event`. + pub fn send(&mut self, event: E) -> EventId { let event_id = EventId { id: self.event_count, _marker: PhantomData, @@ -202,14 +203,32 @@ impl Events { self.events_b.push(event_instance); self.event_count += 1; + + event_id + } + + /// Sends a list of `events` all at once, which can later be read by [`EventReader`]s. + /// This is more efficient than sending each event individually. + /// This method returns the [IDs](`EventId`) of the sent `events`. + pub fn send_batch(&mut self, events: impl IntoIterator) -> SendBatchIds { + let last_count = self.event_count; + + self.extend(events); + + SendBatchIds { + last_count, + event_count: self.event_count, + _marker: PhantomData, + } } /// Sends the default value of the event. Useful when the event is an empty struct. - pub fn send_default(&mut self) + /// This method returns the [ID](`EventId`) of the sent `event`. + pub fn send_default(&mut self) -> EventId where E: Default, { - self.send(Default::default()); + self.send(Default::default()) } /// Gets a new [`ManualEventReader`]. This will include all events already in the event buffers. @@ -512,26 +531,31 @@ pub struct EventWriter<'w, E: Event> { impl<'w, E: Event> EventWriter<'w, E> { /// Sends an `event`, which can later be read by [`EventReader`]s. + /// This method returns the [ID](`EventId`) of the sent `event`. /// /// See [`Events`] for details. - pub fn send(&mut self, event: E) { - self.events.send(event); + pub fn send(&mut self, event: E) -> EventId { + self.events.send(event) } /// Sends a list of `events` all at once, which can later be read by [`EventReader`]s. /// This is more efficient than sending each event individually. + /// This method returns the [IDs](`EventId`) of the sent `events`. /// /// See [`Events`] for details. - pub fn send_batch(&mut self, events: impl IntoIterator) { - self.events.extend(events); + pub fn send_batch(&mut self, events: impl IntoIterator) -> SendBatchIds { + self.events.send_batch(events) } /// Sends the default value of the event. Useful when the event is an empty struct. - pub fn send_default(&mut self) + /// This method returns the [ID](`EventId`) of the sent `event`. + /// + /// See [`Events`] for details. + pub fn send_default(&mut self) -> EventId where E: Default, { - self.events.send_default(); + self.events.send_default() } } @@ -760,6 +784,38 @@ pub fn event_update_condition(events: Res>) -> bool { !events.events_a.is_empty() || !events.events_b.is_empty() } +/// [`Iterator`] over sent [`EventIds`](`EventId`) from a batch. +pub struct SendBatchIds { + last_count: usize, + event_count: usize, + _marker: PhantomData, +} + +impl Iterator for SendBatchIds { + type Item = EventId; + + fn next(&mut self) -> Option { + if self.last_count >= self.event_count { + return None; + } + + let result = Some(EventId { + id: self.last_count, + _marker: PhantomData, + }); + + self.last_count += 1; + + result + } +} + +impl ExactSizeIterator for SendBatchIds { + fn len(&self) -> usize { + self.event_count.saturating_sub(self.last_count) + } +} + #[cfg(test)] mod tests { use crate::system::assert_is_read_only_system; @@ -1119,4 +1175,43 @@ mod tests { assert_is_read_only_system(reader_system); } + + #[test] + fn test_send_events_ids() { + let mut events = Events::::default(); + let event_0 = TestEvent { i: 0 }; + let event_1 = TestEvent { i: 1 }; + let event_2 = TestEvent { i: 2 }; + + let event_0_id = events.send(event_0); + + assert_eq!( + events.get_event(event_0_id.id), + Some((&event_0, event_0_id)), + "Getting a sent event by ID should return the original event" + ); + + let mut event_ids = events.send_batch([event_1, event_2]); + + let event_id = event_ids.next().expect("Event 1 must have been sent"); + + assert_eq!( + events.get_event(event_id.id), + Some((&event_1, event_id)), + "Getting a sent event by ID should return the original event" + ); + + let event_id = event_ids.next().expect("Event 2 must have been sent"); + + assert_eq!( + events.get_event(event_id.id), + Some((&event_2, event_id)), + "Getting a sent event by ID should return the original event" + ); + + assert!( + event_ids.next().is_none(), + "Only sent two events; got more than two IDs" + ); + } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 995b503788..0919dfd632 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -17,7 +17,7 @@ use crate::{ change_detection::{MutUntyped, TicksMut}, component::{Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, Tick}, entity::{AllocAtWithoutReplacement, Entities, Entity, EntityLocation}, - event::{Event, Events}, + event::{Event, EventId, Events, SendBatchIds}, query::{DebugCheckedUnwrap, QueryEntityError, QueryState, ReadOnlyWorldQuery, WorldQuery}, removal_detection::RemovedComponentEvents, schedule::{Schedule, ScheduleLabel, Schedules}, @@ -1594,27 +1594,37 @@ impl World { } /// Sends an [`Event`]. + /// This method returns the [ID](`EventId`) of the sent `event`, + /// or [`None`] if the `event` could not be sent. #[inline] - pub fn send_event(&mut self, event: E) { - self.send_event_batch(std::iter::once(event)); + pub fn send_event(&mut self, event: E) -> Option> { + self.send_event_batch(std::iter::once(event))?.next() } /// Sends the default value of the [`Event`] of type `E`. + /// This method returns the [ID](`EventId`) of the sent `event`, + /// or [`None`] if the `event` could not be sent. #[inline] - pub fn send_event_default(&mut self) { - self.send_event_batch(std::iter::once(E::default())); + pub fn send_event_default(&mut self) -> Option> { + self.send_event(E::default()) } /// Sends a batch of [`Event`]s from an iterator. + /// This method returns the [IDs](`EventId`) of the sent `events`, + /// or [`None`] if the `event` could not be sent. #[inline] - pub fn send_event_batch(&mut self, events: impl IntoIterator) { - match self.get_resource_mut::>() { - Some(mut events_resource) => events_resource.extend(events), - None => bevy_utils::tracing::error!( - "Unable to send event `{}`\n\tEvent must be added to the app with `add_event()`\n\thttps://docs.rs/bevy/*/bevy/app/struct.App.html#method.add_event ", - std::any::type_name::() - ), - } + pub fn send_event_batch( + &mut self, + events: impl IntoIterator, + ) -> Option> { + let Some(mut events_resource) = self.get_resource_mut::>() else { + bevy_utils::tracing::error!( + "Unable to send event `{}`\n\tEvent must be added to the app with `add_event()`\n\thttps://docs.rs/bevy/*/bevy/app/struct.App.html#method.add_event ", + std::any::type_name::() + ); + return None; + }; + Some(events_resource.send_batch(events)) } /// Inserts a new resource with the given `value`. Will replace the value if it already existed. diff --git a/crates/bevy_gilrs/src/gilrs_system.rs b/crates/bevy_gilrs/src/gilrs_system.rs index 5531928103..7849bc1c80 100644 --- a/crates/bevy_gilrs/src/gilrs_system.rs +++ b/crates/bevy_gilrs/src/gilrs_system.rs @@ -51,8 +51,11 @@ pub fn gilrs_event_system( GamepadConnectionEvent::new(gamepad, GamepadConnection::Connected(info)).into(), ); } - EventType::Disconnected => events - .send(GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected).into()), + EventType::Disconnected => { + events.send( + GamepadConnectionEvent::new(gamepad, GamepadConnection::Disconnected).into(), + ); + } EventType::ButtonChanged(gilrs_button, raw_value, _) => { if let Some(button_type) = convert_button(gilrs_button) { let button = GamepadButton::new(gamepad, button_type); diff --git a/crates/bevy_input/src/gamepad.rs b/crates/bevy_input/src/gamepad.rs index 774548f9be..338458605e 100644 --- a/crates/bevy_input/src/gamepad.rs +++ b/crates/bevy_input/src/gamepad.rs @@ -1263,8 +1263,12 @@ pub fn gamepad_event_system( GamepadEvent::Connection(connection_event) => { connection_events.send(connection_event.clone()); } - GamepadEvent::Button(button_event) => button_events.send(button_event.clone()), - GamepadEvent::Axis(axis_event) => axis_events.send(axis_event.clone()), + GamepadEvent::Button(button_event) => { + button_events.send(button_event.clone()); + } + GamepadEvent::Axis(axis_event) => { + axis_events.send(axis_event.clone()); + } } } } diff --git a/crates/bevy_winit/src/lib.rs b/crates/bevy_winit/src/lib.rs index 91c9858c79..eb58a877fa 100644 --- a/crates/bevy_winit/src/lib.rs +++ b/crates/bevy_winit/src/lib.rs @@ -693,16 +693,22 @@ pub fn winit_runner(mut app: App) { cursor, }); } - event::Ime::Commit(value) => event_writers.ime_input.send(Ime::Commit { - window: window_entity, - value, - }), - event::Ime::Enabled => event_writers.ime_input.send(Ime::Enabled { - window: window_entity, - }), - event::Ime::Disabled => event_writers.ime_input.send(Ime::Disabled { - window: window_entity, - }), + event::Ime::Commit(value) => { + event_writers.ime_input.send(Ime::Commit { + window: window_entity, + value, + }); + } + event::Ime::Enabled => { + event_writers.ime_input.send(Ime::Enabled { + window: window_entity, + }); + } + event::Ime::Disabled => { + event_writers.ime_input.send(Ime::Disabled { + window: window_entity, + }); + } }, WindowEvent::ThemeChanged(theme) => { event_writers.window_theme_changed.send(WindowThemeChanged { diff --git a/examples/games/game_menu.rs b/examples/games/game_menu.rs index 4beaa03c60..226317685c 100644 --- a/examples/games/game_menu.rs +++ b/examples/games/game_menu.rs @@ -804,7 +804,9 @@ mod menu { for (interaction, menu_button_action) in &interaction_query { if *interaction == Interaction::Pressed { match menu_button_action { - MenuButtonAction::Quit => app_exit_events.send(AppExit), + MenuButtonAction::Quit => { + app_exit_events.send(AppExit); + } MenuButtonAction::Play => { game_state.set(GameState::Game); menu_state.set(MenuState::Disabled);