Unify FixedTime and Time while fixing several problems

[nakedible]

Objective

Current FixedTime and Time have several problems. This pull aims to fix many of them at once.

• If there is a longer pause between app updates, time will jump forward a lot at once and fixed time will iterate on FixedUpdate for a large number of steps. If the pause is merely seconds, then this will just mean jerkiness and possible unexpected behaviour in gameplay. If the pause is hours/days as with OS suspend, the game will appear to freeze until it has caught up with real time.
• If calculating a fixed step takes longer than specified fixed step period, the game will enter a death spiral where rendering each frame takes longer and longer due to more and more fixed step updates being run per frame and the game appears to freeze.
• There is no way to see current fixed step elapsed time inside fixed steps. In order to track this, the game designer needs to add a custom system inside FixedUpdate that calculates elapsed or step count in a resource.
• Access to delta time inside fixed step is FixedStep::period rather than Time::delta. This, coupled with the issue that Time::elapsed isn't available at all for fixed steps, makes it that time requiring systems are either implemented to be run in FixedUpdate or Update, but rarely work in both.
• Fixes time.delta() not compatible with .run_if() and timer conditions #8800
• Fixes FixedUpdate schedules that take too long enter a cascading failure mode #8543
• Fixes FixedTime API should match Time directly #7439
• Fixes Prevent windup with FixedTimestep #5692

Solution

• Create a generic Time<T> clock that has no processing logic but which can be instantiated for multiple usages. This is also exposed for users to add custom clocks.
• Create three standard clocks, Time<Real>, Time<Virtual> and Time<Fixed>, all of which contain their individual logic.
• Create one "default" clock, which is just Time (or Time<()>), which will be overwritten from Time<Virtual> on each update, and Time<Fixed> inside FixedUpdate schedule. This way systems that do not care specifically which time they track can work both in Update and FixedUpdate without changes and the behaviour is intuitive.
• Add max_delta to virtual time update, which limits how much can be added to virtual time by a single update. This fixes both the behaviour after a long freeze, and also the death spiral by limiting how many fixed timestep iterations there can be per update. Possible future work could be adding max_accumulator to add a sort of "leaky bucket" time processing to possibly smooth out jumps in time while keeping frame rate stable.
• Many minor tweaks and clarifications to the time functions and their documentation.

Changelog

• Time::raw_delta(), Time::raw_elapsed() and related methods are moved to Time<Real>::delta() and Time<Real>::elapsed() and now match Time API
• FixedTime is now Time<Fixed> and matches Time API.
• Time<Fixed> default timestep is now 64 Hz, or 15625 microseconds.
• Time inside FixedUpdate now reflects fixed timestep time, making systems portable between Update and FixedUpdate.
• Time::pause(), Time::set_relative_speed() and related methods must now be called as Time<Virtual>::pause() etc.
• There is a new max_delta setting in Time<Virtual> that limits how much the clock can jump by a single update. The default value is 0.25 seconds.
• Removed on_fixed_timer() condition as on_timer() does the right thing inside FixedUpdate now.

Migration Guide

• Change all Res<Time> instances that access raw_delta(), raw_elapsed() and related methods to Res<Time<Real>> and delta(), elapsed(), etc.
• Change access to period from Res<FixedTime> to Res<Time<Fixed>> and use delta().
• The default timestep has been changed from 60 Hz to 64 Hz. If you wish to restore the old behaviour, use app.insert_resource(Time::<Fixed>::from_hz(60.0)).
• Change app.insert_resource(FixedTime::new(duration)) to app.insert_resource(Time::<Fixed>::from_duration(duration))
• Change app.insert_resource(FixedTime::new_from_secs(secs)) to app.insert_resource(Time::<Fixed>::from_seconds(secs))
• Change system.on_fixed_timer(duration) to system.on_timer(duration). Timers in systems placed in FixedUpdate schedule automatically use the fixed time clock.
• Change ResMut<Time> calls to pause(), is_paused(), set_relative_speed() and related methods to ResMut<Time<Virtual>> calls. The API is the same, with the exception that relative_speed() will return the actual last ste relative speed, while effective_relative_speed() returns 0.0 if the time is paused and corresponds to the speed that was set when the update for the current frame started.

Todo

• Update pull name and description
• Top level documentation on usage
• Fix examples
• Decide on default max_delta value
• Decide naming of the three clocks: is Real, Virtual, Fixed good?
• Decide if the three clock inner structures should be in prelude
• Decide on best way to configure values at startup: is manually inserting a new clock instance okay, or should there be config struct separately?
• Fix links in docs
• Decide what should be public and what not
• Decide how wrap_period should be handled when it is changed
• Add toggles to disable setting the clock as default? No, separate pull if needed.
• Add tests
• Reformat, ensure adheres to conventions etc.
• Build documentation and see that it looks correct

Contributors

Huge thanks to @alice-i-cecile and @maniwani while building this pull. It was a shared effort!

[github-actions]

Example breakout failed to run, please try running it locally and check the result.

[github-actions]

Example breakout failed to run, please try running it locally and check the result.

[github-actions]

Example breakout failed to run, please try running it locally and check the result.

[github-actions]

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[github-actions]

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[github-actions]

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[cart]

I quite like these changes! The Time overriding behavior is a bit implicit. But I do agree that making systems usable across contexts is useful. And it means 99% of users can just consume Time::delta in any context and it will "just work". This does make the system slightly harder to "fully grasp" as a result. I think some users will think they are consuming the exact same value in both contexts. But I think the tradeoff is worth it. For users that need to care, you wrote really nice docs explaining everything. I cannot conceive of a better system.

Great work!

(I just have one comment on the default fixed timestep value)

[alice-i-cecile]

@nakedible let me know if you want a hand with those docs. Once those are written, we'll do a quick pass of review and then I can merge this in for you :)

[nakedible]

Nah, I'll handle the doc change later today, thanks though!

[nakedible]

Some extra doc lint checks were added in the meantime, so cleaned up those too so that tests pass again. Should be merge ready from my side.

[rlidwka]

Can you give an advice on how I can create a custom Time specialization?

In my own crate, I can't add impl Time<MyTime> {}, because it gives the following error:

error[E0116]: cannot define inherent `impl` for a type outside of the crate where the type is defined
  |
3 | impl Time<MyTime> {
  | ^^^^^^^^^^^^^^^^^ impl for type defined outside of crate.
  |
  = note: define and implement a trait or new type instead

But I'd like users to have the same interface (e.g. Time<MyTime>::from_seconds) as bevy core times do.

Is a custom trait (that people have to explicitly import) the only option here?

[rlidwka]

@nakedible, why did you decide to go with:

pub struct Time<T> { ... }
pub struct Fixed;

fn system(time: Res<Time<Fixed>>) {}

as opposed to this?:

pub struct Time {...}

#[derive(Deref, DerefMut)]
pub struct FixedTime(Time);

fn system(time: Res<FixedTime>) {}

I.e. why Time wraps specialization struct, and not the other way around?

[djeedai]

I think because this allows Time == Time<()> syntax which is more compact and looks like the previous version.

[nakedible]

In the design phase I got some opposition to doing this with Deref, as it was felt to be too implicit. Also, even though Bevy differs in many places already, from Rust docs:

Implementing Deref for smart pointers makes accessing the data behind them convenient, which is why they implement Deref. On the other hand, the rules regarding Deref and DerefMut were designed specifically to accommodate smart pointers. Because of this, Deref should only be implemented for smart pointers to avoid confusion.

Honestly could've gone either way. Deref is more "traditional", where as wrapping the specialization makes it more explicit. If this final design would be converted to Deref, I'm not sure the initial opposition to Deref would still hold.

[maniwani]

Can you give an advice on how I can create a custom Time specialization?

In my own crate, I can't add impl Time<MyTime> {}, because [of the orphan rule].

It's a little clunky, but you could implement the specialized methods using a trait. (edit: didn't see you had already considered that) yeah, I think a custom trait is the way to do it.

pub trait MyTrait {
    /* ... */
}

impl MyTrait for Time<MyTime> {
    /* ... */
}

[regnarock]

There seems to be a slight mistake in the migration guide:

Change system.on_fixed_timer(duration) to system.on_timer(duration). Timers in systems placed in FixedUpdate schedule automatically use the fixed time clock.

Should instead say:

• Change system.on_fixed_timer(duration) to system.run_if(on_timer(duration)). Timers in systems placed in FixedUpdate schedule automatically use the fixed time clock.

This mistake also appears in the migration guide