Merge pull request #31799 from frenzibyte/carousel-v2-spacing

Support variable spacing between beatmap carousel panels

osu.Game.Tests/Visual/Editing/TestScenePlacementBlueprint.cs

@@ -74,7 +74,6 @@ namespace osu.Game.Tests.Visual.Editing
         }
 
         [Test]
-        [Solo]
         public void TestCommitPlacementViaRightClick()
         {
             Playfield playfield = null!;

osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs

@@ -165,7 +165,6 @@ namespace osu.Game.Tests.Visual.Navigation
         }
 
         [Test]
-        [Solo]
         public void TestEditorGameplayTestAlwaysUsesOriginalRuleset()
         {
             prepareBeatmap();

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

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
@@ -20,6 +21,7 @@ using osu.Game.Screens.Select;
 using osu.Game.Screens.SelectV2;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Resources;
+using osuTK;
 using osuTK.Graphics;
 using osuTK.Input;
 using BeatmapCarousel = osu.Game.Screens.SelectV2.BeatmapCarousel;
@@ -164,6 +166,15 @@ namespace osu.Game.Tests.Visual.SongSelect
             });
         }
 
+        protected IEnumerable<T> GetVisiblePanels<T>()
+            where T : Drawable
+        {
+            return Carousel.ChildrenOfType<UserTrackingScrollContainer>().Single()
+                           .ChildrenOfType<T>()
+                           .Where(p => ((ICarouselPanel)p).Item?.IsVisible == true)
+                           .OrderBy(p => p.Y);
+        }
+
         protected void ClickVisiblePanel<T>(int index)
             where T : Drawable
         {
@@ -178,6 +189,23 @@ namespace osu.Game.Tests.Visual.SongSelect
             });
         }
 
+        protected void ClickVisiblePanelWithOffset<T>(int index, Vector2 positionOffsetFromCentre)
+            where T : Drawable
+        {
+            AddStep($"move mouse to panel {index} with offset {positionOffsetFromCentre}", () =>
+            {
+                var panel = Carousel.ChildrenOfType<UserTrackingScrollContainer>().Single()
+                                    .ChildrenOfType<T>()
+                                    .Where(p => ((ICarouselPanel)p).Item?.IsVisible == true)
+                                    .OrderBy(p => p.Y)
+                                    .ElementAt(index);
+
+                InputManager.MoveMouseTo(panel.ScreenSpaceDrawQuad.Centre + panel.ToScreenSpace(positionOffsetFromCentre) - panel.ToScreenSpace(Vector2.Zero));
+            });
+
+            AddStep("click", () => InputManager.Click(MouseButton.Left));
+        }
+
         /// <summary>
         /// Add requested beatmap sets count to list.
         /// </summary>

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

@@ -8,6 +8,7 @@ using osu.Game.Beatmaps;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Filter;
 using osu.Game.Screens.SelectV2;
+using osuTK;
 
 namespace osu.Game.Tests.Visual.SongSelect
 {
@@ -146,5 +147,28 @@ namespace osu.Game.Tests.Visual.SongSelect
             SelectPrevGroup();
             WaitForGroupSelection(2, 9);
         }
+
+        [Test]
+        public void TestInputHandlingWithinGaps()
+        {
+            AddAssert("no beatmaps visible", () => !GetVisiblePanels<BeatmapPanel>().Any());
+
+            // Clicks just above the first group panel should not actuate any action.
+            ClickVisiblePanelWithOffset<GroupPanel>(0, new Vector2(0, -(GroupPanel.HEIGHT / 2 + 1)));
+
+            AddAssert("no beatmaps visible", () => !GetVisiblePanels<BeatmapPanel>().Any());
+
+            ClickVisiblePanelWithOffset<GroupPanel>(0, new Vector2(0, -(GroupPanel.HEIGHT / 2)));
+
+            AddUntilStep("wait for beatmaps visible", () => GetVisiblePanels<BeatmapPanel>().Any());
+            CheckNoSelection();
+
+            // Beatmap panels expand their selection area to cover holes from spacing.
+            ClickVisiblePanelWithOffset<BeatmapPanel>(0, new Vector2(0, -(CarouselItem.DEFAULT_HEIGHT / 2 + 1)));
+            WaitForGroupSelection(0, 0);
+
+            ClickVisiblePanelWithOffset<BeatmapPanel>(1, new Vector2(0, (CarouselItem.DEFAULT_HEIGHT / 2 + 1)));
+            WaitForGroupSelection(0, 1);
+        }
     }
 }

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

@@ -1,11 +1,14 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Filter;
+using osu.Game.Screens.SelectV2;
+using osuTK;
 using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.SongSelect
@@ -204,6 +207,36 @@ namespace osu.Game.Tests.Visual.SongSelect
             CheckNoSelection();
         }
 
+        [Test]
+        public void TestInputHandlingWithinGaps()
+        {
+            AddBeatmaps(2, 5);
+            WaitForDrawablePanels();
+
+            AddAssert("no beatmaps visible", () => !GetVisiblePanels<BeatmapPanel>().Any());
+
+            // Clicks just above the first group panel should not actuate any action.
+            ClickVisiblePanelWithOffset<BeatmapSetPanel>(0, new Vector2(0, -(BeatmapSetPanel.HEIGHT / 2 + 1)));
+
+            AddAssert("no beatmaps visible", () => !GetVisiblePanels<BeatmapPanel>().Any());
+
+            ClickVisiblePanelWithOffset<BeatmapSetPanel>(0, new Vector2(0, -(BeatmapSetPanel.HEIGHT / 2)));
+
+            AddUntilStep("wait for beatmaps visible", () => GetVisiblePanels<BeatmapPanel>().Any());
+            WaitForSelection(0, 0);
+
+            // Beatmap panels expand their selection area to cover holes from spacing.
+            ClickVisiblePanelWithOffset<BeatmapPanel>(1, new Vector2(0, -(CarouselItem.DEFAULT_HEIGHT / 2 + 1)));
+            WaitForSelection(0, 0);
+
+            // Panels with higher depth will handle clicks in the gutters for simplicity.
+            ClickVisiblePanelWithOffset<BeatmapPanel>(2, new Vector2(0, (CarouselItem.DEFAULT_HEIGHT / 2 + 1)));
+            WaitForSelection(0, 2);
+
+            ClickVisiblePanelWithOffset<BeatmapPanel>(3, new Vector2(0, (CarouselItem.DEFAULT_HEIGHT / 2 + 1)));
+            WaitForSelection(0, 3);
+        }
+
         private void checkSelectionIterating(bool isIterating)
         {
             object? selection = null;

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

@@ -1239,7 +1239,6 @@ namespace osu.Game.Tests.Visual.SongSelect
         }
 
         [Test]
-        [Solo]
         public void TestHardDeleteHandledCorrectly()
         {
             createSongSelect();

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -20,12 +20,23 @@ namespace osu.Game.Screens.SelectV2
     [Cached]
     public partial class BeatmapCarousel : Carousel<BeatmapInfo>
     {
+        public const float SPACING = 5f;
+
         private IBindableList<BeatmapSetInfo> detachedBeatmaps = null!;
 
         private readonly LoadingLayer loading;
 
         private readonly BeatmapCarouselFilterGrouping grouping;
 
+        protected override float GetSpacingBetweenPanels(CarouselItem top, CarouselItem bottom)
+        {
+            if (top.Model is BeatmapInfo || bottom.Model is BeatmapInfo)
+                // Beatmap difficulty panels do not overlap with themselves or any other panel.
+                return SPACING;
+
+            return -SPACING;
+        }
+
         public BeatmapCarousel()
         {
             DebounceDelay = 100;

osu.Game/Screens/SelectV2/BeatmapPanel.cs

@@ -24,6 +24,19 @@ namespace osu.Game.Screens.SelectV2
         private Box activationFlash = null!;
         private OsuSpriteText text = null!;
 
+        public override bool ReceivePositionalInputAt(Vector2 screenSpacePos)
+        {
+            var inputRectangle = DrawRectangle;
+
+            // Cover the gaps introduced by the spacing between BeatmapPanels so that clicks will not fall through the carousel.
+            //
+            // Caveat is that for simplicity, we are covering the full spacing, so panels with frontmost depth will have a slightly
+            // larger hit target.
+            inputRectangle = inputRectangle.Inflate(new MarginPadding { Vertical = BeatmapCarousel.SPACING });
+
+            return inputRectangle.Contains(ToLocalSpace(screenSpacePos));
+        }
+
         [BackgroundDependencyLoader]
         private void load()
         {

osu.Game/Screens/SelectV2/Carousel.cs

@@ -51,11 +51,6 @@ namespace osu.Game.Screens.SelectV2
         /// </summary>
         public float DistanceOffscreenToPreload { get; set; }
 
-        /// <summary>
-        /// Vertical space between panel layout. Negative value can be used to create an overlapping effect.
-        /// </summary>
-        protected float SpacingBetweenPanels { get; set; } = -5;
-
         /// <summary>
         /// When a new request arrives to change filtering, the number of milliseconds to wait before performing the filter.
         /// Regardless of any external debouncing, this is a safety measure to avoid triggering too many threaded operations.
@@ -130,6 +125,11 @@ namespace osu.Game.Screens.SelectV2
             selectionValid.Invalidate();
         }
 
+        /// <summary>
+        /// Returns the vertical spacing between two given carousel items. Negative value can be used to create an overlapping effect.
+        /// </summary>
+        protected virtual float GetSpacingBetweenPanels(CarouselItem top, CarouselItem bottom) => 0f;
+
         #endregion
 
         #region Properties and methods concerning implementations
@@ -267,7 +267,7 @@ namespace osu.Game.Screens.SelectV2
                     }
 
                     log("Updating Y positions");
-                    updateYPositions(items, visibleHalfHeight, SpacingBetweenPanels);
+                    updateYPositions(items, visibleHalfHeight);
                 }
                 catch (OperationCanceledException)
                 {
@@ -293,17 +293,26 @@ namespace osu.Game.Screens.SelectV2
             void log(string text) => Logger.Log($"Carousel[op {cts.GetHashCode().ToString()}] {stopwatch.ElapsedMilliseconds} ms: {text}");
         }
 
-        private static void updateYPositions(IEnumerable<CarouselItem> carouselItems, float offset, float spacing)
+        private void updateYPositions(IEnumerable<CarouselItem> carouselItems, float offset)
         {
+            CarouselItem? previousVisible = null;
+
             foreach (var item in carouselItems)
-                updateItemYPosition(item, ref offset, spacing);
+                updateItemYPosition(item, ref previousVisible, ref offset);
         }
 
-        private static void updateItemYPosition(CarouselItem item, ref float offset, float spacing)
+        private void updateItemYPosition(CarouselItem item, ref CarouselItem? previousVisible, ref float offset)
         {
+            float spacing = previousVisible == null || !item.IsVisible ? 0 : GetSpacingBetweenPanels(previousVisible, item);
+
+            offset += spacing;
             item.CarouselYPosition = offset;
+
             if (item.IsVisible)
-                offset += item.DrawHeight + spacing;
+            {
+                offset += item.DrawHeight;
+                previousVisible = item;
+            }
         }
 
         #endregion
@@ -461,7 +470,7 @@ namespace osu.Game.Screens.SelectV2
                 return;
             }
 
-            float spacing = SpacingBetweenPanels;
+            CarouselItem? lastVisible = null;
             int count = carouselItems.Count;
 
             Selection prevKeyboard = currentKeyboardSelection;
@@ -473,7 +482,7 @@ namespace osu.Game.Screens.SelectV2
             {
                 var item = carouselItems[i];
 
-                updateItemYPosition(item, ref yPos, spacing);
+                updateItemYPosition(item, ref lastVisible, ref yPos);
 
                 if (ReferenceEquals(item.Model, currentKeyboardSelection.Model))
                     currentKeyboardSelection = new Selection(item.Model, item, item.CarouselYPosition, i);