Fix track restarting when trying to switch track change direction with shuffle active #30215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bdach
added
area:overlay-now-playing
next release
|
Speak of the devil, one of the failing tests is new and it's failing intermittently and I have zero idea where to even begin. I love this. It was supposed to just be a smoke test against a hard crash so I could remove the assertions I guess but I kinda don't want to? |
peppy
approved these changes
Oct 11, 2024
|
Hmm, only just saw the flaky test after hitting button.. I guess we can see how often it happens and adjust? |
|
Yeah I'll handle the test if it acts out. |
bdach
added a commit
to bdach/osu
that referenced
this pull request
Oct 14, 2024
See ppy#30215 (comment) for context. Turns out the test failures were more correct than I'd thought. The long-and-short of it is that both in "pure random" mode and in "permutation" mode, when running out of track history to fall back on, it was possible for the random algorithm to pick the same song twice in a row - which is probably not desired, and which this explicit exclude should make impossible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #30190.
Now with more testing. I previously didn't do tests because they were annoying to do (see "ensure nonzero track duration" steps in the tests), which was a costly mistake.
Recommend viewing this diff without whitespace.