Skip to content

Commit

Permalink
Update Event send methods to return EventId (bevyengine#10551)
Browse files Browse the repository at this point in the history
# Objective

- Fixes bevyengine#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<Request>,
    mut handled: EventReader<Handled>,
    mut pending: Local<HashSet<EventId<Request>>>,
) {
    // 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<Request>,
    mut handled: EventWriter<Handled>,
) {
    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<MyEvent>) {
    events.send_default()
}

// Fix
fn send_my_event(mut events: EventWriter<MyEvent>) {
    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 <alice.i.cecile@gmail.com>
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
  • Loading branch information
3 people authored and Ray Redondo committed Jan 9, 2024
1 parent 768d629 commit 02b18d5
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 37 deletions.
113 changes: 104 additions & 9 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ impl<E: Event> Events<E> {

/// "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<E> {
let event_id = EventId {
id: self.event_count,
_marker: PhantomData,
Expand All @@ -202,14 +203,32 @@ impl<E: Event> Events<E> {

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<Item = E>) -> SendBatchIds<E> {
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<E>
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.
Expand Down Expand Up @@ -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<E> {
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<Item = E>) {
self.events.extend(events);
pub fn send_batch(&mut self, events: impl IntoIterator<Item = E>) -> SendBatchIds<E> {
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<E>
where
E: Default,
{
self.events.send_default();
self.events.send_default()
}
}

Expand Down Expand Up @@ -760,6 +784,38 @@ pub fn event_update_condition<T: Event>(events: Res<Events<T>>) -> bool {
!events.events_a.is_empty() || !events.events_b.is_empty()
}

/// [`Iterator`] over sent [`EventIds`](`EventId`) from a batch.
pub struct SendBatchIds<E> {
last_count: usize,
event_count: usize,
_marker: PhantomData<E>,
}

impl<E: Event> Iterator for SendBatchIds<E> {
type Item = EventId<E>;

fn next(&mut self) -> Option<Self::Item> {
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<E: Event> ExactSizeIterator for SendBatchIds<E> {
fn len(&self) -> usize {
self.event_count.saturating_sub(self.last_count)
}
}

#[cfg(test)]
mod tests {
use crate::system::assert_is_read_only_system;
Expand Down Expand Up @@ -1119,4 +1175,43 @@ mod tests {

assert_is_read_only_system(reader_system);
}

#[test]
fn test_send_events_ids() {
let mut events = Events::<TestEvent>::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"
);
}
}
36 changes: 23 additions & 13 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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<E: Event>(&mut self, event: E) {
self.send_event_batch(std::iter::once(event));
pub fn send_event<E: Event>(&mut self, event: E) -> Option<EventId<E>> {
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<E: Event + Default>(&mut self) {
self.send_event_batch(std::iter::once(E::default()));
pub fn send_event_default<E: Event + Default>(&mut self) -> Option<EventId<E>> {
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<E: Event>(&mut self, events: impl IntoIterator<Item = E>) {
match self.get_resource_mut::<Events<E>>() {
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::<E>()
),
}
pub fn send_event_batch<E: Event>(
&mut self,
events: impl IntoIterator<Item = E>,
) -> Option<SendBatchIds<E>> {
let Some(mut events_resource) = self.get_resource_mut::<Events<E>>() 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::<E>()
);
return None;
};
Some(events_resource.send_batch(events))
}

/// Inserts a new resource with the given `value`. Will replace the value if it already existed.
Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_gilrs/src/gilrs_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_input/src/gamepad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
}
}
Expand Down
26 changes: 16 additions & 10 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion examples/games/game_menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 02b18d5

Please sign in to comment.