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

Simplify gameplay pause sequence #26605

Merged
merged 3 commits into from
Jan 22, 2024
Merged

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 18, 2024

This removes the frequency ramp on pausing. This was something I was personally behind implementing and working to keep around, but nuking in favour of simplifying the process for now. In discussing with nekodex we both agree there is a better way forward than this.

This lays groundwork for doing other things, like allowing seeking during replay pause, and changing the way offsets work without worrying about the weird application considerations from the pause ramp effect.

Closes #26442, closes #26596 and probably other issues.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 18, 2024
@peppy
Copy link
Member Author

peppy commented Jan 18, 2024

I forgot about the one failing test which was going to be very hard to explain. I've basically disabled the test which checks for audio time not incrementing between a pause and resume (c4e9bcd).

This is part of what this PR aims to "fix".

Until now, we have gone out of our way to seek the track (at a bass level) back to the precise point at which the game was paused. This was very relevant when there was a frequency ramp that could mean the user misses some gameplay while the ramp occurs.

With this gone, the delay in which a pause takes is negligible (basically how long BASS takes to do the pause). Accounting for this is in theory still possible, by restoring some of the logic removed in this PR, but it gets us back into a scenario of "game can go backwards while user is playing", which I'm hoping to avoid for simplicity.

The hope is that in normal user gameplay, time does not go backwards ever. Even if that means that the user misses 1-10ms due to a pause operation taking a moment. Why? because if we don't, we end up with completely random issues like #26442, or replay frames which aren't always in forward order. And I don't want to have to deal with that.

This is about simplifying things so we can get gameplay in a good state, and everything we were doing here was unnecessary complexity for a bit of flair.

@bdach bdach self-requested a review January 22, 2024 07:21
@bdach
Copy link
Collaborator

bdach commented Jan 22, 2024

I can't see how this closes #26442 because I can't reproduce the frequency ramp "skipping back" in any way. And I've tried everything mentioned in the issue thread (messing with universal offset, messing with platform offset, messing with ramp duration).

@peppy
Copy link
Member Author

peppy commented Jan 22, 2024

You're definitely right about that, I can't seem to reproduce either. It looks like FrameStabilityContainer immediately gets the message to stop and therefore is not affected by the subsequent backwards seek.

That said, I still feel like this will resolve the issue and there is some way a user is seeing time skip backwards. My hypothesis is that bass is taking some time to perform the seek on unpausing and FrameStabilityContainer plays some forward then skips backwards.

We can untag that issue and wait for user feedback if that works better. Or maybe we can simulate this by performing a delayed BASS seek operation.

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with going with what's here for now as it removes much complication for once.

@bdach bdach merged commit 493d495 into ppy:master Jan 22, 2024
15 of 17 checks passed
@peppy peppy deleted the simplify-gameplay-pause branch January 25, 2024 12:45
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.

ColourHitErrorMeter resets after a pause Pausing gameplay can cause phantom misses
2 participants