Merge pull request #2159 from naoey/fix-unplayable-beatmaps

Make song select ensure current beatmap is always playable in the active ruleset

osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs

@@ -12,6 +12,7 @@ using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
+using osu.Game.Rulesets;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Carousel;
 using osu.Game.Screens.Select.Filter;
@@ -22,6 +23,7 @@ namespace osu.Game.Tests.Visual
     public class TestCaseBeatmapCarousel : OsuTestCase
     {
         private TestBeatmapCarousel carousel;
+        private RulesetStore rulesets;
 
         public override IReadOnlyList<Type> RequiredTypes => new[]
         {
@@ -46,8 +48,10 @@ namespace osu.Game.Tests.Visual
         private const int set_count = 5;
 
         [BackgroundDependencyLoader]
-        private void load()
+        private void load(RulesetStore rulesets)
         {
+            this.rulesets = rulesets;
+
             Add(carousel = new TestBeatmapCarousel
             {
                 RelativeSizeAxes = Axes.Both,
@@ -75,6 +79,7 @@ namespace osu.Game.Tests.Visual
             testRemoveAll();
             testEmptyTraversal();
             testHiding();
+            testSelectingFilteredRuleset();
         }
 
         private void ensureRandomFetchSuccess() =>
@@ -363,6 +368,41 @@ namespace osu.Game.Tests.Visual
             }
         }
 
+        private void testSelectingFilteredRuleset()
+        {
+            var testMixed = createTestBeatmapSet(set_count + 1);
+            AddStep("add mixed ruleset beatmapset", () =>
+            {
+                for (int i = 0; i <= 2; i++)
+                {
+                    testMixed.Beatmaps[i].Ruleset = rulesets.AvailableRulesets.ElementAt(i);
+                    testMixed.Beatmaps[i].RulesetID = i;
+                }
+
+                carousel.UpdateBeatmapSet(testMixed);
+            });
+            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]));
+
+            AddStep("remove mixed set", () =>
+            {
+                carousel.RemoveBeatmapSet(testMixed);
+                testMixed = null;
+            });
+            var testSingle = createTestBeatmapSet(set_count + 2);
+            testSingle.Beatmaps.ForEach(b =>
+            {
+                b.Ruleset = rulesets.AvailableRulesets.ElementAt(1);
+                b.RulesetID = b.Ruleset.ID ?? 1;
+            });
+            AddStep("add single ruleset beatmapset", () => carousel.UpdateBeatmapSet(testSingle));
+            AddStep("select filtered map skipping filtered", () => carousel.SelectBeatmap(testSingle.Beatmaps[0], false));
+            checkNoSelection();
+            AddStep("remove single ruleset set", () => carousel.RemoveBeatmapSet(testSingle));
+        }
+
         private BeatmapSetInfo createTestBeatmapSet(int id)
         {
             return new BeatmapSetInfo

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -169,20 +169,43 @@ namespace osu.Game.Screens.Select
             });
         }
 
-        public void SelectBeatmap(BeatmapInfo beatmap)
+        /// <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>
+        /// <returns>True if a selection was made, False if it wasn't.</returns>
+        public bool SelectBeatmap(BeatmapInfo beatmap, bool bypassFilters = true)
         {
             if (beatmap?.Hidden != false)
-                return;
+                return false;
 
-            foreach (CarouselBeatmapSet group in beatmapSets)
+            foreach (CarouselBeatmapSet set in beatmapSets)
             {
-                var item = group.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap));
+                if (!bypassFilters && set.Filtered)
+                    continue;
+
+                var item = set.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap));
+
+                if (item == null)
+                    // The beatmap that needs to be selected doesn't exist in this set
+                    continue;
+
+                if (!bypassFilters && item.Filtered)
+                    // The beatmap exists in this set but is filtered, so look for the first unfiltered map in the set
+                    item = set.Beatmaps.FirstOrDefault(b => !b.Filtered);
+
                 if (item != null)
                 {
                     select(item);
-                    return;
+                    return true;
                 }
             }
+
+            return false;
         }
 
         /// <summary>

osu.Game/Screens/Select/SongSelect.cs

@@ -214,11 +214,7 @@ namespace osu.Game.Screens.Select
             Beatmap.DisabledChanged += disabled => Carousel.AllowSelection = !disabled;
             Beatmap.TriggerChange();
 
-            Beatmap.ValueChanged += b =>
+            Beatmap.ValueChanged += workingBeatmapChanged;
-            {
-                if (IsCurrentScreen)
-                    Carousel.SelectBeatmap(b?.BeatmapInfo);
-            };
         }
 
         public void Edit(BeatmapInfo beatmap)
@@ -261,6 +257,17 @@ namespace osu.Game.Screens.Select
         // We need to keep track of the last selected beatmap ignoring debounce to play the correct selection sounds.
         private BeatmapInfo beatmapNoDebounce;
 
+        private void workingBeatmapChanged(WorkingBeatmap beatmap)
+        {
+            if (IsCurrentScreen && !Carousel.SelectBeatmap(beatmap?.BeatmapInfo, false))
+                // If selecting new beatmap without bypassing filters failed, there's possibly a ruleset mismatch
+                if (beatmap?.BeatmapInfo?.Ruleset != null && beatmap.BeatmapInfo.Ruleset != Ruleset.Value)
+                {
+                    Ruleset.Value = beatmap.BeatmapInfo.Ruleset;
+                    Carousel.SelectBeatmap(beatmap.BeatmapInfo);
+                }
+        }
+
         /// <summary>
         /// selection has been changed as the result of interaction with the carousel.
         /// </summary>
@@ -450,16 +457,14 @@ namespace osu.Game.Screens.Select
 
         private void carouselBeatmapsLoaded()
         {
-            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))
+                return;
+
+            if (Carousel.SelectedBeatmapSet == null && !Carousel.SelectNextRandom())
             {
-                Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo);
+                // in the case random selection failed, we want to trigger selectionChanged
-            }
+                // to show the dummy beatmap (we have nothing else to display).
-            else if (Carousel.SelectedBeatmapSet == null)
+                carouselSelectionChanged(null);
-            {
-                if (!Carousel.SelectNextRandom())
-                    // in the case random selection failed, we want to trigger selectionChanged
-                    // to show the dummy beatmap (we have nothing else to display).
-                    carouselSelectionChanged(null);
             }
         }