From 12bd516a570191cbb31d271082530eafca1206de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 18 Sep 2024 13:51:45 +0200 Subject: [PATCH 1/4] Shuffle playback order in global playlist by default RFC. Closes https://github.com/ppy/osu/issues/18169. Implements the given proposal of keeping the current stable order but adding a shuffle facility to the now playing overlay, and enabling it by default. There are more changes I want to make here but I'd like this to get discussion first, because I am likely to continue putting this sort of selection logic into `MusicController` and I just want to confirm nobody is going to have a problem with that. In particular this is not sharing the randomisation implementation with beatmap carousel because it doesn't generalise nicely (song select cares about the particular *beatmap difficulties* selected to rewind properly, while the music controller only cares about picking a *beatmap set*). --- .../Menus/TestSceneMusicActionHandling.cs | 2 + osu.Game/Overlays/MusicController.cs | 83 ++++++++++++++++++- osu.Game/Overlays/NowPlayingOverlay.cs | 12 +++ 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs index 03b3b94bd874..4454501a968a 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneMusicActionHandling.cs @@ -34,6 +34,8 @@ public void TestMusicNavigationActions() { Queue<(IWorkingBeatmap working, TrackChangeDirection changeDirection)> trackChangeQueue = null!; + AddStep("disable shuffle", () => Game.MusicController.Shuffle.Value = false); + // 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); diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 63efdd5381b6..a7bca536df29 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -13,8 +14,10 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Logging; using osu.Framework.Threading; +using osu.Framework.Utils; using osu.Game.Audio.Effects; using osu.Game.Beatmaps; +using osu.Game.Configuration; using osu.Game.Database; using osu.Game.Rulesets.Mods; @@ -43,6 +46,8 @@ public partial class MusicController : CompositeDrawable /// public readonly BindableBool AllowTrackControl = new BindableBool(true); + public readonly BindableBool Shuffle = new BindableBool(true); + /// /// Fired when the global has changed. /// Includes direction information for display purposes. @@ -66,12 +71,18 @@ public partial class MusicController : CompositeDrawable private AudioFilter audioDuckFilter = null!; + private readonly Bindable randomSelectAlgorithm = new Bindable(); + private readonly List previousRandomSets = new List(); + private int randomHistoryDirection; + [BackgroundDependencyLoader] - private void load(AudioManager audio) + private void load(AudioManager audio, OsuConfigManager configManager) { AddInternal(audioDuckFilter = new AudioFilter(audio.TrackMixer)); audio.Tracks.AddAdjustment(AdjustableProperty.Volume, audioDuckVolume); sampleVolume = audio.VolumeSample.GetBoundCopy(); + + configManager.BindWith(OsuSetting.RandomSelectAlgorithm, randomSelectAlgorithm); } protected override void LoadComplete() @@ -238,8 +249,15 @@ private PreviousTrackResult prev(bool allowProtectedTracks) queuedDirection = TrackChangeDirection.Prev; - var playableSet = getBeatmapSets().AsEnumerable().TakeWhile(i => !i.Equals(current?.BeatmapSetInfo)).LastOrDefault(s => !s.Protected || allowProtectedTracks) + BeatmapSetInfo? playableSet; + + if (Shuffle.Value) + playableSet = getNextRandom(-1, allowProtectedTracks); + else + { + playableSet = getBeatmapSets().AsEnumerable().TakeWhile(i => !i.Equals(current?.BeatmapSetInfo)).LastOrDefault(s => !s.Protected || allowProtectedTracks) ?? getBeatmapSets().AsEnumerable().LastOrDefault(s => !s.Protected || allowProtectedTracks); + } if (playableSet != null) { @@ -327,8 +345,15 @@ private bool next(bool allowProtectedTracks) queuedDirection = TrackChangeDirection.Next; - var playableSet = getBeatmapSets().AsEnumerable().SkipWhile(i => !i.Equals(current?.BeatmapSetInfo) || (i.Protected && !allowProtectedTracks)).ElementAtOrDefault(1) + BeatmapSetInfo? playableSet; + + if (Shuffle.Value) + playableSet = getNextRandom(1, allowProtectedTracks); + else + { + playableSet = getBeatmapSets().AsEnumerable().SkipWhile(i => !i.Equals(current?.BeatmapSetInfo) || (i.Protected && !allowProtectedTracks)).ElementAtOrDefault(1) ?? getBeatmapSets().AsEnumerable().FirstOrDefault(i => !i.Protected || allowProtectedTracks); + } var playableBeatmap = playableSet?.Beatmaps.FirstOrDefault(); @@ -342,6 +367,58 @@ private bool next(bool allowProtectedTracks) return false; } + private BeatmapSetInfo? getNextRandom(int direction, bool allowProtectedTracks) + { + BeatmapSetInfo result; + + var possibleSets = getBeatmapSets().AsEnumerable().Where(s => !s.Protected || allowProtectedTracks).ToArray(); + + if (possibleSets.Length == 0) + return null; + + // 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 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; + + 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; + } + private void restartTrack() { // if not scheduled, the previously track will be stopped one frame later (see ScheduleAfterChildren logic in GameBase). diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index 76c8c237d581..bcd15a2b7eba 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -47,6 +47,7 @@ public partial class NowPlayingOverlay : OsuFocusedOverlayContainer, INamedOverl private IconButton prevButton = null!; private IconButton playButton = null!; private IconButton nextButton = null!; + private MusicIconButton shuffleButton = null!; private IconButton playlistButton = null!; private ScrollingTextContainer title = null!, artist = null!; @@ -69,6 +70,7 @@ public partial class NowPlayingOverlay : OsuFocusedOverlayContainer, INamedOverl private OsuColour colours { get; set; } = null!; private Bindable allowTrackControl = null!; + private BindableBool shuffle = new BindableBool(true); public NowPlayingOverlay() { @@ -162,6 +164,13 @@ private void load() Action = () => musicController.NextTrack(), Icon = FontAwesome.Solid.StepForward, }, + shuffleButton = new MusicIconButton + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Action = shuffle.Toggle, + Icon = FontAwesome.Solid.Random, + } } }, playlistButton = new MusicIconButton @@ -227,6 +236,9 @@ protected override void LoadComplete() allowTrackControl = musicController.AllowTrackControl.GetBoundCopy(); allowTrackControl.BindValueChanged(_ => Scheduler.AddOnce(updateEnabledStates), true); + shuffle.BindTo(musicController.Shuffle); + shuffle.BindValueChanged(s => shuffleButton.FadeColour(s.NewValue ? colours.Yellow : Color4.White, 200, Easing.OutQuint), true); + musicController.TrackChanged += trackChanged; trackChanged(beatmap.Value); } From 7f52ae883723a684f84f162b6a71b1f8e6dcdcea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 18 Sep 2024 15:30:56 +0200 Subject: [PATCH 2/4] Fix code quality inspection --- osu.Game/Overlays/NowPlayingOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index bcd15a2b7eba..e1e5aa942696 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -70,7 +70,7 @@ public partial class NowPlayingOverlay : OsuFocusedOverlayContainer, INamedOverl private OsuColour colours { get; set; } = null!; private Bindable allowTrackControl = null!; - private BindableBool shuffle = new BindableBool(true); + private readonly BindableBool shuffle = new BindableBool(true); public NowPlayingOverlay() { From a2d9302f4ad472b23c102794a43a4cc9879aa5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Oct 2024 10:12:38 +0200 Subject: [PATCH 3/4] Move shuffle button to left side --- osu.Game/Overlays/NowPlayingOverlay.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game/Overlays/NowPlayingOverlay.cs b/osu.Game/Overlays/NowPlayingOverlay.cs index e1e5aa942696..f4da9a92dcfa 100644 --- a/osu.Game/Overlays/NowPlayingOverlay.cs +++ b/osu.Game/Overlays/NowPlayingOverlay.cs @@ -164,15 +164,16 @@ private void load() Action = () => musicController.NextTrack(), Icon = FontAwesome.Solid.StepForward, }, - shuffleButton = new MusicIconButton - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Action = shuffle.Toggle, - Icon = FontAwesome.Solid.Random, - } } }, + shuffleButton = new MusicIconButton + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.Centre, + Position = new Vector2(bottom_black_area_height / 2, 0), + Action = shuffle.Toggle, + Icon = FontAwesome.Solid.Random, + }, playlistButton = new MusicIconButton { Origin = Anchor.Centre, From 2a214f7c9fb82de43c0b36ef4aab3d4cea301dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Oct 2024 10:19:59 +0200 Subject: [PATCH 4/4] Fix incorrect implementation of next track choice `SkipWhile()` in this context does not correctly ensure that `ElementAtOrDefault(1)` is not a protected track. An explicit `Where()` does. Spotted accidentally when I noticed that skipping to next track can select a protected track, but skipping to previous cannot. --- osu.Game/Overlays/MusicController.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index a7bca536df29..600c014a95af 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -351,7 +351,9 @@ private bool next(bool allowProtectedTracks) playableSet = getNextRandom(1, allowProtectedTracks); else { - playableSet = getBeatmapSets().AsEnumerable().SkipWhile(i => !i.Equals(current?.BeatmapSetInfo) || (i.Protected && !allowProtectedTracks)).ElementAtOrDefault(1) + playableSet = getBeatmapSets().AsEnumerable().SkipWhile(i => !i.Equals(current?.BeatmapSetInfo)) + .Where(i => !i.Protected || allowProtectedTracks) + .ElementAtOrDefault(1) ?? getBeatmapSets().AsEnumerable().FirstOrDefault(i => !i.Protected || allowProtectedTracks); }