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

Keep editor in frame stable mode when possible #26382

Merged
merged 3 commits into from Jan 9, 2024

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Jan 4, 2024

This is intended to be a catch-all fix for frame stable related issues in the editor.

This works around #22353 (renamed issue to cover the remaining portion better), and could help #10455 (but doesn't really in practice because the lenience is too small.. spinners just need fixes locally).

I'm open to adjusting the lenience from 1000 if a better number is arrived on. Going too high causes frame stability to remain on during smooth seeks, though.

@bdach
Copy link
Collaborator

bdach commented Jan 8, 2024

Dunno what to think of this. I don't see this fixing any of the actual issues mentioned, and with one second allowance only for frame-stable seeks this feels like a bandaid that doesn't even do that much. So I'm not exactly convinced this fixes enough to be merged even as a stop-gap.

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 8, 2024

This 100% fixes the sample playback. If it's not deemed a good resolution for that, it'll be back to the drawing board to figure a better fix (probably a rewrite of auto generator, which is much higher effort).

@bdach
Copy link
Collaborator

bdach commented Jan 8, 2024

This 100% fixes the sample playback

I will admit that I misread the OP and what this was intending to fix due to the issue rename but like... the test scene here also doesn't play any samples either? With the fix or without?

I can actually tell in game now that this does something, though. Still not entirely sure why this breakage is worth fixing and other breakage from lack of frame-stability-always isn't. The biggest thing this pull has going for it is that it seems relatively unlikely for things to start growing around it and making it hard or impossible to tear it out (which we presumably will try to do, to fix every other issue that stems from no frame stable clock), but it still is feeling relatively lousy, even for a "temporary" fix.

I dunno if you think it's important I'll yield but I think time would be better spent on just rethinking how the editor should work at some stage later on.

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.

tentative approve, merge if you think it's best

@peppy
Copy link
Sponsor Member Author

peppy commented Jan 9, 2024

Based on the number of times the linked issue this will workaround has been brought up, I wanted to get a tentative fix out. As you say, this is pretty local and can be undone when we come up with a better solution, so let's get things in for now.

@peppy peppy merged commit 6f8a3e1 into ppy:master Jan 9, 2024
17 checks passed
@peppy peppy deleted the editor-more-frame-stable branch January 12, 2024 19:50
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.

None yet

2 participants