Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a failsafe for cascading FixedUpdate time accumulation #8544

Closed
Closed
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions crates/bevy_time/src/fixed_timestep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{tracing::warn, Duration};
use thiserror::Error;

/// The amount of time that must pass before the fixed timestep schedule is run again.
Expand All @@ -34,6 +34,13 @@ 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 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_ticks_per_frame: Option<u8>,
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
}

impl FixedTime {
Expand All @@ -42,6 +49,7 @@ impl FixedTime {
FixedTime {
accumulated: Duration::ZERO,
period,
max_ticks_per_frame: Some(3),
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -50,6 +58,7 @@ impl FixedTime {
FixedTime {
accumulated: Duration::ZERO,
period: Duration::from_secs_f32(period),
max_ticks_per_frame: Some(3),
}
}

Expand Down Expand Up @@ -82,10 +91,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.)
}
}

Expand All @@ -106,9 +112,25 @@ pub fn run_fixed_update_schedule(world: &mut World) {
let mut fixed_time = world.resource_mut::<FixedTime>();
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;

// Run the schedule until we run out of accumulated time
let _ = world.try_schedule_scope(FixedUpdate, |world, schedule| {
while world.resource_mut::<FixedTime>().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.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the game enters the "death spiral", this warning will print out three times per second, constantly. I'm not sure what the policy on warning spam is, so this may be okay.

Also, if the game freezes for a while for any reason (OS freeze for 5 seconds, memory swapping, big blocking loading operation inside game, whatever), these warnings will be printed out for a while (freeze duration seconds * 3 times) until the situation resolves itself. In this case, saying that update schedule is falling behind is incorrect, as it is rather catching up to realtime after a freeze.

And if there is a longer freeze, like OS suspend, that lasts for hours or days, then after resuming we'll be stuck doing maximum amount of fixed updates for a very long time as the game catches up to realtime with 20x acceleration but no faster – and the warnings are printed out for the entire duration it is catching up. However, fixing this is out of scope for this pull, so we don't need to care about the behaviour in this case.

break;
}
}

schedule.run(world);
}
});
Expand Down