diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 4454501a968a..32009dc8c240 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -79,5 +79,114 @@ public void TestMusicNavigationActions() trackChangeQueue.Peek().changeDirection == TrackChangeDirection.Next); AddAssert("track actually changed", () => !trackChangeQueue.First().working.BeatmapInfo.Equals(trackChangeQueue.Last().working.BeatmapInfo)); } + + [Test] + public void TestShuffleBackwards() + { + Queue<(IWorkingBeatmap working, TrackChangeDirection changeDirection)> trackChangeQueue = null!; + + AddStep("enable shuffle", () => Game.MusicController.Shuffle.Value = true); + + // ensure we have at least two beatmaps available to identify the direction the music controller navigated to. + AddRepeatStep("import beatmap", () => Game.BeatmapManager.Import(TestResources.CreateTestBeatmapSetInfo()), 5); + AddStep("ensure nonzero track duration", () => Game.Realm.Write(r => + { + // this was already supposed to be non-zero (see innards of `TestResources.CreateTestBeatmapSetInfo()`), + // but the non-zero value is being overwritten *to* zero by `BeatmapUpdater`. + // do a bit of a hack to change it back again - otherwise tracks are going to switch instantly and we won't be able to assert anything sane anymore. + foreach (var beatmap in r.All().Where(b => b.Length == 0)) + beatmap.Length = 60_000; + })); + + AddStep("bind to track change", () => + { + trackChangeQueue = new Queue<(IWorkingBeatmap, TrackChangeDirection)>(); + Game.MusicController.TrackChanged += (working, changeDirection) => trackChangeQueue.Enqueue((working, changeDirection)); + }); + + AddStep("seek track to 6 second", () => Game.MusicController.SeekTo(6000)); + AddUntilStep("wait for current time to update", () => Game.MusicController.CurrentTrack.CurrentTime > 5000); + + AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev)); + AddAssert("no track change", () => trackChangeQueue.Count == 0); + AddUntilStep("track restarted", () => Game.MusicController.CurrentTrack.CurrentTime < 5000); + + AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev)); + AddUntilStep("track changed", () => trackChangeQueue.Count == 1); + + AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev)); + AddUntilStep("track changed", () => trackChangeQueue.Count == 2); + + AddStep("press next", () => globalActionContainer.TriggerPressed(GlobalAction.MusicNext)); + AddUntilStep("track changed", () => + trackChangeQueue.Count == 3 && !trackChangeQueue.ElementAt(1).working.BeatmapInfo.Equals(trackChangeQueue.Last().working.BeatmapInfo)); + } + + [Test] + public void TestShuffleForwards() + { + Queue<(IWorkingBeatmap working, TrackChangeDirection changeDirection)> trackChangeQueue = null!; + + AddStep("enable shuffle", () => Game.MusicController.Shuffle.Value = true); + + // ensure we have at least two beatmaps available to identify the direction the music controller navigated to. + AddRepeatStep("import beatmap", () => Game.BeatmapManager.Import(TestResources.CreateTestBeatmapSetInfo()), 5); + AddStep("ensure nonzero track duration", () => Game.Realm.Write(r => + { + // this was already supposed to be non-zero (see innards of `TestResources.CreateTestBeatmapSetInfo()`), + // but the non-zero value is being overwritten *to* zero by `BeatmapUpdater`. + // do a bit of a hack to change it back again - otherwise tracks are going to switch instantly and we won't be able to assert anything sane anymore. + foreach (var beatmap in r.All().Where(b => b.Length == 0)) + beatmap.Length = 60_000; + })); + + AddStep("bind to track change", () => + { + trackChangeQueue = new Queue<(IWorkingBeatmap, TrackChangeDirection)>(); + Game.MusicController.TrackChanged += (working, changeDirection) => trackChangeQueue.Enqueue((working, changeDirection)); + }); + + AddStep("press next", () => globalActionContainer.TriggerPressed(GlobalAction.MusicNext)); + AddUntilStep("track changed", () => trackChangeQueue.Count == 1); + + AddStep("press next", () => globalActionContainer.TriggerPressed(GlobalAction.MusicNext)); + AddUntilStep("track changed", () => trackChangeQueue.Count == 2); + + AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev)); + AddUntilStep("track changed", () => + trackChangeQueue.Count == 3 && !trackChangeQueue.ElementAt(1).working.BeatmapInfo.Equals(trackChangeQueue.Last().working.BeatmapInfo)); + } + + [Test] + public void TestShuffleBackAndForth() + { + Queue<(IWorkingBeatmap working, TrackChangeDirection changeDirection)> trackChangeQueue = null!; + + AddStep("enable shuffle", () => Game.MusicController.Shuffle.Value = true); + + // ensure we have at least two beatmaps available to identify the direction the music controller navigated to. + AddRepeatStep("import beatmap", () => Game.BeatmapManager.Import(TestResources.CreateTestBeatmapSetInfo()), 5); + AddStep("ensure nonzero track duration", () => Game.Realm.Write(r => + { + // this was already supposed to be non-zero (see innards of `TestResources.CreateTestBeatmapSetInfo()`), + // but the non-zero value is being overwritten *to* zero by `BeatmapUpdater`. + // do a bit of a hack to change it back again - otherwise tracks are going to switch instantly and we won't be able to assert anything sane anymore. + foreach (var beatmap in r.All().Where(b => b.Length == 0)) + beatmap.Length = 60_000; + })); + + AddStep("bind to track change", () => + { + trackChangeQueue = new Queue<(IWorkingBeatmap, TrackChangeDirection)>(); + Game.MusicController.TrackChanged += (working, changeDirection) => trackChangeQueue.Enqueue((working, changeDirection)); + }); + + AddStep("press next", () => globalActionContainer.TriggerPressed(GlobalAction.MusicNext)); + AddUntilStep("track changed", () => trackChangeQueue.Count == 1); + + AddStep("press previous", () => globalActionContainer.TriggerPressed(GlobalAction.MusicPrev)); + AddUntilStep("track changed", () => + trackChangeQueue.Count == 2 && !trackChangeQueue.First().working.BeatmapInfo.Equals(trackChangeQueue.Last().working.BeatmapInfo)); + } } } diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 971503ca8b66..a269dbdf4fbb 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -74,6 +74,7 @@ public partial class MusicController : CompositeDrawable private readonly Bindable randomSelectAlgorithm = new Bindable(); private readonly List> previousRandomSets = new List>(); private int randomHistoryDirection; + private int lastRandomTrackDirection; [BackgroundDependencyLoader] private void load(AudioManager audio, OsuConfigManager configManager) @@ -371,54 +372,72 @@ private bool next(bool allowProtectedTracks) private Live? getNextRandom(int direction, bool allowProtectedTracks) { - Live result; - - var possibleSets = getBeatmapSets().Where(s => !s.Value.Protected || allowProtectedTracks).ToArray(); + try + { + Live result; - if (possibleSets.Length == 0) - return null; + var possibleSets = getBeatmapSets().Where(s => !s.Value.Protected || allowProtectedTracks).ToArray(); - // condition below checks if the signs of `randomHistoryDirection` and `direction` are opposite and not zero. - // if that is the case, it means that the user had previously chosen next track `randomHistoryDirection` times and wants to go back, - // or that the user had previously chosen previous track `randomHistoryDirection` times and wants to go forward. - // in both cases, it means that we have a history of previous random selections that we can rewind. - if (randomHistoryDirection * direction < 0) - { - Debug.Assert(Math.Abs(randomHistoryDirection) == previousRandomSets.Count); - result = previousRandomSets[^1]; - previousRandomSets.RemoveAt(previousRandomSets.Count - 1); - randomHistoryDirection += direction; - return result; - } + if (possibleSets.Length == 0) + return null; - // if the early-return above didn't cover it, it means that we have no history to fall back on - // and need to actually choose something random. - switch (randomSelectAlgorithm.Value) - { - case RandomSelectAlgorithm.Random: - result = possibleSets[RNG.Next(possibleSets.Length)]; - break; + // condition below checks if the signs of `randomHistoryDirection` and `direction` are opposite and not zero. + // if that is the case, it means that the user had previously chosen next track `randomHistoryDirection` times and wants to go back, + // or that the user had previously chosen previous track `randomHistoryDirection` times and wants to go forward. + // in both cases, it means that we have a history of previous random selections that we can rewind. + if (randomHistoryDirection * direction < 0) + { + Debug.Assert(Math.Abs(randomHistoryDirection) == previousRandomSets.Count); - case RandomSelectAlgorithm.RandomPermutation: - var notYetPlayedSets = possibleSets.Except(previousRandomSets).ToArray(); + // if the user has been shuffling backwards and now going forwards (or vice versa), + // the topmost item from history needs to be discarded because it's the *current* track. + if (direction * lastRandomTrackDirection < 0) + { + previousRandomSets.RemoveAt(previousRandomSets.Count - 1); + randomHistoryDirection += direction; + } - if (notYetPlayedSets.Length == 0) + if (previousRandomSets.Count > 0) { - notYetPlayedSets = possibleSets; - previousRandomSets.Clear(); - randomHistoryDirection = 0; + result = previousRandomSets[^1]; + previousRandomSets.RemoveAt(previousRandomSets.Count - 1); + return result; } + } - result = notYetPlayedSets[RNG.Next(notYetPlayedSets.Length)]; - break; + // if the early-return above didn't cover it, it means that we have no history to fall back on + // and need to actually choose something random. + switch (randomSelectAlgorithm.Value) + { + case RandomSelectAlgorithm.Random: + result = possibleSets[RNG.Next(possibleSets.Length)]; + break; - default: - throw new ArgumentOutOfRangeException(nameof(randomSelectAlgorithm), randomSelectAlgorithm.Value, "Unsupported random select algorithm"); - } + case RandomSelectAlgorithm.RandomPermutation: + var notYetPlayedSets = possibleSets.Except(previousRandomSets).ToArray(); + + if (notYetPlayedSets.Length == 0) + { + notYetPlayedSets = possibleSets; + previousRandomSets.Clear(); + randomHistoryDirection = 0; + } + + result = notYetPlayedSets[RNG.Next(notYetPlayedSets.Length)]; + break; + + default: + throw new ArgumentOutOfRangeException(nameof(randomSelectAlgorithm), randomSelectAlgorithm.Value, "Unsupported random select algorithm"); + } - previousRandomSets.Add(result); - randomHistoryDirection += direction; - return result; + previousRandomSets.Add(result); + return result; + } + finally + { + randomHistoryDirection += direction; + lastRandomTrackDirection = direction; + } } private void restartTrack()