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

Fix spinners potentially displaying incorrect SPM numbers #22302

Merged
merged 2 commits into from
Jan 21, 2023

Conversation

ekrctb
Copy link
Collaborator

@ekrctb ekrctb commented Jan 20, 2023

Fix #21221

@frenzibyte frenzibyte changed the title Fix spinners can show a huge negative SPM number Fix spinners potentially displaying incorrect SPM number Jan 21, 2023
@frenzibyte frenzibyte changed the title Fix spinners potentially displaying incorrect SPM number Fix spinners potentially displaying incorrect SPM numbers Jan 21, 2023
@frenzibyte frenzibyte self-requested a review January 21, 2023 11:59
@frenzibyte
Copy link
Member

frenzibyte commented Jan 21, 2023

It would be helpful to provide a description as to why the original check wasn't enough. But upon investigating this myself, it turns out that the bug is caused by the following scenario:

  • User seeks to end and finishes seeking
  • SPM calculator adds a rotation record of current time
  • User presses right arrow, which causes the game to actually seek backwards because the clock already passed gameplay time and the seek got clamped to gameplay time (last object's end time).
  • Elapsed frame time becomes negative, and the calculator bypasses the original elapsed != 0 check in master
  • SPM calculator adds rotation record of current time, which is actually equal to last record's time

We can't solve this by changing the orgiinal check to account for negative elapsed time, because that would break rewind. Therefore the change here to check on current time rather than elapsed looks sound.

Such scenario is very dependant on time, so a test scene doesn't look to be easily feasible.

@frenzibyte frenzibyte enabled auto-merge January 21, 2023 12:42
@frenzibyte frenzibyte merged commit 292fd34 into ppy:master Jan 21, 2023
@ekrctb ekrctb deleted the fix-infinity-spm branch January 22, 2023 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spinners can show a huge negative SPM number
2 participants