Skip to content

Commit

Permalink
Wait until FixedUpdate can see events before dropping them (#10077)
Browse files Browse the repository at this point in the history
## Objective
 
Currently, events are dropped after two frames. This cadence wasn't
*chosen* for a specific reason, double buffering just lets events
persist for at least two frames. Events only need to be dropped at a
predictable point so that the event queues don't grow forever (i.e.
events should never cause a memory leak).
 
Events (and especially input events) need to be observable by systems in
`FixedUpdate`, but as-is events are dropped before those systems even
get a chance to see them.
 
## Solution
 
Instead of unconditionally dropping events in `First`, require
`FixedUpdate` to first queue the buffer swap (if the `TimePlugin` has
been installed). This way, events are only dropped after a frame that
runs `FixedUpdate`.
 
## Future Work

In the same way we have independent copies of `Time` for tracking time
in `Main` and `FixedUpdate`, we will need independent copies of `Input`
for tracking press/release status correctly in `Main` and `FixedUpdate`.

--
 
Every run of `FixedUpdate` covers a specific timespan. For example, if
the fixed timestep `Δt` is 10ms, the first three `FixedUpdate` runs
cover `[0ms, 10ms)`, `[10ms, 20ms)`, and `[20ms, 30ms)`.
 
`FixedUpdate` can run many times in one frame. For truly
framerate-independent behavior, each `FixedUpdate` should only see the
events that occurred in its covered timespan, but what happens right now
is the first step in the frame reads all pending events.

Fixing that will require timestamped events.

---------

Co-authored-by: Alice Cecile <[email protected]>
  • Loading branch information
maniwani and alice-i-cecile authored Nov 26, 2023
1 parent 9849221 commit 53919c3
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
1 change: 1 addition & 0 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ impl Default for App {
app.init_resource::<AppTypeRegistry>();

app.add_plugins(MainSchedulePlugin);

app.add_event::<AppExit>();

#[cfg(feature = "bevy_ci_testing")]
Expand Down
26 changes: 24 additions & 2 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,8 +733,30 @@ impl<'a, E: Event> ExactSizeIterator for EventIteratorWithId<'a, E> {
}
}

/// A system that calls [`Events::update`] once per frame.
pub fn event_update_system<T: Event>(mut events: ResMut<Events<T>>) {
#[doc(hidden)]
#[derive(Resource, Default)]
pub struct EventUpdateSignal(bool);

/// A system that queues a call to [`Events::update`].
pub fn event_queue_update_system(signal: Option<ResMut<EventUpdateSignal>>) {
if let Some(mut s) = signal {
s.0 = true;
}
}

/// A system that calls [`Events::update`].
pub fn event_update_system<T: Event>(
signal: Option<ResMut<EventUpdateSignal>>,
mut events: ResMut<Events<T>>,
) {
if let Some(mut s) = signal {
// If we haven't got a signal to update the events, but we *could* get such a signal
// return early and update the events later.
if !std::mem::replace(&mut s.0, false) {
return;
}
}

events.update();
}

Expand Down
13 changes: 8 additions & 5 deletions crates/bevy_time/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,18 @@ pub use time::*;
pub use timer::*;
pub use virt::*;

use bevy_ecs::system::{Res, ResMut};
use bevy_utils::{tracing::warn, Duration, Instant};
pub use crossbeam_channel::TrySendError;
use crossbeam_channel::{Receiver, Sender};

pub mod prelude {
//! The Bevy Time Prelude.
#[doc(hidden)]
pub use crate::{Fixed, Real, Time, Timer, TimerMode, Virtual};
}

use bevy_app::{prelude::*, RunFixedUpdateLoop};
use bevy_ecs::event::{event_queue_update_system, EventUpdateSignal};
use bevy_ecs::prelude::*;
use bevy_utils::{tracing::warn, Duration, Instant};
pub use crossbeam_channel::TrySendError;
use crossbeam_channel::{Receiver, Sender};

/// Adds time functionality to Apps.
#[derive(Default)]
Expand Down Expand Up @@ -60,6 +59,10 @@ impl Plugin for TimePlugin {
)
.add_systems(RunFixedUpdateLoop, run_fixed_update_schedule);

// ensure the events are not dropped until `FixedUpdate` systems can observe them
app.init_resource::<EventUpdateSignal>()
.add_systems(FixedUpdate, event_queue_update_system);

#[cfg(feature = "bevy_ci_testing")]
if let Some(ci_testing_config) = app
.world
Expand Down

0 comments on commit 53919c3

Please sign in to comment.