Skip to content

Commit

Permalink
Unify FixedTime and Time while fixing several problems (bevyengin…
Browse files Browse the repository at this point in the history
…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 <alice.i.cecile@gmail.com>
Co-authored-by: Cameron <51241057+maniwani@users.noreply.github.com>
Co-authored-by: Jerome Humbert <djeedai@gmail.com>
  • Loading branch information
4 people authored and ameknite committed Nov 6, 2023
1 parent d8b3dd9 commit 102bd52
Show file tree
Hide file tree
Showing 20 changed files with 1,586 additions and 855 deletions.
10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,16 @@ description = "Illustrates creating custom system parameters with `SystemParam`"
category = "ECS (Entity Component System)"
wasm = false

[[example]]
name = "time"
path = "examples/ecs/time.rs"

[package.metadata.example.time]
name = "Time handling"
description = "Explains how Time is handled in ECS"
category = "ECS (Entity Component System)"
wasm = false

[[example]]
name = "timers"
path = "examples/ecs/timers.rs"
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_diagnostic/src/frame_time_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{Diagnostic, DiagnosticId, Diagnostics, RegisterDiagnostic};
use bevy_app::prelude::*;
use bevy_core::FrameCount;
use bevy_ecs::prelude::*;
use bevy_time::Time;
use bevy_time::{Real, Time};

/// Adds "frame time" diagnostic to an App, specifically "frame time", "fps" and "frame count"
#[derive(Default)]
Expand Down Expand Up @@ -30,12 +30,12 @@ impl FrameTimeDiagnosticsPlugin {

pub fn diagnostic_system(
mut diagnostics: Diagnostics,
time: Res<Time>,
time: Res<Time<Real>>,
frame_count: Res<FrameCount>,
) {
diagnostics.add_measurement(Self::FRAME_COUNT, || frame_count.0 as f64);

let delta_seconds = time.raw_delta_seconds_f64();
let delta_seconds = time.delta_seconds_f64();
if delta_seconds == 0.0 {
return;
}
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_diagnostic/src/log_diagnostics_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::{Diagnostic, DiagnosticId, DiagnosticsStore};
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use bevy_log::{debug, info};
use bevy_time::{Time, Timer, TimerMode};
use bevy_time::{Real, Time, Timer, TimerMode};
use bevy_utils::Duration;

/// An App Plugin that logs diagnostics to the console
Expand Down Expand Up @@ -82,10 +82,10 @@ impl LogDiagnosticsPlugin {

fn log_diagnostics_system(
mut state: ResMut<LogDiagnosticsState>,
time: Res<Time>,
time: Res<Time<Real>>,
diagnostics: Res<DiagnosticsStore>,
) {
if state.timer.tick(time.raw_delta()).finished() {
if state.timer.tick(time.delta()).finished() {
if let Some(ref filter) = state.filter {
for diagnostic in filter.iter().flat_map(|id| {
diagnostics
Expand All @@ -107,10 +107,10 @@ impl LogDiagnosticsPlugin {

fn log_diagnostics_debug_system(
mut state: ResMut<LogDiagnosticsState>,
time: Res<Time>,
time: Res<Time<Real>>,
diagnostics: Res<DiagnosticsStore>,
) {
if state.timer.tick(time.raw_delta()).finished() {
if state.timer.tick(time.delta()).finished() {
if let Some(ref filter) = state.filter {
for diagnostic in filter.iter().flat_map(|id| {
diagnostics
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_gilrs/src/rumble.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bevy_ecs::{
};
use bevy_input::gamepad::{GamepadRumbleIntensity, GamepadRumbleRequest};
use bevy_log::{debug, warn};
use bevy_time::Time;
use bevy_time::{Real, Time};
use bevy_utils::{Duration, HashMap};
use gilrs::{
ff::{self, BaseEffect, BaseEffectType, Repeat, Replay},
Expand Down Expand Up @@ -120,12 +120,12 @@ fn handle_rumble_request(
Ok(())
}
pub(crate) fn play_gilrs_rumble(
time: Res<Time>,
time: Res<Time<Real>>,
mut gilrs: NonSendMut<Gilrs>,
mut requests: EventReader<GamepadRumbleRequest>,
mut running_rumbles: NonSendMut<RunningRumbleEffects>,
) {
let current_time = time.raw_elapsed();
let current_time = time.elapsed();
// Remove outdated rumble effects.
for rumbles in running_rumbles.rumbles.values_mut() {
// `ff::Effect` uses RAII, dropping = deactivating
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn extract_frame_count(mut commands: Commands, frame_count: Extract<Res<FrameCou
}

fn extract_time(mut commands: Commands, time: Extract<Res<Time>>) {
commands.insert_resource(time.clone());
commands.insert_resource(**time);
}

/// Contains global values useful when writing shaders.
Expand Down
42 changes: 2 additions & 40 deletions crates/bevy_time/src/common_conditions.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use crate::{fixed_timestep::FixedTime, Time, Timer, TimerMode};
use crate::{Time, Timer, TimerMode};
use bevy_ecs::system::Res;
use bevy_utils::Duration;

/// Run condition that is active on a regular time interval, using [`Time`] to advance
/// the timer.
///
/// If used for a fixed timestep system, use [`on_fixed_timer`] instead.
///
/// ```rust,no_run
/// # use bevy_app::{App, NoopPluginGroup as DefaultPlugins, PluginGroup, Update};
/// # use bevy_ecs::schedule::IntoSystemConfigs;
Expand Down Expand Up @@ -40,40 +38,6 @@ pub fn on_timer(duration: Duration) -> impl FnMut(Res<Time>) -> bool + Clone {
}
}

/// Run condition that is active on a regular time interval, using [`FixedTime`] to
/// advance the timer.
///
/// If used for a non-fixed timestep system, use [`on_timer`] instead.
///
/// ```rust,no_run
/// # use bevy_app::{App, NoopPluginGroup as DefaultPlugins, PluginGroup, FixedUpdate};
/// # use bevy_ecs::schedule::IntoSystemConfigs;
/// # use bevy_utils::Duration;
/// # use bevy_time::common_conditions::on_fixed_timer;
/// fn main() {
/// App::new()
/// .add_plugins(DefaultPlugins)
/// .add_systems(FixedUpdate,
/// tick.run_if(on_fixed_timer(Duration::from_secs(1))),
/// )
/// .run();
/// }
/// fn tick() {
/// // ran once a second
/// }
/// ```
///
/// Note that this run condition may not behave as expected if `duration` is smaller
/// than the fixed timestep period, since the timer may complete multiple times in
/// one fixed update.
pub fn on_fixed_timer(duration: Duration) -> impl FnMut(Res<FixedTime>) -> bool + Clone {
let mut timer = Timer::new(duration, TimerMode::Repeating);
move |time: Res<FixedTime>| {
timer.tick(time.period);
timer.just_finished()
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -85,9 +49,7 @@ mod tests {
#[test]
fn distributive_run_if_compiles() {
Schedule::default().add_systems(
(test_system, test_system)
.distributive_run_if(on_timer(Duration::new(1, 0)))
.distributive_run_if(on_fixed_timer(Duration::new(1, 0))),
(test_system, test_system).distributive_run_if(on_timer(Duration::new(1, 0))),
);
}
}
Loading

0 comments on commit 102bd52

Please sign in to comment.