Merge pull request #2332 from LittleEndu/alt-eagerselect

Make beatmap carousel select a random beatmap when there was no previous selection

osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs

@@ -42,6 +42,7 @@ namespace osu.Game.Tests.Visual
 
 
         private readonly Stack<BeatmapSetInfo> selectedSets = new Stack<BeatmapSetInfo>();
+        private readonly HashSet<int> eagerSelectedIDs = new HashSet<int>();
 
         private BeatmapInfo currentSelection;
 
@@ -80,6 +81,7 @@ namespace osu.Game.Tests.Visual
             testEmptyTraversal();
             testHiding();
             testSelectingFilteredRuleset();
+            testCarouselRootIsRandom();
         }
 
         private void ensureRandomFetchSuccess() =>
@@ -151,6 +153,17 @@ namespace osu.Game.Tests.Visual
             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);
+            }
+            );
+        }
+
         /// <summary>
         /// Test keyboard traversal
         /// </summary>
@@ -403,6 +416,23 @@ namespace osu.Game.Tests.Visual
             AddStep("remove single ruleset set", () => carousel.RemoveBeatmapSet(testSingle));
         }
 
+        private void testCarouselRootIsRandom()
+        {
+            List<BeatmapSetInfo> beatmapSets = new List<BeatmapSetInfo>();
+
+            for (int i = 1; i <= 50; i++)
+                beatmapSets.Add(createTestBeatmapSet(i));
+
+            AddStep("Load 50 Beatmaps", () => { carousel.BeatmapSets = beatmapSets; });
+            advanceSelection(direction: 1, diff: false);
+            checkNonmatchingFilter();
+            checkNonmatchingFilter();
+            checkNonmatchingFilter();
+            checkNonmatchingFilter();
+            checkNonmatchingFilter();
+            AddAssert("Selection was random", () => eagerSelectedIDs.Count > 1);
+        }
+
         private BeatmapSetInfo createTestBeatmapSet(int id)
         {
             return new BeatmapSetInfo

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -66,7 +66,7 @@ namespace osu.Game.Screens.Select
             get { return beatmapSets.Select(g => g.BeatmapSet); }
             set
             {
-                CarouselGroup newRoot = new CarouselGroupEagerSelect();
+                CarouselRoot newRoot = new CarouselRoot(this);
 
                 Task.Run(() =>
                 {
@@ -102,10 +102,11 @@ namespace osu.Game.Screens.Select
         private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
 
         protected List<DrawableCarouselItem> Items = new List<DrawableCarouselItem>();
-        private CarouselGroup root = new CarouselGroupEagerSelect();
+        private CarouselRoot root;
 
         public BeatmapCarousel()
         {
+            root = new CarouselRoot(this);
             Child = new OsuContextMenuContainer
             {
                 RelativeSizeAxes = Axes.X,
@@ -627,5 +628,23 @@ namespace osu.Game.Screens.Select
             // layer transformations on top, with a similar reasoning to the previous comment.
             p.SetMultiplicativeAlpha(MathHelper.Clamp(1.75f - 1.5f * dist, 0, 1));
         }
+
+        private class CarouselRoot : CarouselGroupEagerSelect
+        {
+            private readonly BeatmapCarousel carousel;
+
+            public CarouselRoot(BeatmapCarousel carousel)
+            {
+                this.carousel = carousel;
+            }
+
+            protected override void PerformSelection()
+            {
+                if (LastSelected == null)
+                    carousel.SelectNextRandom();
+                else
+                    base.PerformSelection();
+            }
+        }
     }
 }

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

@@ -20,13 +20,16 @@ namespace osu.Game.Screens.Select.Carousel
             };
         }
 
+        /// <summary>
+        /// The last selected item.
+        /// </summary>
+        protected CarouselItem LastSelected { get; private set; }
+
         /// <summary>
         /// We need to keep track of the index for cases where the selection is removed but we want to select a new item based on its old location.
         /// </summary>
         private int lastSelectedIndex;
 
-        private CarouselItem lastSelected;
-
         /// <summary>
         /// To avoid overhead during filter operations, we don't attempt any selections until after all
         /// children have been filtered. This bool will be true during the base <see cref="Filter(FilterCriteria)"/>
@@ -47,7 +50,7 @@ namespace osu.Game.Screens.Select.Carousel
         {
             base.RemoveChild(i);
 
-            if (i != lastSelected)
+            if (i != LastSelected)
                 updateSelectedIndex();
         }
 
@@ -83,6 +86,11 @@ namespace osu.Game.Screens.Select.Carousel
             // we only perform eager selection if none of our children are in a selected state already.
             if (Children.Any(i => i.State == CarouselItemState.Selected)) return;
 
+            PerformSelection();
+        }
+
+        protected virtual void PerformSelection()
+        {
             CarouselItem nextToSelect =
                 Children.Skip(lastSelectedIndex).FirstOrDefault(i => !i.Filtered) ??
                 Children.Reverse().Skip(InternalChildren.Count - lastSelectedIndex).FirstOrDefault(i => !i.Filtered);
@@ -95,10 +103,10 @@ namespace osu.Game.Screens.Select.Carousel
 
         private void updateSelected(CarouselItem newSelection)
         {
-            lastSelected = newSelection;
+            LastSelected = newSelection;
             updateSelectedIndex();
         }
 
-        private void updateSelectedIndex() => lastSelectedIndex = lastSelected == null ? 0 : Math.Max(0, InternalChildren.IndexOf(lastSelected));
+        private void updateSelectedIndex() => lastSelectedIndex = LastSelected == null ? 0 : Math.Max(0, InternalChildren.IndexOf(LastSelected));
     }
 }