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

Change frame stable catch-up method to allow for much faster sync #26600

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 17, 2024

Especially noticeable on single-threaded mode (hello macOS).

Before:

2024-01-18.02.24.19.mp4

After:

2024-01-18.02.23.19.mp4

Closes #5119.

private void createStabilityContainer(double gameplayStartTime = double.MinValue) => AddStep("create container", () =>
mainContainer.Child = new FrameStabilityContainer(gameplayStartTime) { MaxCatchUpFrames = max_frames_catchup }
mainContainer.Child = new FrameStabilityContainer(gameplayStartTime)
Copy link
Member Author

Choose a reason for hiding this comment

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

I stress tested both tests which had custom overrides and they still pass. For whatever that's worth.

{
MaxCatchUpFrames = 1
}
Child = frameStabilityContainer = new FrameStabilityContainer()
Copy link
Member Author

Choose a reason for hiding this comment

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

We may want to look at exactly why this was limiting to one frame just to be certain though.

Copy link
Member Author

Choose a reason for hiding this comment

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

5ead85f

This was just done for visual testing purposes (to actually show the game catching up in the test scene). I've added back a Thread.Sleep to make this still look correct.

/// </summary>
public int MaxCatchUpFrames { get; set; } = 5;
private const double max_catchup_milliseconds = 10;
Copy link
Member Author

Choose a reason for hiding this comment

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

This value is just a ballpark good place to be. The game should always maintain at least 60 fps, and this will give enough headroom to allow that.

@Joehuu
Copy link
Member

Joehuu commented Jan 18, 2024

Probably closes #5119?

@peppy
Copy link
Member Author

peppy commented Jan 18, 2024

Probably closes #5119?

Yeah maybe. There's still more I want to do, but it's a good improvement.

@peppy peppy force-pushed the frame-stable-catchup-zoom branch from 5295eb6 to fb4efd9 Compare January 18, 2024 05:01
@bdach bdach self-requested a review January 22, 2024 09:39
@bdach bdach merged commit c8053a8 into ppy:master Jan 22, 2024
15 of 17 checks passed
@peppy peppy deleted the frame-stable-catchup-zoom 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.

Scrubbing Autoplay is not very good
3 participants