Prevent windup with FixedTimestep #5692

HackerFoo · 2022-08-14T22:09:05Z

Objective

After being suspended for a while and then resuming, FixedTimestep systems will run full speed until caught up, which is undesirable in many (most?) cases, such as my physics system in Noumenal.

Solution

Add a new constructor step_with_max_loops for creating a system that will limit the number of loops to catch up.

Changelog

Added: FixedTimestep::step_with_max_loops for creating a system that will limit the number of loops to catch up.

alice-i-cecile · 2022-08-14T22:29:48Z

Related to #1353. I think this is the wrong fix; IMO time simply shouldn't be accumulated when the app is paused.

maniwani

IMO time simply shouldn't be accumulated when the app is paused.

Yeah, this should be handled by suspending Time, which will suspend everything that inherits from it.

I would address this with the time scaling added in #3002. Setting the relative speed to 0.0 would stop Time and, by proxy, the fixed timestep.

Edit: Maybe that can be carved out and merged independently? Without the "just one built-in timestep" part, does it make sense for scaling to affect all of the timestep states in FixedTimesteps?

maniwani · 2022-08-14T22:56:48Z

crates/bevy_time/src/fixed_timestep.rs

+            let time_delta = time.delta_seconds_f64();
+            self.accumulator += if time_delta > self.max_time_delta {
+                self.max_time_delta
+            } else {
+                time_delta
+            };


For the fixed timestep itself, it would be better to allow limiting the number of steps that can be run per frame rather than limit the amount of time that can be accrued.

The current change in this PR would prevent us from keeping the fixed timestep in sync with Time.

That wouldn't achieve the desired goal, because the system would run the maximum number of steps (up to the limit) until it has caught up to the accumulator.

I meant that we should enable limiting the number of steps per frame as a supplement to pausing Time.

I think it's important that the fixed timestep be kept in sync with Time. Your change would break that connection.

HackerFoo · 2022-08-14T23:08:22Z

This PR might be useful even with the ability to pause Time where a soft FixedTimestep is desired, because it can be surprising when a CPU intensive system falls behind and then starts running full throttle.

maniwani · 2022-08-14T23:36:53Z

I think the solution is to add pausing methods to Time (forget the scaling stuff) and put the limit on the number of steps that can be run per frame.

That limit will allow mitigating windup that results from temporary frame lag by spreading the load over multiple frames. If a device still can't keep up, doesn't that mean the simulation is just too intensive for that device?

It's important to keep the fixed timestep in sync with Time so that all systems, inside or outside, observe a single timeframe. I believe that property is important for interpolation and animation (and timekeeping in general). At the very least, breaking that connection is going off the beaten path, since AFAIK both Godot and Unity (not sure about Unreal) keep them in sync.

(Edit: To clarify, "in sync" means that every step seconds Time advances will result in 1 iteration of the fixed timestep logic.)

@alice-i-cecile

# 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 #8800 - Fixes #8543 - Fixes #7439 - Fixes #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 - [x] Update pull name and description - [x] Top level documentation on usage - [x] Fix examples - [x] Decide on default `max_delta` value - [x] Decide naming of the three clocks: is `Real`, `Virtual`, `Fixed` good? - [x] Decide if the three clock inner structures should be in prelude - [x] Decide on best way to configure values at startup: is manually inserting a new clock instance okay, or should there be config struct separately? - [x] Fix links in docs - [x] Decide what should be public and what not - [x] Decide how `wrap_period` should be handled when it is changed - [x] ~~Add toggles to disable setting the clock as default?~~ No, separate pull if needed. - [x] Add tests - [x] Reformat, ensure adheres to conventions etc. - [x] 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! --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Cameron <[email protected]> Co-authored-by: Jerome Humbert <[email protected]>

@alice-i-cecile

…e#8964) # 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 bevyengine#8800 - Fixes bevyengine#8543 - Fixes bevyengine#7439 - Fixes bevyengine#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 - [x] Update pull name and description - [x] Top level documentation on usage - [x] Fix examples - [x] Decide on default `max_delta` value - [x] Decide naming of the three clocks: is `Real`, `Virtual`, `Fixed` good? - [x] Decide if the three clock inner structures should be in prelude - [x] Decide on best way to configure values at startup: is manually inserting a new clock instance okay, or should there be config struct separately? - [x] Fix links in docs - [x] Decide what should be public and what not - [x] Decide how `wrap_period` should be handled when it is changed - [x] ~~Add toggles to disable setting the clock as default?~~ No, separate pull if needed. - [x] Add tests - [x] Reformat, ensure adheres to conventions etc. - [x] 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! --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Cameron <[email protected]> Co-authored-by: Jerome Humbert <[email protected]>

@alice-i-cecile

…e#8964) # 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 bevyengine#8800 - Fixes bevyengine#8543 - Fixes bevyengine#7439 - Fixes bevyengine#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 - [x] Update pull name and description - [x] Top level documentation on usage - [x] Fix examples - [x] Decide on default `max_delta` value - [x] Decide naming of the three clocks: is `Real`, `Virtual`, `Fixed` good? - [x] Decide if the three clock inner structures should be in prelude - [x] Decide on best way to configure values at startup: is manually inserting a new clock instance okay, or should there be config struct separately? - [x] Fix links in docs - [x] Decide what should be public and what not - [x] Decide how `wrap_period` should be handled when it is changed - [x] ~~Add toggles to disable setting the clock as default?~~ No, separate pull if needed. - [x] Add tests - [x] Reformat, ensure adheres to conventions etc. - [x] 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! --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Cameron <[email protected]> Co-authored-by: Jerome Humbert <[email protected]>

Prevent windup with FixedTimeStep

b88a115

HackerFoo mentioned this pull request Aug 14, 2022

Handles to Assets leaking #4589

Open

HackerFoo changed the title ~~Prevent windup with FixedTimeStep~~ Prevent windup with FixedTimestep Aug 14, 2022

alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Aug 14, 2022

alice-i-cecile assigned maniwani Aug 14, 2022

alice-i-cecile requested a review from hymm August 14, 2022 22:30

alice-i-cecile unassigned maniwani Aug 14, 2022

alice-i-cecile requested a review from maniwani August 14, 2022 22:30

maniwani reviewed Aug 14, 2022

View reviewed changes

alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Sep 3, 2022

alice-i-cecile added the A-Time Involves time keeping and reporting label Sep 14, 2022

nakedible mentioned this pull request Jun 28, 2023

Unify FixedTime and Time while fixing several problems #8964

Merged

14 tasks

alice-i-cecile closed this in #8964 Oct 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent windup with FixedTimestep #5692

Prevent windup with FixedTimestep #5692

HackerFoo commented Aug 14, 2022 •

edited

Loading

alice-i-cecile commented Aug 14, 2022

maniwani left a comment •

edited

Loading

maniwani Aug 14, 2022 •

edited

Loading

HackerFoo Aug 14, 2022

maniwani Aug 14, 2022 •

edited

Loading

HackerFoo commented Aug 14, 2022

maniwani commented Aug 14, 2022 •

edited

Loading

Prevent windup with FixedTimestep #5692

Prevent windup with FixedTimestep #5692

Conversation

HackerFoo commented Aug 14, 2022 • edited Loading

Objective

Solution

Changelog

alice-i-cecile commented Aug 14, 2022

maniwani left a comment • edited Loading

Choose a reason for hiding this comment

maniwani Aug 14, 2022 • edited Loading

Choose a reason for hiding this comment

HackerFoo Aug 14, 2022

Choose a reason for hiding this comment

maniwani Aug 14, 2022 • edited Loading

Choose a reason for hiding this comment

HackerFoo commented Aug 14, 2022

maniwani commented Aug 14, 2022 • edited Loading

HackerFoo commented Aug 14, 2022 •

edited

Loading

maniwani left a comment •

edited

Loading

maniwani Aug 14, 2022 •

edited

Loading

maniwani Aug 14, 2022 •

edited

Loading

maniwani commented Aug 14, 2022 •

edited

Loading