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

Unify FixedTime and Time (ver. B) (proof-of-concept) #8946

Closed
wants to merge 6 commits into from

Conversation

maniwani
Copy link
Contributor

@maniwani maniwani commented Jun 24, 2023

This is an alternative to #8930 (pretty similar though).

(credit to @devil-ira for suggesting this idea to me months ago)

As a quick background, both Update and FixedUpdate follow "virtual" time (time that can be sped up, slowed down, and paused) but each needs its own clock to measure it because they advance at different rates.

Objective

I wrote this PR to make the Time API usable in FixedUpdate, with the goal of making plugin systems more portable. If Time is setup so the clock is correct in both Update and FixedUpdate, then users can write and import systems that access Res<Time> and they'll always work no matter which scheduled they're added to.

This would take us off a path that leads to some really annoying compatibility issues that plague third-party code in other engines (e.g. store assets for Unity) because their logic can't be freely moved between their Update and FixedUpdate equivalents.

Some other issues:

  • If running FixedUpdate takes longer than the fixed timestep, the app can enter a "death spiral" and ultimately freeze as each frame sees an ever-increasing queue of FixedUpdate steps and never catches up.
  • If there is a particularly long gap between app updates (e.g. the program was suspended), time will jump1 forward a lot all at once, which translates into a huge FixedUpdate simulation debt. The app will seem to freeze until it catches up.

Solution

  • Split common functionality into a Clock data structure to reduce code duplication.
  • Make Time report "fixed time" while running FixedUpdate.
  • Add a setting to directly limit the number of FixedUpdate steps per frame. This will spread "paying off a large simulation debt" over multiple frames.
  • Add a setting to set a maximum delta time per frame. This will limit the impact of program suspends (by indirectly limiting how many FixedUpdate steps can build up between two updates).

I moved the "real time" (same speed as Instant) clock into a separate RealTime resource. Maybe having all three clocks crammed inside Time would have been better, but (1) I didn't like the look of having clock-specific methods (e.g. time.delta(), time.raw_delta(), time.fixed_delta()) and (2) "real time" is pretty meaningless in FixedUpdate. More breaking changes though.

I've also kept a FixedTimestep struct because keeping the step size and accumulator separate from the clock is much cleaner IMO and leaves room for, say, a networking plugin to sub in its own handling (without requiring more API from us).

Migration Guide

  • Use FixedTimestep and Time instead of hard-coded constants.
  • Replace FixedTime::period with Time::delta.
  • Replace other FixedTime methods with FixedTimestep equivalents.
  • on_fixed_timer has been removed. Replace it with on_timer as Time now works correctly in both schedules.

Footnotes

  1. On some platforms.

@alice-i-cecile alice-i-cecile added C-Usability A simple quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR A-Time Involves time keeping and reporting labels Jun 25, 2023
@B-head B-head mentioned this pull request Jul 4, 2023
8 tasks
@maniwani
Copy link
Contributor Author

Closing in favor of #8964.

@maniwani maniwani closed this Jul 20, 2023
@maniwani maniwani deleted the time_api_rework branch October 21, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Time Involves time keeping and reporting C-Usability A simple quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants