Add tests for (and fix) removal of last item in carousel

osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs

@@ -39,7 +39,9 @@ namespace osu.Game.Tests.Visual
 
         private readonly Stack<BeatmapSetInfo> selectedSets = new Stack<BeatmapSetInfo>();
 
-        private const int set_count = 1000;
+        private BeatmapInfo currentSelection;
+
+        private const int set_count = 5;
 
         [BackgroundDependencyLoader]
         private void load()
@@ -54,6 +56,8 @@ namespace osu.Game.Tests.Visual
             for (int i = 1; i <= set_count; i++)
                 beatmapSets.Add(createTestBeatmapSet(i));
 
+            carousel.SelectionChanged = s => currentSelection = s;
+
             AddStep("Load Beatmaps", () => { carousel.BeatmapSets = beatmapSets; });
 
             AddUntilStep(() => carousel.BeatmapSets.Any(), "Wait for load");
@@ -63,6 +67,8 @@ namespace osu.Game.Tests.Visual
             testRandom();
             testAddRemove();
             testSorting();
+
+            testRemoveAll();
         }
 
         private void ensureRandomFetchSuccess() =>
@@ -145,6 +151,8 @@ namespace osu.Game.Tests.Visual
         {
             // basic filtering
 
+            setSelected(1, 1);
+
             AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3!" }, false));
             checkVisibleItemCount(diff: false, count: 1);
             checkVisibleItemCount(diff: true, count: 3);
@@ -163,8 +171,19 @@ namespace osu.Game.Tests.Visual
             setSelected(1, 2);
             AddStep("Filter some difficulties", () => carousel.Filter(new FilterCriteria { SearchText = "Normal" }, false));
             checkSelected(1, 1);
+
             AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false));
             checkSelected(1, 1);
+
+            AddStep("Filter all", () => carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false));
+
+            checkVisibleItemCount(false, 0);
+            checkVisibleItemCount(true, 0);
+            AddAssert("Selection is null", () => currentSelection == null);
+
+            AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false));
+
+            AddAssert("Selection is non-null", () => currentSelection != null);
         }
 
         /// <summary>
@@ -225,6 +244,21 @@ namespace osu.Game.Tests.Visual
             AddAssert($"Check #{set_count} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith($"#{set_count}!"));
         }
 
+        private void testRemoveAll()
+        {
+            setSelected(2, 1);
+
+            AddAssert("Selection is non-null", () => currentSelection != null);
+
+            AddUntilStep(() =>
+            {
+                carousel.RemoveBeatmapSet(carousel.BeatmapSets.Last());
+                return !carousel.BeatmapSets.Any();
+            }, "Remove all");
+
+            AddAssert("Selection is null", () => currentSelection == null);
+        }
+
 
         private BeatmapSetInfo createTestBeatmapSet(int i)
         {
@@ -237,9 +271,9 @@ namespace osu.Game.Tests.Visual
                 {
                     OnlineBeatmapSetID = i,
                     // Create random metadata, then we can check if sorting works based on these
-                    Artist = $"peppy{i.ToString().PadLeft(6,'0')}",
+                    Artist = $"peppy{i.ToString().PadLeft(6, '0')}",
                     Title = $"test set #{i}!",
-                    AuthorString = string.Concat(Enumerable.Repeat((char)('z' - Math.Min(25,i - 1)), 5))
+                    AuthorString = string.Concat(Enumerable.Repeat((char)('z' - Math.Min(25, i - 1)), 5))
                 },
                 Beatmaps = new List<BeatmapInfo>(new[]
                 {

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -62,7 +62,7 @@ namespace osu.Game.Screens.Select
             {
                 List<CarouselBeatmapSet> newSets = null;
 
-                CarouselGroup newRoot = new CarouselGroup();
+                CarouselGroup newRoot = new CarouselGroupEagerSelect();
 
                 Task.Run(() =>
                 {
@@ -102,7 +102,7 @@ namespace osu.Game.Screens.Select
         private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
 
         protected List<DrawableCarouselItem> Items = new List<DrawableCarouselItem>();
-        private CarouselGroup root = new CarouselGroup();
+        private CarouselGroup root = new CarouselGroupEagerSelect();
 
         public BeatmapCarousel()
         {
@@ -189,7 +189,7 @@ namespace osu.Game.Screens.Select
         public void SelectNext(int direction = 1, bool skipDifficulties = true)
         {
             // todo: we may want to refactor and remove this as an optimisation in the future.
-            if (beatmapSets.All(g => !g.Visible))
+            if (beatmapSets.All(g => g.Filtered))
             {
                 SelectionChanged?.Invoke(null);
                 return;
@@ -304,7 +304,7 @@ namespace osu.Game.Screens.Select
                 root.Filter(criteria);
                 updateItems();
 
-                if (selectedBeatmap?.Filtered == false)
+                if (selectedBeatmap != null && !selectedBeatmap.Filtered)
                     select(selectedBeatmap);
                 else if (lastBeatmap != null && !lastSet.Filtered)
                 {

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

@@ -19,7 +19,7 @@ namespace osu.Game.Screens.Select.Carousel
 
         public override void AddChild(CarouselItem i)
         {
-            i.State.ValueChanged += v => itemStateChanged(i, v);
+            i.State.ValueChanged += v => ItemStateChanged(i, v);
             base.AddChild(i);
         }
 
@@ -28,7 +28,7 @@ namespace osu.Game.Screens.Select.Carousel
             if (items != null) InternalChildren = items;
         }
 
-        private void itemStateChanged(CarouselItem item, CarouselItemState value)
+        protected virtual void ItemStateChanged(CarouselItem item, CarouselItemState value)
         {
             // todo: check state of selected item.
 

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

@@ -24,5 +24,22 @@ namespace osu.Game.Screens.Select.Carousel
                 }
             };
         }
+
+        protected override void ItemStateChanged(CarouselItem item, CarouselItemState value)
+        {
+            base.ItemStateChanged(item, value);
+
+            if (value == CarouselItemState.NotSelected)
+            {
+                if (Children.All(i => i.State != CarouselItemState.Selected))
+                {
+                    var first = Children.FirstOrDefault(i => !i.Filtered);
+                    if (first != null)
+                    {
+                        first.State.Value = CarouselItemState.Selected;
+                    }
+                }
+            }
+        }
     }
 }

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

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using osu.Framework.Configuration;
+using osu.Framework.Logging;
 
 namespace osu.Game.Screens.Select.Carousel
 {
@@ -48,6 +49,8 @@ namespace osu.Game.Screens.Select.Carousel
             {
                 if (InternalChildren == null) return;
 
+                Logger.Log($"State changed to {v}");
+
                 switch (v)
                 {
                     case CarouselItemState.Hidden:
@@ -56,6 +59,12 @@ namespace osu.Game.Screens.Select.Carousel
                         break;
                 }
             };
+
+            Filtered.ValueChanged += v =>
+            {
+                if (v && State == CarouselItemState.Selected)
+                    State.Value = CarouselItemState.NotSelected;
+            };
         }
 
         private readonly Lazy<DrawableCarouselItem> drawableRepresentation;