Merge pull request #8246 from peppy/carousel-selection-fallback-improvement

Improve beatmap carousel / song select fallback logic pathing

osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs

@@ -399,7 +399,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddStep("filter to ruleset 0", () =>
                 carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) }, false));
             AddStep("select filtered map skipping filtered", () => carousel.SelectBeatmap(testMixed.Beatmaps[1], false));
-            AddAssert("unfiltered beatmap selected", () => carousel.SelectedBeatmap.Equals(testMixed.Beatmaps[0]));
+            AddAssert("unfiltered beatmap not selected", () => carousel.SelectedBeatmap == null);
 
             AddStep("remove mixed set", () =>
             {

osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs

@@ -591,16 +591,16 @@ namespace osu.Game.Tests.Visual.SongSelect
                 }
             }));
 
+            BeatmapInfo filteredBeatmap = null;
             DrawableCarouselBeatmapSet.FilterableDifficultyIcon filteredIcon = null;
+
             AddStep("Get filtered icon", () =>
             {
-                var filteredBeatmap = songSelect.Carousel.SelectedBeatmapSet.Beatmaps.Find(b => b.BPM < maxBPM);
+                filteredBeatmap = songSelect.Carousel.SelectedBeatmapSet.Beatmaps.Find(b => b.BPM < maxBPM);
                 int filteredBeatmapIndex = getBeatmapIndex(filteredBeatmap.BeatmapSet, filteredBeatmap);
                 filteredIcon = set.ChildrenOfType<DrawableCarouselBeatmapSet.FilterableDifficultyIcon>().ElementAt(filteredBeatmapIndex);
             });
 
-            int? previousID = null;
-            AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID);
             AddStep("Click on a filtered difficulty", () =>
             {
                 InputManager.MoveMouseTo(filteredIcon);
@@ -608,7 +608,9 @@ namespace osu.Game.Tests.Visual.SongSelect
                 InputManager.PressButton(MouseButton.Left);
                 InputManager.ReleaseButton(MouseButton.Left);
             });
-            AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID);
+
+            // todo: this logic is changed in follow up PR.
+            AddAssert("Selected beatmap not changed", () => songSelect.Carousel.SelectedBeatmap != filteredBeatmap);
         }
 
         private int getBeatmapIndex(BeatmapSetInfo set, BeatmapInfo info) => set.Beatmaps.FindIndex(b => b == info);

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -203,9 +203,6 @@ namespace osu.Game.Screens.Select
 
         /// <summary>
         /// Selects a given beatmap on the carousel.
-        ///
-        /// If bypassFilters is false, we will try to select another unfiltered beatmap in the same set. If the
-        /// entire set is filtered, no selection is made.
         /// </summary>
         /// <param name="beatmap">The beatmap to select.</param>
         /// <param name="bypassFilters">Whether to select the beatmap even if it is filtered (i.e., not visible on carousel).</param>
@@ -227,11 +224,8 @@ namespace osu.Game.Screens.Select
                     continue;
 
                 if (!bypassFilters && item.Filtered.Value)
-                    // The beatmap exists in this set but is filtered, so look for the first unfiltered map in the set
+                    return false;
-                    item = set.Beatmaps.FirstOrDefault(b => !b.Filtered.Value);
 
-                if (item != null)
-                {
                 select(item);
 
                 // if we got here and the set is filtered, it means we were bypassing filters.
@@ -246,7 +240,6 @@ namespace osu.Game.Screens.Select
 
                 return true;
             }
-            }
 
             return false;
         }

osu.Game/Screens/Select/SongSelect.cs

@@ -380,6 +380,8 @@ namespace osu.Game.Screens.Select
         {
             if (e.NewValue is DummyWorkingBeatmap || !this.IsCurrentScreen()) return;
 
+            Logger.Log($"working beatmap updated to {e.NewValue}");
+
             if (!Carousel.SelectBeatmap(e.NewValue.BeatmapInfo, false))
             {
                 // A selection may not have been possible with filters applied.
@@ -446,8 +448,10 @@ namespace osu.Game.Screens.Select
 
                 if (transferRulesetValue())
                 {
-                    // if the ruleset changed, the rest of the selection update will happen via updateSelectedRuleset.
                     Mods.Value = Array.Empty<Mod>();
+
+                    // required to return once in order to have the carousel in a good state.
+                    // if the ruleset changed, the rest of the selection update will happen via updateSelectedRuleset.
                     return;
                 }
 
@@ -472,7 +476,7 @@ namespace osu.Game.Screens.Select
                 if (this.IsCurrentScreen())
                     ensurePlayingSelected();
 
-                UpdateBeatmap(Beatmap.Value);
+                updateComponentFromBeatmap(Beatmap.Value);
             }
         }
 
@@ -547,7 +551,7 @@ namespace osu.Game.Screens.Select
 
             if (Beatmap != null && !Beatmap.Value.BeatmapSetInfo.DeletePending)
             {
-                UpdateBeatmap(Beatmap.Value);
+                updateComponentFromBeatmap(Beatmap.Value);
 
                 // restart playback on returning to song select, regardless.
                 music?.Play();
@@ -610,10 +614,8 @@ namespace osu.Game.Screens.Select
         /// This is a debounced call (unlike directly binding to WorkingBeatmap.ValueChanged).
         /// </summary>
         /// <param name="beatmap">The working beatmap.</param>
-        protected virtual void UpdateBeatmap(WorkingBeatmap beatmap)
+        private void updateComponentFromBeatmap(WorkingBeatmap beatmap)
         {
-            Logger.Log($"working beatmap updated to {beatmap}");
-
             if (Background is BackgroundScreenBeatmap backgroundModeBeatmap)
             {
                 backgroundModeBeatmap.Beatmap = beatmap;
@@ -658,10 +660,18 @@ namespace osu.Game.Screens.Select
                 return;
 
             // Attempt to select the current beatmap on the carousel, if it is valid to be selected.
-            if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false
+            if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false)
-                && Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false))
+            {
+                if (Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false))
                     return;
 
+                // prefer not changing ruleset at this point, so look for another difficulty in the currently playing beatmap
+                var found = Beatmap.Value.BeatmapSetInfo.Beatmaps.FirstOrDefault(b => b.Ruleset.Equals(decoupledRuleset.Value));
+
+                if (found != null && Carousel.SelectBeatmap(found, false))
+                    return;
+            }
+
             // If the current active beatmap could not be selected, select a new random beatmap.
             if (!Carousel.SelectNextRandom())
             {