osu
osu copied to clipboard
Skip song intro on quick restart
Closes #5745
~~Also to note: In case of a map restart I reduced the duration it takes for the PlayerLoader. I prefer having a shorter wait time on each map restart but that is just my personal preference so I'm not sure if this should be changed at all.~~
The song intro gets skipped if the user has requested a restart via the quick retry hotkey.
Before this PR:
https://user-images.githubusercontent.com/34138240/183213704-9d783a7d-7e8f-4d51-a70b-9a85890e76a5.mp4
~~After this PR (without the reduced wait time commit):~~
https://user-images.githubusercontent.com/34138240/183214156-b469b603-2f30-4d1a-b891-4e12e2aaf2c3.mp4
~~After this PR (including the reduced wait time commit):~~
https://user-images.githubusercontent.com/34138240/183214276-d8814df6-4991-47db-84f8-eb6557e2b0fe.mp4
After this PR:
https://user-images.githubusercontent.com/34138240/183254939-fdae75c1-7d3f-4fb8-8930-ce668120c9ea.mp4
I don’t quite understand. This completely breaks multiple user interaction flows. How does a use change settings? How does a user restart without skipping if they want to have the intro time?
I guess this should be like a specific key binding instead of game-wide function
I think this should only apply to quick restart, like it is in stable
I don't know, but if this were to be implemented, there should be a toggle to enable it. The toggle could be under the gameplay tab in the options menu, somewhere near the fade playfield to red option.
It also looks bad and I wouldn't see it getting merged in the current state (the player loading screen wasn't made to show for only 100ms).
Retry feeling slow would be fixed when https://github.com/ppy/osu/issues/9039 gets implemented. This PR should only change skip behaviour
Okay I did what @LittleEndu and @uzervlad suggested. This isn't breaking user interaction/UX. Are there any other issues?
This PR requires test coverage. Please add tests.
I don't think I agree with this behaviour in principle. Is it just me?
The auto-skip as a whole? It's a stable behaviour which people expect to have available.
I'm fine with having it as a separate hotkey or similar, because I agree that not all users would want this. Open to ideas.
Hmm, maybe I've played stable all these years and never noticed that was the case. If so I guess I can't complain.
Could quick retrying just get rid of the whole player loading screen as a whole and just start at the start of the song just like stable? It doesn't really feel like a quick retry when it still takes a while to get back into the song.
Yeah, that's something we've had in mind for a while.
Had to rewrite every line (sorry).
@BlauFx please check my commits to see how I'd expect this to have been implemented. A couple of notes:
- Your tests didn't actually test anything
- You were using bindables in places where bindable flow was not required
- As mentioned in my "i don't know what's going on" comment, the flow of transferring state between
Player
andPlayerLoder
was weird. See how I've done it, hopefully it feels better.
Will require a fresh review from someone else