Fix backwards traversal of groupings and allow toggling groups without updating selection

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -102,23 +102,15 @@ namespace osu.Game.Screens.SelectV2
             switch (model)
             {
                 case GroupDefinition group:
-                    if (lastSelectedGroup != null)
+                    // Special case – collapsing an open group.
-                        setVisibilityOfGroupItems(lastSelectedGroup, false);
-
-                    // Collapsing an open group.
                     if (lastSelectedGroup == group)
                     {
+                        setVisibilityOfGroupItems(lastSelectedGroup, false);
                         lastSelectedGroup = null;
                         return false;
                     }
 
-                    lastSelectedGroup = group;
+                    setVisibleGroup(group);
-
-                    setVisibilityOfGroupItems(group, true);
-
-                    // 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 false;
 
                 case BeatmapSetInfo setInfo:
@@ -127,28 +119,52 @@ namespace osu.Game.Screens.SelectV2
                     return false;
 
                 case BeatmapInfo beatmapInfo:
-                    if (lastSelectedBeatmap != null)
-                        setVisibilityOfSetItems(lastSelectedBeatmap.BeatmapSet!, false);
-                    lastSelectedBeatmap = beatmapInfo;
 
                     // If we have groups, we need to account for them.
-                    if (grouping.GroupItems.Count > 0)
+                    if (Criteria.SplitOutDifficulties)
                     {
                         // 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;
+                        GroupDefinition group = grouping.GroupItems.Single(kvp => kvp.Value.Any(i => ReferenceEquals(i.Model, beatmapInfo))).Key;
-                        setVisibilityOfGroupItems(group, true);
+
+                        setVisibleGroup(group);
                     }
                     else
-                        setVisibilityOfSetItems(beatmapInfo.BeatmapSet!, true);
+                    {
+                        setVisibleSet(beatmapInfo);
+                    }
 
-                    // Ensure the group containing this beatmap is also visible.
-                    // TODO: need to update visibility of correct group?
                     return true;
             }
 
             return true;
         }
 
+        protected override bool CheckValidForGroupSelection(CarouselItem item)
+        {
+            switch (item.Model)
+            {
+                case BeatmapSetInfo:
+                    return true;
+
+                case BeatmapInfo:
+                    return Criteria.SplitOutDifficulties;
+
+                case GroupDefinition:
+                    return false;
+
+                default:
+                    throw new ArgumentException($"Unsupported model type {item.Model}");
+            }
+        }
+
+        private void setVisibleGroup(GroupDefinition group)
+        {
+            if (lastSelectedGroup != null)
+                setVisibilityOfGroupItems(lastSelectedGroup, false);
+            lastSelectedGroup = group;
+            setVisibilityOfGroupItems(group, true);
+        }
+
         private void setVisibilityOfGroupItems(GroupDefinition group, bool visible)
         {
             if (grouping.GroupItems.TryGetValue(group, out var items))
@@ -158,6 +174,14 @@ namespace osu.Game.Screens.SelectV2
             }
         }
 
+        private void setVisibleSet(BeatmapInfo beatmapInfo)
+        {
+            if (lastSelectedBeatmap != null)
+                setVisibilityOfSetItems(lastSelectedBeatmap.BeatmapSet!, false);
+            lastSelectedBeatmap = beatmapInfo;
+            setVisibilityOfSetItems(beatmapInfo.BeatmapSet!, true);
+        }
+
         private void setVisibilityOfSetItems(BeatmapSetInfo set, bool visible)
         {
             if (grouping.SetItems.TryGetValue(set, out var items))

osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs

@@ -56,11 +56,7 @@ namespace osu.Game.Screens.SelectV2
                     {
                         starGroup = (int)Math.Floor(b.StarRating);
                         group = new GroupDefinition($"{starGroup} - {++starGroup} *");
-                        diffItems.Add(new CarouselItem(group)
+                        diffItems.Add(new CarouselItem(group) { DrawHeight = GroupPanel.HEIGHT });
-                        {
-                            DrawHeight = GroupPanel.HEIGHT,
-                            IsGroupSelectionTarget = true
-                        });
                     }
 
                     if (!groupItems.TryGetValue(group!, out var related))
@@ -70,7 +66,6 @@ namespace osu.Game.Screens.SelectV2
                     diffItems.Add(item);
 
                     item.IsVisible = false;
-                    item.IsGroupSelectionTarget = true;
                 }
 
                 return diffItems;
@@ -92,7 +87,6 @@ namespace osu.Game.Screens.SelectV2
                         newItems.Add(new CarouselItem(b.BeatmapSet!)
                         {
                             DrawHeight = BeatmapSetPanel.HEIGHT,
-                            IsGroupSelectionTarget = true
                         });
                     }
 
@@ -104,7 +98,6 @@ namespace osu.Game.Screens.SelectV2
                 newItems.Add(item);
                 lastItem = item;
 
-                item.IsGroupSelectionTarget = false;
                 item.IsVisible = false;
             }
 

osu.Game/Screens/SelectV2/BeatmapSetPanel.cs

@@ -67,7 +67,6 @@ namespace osu.Game.Screens.SelectV2
             base.PrepareForUse();
 
             Debug.Assert(Item != null);
-            Debug.Assert(Item.IsGroupSelectionTarget);
 
             var beatmapSetInfo = (BeatmapSetInfo)Item.Model;
 

osu.Game/Screens/SelectV2/Carousel.cs

@@ -168,6 +168,13 @@ namespace osu.Game.Screens.SelectV2
         protected Drawable? GetMaterialisedDrawableForItem(CarouselItem item) =>
             scroll.Panels.SingleOrDefault(p => ((ICarouselPanel)p).Item == item);
 
+        /// <summary>
+        /// When a user is traversing the carousel via group selection keys, assert whether the item provided is a valid target.
+        /// </summary>
+        /// <param name="item">The candidate item.</param>
+        /// <returns>Whether the provided item is a valid group target. If <c>false</c>, more panels will be checked in the user's requested direction until a valid target is found.</returns>
+        protected virtual bool CheckValidForGroupSelection(CarouselItem item) => true;
+
         /// <summary>
         /// Called when an item is "selected".
         /// </summary>
@@ -373,7 +380,7 @@ namespace osu.Game.Screens.SelectV2
             // make sure to go back to the group header this item belongs to, so that the block below doesn't find it and stop too early.
             if (isGroupSelection && direction < 0)
             {
-                while (!carouselItems[selectionIndex].IsGroupSelectionTarget)
+                while (!CheckValidForGroupSelection(carouselItems[selectionIndex]))
                     selectionIndex--;
             }
 
@@ -394,7 +401,11 @@ namespace osu.Game.Screens.SelectV2
 
             bool attemptSelection(CarouselItem item)
             {
-                if (!item.IsVisible || (isGroupSelection && !item.IsGroupSelectionTarget))
+                // Keyboard (non-group) selection should only consider visible items.
+                if (!isGroupSelection && !item.IsVisible)
+                    return false;
+
+                if (isGroupSelection && !CheckValidForGroupSelection(item))
                     return false;
 
                 if (isGroupSelection)
@@ -427,8 +438,9 @@ namespace osu.Game.Screens.SelectV2
 
                 currentKeyboardSelection = new Selection(model);
                 currentSelection = currentKeyboardSelection;
-                selectionValid.Invalidate();
             }
+
+            selectionValid.Invalidate();
         }
 
         private void setKeyboardSelection(object? model)

osu.Game/Screens/SelectV2/CarouselItem.cs

@@ -29,11 +29,6 @@ namespace osu.Game.Screens.SelectV2
         /// </summary>
         public float DrawHeight { get; set; } = DEFAULT_HEIGHT;
 
-        /// <summary>
-        /// Whether this item should be a valid target for user group selection hotkeys.
-        /// </summary>
-        public bool IsGroupSelectionTarget { get; set; }
-
         /// <summary>
         /// Whether this item is visible or collapsed (hidden).
         /// </summary>

osu.Game/Screens/SelectV2/GroupPanel.cs

@@ -79,7 +79,6 @@ namespace osu.Game.Screens.SelectV2
             base.PrepareForUse();
 
             Debug.Assert(Item != null);
-            Debug.Assert(Item.IsGroupSelectionTarget);
 
             GroupDefinition group = (GroupDefinition)Item.Model;