From ca994a4b0d69449c397a6edc54e9e6e26eafdfc3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 13:16:26 -0400 Subject: [PATCH 1/6] Add a failsafe for cascading FixedUpdate time accumulation --- crates/bevy_time/src/fixed_timestep.rs | 37 ++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 66be2f41ef2ff..e546c7d5b94dc 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -24,7 +24,7 @@ use crate::Time; use bevy_app::FixedUpdate; use bevy_ecs::{system::Resource, world::World}; -use bevy_utils::Duration; +use bevy_utils::{Duration, tracing::warn}; use thiserror::Error; /// The amount of time that must pass before the fixed timestep schedule is run again. @@ -34,14 +34,25 @@ pub struct FixedTime { /// Defaults to 1/60th of a second. /// To configure this value, simply mutate or overwrite this resource. pub period: Duration, + /// The maximum amount of time that can be expended in a single frame. + /// Defaults to 3 times the period. + /// + /// If this value is set to `None`, the schedule will run as many times as possible. + /// Be careful when setting this value to `None`, as it can cause the app to stop rendering + /// as the fixed update schedule will run an increasing number of times per frame as it falls behind. + pub max_time_per_frame: Option, } impl FixedTime { + /// The default ratio between the period and the max allocated time. + const DEFAULT_MAX_TIME_RATIO: u32 = 3; + /// Creates a new [`FixedTime`] struct pub fn new(period: Duration) -> Self { FixedTime { accumulated: Duration::ZERO, period, + max_time_per_frame: Some(period * Self::DEFAULT_MAX_TIME_RATIO), } } @@ -50,6 +61,9 @@ impl FixedTime { FixedTime { accumulated: Duration::ZERO, period: Duration::from_secs_f32(period), + max_time_per_frame: Some(Duration::from_secs_f32( + period * Self::DEFAULT_MAX_TIME_RATIO as f32, + )), } } @@ -82,10 +96,7 @@ impl FixedTime { impl Default for FixedTime { fn default() -> Self { - FixedTime { - accumulated: Duration::ZERO, - period: Duration::from_secs_f32(1. / 60.), - } + FixedTime::new_from_secs(1. / 60.) } } @@ -106,9 +117,25 @@ pub fn run_fixed_update_schedule(world: &mut World) { let mut fixed_time = world.resource_mut::(); fixed_time.tick(delta_time); + // Copy these out to appease the borrow checker + let maybe_max_time = fixed_time.max_time_per_frame; + let period = fixed_time.period; + let mut time_this_frame = Duration::ZERO; + // Run the schedule until we run out of accumulated time let _ = world.try_schedule_scope(FixedUpdate, |world, schedule| { while world.resource_mut::().expend().is_ok() { + time_this_frame += period; + + // This is required to avoid entering a death spiral where the fixed update schedule falls behind + // and needs to run more and more times per frame to catch up. + if let Some(max_time) = maybe_max_time { + if time_this_frame > max_time { + warn!("The fixed update schedule is falling behind. Consider increasing the period or decreasing the amount of work done in the fixed update schedule."); + break; + } + } + schedule.run(world); } }); From 0cc49e523acd7ee3e7ccc5c2c15c0c098ae0ae4d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 13:26:08 -0400 Subject: [PATCH 2/6] Use a simpler tick counting mechanism --- crates/bevy_time/src/fixed_timestep.rs | 29 +++++++++++--------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index e546c7d5b94dc..033f9809215ff 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -34,25 +34,22 @@ pub struct FixedTime { /// Defaults to 1/60th of a second. /// To configure this value, simply mutate or overwrite this resource. pub period: Duration, - /// The maximum amount of time that can be expended in a single frame. - /// Defaults to 3 times the period. + /// The maximum number of times that the [`FixedUpdate`] schedule will be allowed to run per main schedule pass. + /// Defaults to 3. /// /// If this value is set to `None`, the schedule will run as many times as possible. /// Be careful when setting this value to `None`, as it can cause the app to stop rendering /// as the fixed update schedule will run an increasing number of times per frame as it falls behind. - pub max_time_per_frame: Option, + pub max_ticks_per_frame: Option, } impl FixedTime { - /// The default ratio between the period and the max allocated time. - const DEFAULT_MAX_TIME_RATIO: u32 = 3; - /// Creates a new [`FixedTime`] struct pub fn new(period: Duration) -> Self { FixedTime { accumulated: Duration::ZERO, period, - max_time_per_frame: Some(period * Self::DEFAULT_MAX_TIME_RATIO), + max_ticks_per_frame: Some(3), } } @@ -61,9 +58,7 @@ impl FixedTime { FixedTime { accumulated: Duration::ZERO, period: Duration::from_secs_f32(period), - max_time_per_frame: Some(Duration::from_secs_f32( - period * Self::DEFAULT_MAX_TIME_RATIO as f32, - )), + max_ticks_per_frame: Some(3), } } @@ -117,20 +112,20 @@ pub fn run_fixed_update_schedule(world: &mut World) { let mut fixed_time = world.resource_mut::(); fixed_time.tick(delta_time); - // Copy these out to appease the borrow checker - let maybe_max_time = fixed_time.max_time_per_frame; - let period = fixed_time.period; - let mut time_this_frame = Duration::ZERO; + let mut ticks_this_frame = 0; + + // Copy this out to appease the borrow checker + let maybe_max_ticks = fixed_time.max_ticks_per_frame; // Run the schedule until we run out of accumulated time let _ = world.try_schedule_scope(FixedUpdate, |world, schedule| { while world.resource_mut::().expend().is_ok() { - time_this_frame += period; + ticks_this_frame += 1; // This is required to avoid entering a death spiral where the fixed update schedule falls behind // and needs to run more and more times per frame to catch up. - if let Some(max_time) = maybe_max_time { - if time_this_frame > max_time { + if let Some(max_ticks) = maybe_max_ticks { + if ticks_this_frame > max_ticks { warn!("The fixed update schedule is falling behind. Consider increasing the period or decreasing the amount of work done in the fixed update schedule."); break; } From ece5f64a079c9716dbfd92ea6aa4f18d561a92c1 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 13:29:05 -0400 Subject: [PATCH 3/6] Formatting --- crates/bevy_time/src/fixed_timestep.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 033f9809215ff..33af2762615af 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -24,7 +24,7 @@ use crate::Time; use bevy_app::FixedUpdate; use bevy_ecs::{system::Resource, world::World}; -use bevy_utils::{Duration, tracing::warn}; +use bevy_utils::{tracing::warn, Duration}; use thiserror::Error; /// The amount of time that must pass before the fixed timestep schedule is run again. @@ -113,7 +113,7 @@ pub fn run_fixed_update_schedule(world: &mut World) { fixed_time.tick(delta_time); let mut ticks_this_frame = 0; - + // Copy this out to appease the borrow checker let maybe_max_ticks = fixed_time.max_ticks_per_frame; From 9d50054a369a02e496548c7d6ca9fe942ea459e4 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 13:29:40 -0400 Subject: [PATCH 4/6] Help out cargo fmt --- crates/bevy_time/src/fixed_timestep.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 33af2762615af..68110cfc2d240 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -121,14 +121,14 @@ pub fn run_fixed_update_schedule(world: &mut World) { let _ = world.try_schedule_scope(FixedUpdate, |world, schedule| { while world.resource_mut::().expend().is_ok() { ticks_this_frame += 1; - + // This is required to avoid entering a death spiral where the fixed update schedule falls behind // and needs to run more and more times per frame to catch up. if let Some(max_ticks) = maybe_max_ticks { if ticks_this_frame > max_ticks { warn!("The fixed update schedule is falling behind. Consider increasing the period or decreasing the amount of work done in the fixed update schedule."); break; - } + } } schedule.run(world); From edb8da3d6b0dfeb59b14e54a60987865e0f28a5d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 15:59:27 -0400 Subject: [PATCH 5/6] Increase default and move to a const --- crates/bevy_time/src/fixed_timestep.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 68110cfc2d240..9edd0ca38feae 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -35,7 +35,7 @@ pub struct FixedTime { /// To configure this value, simply mutate or overwrite this resource. pub period: Duration, /// The maximum number of times that the [`FixedUpdate`] schedule will be allowed to run per main schedule pass. - /// Defaults to 3. + /// Defaults to 20: this may be too high for your game and will result in very low (but stable) FPS with a long fixed update period. /// /// If this value is set to `None`, the schedule will run as many times as possible. /// Be careful when setting this value to `None`, as it can cause the app to stop rendering @@ -44,12 +44,15 @@ pub struct FixedTime { } impl FixedTime { + /// Sets the default maximum number of times that the [`FixedUpdate`] schedule will be allowed to run per main schedule pass. + const DEFAULT_MAX_TICKS_PER_FRAME: u8 = 20; + /// Creates a new [`FixedTime`] struct pub fn new(period: Duration) -> Self { FixedTime { accumulated: Duration::ZERO, period, - max_ticks_per_frame: Some(3), + max_ticks_per_frame: Some(Self::DEFAULT_MAX_TICKS_PER_FRAME), } } @@ -58,7 +61,7 @@ impl FixedTime { FixedTime { accumulated: Duration::ZERO, period: Duration::from_secs_f32(period), - max_ticks_per_frame: Some(3), + max_ticks_per_frame: Some(Self::DEFAULT_MAX_TICKS_PER_FRAME), } } From fa9e2ea199cd4956160ae2334006f1c313210d25 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 4 May 2023 22:47:46 -0400 Subject: [PATCH 6/6] Swap to a usize --- crates/bevy_time/src/fixed_timestep.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 9edd0ca38feae..4da69dc438d2b 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -40,12 +40,12 @@ pub struct FixedTime { /// If this value is set to `None`, the schedule will run as many times as possible. /// Be careful when setting this value to `None`, as it can cause the app to stop rendering /// as the fixed update schedule will run an increasing number of times per frame as it falls behind. - pub max_ticks_per_frame: Option, + pub max_ticks_per_frame: Option, } impl FixedTime { /// Sets the default maximum number of times that the [`FixedUpdate`] schedule will be allowed to run per main schedule pass. - const DEFAULT_MAX_TICKS_PER_FRAME: u8 = 20; + const DEFAULT_MAX_TICKS_PER_FRAME: usize = 20; /// Creates a new [`FixedTime`] struct pub fn new(period: Duration) -> Self {