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 (proof-of-concept) #8930

Closed

Conversation

nakedible
Copy link
Contributor

@nakedible nakedible commented Jun 22, 2023

PULL REQUEST ONLY FOR DISCUSSION, NOT MERGE READY BY FAR.

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.
  • If you're fixing a specific issue, say "Fixes #X".

Solution

  • Add a maximum delta time that can limits how much can be added to the virtual game clock by a single frame update. This also limits how many fixed update periods are run per frame.
  • Make Time report "fixed step clock" time while inside FixedUpdate.

...

  • Add maximum_delta to Time that limits how much the virtual game clock can be advanced with a single update. Does not affect raw clock.
  • Move FixedTime::period to be Time::fixed_period and remove FixedTime structure entirely.
  • Make Time::delta, Time::elapsed and friends report time according to fixed steps inside FixedUpdate.
  • Implementation detail: Separate out Clock data structure to make it more obvious what the independent tracked values are and to reduce code duplication.

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

  • What changed as a result of this PR?
  • If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
  • Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section
    • If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

  • If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
  • Simply adding new functionality is not a breaking change.
  • Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Time Involves time keeping and reporting labels Jun 22, 2023
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

3 similar comments
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@nakedible
Copy link
Contributor Author

Closing in favor of #8964

@nakedible nakedible closed this Jun 28, 2023
@B-head B-head mentioned this pull request Jul 4, 2023
8 tasks
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-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants