Merge pull request #8368 from peppy/fix-carousel-restore-previous-selection

Fix carousel not returning to previous selection after filter

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

@@ -419,7 +419,7 @@ namespace osu.Game.Tests.Visual.SongSelect
         }
 
         [Test]
-        public void TestCarouselRootIsRandom()
+        public void TestCarouselRemembersSelection()
         {
             List<BeatmapSetInfo> manySets = new List<BeatmapSetInfo>();
 
@@ -429,12 +429,74 @@ namespace osu.Game.Tests.Visual.SongSelect
             loadBeatmaps(manySets);
 
             advanceSelection(direction: 1, diff: false);
-            checkNonmatchingFilter();
+
-            checkNonmatchingFilter();
+            for (int i = 0; i < 5; i++)
-            checkNonmatchingFilter();
+            {
-            checkNonmatchingFilter();
+                AddStep("Toggle non-matching filter", () =>
-            checkNonmatchingFilter();
+                {
-            AddAssert("Selection was random", () => eagerSelectedIDs.Count > 1);
+                    carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false);
+                });
+
+                AddStep("Restore no filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria(), false);
+                    eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
+                });
+            }
+
+            // always returns to same selection as long as it's available.
+            AddAssert("Selection was remembered", () => eagerSelectedIDs.Count == 1);
+        }
+
+        [Test]
+        public void TestRandomFallbackOnNonMatchingPrevious()
+        {
+            List<BeatmapSetInfo> manySets = new List<BeatmapSetInfo>();
+
+            AddStep("populate maps", () =>
+            {
+                for (int i = 0; i < 10; i++)
+                {
+                    var set = createTestBeatmapSet(i);
+
+                    foreach (var b in set.Beatmaps)
+                    {
+                        // all taiko except for first
+                        int ruleset = i > 0 ? 1 : 0;
+
+                        b.Ruleset = rulesets.GetRuleset(ruleset);
+                        b.RulesetID = ruleset;
+                    }
+
+                    manySets.Add(set);
+                }
+            });
+
+            loadBeatmaps(manySets);
+
+            for (int i = 0; i < 10; i++)
+            {
+                AddStep("Reset filter", () => carousel.Filter(new FilterCriteria(), false));
+
+                AddStep("select first beatmap", () => carousel.SelectBeatmap(manySets.First().Beatmaps.First()));
+
+                AddStep("Toggle non-matching filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false);
+                });
+
+                AddAssert("selection lost", () => carousel.SelectedBeatmap == null);
+
+                AddStep("Restore different ruleset filter", () =>
+                {
+                    carousel.Filter(new FilterCriteria { Ruleset = rulesets.GetRuleset(1) }, false);
+                    eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
+                });
+
+                AddAssert("selection changed", () => carousel.SelectedBeatmap != manySets.First().Beatmaps.First());
+            }
+
+            AddAssert("Selection was random", () => eagerSelectedIDs.Count > 2);
         }
 
         [Test]
@@ -593,16 +655,6 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("Selection is visible", selectedBeatmapVisible);
         }
 
-        private void checkNonmatchingFilter()
-        {
-            AddStep("Toggle non-matching filter", () =>
-            {
-                carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false);
-                carousel.Filter(new FilterCriteria(), false);
-                eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID);
-            });
-        }
-
         private BeatmapSetInfo createTestBeatmapSet(int id)
         {
             return new BeatmapSetInfo

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -333,8 +333,7 @@ namespace osu.Game.Screens.Select
             else
                 set = visibleSets.ElementAt(RNG.Next(visibleSets.Count));
 
-            var visibleBeatmaps = set.Beatmaps.Where(s => !s.Filtered.Value).ToList();
+            select(set);
-            select(visibleBeatmaps[RNG.Next(visibleBeatmaps.Count)]);
             return true;
         }
 
@@ -756,7 +755,7 @@ namespace osu.Game.Screens.Select
 
             protected override void PerformSelection()
             {
-                if (LastSelected == null)
+                if (LastSelected == null || LastSelected.Filtered.Value)
                     carousel.SelectNextRandom();
                 else
                     base.PerformSelection();

osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs

@@ -104,7 +104,8 @@ namespace osu.Game.Screens.Select.Carousel
 
         private void updateSelected(CarouselItem newSelection)
         {
-            LastSelected = newSelection;
+            if (newSelection != null)
+                LastSelected = newSelection;
             updateSelectedIndex();
         }