From bdd120f78a230ed3dd202fd02091c3b5565c2a2c Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 00:54:50 -0400 Subject: [PATCH 01/15] both tests pass --- src/action_state.rs | 27 +++++++++ src/plugin.rs | 27 ++++++++- src/systems.rs | 56 +++++++++++++++++++ tests/fixed_update.rs | 125 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 tests/fixed_update.rs diff --git a/src/action_state.rs b/src/action_state.rs index 8771b344..5a5eab1c 100644 --- a/src/action_state.rs +++ b/src/action_state.rs @@ -18,6 +18,9 @@ use serde::{Deserialize, Serialize}; pub struct ActionData { /// Is the action pressed or released? pub state: ButtonState, + + pub(crate) update_state: ButtonState, + pub(crate) fixed_update_state: ButtonState, /// The "value" of the binding that triggered the action. /// /// See [`ActionState::value`] for more details. @@ -98,6 +101,30 @@ impl Default for ActionState { } impl ActionState { + pub(crate) fn swap_update_into_state(&mut self) { + for (_action, action_datum) in self.action_data.iter_mut() { + action_datum.state = action_datum.update_state; + } + } + + pub(crate) fn swap_state_into_update(&mut self) { + for (_action, action_datum) in self.action_data.iter_mut() { + action_datum.update_state = action_datum.state; + } + } + + pub(crate) fn swap_fixed_update_into_state(&mut self) { + for (_action, action_datum) in self.action_data.iter_mut() { + action_datum.state = action_datum.fixed_update_state; + } + } + + pub(crate) fn swap_state_into_fixed_update(&mut self) { + for (_action, action_datum) in self.action_data.iter_mut() { + action_datum.fixed_update_state = action_datum.state; + } + } + /// Updates the [`ActionState`] based on a vector of [`ActionData`], ordered by [`Actionlike::id`](Actionlike). /// /// The `action_data` is typically constructed from [`InputMap::which_pressed`](crate::input_map::InputMap), diff --git a/src/plugin.rs b/src/plugin.rs index 70644e4a..003c2575 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -16,11 +16,12 @@ use core::hash::Hash; use core::marker::PhantomData; use std::fmt::Debug; -use bevy::app::{App, Plugin}; +use bevy::app::{App, Plugin, RunFixedMainLoop}; use bevy::ecs::prelude::*; use bevy::input::{ButtonState, InputSystem}; -use bevy::prelude::{PostUpdate, PreUpdate}; +use bevy::prelude::{FixedPostUpdate, PostUpdate, PreUpdate}; use bevy::reflect::TypePath; +use bevy::time::run_fixed_main_schedule; #[cfg(feature = "ui")] use bevy::ui::UiSystem; @@ -97,6 +98,10 @@ impl Plugin for InputManagerPlugin { match self.machine { Machine::Client => { + app.add_systems(PreUpdate, swap_update_into_state:: + .run_if(run_if_enabled::) + .before(InputManagerSystem::Tick) + ); app.add_systems( PreUpdate, tick_action_state:: @@ -150,6 +155,24 @@ impl Plugin for InputManagerPlugin { .run_if(run_if_enabled::) .in_set(InputManagerSystem::ManualControl), ); + app.add_systems(PreUpdate, swap_state_into_update:: + .run_if(run_if_enabled::) + .after(InputManagerSystem::ManualControl) + ); + + app.add_systems(RunFixedMainLoop, (swap_fixed_update_into_state::, update_action_state::, release_on_disable::).chain() + .run_if(run_if_enabled::) + .before(run_fixed_main_schedule)); + app.add_systems( + FixedPostUpdate, + tick_action_state:: + .run_if(run_if_enabled::) + .in_set(InputManagerSystem::Tick) + .before(InputManagerSystem::Update), + ); + app.add_systems(RunFixedMainLoop, (swap_state_into_fixed_update::, swap_update_into_state::).chain() + .run_if(run_if_enabled::) + .after(run_fixed_main_schedule)); } Machine::Server => { app.add_systems( diff --git a/src/systems.rs b/src/systems.rs index f77a7071..fbfaabf9 100644 --- a/src/systems.rs +++ b/src/systems.rs @@ -28,6 +28,62 @@ use bevy::ui::Interaction; #[cfg(feature = "egui")] use bevy_egui::EguiContext; +/// a +pub fn swap_update_into_state( + mut query: Query<&mut ActionState>, + action_state: Option>>, +) { + if let Some(mut action_state) = action_state { + action_state.swap_update_into_state(); + } + + for mut action_state in query.iter_mut() { + action_state.swap_update_into_state(); + } +} + +/// b +pub fn swap_state_into_update( + mut query: Query<&mut ActionState>, + action_state: Option>>, +) { + if let Some(mut action_state) = action_state { + action_state.swap_state_into_update(); + } + + for mut action_state in query.iter_mut() { + action_state.swap_state_into_update(); + } +} + +/// c +pub fn swap_fixed_update_into_state( + mut query: Query<&mut ActionState>, + action_state: Option>>, +) { + if let Some(mut action_state) = action_state { + action_state.swap_fixed_update_into_state(); + } + + for mut action_state in query.iter_mut() { + action_state.swap_fixed_update_into_state(); + } +} + +/// d +pub fn swap_state_into_fixed_update( + mut query: Query<&mut ActionState>, + action_state: Option>>, +) { + if let Some(mut action_state) = action_state { + action_state.swap_state_into_fixed_update(); + } + + for mut action_state in query.iter_mut() { + action_state.swap_state_into_fixed_update(); + } +} + /// Advances actions timer. /// /// Clears the just-pressed and just-released values of all [`ActionState`]s. diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs new file mode 100644 index 00000000..a45f53ac --- /dev/null +++ b/tests/fixed_update.rs @@ -0,0 +1,125 @@ +use std::time::Duration; +use bevy::input::InputPlugin; +use bevy::MinimalPlugins; +use bevy::prelude::{App, Fixed, FixedUpdate, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update}; +use bevy::time::TimeUpdateStrategy; +use leafwing_input_manager::action_state::ActionState; +use leafwing_input_manager::input_map::InputMap; +use leafwing_input_manager::input_mocking::MockInput; +use leafwing_input_manager::plugin::InputManagerPlugin; +use leafwing_input_manager_macros::Actionlike; + + +#[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] +enum TestAction { + Up, + Down, +} + +#[derive(Resource, Default)] +struct UpdateCounter { + just_pressed: usize, +} + +#[derive(Resource, Default)] +struct FixedUpdateCounter{ + /// how many times did the FixedUpdate schedule run? + run: usize, + /// how many times did the Up button get just_pressed? + just_pressed: usize, +} + +fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { + let mut app = App::new(); + app + .add_plugins(MinimalPlugins) + .add_plugins(InputPlugin) + .add_plugins(InputManagerPlugin::::default()) + .init_resource::() + .init_resource::() + .init_resource::>() + .insert_resource(InputMap::::new([ + (TestAction::Up, KeyCode::ArrowUp), + (TestAction::Down, KeyCode::ArrowDown) + ])) + .insert_resource(Time::::from_duration(fixed_timestep)) + .insert_resource(TimeUpdateStrategy::ManualDuration(frame_timestep)); + + app.add_systems(Update, update_counter); + app.add_systems(FixedUpdate, fixed_update_counter); + + // initialize time (needed to set a starting time for tests) + let now = bevy::utils::Instant::now(); + app + .world + .get_resource_mut::>() + .unwrap() + .update_with_instant(now); + app +} + +fn fixed_update_counter(mut counter: ResMut, action: Res>) { + if action.just_pressed(&TestAction::Up) { + counter.just_pressed += 1; + } + counter.run += 1; +} + +fn update_counter(mut counter: ResMut, action: Res>) { + if action.just_pressed(&TestAction::Up) { + counter.just_pressed += 1; + } +} + + +/// We have 2 frames without a FixedUpdate schedule in between (F1 - F2 - FU - F3) +/// +/// A button pressed in F1 should still be marked as `just_pressed` in FU +#[test] +fn frame_without_fixed_timestep() { + let mut app = build_app(Duration::from_millis(10), Duration::from_millis(9)); + + app.press_input(KeyCode::ArrowUp); + + // the FixedUpdate schedule shouldn't run + app.update(); + assert_eq!(app.world.get_resource::().unwrap().run, 0); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + + // the FixedUpdate schedule should run once and should still the button as just_pressed + app.update(); + assert_eq!(app.world.get_resource::().unwrap().run, 1); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + + // the FixedUpdate schedule should run once + app.update(); + assert_eq!(app.world.get_resource::().unwrap().run, 2); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); +} + + +/// We have a frames with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) +/// +/// A button pressed in F1 should still be marked as `just_pressed` in FU1, and should just be +/// `pressed` in FU2 +#[test] +fn frame_with_two_fixed_timestep() { + let mut app = build_app(Duration::from_millis(4), Duration::from_millis(9)); + + app.press_input(KeyCode::ArrowUp); + + // the FixedUpdate schedule should run twice + // the button should be just_pressed only once + app.update(); + assert_eq!(app.world.get_resource::().unwrap().run, 2); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + + // the FixedUpdate schedule should run twice + app.update(); + assert_eq!(app.world.get_resource::().unwrap().run, 4); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); +} \ No newline at end of file From ebf4e32384e676bf9772e5e7d274897ddfcc3267 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 12:32:46 -0400 Subject: [PATCH 02/15] simplify implementation --- src/action_state.rs | 27 +++++++++++++------------ src/plugin.rs | 40 ++++++++++++++++++++++++++----------- src/systems.rs | 46 +++++++++++-------------------------------- tests/fixed_update.rs | 4 ++-- 4 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/action_state.rs b/src/action_state.rs index 5a5eab1c..e90c2631 100644 --- a/src/action_state.rs +++ b/src/action_state.rs @@ -101,30 +101,31 @@ impl Default for ActionState { } impl ActionState { - pub(crate) fn swap_update_into_state(&mut self) { + + /// We are about to enter the Main schedule, so we: + /// - save all the changes applied to `state` into the `fixed_update_state` + /// - switch to loading the `update_state` + pub(crate) fn swap_to_update_state(&mut self) { for (_action, action_datum) in self.action_data.iter_mut() { + // save the changes applied to `state` into `fixed_update_state` + action_datum.fixed_update_state = action_datum.state; + // switch to loading the `update_state` into `state` action_datum.state = action_datum.update_state; } } - pub(crate) fn swap_state_into_update(&mut self) { + /// We are about to enter the FixedMain schedule, so we: + /// - save all the changes applied to `state` into the `update_state` + /// - switch to loading the `fixed_update_state` + pub(crate) fn swap_to_fixed_update_state(&mut self) { for (_action, action_datum) in self.action_data.iter_mut() { + // save the changes applied to `state` into `update_state` action_datum.update_state = action_datum.state; - } - } - - pub(crate) fn swap_fixed_update_into_state(&mut self) { - for (_action, action_datum) in self.action_data.iter_mut() { + // switch to loading the `fixed_update_state` into `state` action_datum.state = action_datum.fixed_update_state; } } - pub(crate) fn swap_state_into_fixed_update(&mut self) { - for (_action, action_datum) in self.action_data.iter_mut() { - action_datum.fixed_update_state = action_datum.state; - } - } - /// Updates the [`ActionState`] based on a vector of [`ActionData`], ordered by [`Actionlike::id`](Actionlike). /// /// The `action_data` is typically constructed from [`InputMap::which_pressed`](crate::input_map::InputMap), diff --git a/src/plugin.rs b/src/plugin.rs index 003c2575..7ccf0341 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -19,7 +19,7 @@ use std::fmt::Debug; use bevy::app::{App, Plugin, RunFixedMainLoop}; use bevy::ecs::prelude::*; use bevy::input::{ButtonState, InputSystem}; -use bevy::prelude::{FixedPostUpdate, PostUpdate, PreUpdate}; +use bevy::prelude::{FixedPostUpdate, FixedPreUpdate, PostUpdate, PreUpdate}; use bevy::reflect::TypePath; use bevy::time::run_fixed_main_schedule; #[cfg(feature = "ui")] @@ -98,10 +98,7 @@ impl Plugin for InputManagerPlugin { match self.machine { Machine::Client => { - app.add_systems(PreUpdate, swap_update_into_state:: - .run_if(run_if_enabled::) - .before(InputManagerSystem::Tick) - ); + // Main schedule app.add_systems( PreUpdate, tick_action_state:: @@ -155,14 +152,35 @@ impl Plugin for InputManagerPlugin { .run_if(run_if_enabled::) .in_set(InputManagerSystem::ManualControl), ); - app.add_systems(PreUpdate, swap_state_into_update:: - .run_if(run_if_enabled::) - .after(InputManagerSystem::ManualControl) - ); - app.add_systems(RunFixedMainLoop, (swap_fixed_update_into_state::, update_action_state::, release_on_disable::).chain() + // FixedMain schedule + app.add_systems(RunFixedMainLoop, + ( + swap_to_fixed_update::, + // we want to update the ActionState only once, even if the FixedMain schedule runs multiple times + update_action_state::, + ).chain() .run_if(run_if_enabled::) .before(run_fixed_main_schedule)); + + app.add_systems( + FixedPreUpdate, release_on_disable:: + .in_set(InputManagerSystem::ReleaseOnDisable) + ); + #[cfg(feature = "ui")] + app.configure_sets( + FixedPreUpdate, + InputManagerSystem::ManualControl + .before(InputManagerSystem::ReleaseOnDisable) + ); + #[cfg(feature = "ui")] + app.add_systems( + FixedPreUpdate, + update_action_state_from_interaction:: + .run_if(run_if_enabled::) + .in_set(InputManagerSystem::ManualControl), + ); + app.add_systems(FixedPostUpdate, release_on_input_map_removed::); app.add_systems( FixedPostUpdate, tick_action_state:: @@ -170,7 +188,7 @@ impl Plugin for InputManagerPlugin { .in_set(InputManagerSystem::Tick) .before(InputManagerSystem::Update), ); - app.add_systems(RunFixedMainLoop, (swap_state_into_fixed_update::, swap_update_into_state::).chain() + app.add_systems(RunFixedMainLoop, swap_to_update:: .run_if(run_if_enabled::) .after(run_fixed_main_schedule)); } diff --git a/src/systems.rs b/src/systems.rs index fbfaabf9..2605ee8e 100644 --- a/src/systems.rs +++ b/src/systems.rs @@ -28,61 +28,39 @@ use bevy::ui::Interaction; #[cfg(feature = "egui")] use bevy_egui::EguiContext; -/// a -pub fn swap_update_into_state( - mut query: Query<&mut ActionState>, - action_state: Option>>, -) { - if let Some(mut action_state) = action_state { - action_state.swap_update_into_state(); - } - - for mut action_state in query.iter_mut() { - action_state.swap_update_into_state(); - } -} -/// b -pub fn swap_state_into_update( +/// We are about to enter the Main schedule, so we: +/// - save all the changes applied to `state` into the `fixed_update_state` +/// - switch to loading the `update_state` +pub fn swap_to_update( mut query: Query<&mut ActionState>, action_state: Option>>, ) { if let Some(mut action_state) = action_state { - action_state.swap_state_into_update(); + action_state.swap_to_update_state(); } for mut action_state in query.iter_mut() { - action_state.swap_state_into_update(); + action_state.swap_to_update_state(); } } -/// c -pub fn swap_fixed_update_into_state( +/// We are about to enter the FixedMain schedule, so we: +/// - save all the changes applied to `state` into the `update_state` +/// - switch to loading the `fixed_update_state` +pub fn swap_to_fixed_update( mut query: Query<&mut ActionState>, action_state: Option>>, ) { if let Some(mut action_state) = action_state { - action_state.swap_fixed_update_into_state(); + action_state.swap_to_fixed_update_state(); } for mut action_state in query.iter_mut() { - action_state.swap_fixed_update_into_state(); + action_state.swap_to_fixed_update_state(); } } -/// d -pub fn swap_state_into_fixed_update( - mut query: Query<&mut ActionState>, - action_state: Option>>, -) { - if let Some(mut action_state) = action_state { - action_state.swap_state_into_fixed_update(); - } - - for mut action_state in query.iter_mut() { - action_state.swap_state_into_fixed_update(); - } -} /// Advances actions timer. /// diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index a45f53ac..082fe89c 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -86,13 +86,13 @@ fn frame_without_fixed_timestep() { assert_eq!(app.world.get_resource::().unwrap().run, 0); assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - // the FixedUpdate schedule should run once and should still the button as just_pressed + // the FixedUpdate schedule should run once and the action still be counted as `just_pressed` app.update(); assert_eq!(app.world.get_resource::().unwrap().run, 1); assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - // the FixedUpdate schedule should run once + // the FixedUpdate schedule should run once, the button should now be `pressed` app.update(); assert_eq!(app.world.get_resource::().unwrap().run, 2); assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); From 71148acb676fb0f4d404325fe63f9b1c8a8aed38 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 12:40:29 -0400 Subject: [PATCH 03/15] clippy --- src/action_state.rs | 5 +- src/plugin.rs | 35 +++++++----- src/systems.rs | 6 +- tests/fixed_update.rs | 125 ++++++++++++++++++++++++++++++++---------- 4 files changed, 121 insertions(+), 50 deletions(-) diff --git a/src/action_state.rs b/src/action_state.rs index e90c2631..8bb4d01b 100644 --- a/src/action_state.rs +++ b/src/action_state.rs @@ -101,8 +101,7 @@ impl Default for ActionState { } impl ActionState { - - /// We are about to enter the Main schedule, so we: + /// We are about to enter the `Main` schedule, so we: /// - save all the changes applied to `state` into the `fixed_update_state` /// - switch to loading the `update_state` pub(crate) fn swap_to_update_state(&mut self) { @@ -114,7 +113,7 @@ impl ActionState { } } - /// We are about to enter the FixedMain schedule, so we: + /// We are about to enter the `FixedMain` schedule, so we: /// - save all the changes applied to `state` into the `update_state` /// - switch to loading the `fixed_update_state` pub(crate) fn swap_to_fixed_update_state(&mut self) { diff --git a/src/plugin.rs b/src/plugin.rs index 7ccf0341..2474dab1 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -154,24 +154,26 @@ impl Plugin for InputManagerPlugin { ); // FixedMain schedule - app.add_systems(RunFixedMainLoop, - ( - swap_to_fixed_update::, - // we want to update the ActionState only once, even if the FixedMain schedule runs multiple times - update_action_state::, - ).chain() - .run_if(run_if_enabled::) - .before(run_fixed_main_schedule)); + app.add_systems( + RunFixedMainLoop, + ( + swap_to_fixed_update::, + // we want to update the ActionState only once, even if the FixedMain schedule runs multiple times + update_action_state::, + ) + .chain() + .run_if(run_if_enabled::) + .before(run_fixed_main_schedule), + ); app.add_systems( - FixedPreUpdate, release_on_disable:: - .in_set(InputManagerSystem::ReleaseOnDisable) + FixedPreUpdate, + release_on_disable::.in_set(InputManagerSystem::ReleaseOnDisable), ); #[cfg(feature = "ui")] app.configure_sets( FixedPreUpdate, - InputManagerSystem::ManualControl - .before(InputManagerSystem::ReleaseOnDisable) + InputManagerSystem::ManualControl.before(InputManagerSystem::ReleaseOnDisable), ); #[cfg(feature = "ui")] app.add_systems( @@ -188,9 +190,12 @@ impl Plugin for InputManagerPlugin { .in_set(InputManagerSystem::Tick) .before(InputManagerSystem::Update), ); - app.add_systems(RunFixedMainLoop, swap_to_update:: - .run_if(run_if_enabled::) - .after(run_fixed_main_schedule)); + app.add_systems( + RunFixedMainLoop, + swap_to_update:: + .run_if(run_if_enabled::) + .after(run_fixed_main_schedule), + ); } Machine::Server => { app.add_systems( diff --git a/src/systems.rs b/src/systems.rs index 2605ee8e..6aaf7542 100644 --- a/src/systems.rs +++ b/src/systems.rs @@ -28,8 +28,7 @@ use bevy::ui::Interaction; #[cfg(feature = "egui")] use bevy_egui::EguiContext; - -/// We are about to enter the Main schedule, so we: +/// We are about to enter the `Main` schedule, so we: /// - save all the changes applied to `state` into the `fixed_update_state` /// - switch to loading the `update_state` pub fn swap_to_update( @@ -45,7 +44,7 @@ pub fn swap_to_update( } } -/// We are about to enter the FixedMain schedule, so we: +/// We are about to enter the `FixedMain` schedule, so we: /// - save all the changes applied to `state` into the `update_state` /// - switch to loading the `fixed_update_state` pub fn swap_to_fixed_update( @@ -61,7 +60,6 @@ pub fn swap_to_fixed_update( } } - /// Advances actions timer. /// /// Clears the just-pressed and just-released values of all [`ActionState`]s. diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 082fe89c..24882b28 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,14 +1,15 @@ -use std::time::Duration; use bevy::input::InputPlugin; -use bevy::MinimalPlugins; -use bevy::prelude::{App, Fixed, FixedUpdate, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update}; +use bevy::prelude::{ + App, Fixed, FixedUpdate, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update, +}; use bevy::time::TimeUpdateStrategy; +use bevy::MinimalPlugins; use leafwing_input_manager::action_state::ActionState; use leafwing_input_manager::input_map::InputMap; use leafwing_input_manager::input_mocking::MockInput; use leafwing_input_manager::plugin::InputManagerPlugin; use leafwing_input_manager_macros::Actionlike; - +use std::time::Duration; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { @@ -22,7 +23,7 @@ struct UpdateCounter { } #[derive(Resource, Default)] -struct FixedUpdateCounter{ +struct FixedUpdateCounter { /// how many times did the FixedUpdate schedule run? run: usize, /// how many times did the Up button get just_pressed? @@ -31,8 +32,7 @@ struct FixedUpdateCounter{ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { let mut app = App::new(); - app - .add_plugins(MinimalPlugins) + app.add_plugins(MinimalPlugins) .add_plugins(InputPlugin) .add_plugins(InputManagerPlugin::::default()) .init_resource::() @@ -40,7 +40,7 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { .init_resource::>() .insert_resource(InputMap::::new([ (TestAction::Up, KeyCode::ArrowUp), - (TestAction::Down, KeyCode::ArrowDown) + (TestAction::Down, KeyCode::ArrowDown), ])) .insert_resource(Time::::from_duration(fixed_timestep)) .insert_resource(TimeUpdateStrategy::ManualDuration(frame_timestep)); @@ -50,15 +50,17 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { // initialize time (needed to set a starting time for tests) let now = bevy::utils::Instant::now(); - app - .world + app.world .get_resource_mut::>() .unwrap() .update_with_instant(now); app } -fn fixed_update_counter(mut counter: ResMut, action: Res>) { +fn fixed_update_counter( + mut counter: ResMut, + action: Res>, +) { if action.just_pressed(&TestAction::Up) { counter.just_pressed += 1; } @@ -71,7 +73,6 @@ fn update_counter(mut counter: ResMut, action: Res().unwrap().run, 0); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!( + app.world.get_resource::().unwrap().run, + 0 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); // the FixedUpdate schedule should run once and the action still be counted as `just_pressed` app.update(); - assert_eq!(app.world.get_resource::().unwrap().run, 1); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!( + app.world.get_resource::().unwrap().run, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); // the FixedUpdate schedule should run once, the button should now be `pressed` app.update(); - assert_eq!(app.world.get_resource::().unwrap().run, 2); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!( + app.world.get_resource::().unwrap().run, + 2 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); } - /// We have a frames with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) /// /// A button pressed in F1 should still be marked as `just_pressed` in FU1, and should just be @@ -113,13 +152,43 @@ fn frame_with_two_fixed_timestep() { // the FixedUpdate schedule should run twice // the button should be just_pressed only once app.update(); - assert_eq!(app.world.get_resource::().unwrap().run, 2); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); + assert_eq!( + app.world.get_resource::().unwrap().run, + 2 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); // the FixedUpdate schedule should run twice app.update(); - assert_eq!(app.world.get_resource::().unwrap().run, 4); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); - assert_eq!(app.world.get_resource::().unwrap().just_pressed, 1); -} \ No newline at end of file + assert_eq!( + app.world.get_resource::().unwrap().run, + 4 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); +} From 83f332a4aaa0511b8e22627cfb6ef1b8bdd1d8c8 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 13:01:12 -0400 Subject: [PATCH 04/15] update benchmarks --- benches/action_state.rs | 2 ++ src/action_state.rs | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/benches/action_state.rs b/benches/action_state.rs index 7c194175..340a272c 100644 --- a/benches/action_state.rs +++ b/benches/action_state.rs @@ -63,6 +63,8 @@ fn criterion_benchmark(c: &mut Criterion) { action, ActionData { state: ButtonState::JustPressed, + update_state: ButtonState::Released, + fixed_update_state: ButtonState::Released, value: 0.0, axis_pair: None, timing: Timing::default(), diff --git a/src/action_state.rs b/src/action_state.rs index 8bb4d01b..f1d26439 100644 --- a/src/action_state.rs +++ b/src/action_state.rs @@ -18,9 +18,10 @@ use serde::{Deserialize, Serialize}; pub struct ActionData { /// Is the action pressed or released? pub state: ButtonState, - - pub(crate) update_state: ButtonState, - pub(crate) fixed_update_state: ButtonState, + /// The `state` of the action in the `Main` schedule + pub update_state: ButtonState, + /// The `state` of the action in the `FixedMain` schedule + pub fixed_update_state: ButtonState, /// The "value" of the binding that triggered the action. /// /// See [`ActionState::value`] for more details. From 6d2107c0b1b119830d5f89efdbad143e46d41a49 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 13:44:53 -0400 Subject: [PATCH 05/15] add consume tests --- tests/fixed_update.rs | 90 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 81 insertions(+), 9 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 24882b28..0104399d 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,15 +1,14 @@ use bevy::input::InputPlugin; -use bevy::prelude::{ - App, Fixed, FixedUpdate, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update, -}; +use bevy::prelude::{App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update}; use bevy::time::TimeUpdateStrategy; use bevy::MinimalPlugins; use leafwing_input_manager::action_state::ActionState; use leafwing_input_manager::input_map::InputMap; use leafwing_input_manager::input_mocking::MockInput; -use leafwing_input_manager::plugin::InputManagerPlugin; +use leafwing_input_manager::plugin::{InputManagerPlugin, InputManagerSystem}; use leafwing_input_manager_macros::Actionlike; use std::time::Duration; +use bevy::app::PreUpdate; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { @@ -48,12 +47,11 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { app.add_systems(Update, update_counter); app.add_systems(FixedUpdate, fixed_update_counter); - // initialize time (needed to set a starting time for tests) - let now = bevy::utils::Instant::now(); + // we have to set an initial time for TimeUpdateStrategy::ManualDuration to work properly + let startup = app.world.resource::>().startup(); app.world - .get_resource_mut::>() - .unwrap() - .update_with_instant(now); + .resource_mut::>() + .update_with_instant(startup); app } @@ -137,6 +135,11 @@ fn frame_without_fixed_timestep() { .just_pressed, 1 ); + + // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) + // (the `tick` function has been called twice, but it uses `Time` to update the time, + // which is only updated in `PreUpdate`, which is what we want) + assert_eq!(app.world.get_resource::>().unwrap().current_duration(&TestAction::Up), Duration::from_millis(18)); } /// We have a frames with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) @@ -191,4 +194,73 @@ fn frame_with_two_fixed_timestep() { .just_pressed, 1 ); + + // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) + // (the `tick` function has been called twice, but it uses `Time` to update the time, + // which is only updated in `PreUpdate`, which is what we want) + assert_eq!(app.world.get_resource::>().unwrap().current_duration(&TestAction::Up), Duration::from_millis(18)); +} + + +/// Check that if the action is consumed in FU1, it will still be consumed in F2. +/// (i.e. consuming is shared between the `FixedMain` and `Main` schedules) +#[test] +fn test_consume_in_fixed_update() { + let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); + + app.add_systems(FixedPostUpdate, |mut action: ResMut>| { + action.consume(&TestAction::Up); + }); + + app.press_input(KeyCode::ArrowUp); + + // the FixedUpdate schedule should run once + // the button should be just_pressed only once + app.update(); + assert_eq!( + app.world.get_resource::().unwrap().run, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + assert_eq!( + app.world + .get_resource::() + .unwrap() + .just_pressed, + 1 + ); + + // the button should still be consumed, even after we exit the FixedUpdate schedule + assert!( + app.world.get_resource::>().unwrap().action_data(&TestAction::Up).unwrap().consumed, + ); } + +/// Check that if the action is consumed in F1, it will still be consumed in FU1. +/// (i.e. consuming is shared between the `FixedMain` and `Main` schedules) +#[test] +fn test_consume_in_update() { + let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); + + + app.press_input(KeyCode::ArrowUp); + fn consume_action(mut action: ResMut>) { + action.consume(&TestAction::Up); + } + + app.add_systems(PreUpdate, consume_action.in_set(InputManagerSystem::ManualControl)); + + app.add_systems(FixedUpdate, |action: Res>| { + // check that the action is still consumed in the FixedMain schedule + assert!(action.consumed(&TestAction::Up), "Action should still be consumed in FixedUpdate"); + }); + + // the FixedUpdate schedule should run once + app.update(); +} \ No newline at end of file From 94aeeb04c793a36739841be1d9747ce165846a0d Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Fri, 17 May 2024 13:52:33 -0400 Subject: [PATCH 06/15] fmt --- tests/fixed_update.rs | 53 ++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 0104399d..1e8007a4 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,5 +1,9 @@ +use bevy::app::PreUpdate; use bevy::input::InputPlugin; -use bevy::prelude::{App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, ResMut, Resource, Time, Update}; +use bevy::prelude::{ + App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, + ResMut, Resource, Time, Update, +}; use bevy::time::TimeUpdateStrategy; use bevy::MinimalPlugins; use leafwing_input_manager::action_state::ActionState; @@ -8,7 +12,6 @@ use leafwing_input_manager::input_mocking::MockInput; use leafwing_input_manager::plugin::{InputManagerPlugin, InputManagerSystem}; use leafwing_input_manager_macros::Actionlike; use std::time::Duration; -use bevy::app::PreUpdate; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { @@ -139,7 +142,13 @@ fn frame_without_fixed_timestep() { // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, // which is only updated in `PreUpdate`, which is what we want) - assert_eq!(app.world.get_resource::>().unwrap().current_duration(&TestAction::Up), Duration::from_millis(18)); + assert_eq!( + app.world + .get_resource::>() + .unwrap() + .current_duration(&TestAction::Up), + Duration::from_millis(18) + ); } /// We have a frames with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) @@ -198,19 +207,27 @@ fn frame_with_two_fixed_timestep() { // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, // which is only updated in `PreUpdate`, which is what we want) - assert_eq!(app.world.get_resource::>().unwrap().current_duration(&TestAction::Up), Duration::from_millis(18)); + assert_eq!( + app.world + .get_resource::>() + .unwrap() + .current_duration(&TestAction::Up), + Duration::from_millis(18) + ); } - /// Check that if the action is consumed in FU1, it will still be consumed in F2. /// (i.e. consuming is shared between the `FixedMain` and `Main` schedules) #[test] fn test_consume_in_fixed_update() { let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); - app.add_systems(FixedPostUpdate, |mut action: ResMut>| { - action.consume(&TestAction::Up); - }); + app.add_systems( + FixedPostUpdate, + |mut action: ResMut>| { + action.consume(&TestAction::Up); + }, + ); app.press_input(KeyCode::ArrowUp); @@ -238,7 +255,12 @@ fn test_consume_in_fixed_update() { // the button should still be consumed, even after we exit the FixedUpdate schedule assert!( - app.world.get_resource::>().unwrap().action_data(&TestAction::Up).unwrap().consumed, + app.world + .get_resource::>() + .unwrap() + .action_data(&TestAction::Up) + .unwrap() + .consumed, ); } @@ -248,19 +270,24 @@ fn test_consume_in_fixed_update() { fn test_consume_in_update() { let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); - app.press_input(KeyCode::ArrowUp); fn consume_action(mut action: ResMut>) { action.consume(&TestAction::Up); } - app.add_systems(PreUpdate, consume_action.in_set(InputManagerSystem::ManualControl)); + app.add_systems( + PreUpdate, + consume_action.in_set(InputManagerSystem::ManualControl), + ); app.add_systems(FixedUpdate, |action: Res>| { // check that the action is still consumed in the FixedMain schedule - assert!(action.consumed(&TestAction::Up), "Action should still be consumed in FixedUpdate"); + assert!( + action.consumed(&TestAction::Up), + "Action should still be consumed in FixedUpdate" + ); }); // the FixedUpdate schedule should run once app.update(); -} \ No newline at end of file +} From 0ade4d632217d2dd6f2ae2a6550632ba82d61f0d Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 1 Jun 2024 10:42:21 -0400 Subject: [PATCH 07/15] clean up tests --- tests/fixed_update.rs | 192 +++++++++++++++++------------------------- 1 file changed, 79 insertions(+), 113 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 1e8007a4..8951cf75 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,4 +1,4 @@ -use bevy::app::PreUpdate; +use bevy::app::{PreUpdate}; use bevy::input::InputPlugin; use bevy::prelude::{ App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, @@ -16,7 +16,6 @@ use std::time::Duration; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { Up, - Down, } #[derive(Resource, Default)] @@ -42,7 +41,6 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { .init_resource::>() .insert_resource(InputMap::::new([ (TestAction::Up, KeyCode::ArrowUp), - (TestAction::Down, KeyCode::ArrowDown), ])) .insert_resource(Time::::from_duration(fixed_timestep)) .insert_resource(TimeUpdateStrategy::ManualDuration(frame_timestep)); @@ -58,6 +56,9 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { app } +/// Keep track of some events that happen in the FixedUpdate schedule +/// - did the FixedUpdate schedule run during the frame? +/// - was the button marked as `just_pressed` in the FixedUpdate schedule? fn fixed_update_counter( mut counter: ResMut, action: Res>, @@ -68,12 +69,53 @@ fn fixed_update_counter( counter.run += 1; } +/// Keep track of some events that happen in the Update schedule +/// - was the button marked as `just_pressed` in the Update schedule? fn update_counter(mut counter: ResMut, action: Res>) { if action.just_pressed(&TestAction::Up) { counter.just_pressed += 1; } } +/// Reset the counters at the end of the frame +fn reset_counters(app: &mut App) { + let mut fixed_update_counters = app.world.resource_mut::(); + fixed_update_counters.run = 0; + fixed_update_counters.just_pressed = 0; + let mut update_counters = app.world.resource_mut::(); + update_counters.just_pressed = 0; +} + +/// Assert that the FixedUpdate schedule run the expected number of times +fn check_fixed_update_run_count(app: &mut App, expected: usize) { + assert_eq!( + app.world.get_resource::().unwrap().run, + expected, + "FixedUpdate schedule should have run {} times", + expected + ); +} + +/// Assert that the button was just_pressed the expected number of times during the FixedUpdate schedule +fn check_fixed_update_just_pressed_count(app: &mut App, expected: usize) { + assert_eq!( + app.world.get_resource::().unwrap().just_pressed, + expected, + "Button should have been just_pressed {} times during the FixedUpdate schedule", + expected + ); +} + +/// Assert that the button was just_pressed the expected number of times during the Update schedule +fn check_update_just_pressed_count(app: &mut App, expected: usize) { + assert_eq!( + app.world.get_resource::().unwrap().just_pressed, + expected, + "Button should have been just_pressed {} times during the Update schedule", + expected + ); +} + /// We have 2 frames without a FixedUpdate schedule in between (F1 - F2 - FU - F3) /// /// A button pressed in F1 should still be marked as `just_pressed` in FU @@ -83,61 +125,28 @@ fn frame_without_fixed_timestep() { app.press_input(KeyCode::ArrowUp); - // the FixedUpdate schedule shouldn't run + // Frame 1: button is just_pressed and the FixedUpdate schedule does not run app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 0 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 1); + check_fixed_update_run_count(&mut app, 0); + reset_counters(&mut app); + - // the FixedUpdate schedule should run once and the action still be counted as `just_pressed` + // Frame 2: the FixedUpdate schedule should run once, the button is `just_pressed` for FixedUpdate + // because we tick independently in FixedUpdate and Update. + // Button is not `just_pressed` anymore in the Update schedule. app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 0); + check_fixed_update_run_count(&mut app, 1); + check_fixed_update_just_pressed_count(&mut app, 1); + reset_counters(&mut app); - // the FixedUpdate schedule should run once, the button should now be `pressed` + // Frame 3: the FixedUpdate schedule should run once, the button should now be `pressed` in both schedules app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 2 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 0); + check_fixed_update_run_count(&mut app, 1); + check_fixed_update_just_pressed_count(&mut app, 0); + reset_counters(&mut app); // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, @@ -151,7 +160,7 @@ fn frame_without_fixed_timestep() { ); } -/// We have a frames with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) +/// We have a frame with two FixedUpdate schedule executions in between (F1 - FU1 - FU2 - F2) /// /// A button pressed in F1 should still be marked as `just_pressed` in FU1, and should just be /// `pressed` in FU2 @@ -161,48 +170,20 @@ fn frame_with_two_fixed_timestep() { app.press_input(KeyCode::ArrowUp); - // the FixedUpdate schedule should run twice - // the button should be just_pressed only once + // Frame 1: the FixedUpdate schedule should run twice, but the button should be just_pressed only once + // in FixedUpdate app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 2 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 1); + check_fixed_update_run_count(&mut app, 2); + check_fixed_update_just_pressed_count(&mut app, 1); + reset_counters(&mut app); - // the FixedUpdate schedule should run twice + // Frame 2: the FixedUpdate schedule should run twice, but the buttton is not just_pressed anymore app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 4 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 0); + check_fixed_update_run_count(&mut app, 2); + check_fixed_update_just_pressed_count(&mut app, 0); + reset_counters(&mut app); // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, @@ -231,27 +212,12 @@ fn test_consume_in_fixed_update() { app.press_input(KeyCode::ArrowUp); - // the FixedUpdate schedule should run once - // the button should be just_pressed only once + // Frame 1: the FixedUpdate schedule should run once and the button should be just_pressed only once app.update(); - assert_eq!( - app.world.get_resource::().unwrap().run, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); - assert_eq!( - app.world - .get_resource::() - .unwrap() - .just_pressed, - 1 - ); + check_update_just_pressed_count(&mut app, 1); + check_fixed_update_run_count(&mut app, 1); + check_fixed_update_just_pressed_count(&mut app, 1); + reset_counters(&mut app); // the button should still be consumed, even after we exit the FixedUpdate schedule assert!( From 29ecd17f5e615aac448a4876518f1c16f7753ec8 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 1 Jun 2024 16:10:53 -0400 Subject: [PATCH 08/15] fmt --- tests/fixed_update.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 8951cf75..c1fce9e7 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,4 +1,4 @@ -use bevy::app::{PreUpdate}; +use bevy::app::PreUpdate; use bevy::input::InputPlugin; use bevy::prelude::{ App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, @@ -39,9 +39,10 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { .init_resource::() .init_resource::() .init_resource::>() - .insert_resource(InputMap::::new([ - (TestAction::Up, KeyCode::ArrowUp), - ])) + .insert_resource(InputMap::::new([( + TestAction::Up, + KeyCode::ArrowUp, + )])) .insert_resource(Time::::from_duration(fixed_timestep)) .insert_resource(TimeUpdateStrategy::ManualDuration(frame_timestep)); @@ -99,7 +100,10 @@ fn check_fixed_update_run_count(app: &mut App, expected: usize) { /// Assert that the button was just_pressed the expected number of times during the FixedUpdate schedule fn check_fixed_update_just_pressed_count(app: &mut App, expected: usize) { assert_eq!( - app.world.get_resource::().unwrap().just_pressed, + app.world + .get_resource::() + .unwrap() + .just_pressed, expected, "Button should have been just_pressed {} times during the FixedUpdate schedule", expected @@ -109,7 +113,10 @@ fn check_fixed_update_just_pressed_count(app: &mut App, expected: usize) { /// Assert that the button was just_pressed the expected number of times during the Update schedule fn check_update_just_pressed_count(app: &mut App, expected: usize) { assert_eq!( - app.world.get_resource::().unwrap().just_pressed, + app.world + .get_resource::() + .unwrap() + .just_pressed, expected, "Button should have been just_pressed {} times during the Update schedule", expected @@ -131,7 +138,6 @@ fn frame_without_fixed_timestep() { check_fixed_update_run_count(&mut app, 0); reset_counters(&mut app); - // Frame 2: the FixedUpdate schedule should run once, the button is `just_pressed` for FixedUpdate // because we tick independently in FixedUpdate and Update. // Button is not `just_pressed` anymore in the Update schedule. From 2509167432fd7e1491e846e91bcc9251dd05557a Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 1 Jun 2024 17:24:58 -0400 Subject: [PATCH 09/15] remove disable/enable logic --- src/plugin.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/plugin.rs b/src/plugin.rs index 2474dab1..8a51e60f 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -162,14 +162,9 @@ impl Plugin for InputManagerPlugin { update_action_state::, ) .chain() - .run_if(run_if_enabled::) .before(run_fixed_main_schedule), ); - app.add_systems( - FixedPreUpdate, - release_on_disable::.in_set(InputManagerSystem::ReleaseOnDisable), - ); #[cfg(feature = "ui")] app.configure_sets( FixedPreUpdate, @@ -179,21 +174,18 @@ impl Plugin for InputManagerPlugin { app.add_systems( FixedPreUpdate, update_action_state_from_interaction:: - .run_if(run_if_enabled::) .in_set(InputManagerSystem::ManualControl), ); app.add_systems(FixedPostUpdate, release_on_input_map_removed::); app.add_systems( FixedPostUpdate, tick_action_state:: - .run_if(run_if_enabled::) .in_set(InputManagerSystem::Tick) .before(InputManagerSystem::Update), ); app.add_systems( RunFixedMainLoop, swap_to_update:: - .run_if(run_if_enabled::) .after(run_fixed_main_schedule), ); } @@ -201,7 +193,6 @@ impl Plugin for InputManagerPlugin { app.add_systems( PreUpdate, tick_action_state:: - .run_if(run_if_enabled::) .in_set(InputManagerSystem::Tick), ); } From 7fc6f1438d400f2f32c8561574efb7bd5782f667 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 1 Jun 2024 17:27:07 -0400 Subject: [PATCH 10/15] update --- src/plugin.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/plugin.rs b/src/plugin.rs index 3671cc87..0ee77c7e 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -20,7 +20,7 @@ use std::fmt::Debug; use bevy::app::{App, Plugin, RunFixedMainLoop}; use bevy::ecs::prelude::*; use bevy::input::{ButtonState, InputSystem}; -use bevy::prelude::{FixedPostUpdate, FixedPreUpdate, PostUpdate, PreUpdate}; +use bevy::prelude::{FixedPostUpdate, PostUpdate, PreUpdate}; use bevy::reflect::TypePath; use bevy::time::run_fixed_main_schedule; #[cfg(feature = "ui")] @@ -156,13 +156,10 @@ impl Plugin for InputManagerPlugin { ); #[cfg(feature = "ui")] - app.configure_sets( - FixedPreUpdate, - InputManagerSystem::ManualControl.before(InputManagerSystem::ReleaseOnDisable), - ); + app.configure_sets(bevy::app::FixedPreUpdate, InputManagerSystem::ManualControl); #[cfg(feature = "ui")] app.add_systems( - FixedPreUpdate, + bevy::app::FixedPreUpdate, update_action_state_from_interaction:: .in_set(InputManagerSystem::ManualControl), ); @@ -175,8 +172,7 @@ impl Plugin for InputManagerPlugin { ); app.add_systems( RunFixedMainLoop, - swap_to_update:: - .after(run_fixed_main_schedule), + swap_to_update::.after(run_fixed_main_schedule), ); } Machine::Server => { From a65bb1f07fec32929d946d825119dfec5845eb87 Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Sat, 1 Jun 2024 23:17:50 -0400 Subject: [PATCH 11/15] update timing test --- tests/fixed_update.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index c1fce9e7..c09a188e 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -170,6 +170,7 @@ fn frame_without_fixed_timestep() { /// /// A button pressed in F1 should still be marked as `just_pressed` in FU1, and should just be /// `pressed` in FU2 +#[cfg(feature = "timing")] #[test] fn frame_with_two_fixed_timestep() { let mut app = build_app(Duration::from_millis(4), Duration::from_millis(9)); @@ -205,6 +206,7 @@ fn frame_with_two_fixed_timestep() { /// Check that if the action is consumed in FU1, it will still be consumed in F2. /// (i.e. consuming is shared between the `FixedMain` and `Main` schedules) +#[cfg(feature = "timing")] #[test] fn test_consume_in_fixed_update() { let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); From d05b877d8da78548f7e4b2428c305df94dcbe65d Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Mon, 10 Jun 2024 10:05:27 -0400 Subject: [PATCH 12/15] fix tests for timing feature --- tests/fixed_update.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index c09a188e..4cc0da45 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,9 +1,4 @@ -use bevy::app::PreUpdate; -use bevy::input::InputPlugin; -use bevy::prelude::{ - App, Fixed, FixedPostUpdate, FixedUpdate, IntoSystemConfigs, KeyCode, Real, Reflect, Res, - ResMut, Resource, Time, Update, -}; +use bevy::prelude::*; use bevy::time::TimeUpdateStrategy; use bevy::MinimalPlugins; use leafwing_input_manager::action_state::ActionState; @@ -154,9 +149,11 @@ fn frame_without_fixed_timestep() { check_fixed_update_just_pressed_count(&mut app, 0); reset_counters(&mut app); + // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, // which is only updated in `PreUpdate`, which is what we want) + #[cfg(feature = "timing")] assert_eq!( app.world .get_resource::>() @@ -170,7 +167,6 @@ fn frame_without_fixed_timestep() { /// /// A button pressed in F1 should still be marked as `just_pressed` in FU1, and should just be /// `pressed` in FU2 -#[cfg(feature = "timing")] #[test] fn frame_with_two_fixed_timestep() { let mut app = build_app(Duration::from_millis(4), Duration::from_millis(9)); @@ -195,6 +191,7 @@ fn frame_with_two_fixed_timestep() { // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, // which is only updated in `PreUpdate`, which is what we want) + #[cfg(feature = "timing")] assert_eq!( app.world .get_resource::>() @@ -206,7 +203,6 @@ fn frame_with_two_fixed_timestep() { /// Check that if the action is consumed in FU1, it will still be consumed in F2. /// (i.e. consuming is shared between the `FixedMain` and `Main` schedules) -#[cfg(feature = "timing")] #[test] fn test_consume_in_fixed_update() { let mut app = build_app(Duration::from_millis(5), Duration::from_millis(5)); From ae77c77efecea56d01de10e4b064ae6baba9c72e Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Mon, 10 Jun 2024 10:07:48 -0400 Subject: [PATCH 13/15] lint --- tests/fixed_update.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 4cc0da45..f101f238 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -149,7 +149,6 @@ fn frame_without_fixed_timestep() { check_fixed_update_just_pressed_count(&mut app, 0); reset_counters(&mut app); - // make sure that the timings didn't get updated twice (once in Update and once in FixedUpdate) // (the `tick` function has been called twice, but it uses `Time` to update the time, // which is only updated in `PreUpdate`, which is what we want) From ddc4f1c1afe0db28da9145279beee0087ee62c7d Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Mon, 10 Jun 2024 10:29:45 -0400 Subject: [PATCH 14/15] add release notes --- RELEASES.md | 3 +++ tests/fixed_update.rs | 21 +++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 5b3abc9a..aef93d96 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -21,6 +21,9 @@ ### Enhancements +- Inputs are now handled correctly in the `FixedUpdate` schedule! Previously, the `ActionState`s were only updated in the `PreUpdate` schedule, so you could have situations where an action was + marked as `just_pressed` multiple times in a row (if the `FixedUpdate` schedule ran multiple times in a frame) or was missed entirely (if the `FixedUpdate` schedule ran 0 times in a frame). + #### Input Processors Input processors allow you to create custom logic for axis-like input manipulation. diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index f101f238..0b494846 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -7,6 +7,7 @@ use leafwing_input_manager::input_mocking::MockInput; use leafwing_input_manager::plugin::{InputManagerPlugin, InputManagerSystem}; use leafwing_input_manager_macros::Actionlike; use std::time::Duration; +use bevy::input::InputPlugin; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { @@ -45,8 +46,8 @@ fn build_app(fixed_timestep: Duration, frame_timestep: Duration) -> App { app.add_systems(FixedUpdate, fixed_update_counter); // we have to set an initial time for TimeUpdateStrategy::ManualDuration to work properly - let startup = app.world.resource::>().startup(); - app.world + let startup = app.world().resource::>().startup(); + app.world_mut() .resource_mut::>() .update_with_instant(startup); app @@ -75,17 +76,17 @@ fn update_counter(mut counter: ResMut, action: Res(); + let mut fixed_update_counters = app.world_mut().resource_mut::(); fixed_update_counters.run = 0; fixed_update_counters.just_pressed = 0; - let mut update_counters = app.world.resource_mut::(); + let mut update_counters = app.world_mut().resource_mut::(); update_counters.just_pressed = 0; } /// Assert that the FixedUpdate schedule run the expected number of times fn check_fixed_update_run_count(app: &mut App, expected: usize) { assert_eq!( - app.world.get_resource::().unwrap().run, + app.world().get_resource::().unwrap().run, expected, "FixedUpdate schedule should have run {} times", expected @@ -95,7 +96,7 @@ fn check_fixed_update_run_count(app: &mut App, expected: usize) { /// Assert that the button was just_pressed the expected number of times during the FixedUpdate schedule fn check_fixed_update_just_pressed_count(app: &mut App, expected: usize) { assert_eq!( - app.world + app.world() .get_resource::() .unwrap() .just_pressed, @@ -108,7 +109,7 @@ fn check_fixed_update_just_pressed_count(app: &mut App, expected: usize) { /// Assert that the button was just_pressed the expected number of times during the Update schedule fn check_update_just_pressed_count(app: &mut App, expected: usize) { assert_eq!( - app.world + app.world() .get_resource::() .unwrap() .just_pressed, @@ -154,7 +155,7 @@ fn frame_without_fixed_timestep() { // which is only updated in `PreUpdate`, which is what we want) #[cfg(feature = "timing")] assert_eq!( - app.world + app.world() .get_resource::>() .unwrap() .current_duration(&TestAction::Up), @@ -192,7 +193,7 @@ fn frame_with_two_fixed_timestep() { // which is only updated in `PreUpdate`, which is what we want) #[cfg(feature = "timing")] assert_eq!( - app.world + app.world() .get_resource::>() .unwrap() .current_duration(&TestAction::Up), @@ -224,7 +225,7 @@ fn test_consume_in_fixed_update() { // the button should still be consumed, even after we exit the FixedUpdate schedule assert!( - app.world + app.world() .get_resource::>() .unwrap() .action_data(&TestAction::Up) From 338dcde046b6a9548f018d29b56732373163978d Mon Sep 17 00:00:00 2001 From: Charles Bournhonesque Date: Mon, 10 Jun 2024 10:37:26 -0400 Subject: [PATCH 15/15] lint --- tests/fixed_update.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/fixed_update.rs b/tests/fixed_update.rs index 0b494846..71223dea 100644 --- a/tests/fixed_update.rs +++ b/tests/fixed_update.rs @@ -1,3 +1,4 @@ +use bevy::input::InputPlugin; use bevy::prelude::*; use bevy::time::TimeUpdateStrategy; use bevy::MinimalPlugins; @@ -7,7 +8,6 @@ use leafwing_input_manager::input_mocking::MockInput; use leafwing_input_manager::plugin::{InputManagerPlugin, InputManagerSystem}; use leafwing_input_manager_macros::Actionlike; use std::time::Duration; -use bevy::input::InputPlugin; #[derive(Actionlike, Clone, Copy, Debug, Reflect, PartialEq, Eq, Hash)] enum TestAction { @@ -86,7 +86,10 @@ fn reset_counters(app: &mut App) { /// Assert that the FixedUpdate schedule run the expected number of times fn check_fixed_update_run_count(app: &mut App, expected: usize) { assert_eq!( - app.world().get_resource::().unwrap().run, + app.world() + .get_resource::() + .unwrap() + .run, expected, "FixedUpdate schedule should have run {} times", expected