diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index ccee94ec544ee4..1391250ac9b4f1 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -925,7 +925,7 @@ mod tests { change_detection::{DetectChanges, ResMut}, component::Component, entity::Entity, - event::EventWriter, + event::{Event, EventWriter, Events}, query::With, removal_detection::RemovedComponents, schedule::{IntoSystemConfigs, ScheduleLabel}, @@ -1274,4 +1274,41 @@ mod tests { .init_non_send_resource::() .init_resource::(); } + + #[test] + fn events_should_be_updated_once_per_update() { + #[derive(Event, Clone)] + struct TestEvent; + + let mut app = App::new(); + app.add_event::(); + + // Starts empty + let test_events = app.world().resource::>(); + assert_eq!(test_events.len(), 0); + assert_eq!(test_events.iter_current_update_events().count(), 0); + app.update(); + + // Sending one event + app.world_mut().send_event(TestEvent); + + let test_events = app.world().resource::>(); + assert_eq!(test_events.len(), 1); + assert_eq!(test_events.iter_current_update_events().count(), 1); + app.update(); + + // Sending two events on the next frame + app.world_mut().send_event(TestEvent); + app.world_mut().send_event(TestEvent); + + let test_events = app.world().resource::>(); + assert_eq!(test_events.len(), 3); // Events are double-buffered, so we see 1 + 2 = 3 + assert_eq!(test_events.iter_current_update_events().count(), 2); + app.update(); + + // Sending zero events + let test_events = app.world().resource::>(); + assert_eq!(test_events.len(), 2); // Events are double-buffered, so we see 2 + 0 = 2 + assert_eq!(test_events.iter_current_update_events().count(), 0); + } } diff --git a/crates/bevy_ecs/src/event.rs b/crates/bevy_ecs/src/event.rs index 18f491a074d4da..faab4fd2303db8 100644 --- a/crates/bevy_ecs/src/event.rs +++ b/crates/bevy_ecs/src/event.rs @@ -1010,10 +1010,25 @@ struct RegisteredEvent { /// to update all of the events. #[derive(Resource, Default)] pub struct EventRegistry { - needs_update: bool, + /// Should the events be updated? + /// + /// This field is generally automatically updated by the [`signal_event_update_system`](crate::event::update::signal_event_update_system). + pub should_update: ShouldUpdateEvents, event_updates: Vec, } +/// Controls whether or not the events in an [`EventRegistry`] should be updated. +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)] +pub enum ShouldUpdateEvents { + /// Without any fixed timestep, events should always be updated each frame. + #[default] + Always, + /// We need to wait until at least one pass of the fixed update schedules to update the events. + Waiting, + /// At least one pass of the fixed update schedules has occurred, and the events are ready to be updated. + Ready, +} + impl EventRegistry { /// Registers an event type to be updated. pub fn register_event(world: &mut World) { @@ -1058,9 +1073,12 @@ impl EventRegistry { pub struct EventUpdates; /// Signals the [`event_update_system`] to run after `FixedUpdate` systems. +/// +/// This will change the behavior of the [`EventRegistry`] to only run after a fixed update cycle has passed. +/// Normally, this will simply run every frame. pub fn signal_event_update_system(signal: Option>) { if let Some(mut registry) = signal { - registry.needs_update = true; + registry.should_update = ShouldUpdateEvents::Ready; } } @@ -1069,18 +1087,34 @@ pub fn event_update_system(world: &mut World, mut last_change_tick: Local) if world.contains_resource::() { world.resource_scope(|world, mut registry: Mut| { registry.run_updates(world, *last_change_tick); - // Disable the system until signal_event_update_system runs again. - registry.needs_update = false; + + registry.should_update = match registry.should_update { + // If we're always updating, keep doing so. + ShouldUpdateEvents::Always => ShouldUpdateEvents::Always, + // Disable the system until signal_event_update_system runs again. + ShouldUpdateEvents::Waiting | ShouldUpdateEvents::Ready => { + ShouldUpdateEvents::Waiting + } + }; }); } *last_change_tick = world.change_tick(); } /// A run condition for [`event_update_system`]. -pub fn event_update_condition(signal: Option>) -> bool { - // 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. - signal.map_or(false, |signal| signal.needs_update) +/// +/// If [`signal_event_update_system`] has been run at least once, +/// we will wait for it to be run again before updating the events. +/// +/// Otherwise, we will always update the events. +pub fn event_update_condition(maybe_signal: Option>) -> bool { + match maybe_signal { + Some(signal) => match signal.should_update { + ShouldUpdateEvents::Always | ShouldUpdateEvents::Ready => true, + ShouldUpdateEvents::Waiting => false, + }, + None => true, + } } /// [`Iterator`] over sent [`EventIds`](`EventId`) from a batch. @@ -1541,4 +1575,42 @@ mod tests { }); schedule.run(&mut world); } + + #[test] + fn iter_current_update_events_iterates_over_current_events() { + #[derive(Event, Clone)] + struct TestEvent; + + let mut test_events = Events::::default(); + + // Starting empty + assert_eq!(test_events.len(), 0); + assert_eq!(test_events.iter_current_update_events().count(), 0); + test_events.update(); + + // Sending one event + test_events.send(TestEvent); + + assert_eq!(test_events.len(), 1); + assert_eq!(test_events.iter_current_update_events().count(), 1); + test_events.update(); + + // Sending two events on the next frame + test_events.send(TestEvent); + test_events.send(TestEvent); + + assert_eq!(test_events.len(), 3); // Events are double-buffered, so we see 1 + 2 = 3 + assert_eq!(test_events.iter_current_update_events().count(), 2); + test_events.update(); + + // Sending zero events + assert_eq!(test_events.len(), 2); // Events are double-buffered, so we see 2 + 0 = 2 + assert_eq!(test_events.iter_current_update_events().count(), 0); + } } + +// #[cfg(test)] +// mod tests { +// use crate::{self as bevy_ecs, event::Events}; +// use bevy_ecs_macros::Event; +// } diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index e6cabde3981539..689ee8967e5d6a 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -45,7 +45,7 @@ pub mod prelude { change_detection::{DetectChanges, DetectChangesMut, Mut, Ref}, component::Component, entity::{Entity, EntityMapper}, - event::{Event, EventReader, EventWriter, Events}, + event::{Event, EventReader, EventWriter, Events, ShouldUpdateEvents}, query::{Added, AnyOf, Changed, Has, Or, QueryBuilder, QueryState, With, Without}, removal_detection::RemovedComponents, schedule::{ diff --git a/crates/bevy_time/src/lib.rs b/crates/bevy_time/src/lib.rs index 9778e1517efc70..9b1bfc2e7635ca 100644 --- a/crates/bevy_time/src/lib.rs +++ b/crates/bevy_time/src/lib.rs @@ -30,7 +30,7 @@ pub mod prelude { } use bevy_app::{prelude::*, RunFixedMainLoop}; -use bevy_ecs::event::signal_event_update_system; +use bevy_ecs::event::{signal_event_update_system, EventRegistry, ShouldUpdateEvents}; use bevy_ecs::prelude::*; use bevy_utils::{tracing::warn, Duration, Instant}; pub use crossbeam_channel::TrySendError; @@ -65,8 +65,11 @@ impl Plugin for TimePlugin { app.add_systems(First, time_system.in_set(TimeSystem)) .add_systems(RunFixedMainLoop, run_fixed_main_schedule); - // ensure the events are not dropped until `FixedMain` systems can observe them + // Ensure the events are not dropped until `FixedMain` systems can observe them app.add_systems(FixedPostUpdate, signal_event_update_system); + let mut event_registry = app.world_mut().resource_mut::(); + // We need to start in a waiting state so that the events are not updated until the first fixed update + event_registry.should_update = ShouldUpdateEvents::Waiting; } } @@ -141,9 +144,13 @@ fn time_system( #[cfg(test)] mod tests { - use crate::{Fixed, Time, TimePlugin, TimeUpdateStrategy}; - use bevy_app::{App, Startup, Update}; - use bevy_ecs::event::{Event, EventReader, EventWriter}; + use crate::{Fixed, Time, TimePlugin, TimeUpdateStrategy, Virtual}; + use bevy_app::{App, FixedUpdate, Startup, Update}; + use bevy_ecs::{ + event::{Event, EventReader, EventRegistry, EventWriter, Events, ShouldUpdateEvents}, + system::{Local, Res, ResMut, Resource}, + }; + use bevy_utils::Duration; use std::error::Error; #[derive(Event)] @@ -159,6 +166,91 @@ mod tests { } } + #[derive(Event)] + struct DummyEvent; + + #[derive(Resource, Default)] + struct FixedUpdateCounter(u8); + + fn count_fixed_updates(mut counter: ResMut) { + counter.0 += 1; + } + + fn report_time( + mut frame_count: Local, + virtual_time: Res>, + fixed_time: Res>, + ) { + println!( + "Virtual time on frame {}: {:?}", + *frame_count, + virtual_time.elapsed() + ); + println!( + "Fixed time on frame {}: {:?}", + *frame_count, + fixed_time.elapsed() + ); + + *frame_count += 1; + } + + #[test] + fn fixed_main_schedule_should_run_with_time_plugin_enabled() { + // Set the time step to just over half the fixed update timestep + // This way, it will have not accumulated enough time to run the fixed update after one update + // But will definitely have enough time after two updates + let fixed_update_timestep = Time::::default().timestep(); + let time_step = fixed_update_timestep / 2 + Duration::from_millis(1); + + let mut app = App::new(); + app.add_plugins(TimePlugin) + .add_systems(FixedUpdate, count_fixed_updates) + .add_systems(Update, report_time) + .init_resource::() + .insert_resource(TimeUpdateStrategy::ManualDuration(time_step)); + + // Frame 0 + // Fixed update should not have run yet + app.update(); + + assert!(Duration::ZERO < fixed_update_timestep); + let counter = app.world().resource::(); + assert_eq!(counter.0, 0, "Fixed update should not have run yet"); + + // Frame 1 + // Fixed update should not have run yet + app.update(); + + assert!(time_step < fixed_update_timestep); + let counter = app.world().resource::(); + assert_eq!(counter.0, 0, "Fixed update should not have run yet"); + + // Frame 2 + // Fixed update should have run now + app.update(); + + assert!(2 * time_step > fixed_update_timestep); + let counter = app.world().resource::(); + assert_eq!(counter.0, 1, "Fixed update should have run once"); + + // Frame 3 + // Fixed update should have run exactly once still + app.update(); + + assert!(3 * time_step < 2 * fixed_update_timestep); + let counter = app.world().resource::(); + assert_eq!(counter.0, 1, "Fixed update should have run once"); + + // Frame 4 + // Fixed update should have run twice now + app.update(); + + assert!(4 * time_step > 2 * fixed_update_timestep); + let counter = app.world().resource::(); + assert_eq!(counter.0, 2, "Fixed update should have run twice"); + } + #[test] fn events_get_dropped_regression_test_11528() -> Result<(), impl Error> { let (tx1, rx1) = std::sync::mpsc::channel(); @@ -199,4 +291,75 @@ mod tests { // Check event type 2 has been dropped rx2.try_recv() } + + #[test] + fn event_update_should_wait_for_fixed_main() { + // Set the time step to just over half the fixed update timestep + // This way, it will have not accumulated enough time to run the fixed update after one update + // But will definitely have enough time after two updates + let fixed_update_timestep = Time::::default().timestep(); + let time_step = fixed_update_timestep / 2 + Duration::from_millis(1); + + fn send_event(mut events: ResMut>) { + events.send(DummyEvent); + } + + let mut app = App::new(); + app.add_plugins(TimePlugin) + .add_event::() + .init_resource::() + .add_systems(Startup, send_event) + .add_systems(FixedUpdate, count_fixed_updates) + .insert_resource(TimeUpdateStrategy::ManualDuration(time_step)); + + for frame in 0..10 { + app.update(); + let fixed_updates_seen = app.world().resource::().0; + let events = app.world().resource::>(); + let n_total_events = events.len(); + let n_current_events = events.iter_current_update_events().count(); + let event_registry = app.world().resource::(); + let should_update = event_registry.should_update; + + println!("Frame {frame}, {fixed_updates_seen} fixed updates seen. Should update: {should_update:?}"); + println!("Total events: {n_total_events} | Current events: {n_current_events}",); + + match frame { + 0 | 1 => { + assert_eq!(fixed_updates_seen, 0); + assert_eq!(n_total_events, 1); + assert_eq!(n_current_events, 1); + assert_eq!(should_update, ShouldUpdateEvents::Waiting); + } + 2 => { + assert_eq!(fixed_updates_seen, 1); // Time to trigger event updates + assert_eq!(n_total_events, 1); + assert_eq!(n_current_events, 1); + assert_eq!(should_update, ShouldUpdateEvents::Ready); // Prepping first update + } + 3 => { + assert_eq!(fixed_updates_seen, 1); + assert_eq!(n_total_events, 1); + assert_eq!(n_current_events, 0); // First update has occurred + assert_eq!(should_update, ShouldUpdateEvents::Waiting); + } + 4 => { + assert_eq!(fixed_updates_seen, 2); // Time to trigger the second update + assert_eq!(n_total_events, 1); + assert_eq!(n_current_events, 0); + assert_eq!(should_update, ShouldUpdateEvents::Ready); // Prepping second update + } + 5 => { + assert_eq!(fixed_updates_seen, 2); + assert_eq!(n_total_events, 0); // Second update has occurred + assert_eq!(n_current_events, 0); + assert_eq!(should_update, ShouldUpdateEvents::Waiting); + } + _ => { + assert_eq!(n_total_events, 0); // No more events are sent + assert_eq!(n_current_events, 0); + } + } + } + } }