Improve selection flow using early exit and invalidation

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -95,7 +95,7 @@ namespace osu.Game.Screens.SelectV2
         private GroupDefinition? lastSelectedGroup;
         private BeatmapInfo? lastSelectedBeatmap;
 
-        protected override void HandleItemSelected(object? model)
+        protected override bool HandleItemSelected(object? model)
         {
             base.HandleItemSelected(model);
 
@@ -104,6 +104,14 @@ namespace osu.Game.Screens.SelectV2
                 case GroupDefinition group:
                     if (lastSelectedGroup != null)
                         setVisibilityOfGroupItems(lastSelectedGroup, false);
+
+                    // Collapsing an open group.
+                    if (lastSelectedGroup == group)
+                    {
+                        lastSelectedGroup = null;
+                        return false;
+                    }
+
                     lastSelectedGroup = group;
 
                     setVisibilityOfGroupItems(group, true);
@@ -111,21 +119,34 @@ namespace osu.Game.Screens.SelectV2
                     // In stable, you can kinda select a group (expand without changing selection)
                     // For simplicity, let's not do that for now and handle similar to a beatmap set header.
                     CurrentSelection = grouping.GroupItems[group].First().Model;
-                    return;
+                    return false;
 
                 case BeatmapSetInfo setInfo:
                     // Selecting a set isn't valid – let's re-select the first difficulty.
                     CurrentSelection = setInfo.Beatmaps.First();
-                    return;
+                    return false;
 
                 case BeatmapInfo beatmapInfo:
                     if (lastSelectedBeatmap != null)
                         setVisibilityOfSetItems(lastSelectedBeatmap.BeatmapSet!, false);
                     lastSelectedBeatmap = beatmapInfo;
 
-                    setVisibilityOfSetItems(beatmapInfo.BeatmapSet!, true);
+                    // If we have groups, we need to account for them.
-                    break;
+                    if (grouping.GroupItems.Count > 0)
+                    {
+                        // Find the containing group. There should never be too many groups so iterating is efficient enough.
+                        var group = grouping.GroupItems.Single(kvp => kvp.Value.Any(i => ReferenceEquals(i.Model, beatmapInfo))).Key;
+                        setVisibilityOfGroupItems(group, true);
+                    }
+                    else
+                        setVisibilityOfSetItems(beatmapInfo.BeatmapSet!, true);
+
+                    // Ensure the group containing this beatmap is also visible.
+                    // TODO: need to update visibility of correct group?
+                    return true;
             }
+
+            return true;
         }
 
         private void setVisibilityOfGroupItems(GroupDefinition group, bool visible)

osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs

@@ -35,6 +35,9 @@ namespace osu.Game.Screens.SelectV2
 
         public async Task<IEnumerable<CarouselItem>> Run(IEnumerable<CarouselItem> items, CancellationToken cancellationToken) => await Task.Run(() =>
         {
+            setItems.Clear();
+            groupItems.Clear();
+
             var criteria = getCriteria();
 
             int starGroup = int.MinValue;

osu.Game/Screens/SelectV2/Carousel.cs

@@ -8,6 +8,7 @@ using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using osu.Framework.Bindables;
+using osu.Framework.Caching;
 using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
@@ -170,9 +171,8 @@ namespace osu.Game.Screens.SelectV2
         /// <summary>
         /// Called when an item is "selected".
         /// </summary>
-        protected virtual void HandleItemSelected(object? model)
+        /// <returns>Whether the item should be selected.</returns>
-        {
+        protected virtual bool HandleItemSelected(object? model) => true;
-        }
 
         /// <summary>
         /// Called when an item is "deselected".
@@ -410,6 +410,8 @@ namespace osu.Game.Screens.SelectV2
 
         #region Selection handling
 
+        private readonly Cached selectionValid = new Cached();
+
         private Selection currentKeyboardSelection = new Selection();
         private Selection currentSelection = new Selection();
 
@@ -418,29 +420,21 @@ namespace osu.Game.Screens.SelectV2
             if (currentSelection.Model == model)
                 return;
 
-            var previousSelection = currentSelection;
+            if (HandleItemSelected(model))
+            {
+                if (currentSelection.Model != null)
+                    HandleItemDeselected(currentSelection.Model);
 
-            if (previousSelection.Model != null)
+                currentKeyboardSelection = new Selection(model);
-                HandleItemDeselected(previousSelection.Model);
+                currentSelection = currentKeyboardSelection;
-
+                selectionValid.Invalidate();
-            currentSelection = currentKeyboardSelection = new Selection(model);
+            }
-            HandleItemSelected(currentSelection.Model);
-
-            // `HandleItemSelected` can alter `CurrentSelection`, which will recursively call `setSelection()` again.
-            // if that happens, the rest of this method should be a no-op.
-            if (currentSelection.Model != model)
-                return;
-
-            refreshAfterSelection();
-            scrollToSelection();
         }
 
         private void setKeyboardSelection(object? model)
         {
             currentKeyboardSelection = new Selection(model);
-
+            selectionValid.Invalidate();
-            refreshAfterSelection();
-            scrollToSelection();
         }
 
         /// <summary>
@@ -525,6 +519,13 @@ namespace osu.Game.Screens.SelectV2
             if (carouselItems == null)
                 return;
 
+            if (!selectionValid.IsValid)
+            {
+                refreshAfterSelection();
+                scrollToSelection();
+                selectionValid.Validate();
+            }
+
             var range = getDisplayRange();
 
             if (range != displayedRange)