Merge branch 'master' into carousel-v2-spacing

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

@@ -1,6 +1,7 @@
 // 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;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
@@ -16,7 +17,6 @@ using osu.Game.Database;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Screens.Select;
-using osu.Game.Screens.Select.Filter;
 using osu.Game.Screens.SelectV2;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Resources;
@@ -53,16 +53,6 @@ namespace osu.Game.Tests.Visual.SongSelect
             Scheduler.AddDelayed(updateStats, 100, true);
         }
 
-        [SetUpSteps]
-        public virtual void SetUpSteps()
-        {
-            RemoveAllBeatmaps();
-
-            CreateCarousel();
-
-            SortBy(new FilterCriteria { Sort = SortMode.Title });
-        }
-
         protected void CreateCarousel()
         {
             AddStep("create components", () =>
@@ -146,6 +136,9 @@ namespace osu.Game.Tests.Visual.SongSelect
         protected void CheckNoSelection() => AddAssert("has no selection", () => Carousel.CurrentSelection, () => Is.Null);
         protected void CheckHasSelection() => AddAssert("has selection", () => Carousel.CurrentSelection, () => Is.Not.Null);
 
+        protected BeatmapPanel? GetSelectedPanel() => Carousel.ChildrenOfType<BeatmapPanel>().SingleOrDefault(p => p.Selected.Value);
+        protected GroupPanel? GetKeyboardSelectedPanel() => Carousel.ChildrenOfType<GroupPanel>().SingleOrDefault(p => p.KeyboardSelected.Value);
+
         protected void WaitForGroupSelection(int group, int panel)
         {
             AddUntilStep($"selected is group{group} panel{panel}", () =>
@@ -190,12 +183,41 @@ namespace osu.Game.Tests.Visual.SongSelect
         /// </summary>
         /// <param name="count">The count of beatmap sets to add.</param>
         /// <param name="fixedDifficultiesPerSet">If not null, the number of difficulties per set. If null, randomised difficulty count will be used.</param>
-        protected void AddBeatmaps(int count, int? fixedDifficultiesPerSet = null) => AddStep($"add {count} beatmaps", () =>
+        /// <param name="randomMetadata">Whether to randomise the metadata to make groupings more uniform.</param>
+        protected void AddBeatmaps(int count, int? fixedDifficultiesPerSet = null, bool randomMetadata = false) => AddStep($"add {count} beatmaps{(randomMetadata ? " with random data" : "")}", () =>
         {
             for (int i = 0; i < count; i++)
-                BeatmapSets.Add(TestResources.CreateTestBeatmapSetInfo(fixedDifficultiesPerSet ?? RNG.Next(1, 4)));
+            {
+                var beatmapSetInfo = TestResources.CreateTestBeatmapSetInfo(fixedDifficultiesPerSet ?? RNG.Next(1, 4));
+
+                if (randomMetadata)
+                {
+                    char randomCharacter = getRandomCharacter();
+
+                    var metadata = new BeatmapMetadata
+                    {
+                        // Create random metadata, then we can check if sorting works based on these
+                        Artist = $"{randomCharacter}ome Artist " + RNG.Next(0, 9),
+                        Title = $"{randomCharacter}ome Song (set id {beatmapSetInfo.OnlineID:000}) {Guid.NewGuid()}",
+                        Author = { Username = $"{randomCharacter}ome Guy " + RNG.Next(0, 9) },
+                    };
+
+                    foreach (var beatmap in beatmapSetInfo.Beatmaps)
+                        beatmap.Metadata = metadata.DeepClone();
+                }
+
+                BeatmapSets.Add(beatmapSetInfo);
+            }
         });
 
+        private static long randomCharPointer;
+
+        private static char getRandomCharacter()
+        {
+            const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz*";
+            return chars[(int)((randomCharPointer++ / 2) % chars.Length)];
+        }
+
         protected void RemoveAllBeatmaps() => AddStep("clear all beatmaps", () => BeatmapSets.Clear());
 
         protected void RemoveFirstBeatmap() =>

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

@@ -2,100 +2,65 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using NUnit.Framework;
-using osu.Framework.Graphics.Primitives;
-using osu.Framework.Testing;
 using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Filter;
-using osu.Game.Screens.SelectV2;
 using osu.Game.Tests.Resources;
 
 namespace osu.Game.Tests.Visual.SongSelect
 {
     /// <summary>
-    /// Currently covers adding and removing of items and scrolling.
+    /// Covers common steps which can be used for manual testing.
-    /// If we add more tests here, these two categories can likely be split out into separate scenes.
     /// </summary>
     [TestFixture]
-    public partial class TestSceneBeatmapCarouselV2Basics : BeatmapCarouselV2TestScene
+    public partial class TestSceneBeatmapCarouselV2 : BeatmapCarouselV2TestScene
     {
         [Test]
+        [Explicit]
         public void TestBasics()
         {
-            AddBeatmaps(1);
+            CreateCarousel();
+            RemoveAllBeatmaps();
+
+            AddBeatmaps(10, randomMetadata: true);
             AddBeatmaps(10);
+            AddBeatmaps(1);
+        }
+
+        [Test]
+        [Explicit]
+        public void TestSorting()
+        {
+            SortBy(new FilterCriteria { Sort = SortMode.Artist });
+            SortBy(new FilterCriteria { Group = GroupMode.Difficulty, Sort = SortMode.Difficulty });
+            SortBy(new FilterCriteria { Group = GroupMode.Artist, Sort = SortMode.Artist });
+        }
+
+        [Test]
+        [Explicit]
+        public void TestRemovals()
+        {
             RemoveFirstBeatmap();
             RemoveAllBeatmaps();
         }
 
         [Test]
-        public void TestOffScreenLoading()
+        [Explicit]
-        {
+        public void TestAddRemoveRepeatedOps()
-            AddStep("disable masking", () => Scroll.Masking = false);
-            AddStep("enable masking", () => Scroll.Masking = true);
-        }
-
-        [Test]
-        public void TestAddRemoveOneByOne()
         {
             AddRepeatStep("add beatmaps", () => BeatmapSets.Add(TestResources.CreateTestBeatmapSetInfo(RNG.Next(1, 4))), 20);
             AddRepeatStep("remove beatmaps", () => BeatmapSets.RemoveAt(RNG.Next(0, BeatmapSets.Count)), 20);
         }
 
         [Test]
-        public void TestSorting()
+        [Explicit]
+        public void TestMasking()
         {
-            AddBeatmaps(10);
+            AddStep("disable masking", () => Scroll.Masking = false);
-            SortBy(new FilterCriteria { Group = GroupMode.Difficulty, Sort = SortMode.Difficulty });
+            AddStep("enable masking", () => Scroll.Masking = true);
-            SortBy(new FilterCriteria { Sort = SortMode.Artist });
-        }
-
-        [Test]
-        public void TestScrollPositionMaintainedOnAddSecondSelected()
-        {
-            Quad positionBefore = default;
-
-            AddBeatmaps(10);
-            WaitForDrawablePanels();
-
-            AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2));
-            AddStep("scroll to selected item", () => Scroll.ScrollTo(Scroll.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value)));
-
-            WaitForScrolling();
-
-            AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<BeatmapPanel>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
-
-            RemoveFirstBeatmap();
-            WaitForSorting();
-
-            AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
-                () => Is.EqualTo(positionBefore));
-        }
-
-        [Test]
-        public void TestScrollPositionMaintainedOnAddLastSelected()
-        {
-            Quad positionBefore = default;
-
-            AddBeatmaps(10);
-            WaitForDrawablePanels();
-
-            AddStep("scroll to last item", () => Scroll.ScrollToEnd(false));
-
-            AddStep("select last beatmap", () => Carousel.CurrentSelection = BeatmapSets.Last());
-
-            WaitForScrolling();
-
-            AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<BeatmapPanel>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
-
-            RemoveFirstBeatmap();
-            WaitForSorting();
-            AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
-                () => Is.EqualTo(positionBefore));
         }
 
         [Test]

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

@@ -0,0 +1,177 @@
+// 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;
+
+namespace osu.Game.Tests.Visual.SongSelect
+{
+    [TestFixture]
+    public partial class TestSceneBeatmapCarouselV2ArtistGrouping : BeatmapCarouselV2TestScene
+    {
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            RemoveAllBeatmaps();
+            CreateCarousel();
+            SortBy(new FilterCriteria { Group = GroupMode.Artist, Sort = SortMode.Artist });
+
+            AddBeatmaps(10, 3, true);
+            WaitForDrawablePanels();
+        }
+
+        [Test]
+        public void TestOpenCloseGroupWithNoSelectionMouse()
+        {
+            AddAssert("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddUntilStep("no sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            CheckNoSelection();
+
+            ClickVisiblePanel<GroupPanel>(0);
+
+            AddUntilStep("some sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.GreaterThan(0));
+            AddUntilStep("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            CheckNoSelection();
+
+            ClickVisiblePanel<GroupPanel>(0);
+
+            AddUntilStep("no sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddUntilStep("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            CheckNoSelection();
+        }
+
+        [Test]
+        public void TestOpenCloseGroupWithNoSelectionKeyboard()
+        {
+            AddAssert("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddUntilStep("no sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            CheckNoSelection();
+
+            SelectNextPanel();
+            Select();
+
+            AddUntilStep("some sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.GreaterThan(0));
+            AddUntilStep("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddAssert("keyboard selected is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
+            CheckNoSelection();
+
+            Select();
+
+            AddUntilStep("no sets visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddUntilStep("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
+            AddAssert("keyboard selected is collapsed", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.False);
+            CheckNoSelection();
+        }
+
+        [Test]
+        public void TestCarouselRemembersSelection()
+        {
+            SelectNextGroup();
+
+            object? selection = null;
+
+            AddStep("store drawable selection", () => selection = GetSelectedPanel()?.Item?.Model);
+
+            CheckHasSelection();
+            AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null);
+            AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
+
+            RemoveAllBeatmaps();
+            AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null);
+
+            AddBeatmaps(10);
+            WaitForDrawablePanels();
+
+            CheckHasSelection();
+            AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null);
+
+            AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!));
+
+            AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
+            AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection));
+            AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True);
+
+            ClickVisiblePanel<GroupPanel>(0);
+            AddUntilStep("carousel item not visible", GetSelectedPanel, () => Is.Null);
+
+            ClickVisiblePanel<GroupPanel>(0);
+            AddUntilStep("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True);
+        }
+
+        [Test]
+        public void TestGroupSelectionOnHeader()
+        {
+            SelectNextGroup();
+            WaitForGroupSelection(0, 1);
+
+            SelectPrevPanel();
+            SelectPrevPanel();
+
+            AddAssert("keyboard selected panel is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
+
+            SelectPrevGroup();
+
+            WaitForGroupSelection(0, 1);
+            AddAssert("keyboard selected panel is contracted", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.False);
+
+            SelectPrevGroup();
+
+            WaitForGroupSelection(0, 1);
+            AddAssert("keyboard selected panel is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
+        }
+
+        [Test]
+        public void TestKeyboardSelection()
+        {
+            SelectNextPanel();
+            SelectNextPanel();
+            SelectNextPanel();
+            SelectNextPanel();
+            CheckNoSelection();
+
+            // open first group
+            Select();
+            CheckNoSelection();
+            AddUntilStep("some beatmaps visible", () => Carousel.ChildrenOfType<BeatmapSetPanel>().Count(p => p.Alpha > 0), () => Is.GreaterThan(0));
+
+            SelectNextPanel();
+            Select();
+            WaitForGroupSelection(3, 1);
+
+            SelectNextGroup();
+            WaitForGroupSelection(3, 5);
+
+            SelectNextGroup();
+            WaitForGroupSelection(4, 1);
+
+            SelectPrevGroup();
+            WaitForGroupSelection(3, 5);
+
+            SelectNextGroup();
+            WaitForGroupSelection(4, 1);
+
+            SelectNextGroup();
+            WaitForGroupSelection(4, 5);
+
+            SelectNextGroup();
+            WaitForGroupSelection(0, 1);
+
+            SelectNextPanel();
+            SelectNextPanel();
+            SelectNextPanel();
+            SelectNextPanel();
+
+            SelectNextGroup();
+            WaitForGroupSelection(0, 1);
+
+            SelectNextPanel();
+            SelectNextGroup();
+            WaitForGroupSelection(1, 1);
+        }
+    }
+}

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

@@ -15,23 +15,22 @@ using osuTK.Input;
 namespace osu.Game.Tests.Visual.SongSelect
 {
     [TestFixture]
-    public partial class TestSceneBeatmapCarouselV2GroupSelection : BeatmapCarouselV2TestScene
+    public partial class TestSceneBeatmapCarouselV2DifficultyGrouping : BeatmapCarouselV2TestScene
     {
-        public override void SetUpSteps()
+        [SetUpSteps]
+        public void SetUpSteps()
         {
             RemoveAllBeatmaps();
-
             CreateCarousel();
-
             SortBy(new FilterCriteria { Group = GroupMode.Difficulty, Sort = SortMode.Difficulty });
+
+            AddBeatmaps(10, 3);
+            WaitForDrawablePanels();
         }
 
         [Test]
         public void TestOpenCloseGroupWithNoSelectionMouse()
         {
-            AddBeatmaps(10, 5);
-            WaitForDrawablePanels();
-
             AddAssert("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
             CheckNoSelection();
 
@@ -47,86 +46,79 @@ namespace osu.Game.Tests.Visual.SongSelect
         [Test]
         public void TestOpenCloseGroupWithNoSelectionKeyboard()
         {
-            AddBeatmaps(10, 5);
-            WaitForDrawablePanels();
-
             AddAssert("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
             CheckNoSelection();
 
             SelectNextPanel();
             Select();
             AddUntilStep("some beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.GreaterThan(0));
-            AddAssert("keyboard selected is expanded", () => getKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
+            AddAssert("keyboard selected is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
             CheckNoSelection();
 
             Select();
             AddUntilStep("no beatmaps visible", () => Carousel.ChildrenOfType<BeatmapPanel>().Count(p => p.Alpha > 0), () => Is.Zero);
-            AddAssert("keyboard selected is collapsed", () => getKeyboardSelectedPanel()?.Expanded.Value, () => Is.False);
+            AddAssert("keyboard selected is collapsed", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.False);
             CheckNoSelection();
-
-            GroupPanel? getKeyboardSelectedPanel() => Carousel.ChildrenOfType<GroupPanel>().SingleOrDefault(p => p.KeyboardSelected.Value);
         }
 
         [Test]
         public void TestCarouselRemembersSelection()
         {
-            AddBeatmaps(10);
-            WaitForDrawablePanels();
-
             SelectNextGroup();
 
             object? selection = null;
 
-            AddStep("store drawable selection", () => selection = getSelectedPanel()?.Item?.Model);
+            AddStep("store drawable selection", () => selection = GetSelectedPanel()?.Item?.Model);
 
             CheckHasSelection();
             AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null);
             AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
 
             RemoveAllBeatmaps();
-            AddUntilStep("no drawable selection", getSelectedPanel, () => Is.Null);
+            AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null);
 
             AddBeatmaps(10);
             WaitForDrawablePanels();
 
             CheckHasSelection();
-            AddAssert("no drawable selection", getSelectedPanel, () => Is.Null);
+            AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null);
 
             AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!));
 
             AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
-            AddUntilStep("drawable selection restored", () => getSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection));
+            AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection));
-            AddAssert("carousel item is visible", () => getSelectedPanel()?.Item?.IsVisible, () => Is.True);
+            AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True);
 
             ClickVisiblePanel<GroupPanel>(0);
-            AddUntilStep("carousel item not visible", getSelectedPanel, () => Is.Null);
+            AddUntilStep("carousel item not visible", GetSelectedPanel, () => Is.Null);
 
             ClickVisiblePanel<GroupPanel>(0);
-            AddUntilStep("carousel item is visible", () => getSelectedPanel()?.Item?.IsVisible, () => Is.True);
+            AddUntilStep("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True);
-
-            BeatmapPanel? getSelectedPanel() => Carousel.ChildrenOfType<BeatmapPanel>().SingleOrDefault(p => p.Selected.Value);
         }
 
         [Test]
         public void TestGroupSelectionOnHeader()
         {
-            AddBeatmaps(10, 3);
-            WaitForDrawablePanels();
-
             SelectNextGroup();
             WaitForGroupSelection(0, 0);
 
             SelectPrevPanel();
+            AddAssert("keyboard selected panel is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
+
             SelectPrevGroup();
-            WaitForGroupSelection(2, 9);
+
+            WaitForGroupSelection(0, 0);
+            AddAssert("keyboard selected panel is contracted", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.False);
+
+            SelectPrevGroup();
+
+            WaitForGroupSelection(0, 0);
+            AddAssert("keyboard selected panel is expanded", () => GetKeyboardSelectedPanel()?.Expanded.Value, () => Is.True);
         }
 
         [Test]
         public void TestKeyboardSelection()
         {
-            AddBeatmaps(10, 3);
-            WaitForDrawablePanels();
-
             SelectNextPanel();
             SelectNextPanel();
             SelectNextPanel();

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

@@ -6,6 +6,8 @@ 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;
@@ -13,8 +15,16 @@ using osuTK.Input;
 namespace osu.Game.Tests.Visual.SongSelect
 {
     [TestFixture]
-    public partial class TestSceneBeatmapCarouselV2Selection : BeatmapCarouselV2TestScene
+    public partial class TestSceneBeatmapCarouselV2NoGrouping : BeatmapCarouselV2TestScene
     {
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            RemoveAllBeatmaps();
+            CreateCarousel();
+            SortBy(new FilterCriteria { Sort = SortMode.Title });
+        }
+
         /// <summary>
         /// Keyboard selection via up and down arrows doesn't actually change the selection until
         /// the select key is pressed.
@@ -79,27 +89,26 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             object? selection = null;
 
-            AddStep("store drawable selection", () => selection = getSelectedPanel()?.Item?.Model);
+            AddStep("store drawable selection", () => selection = GetSelectedPanel()?.Item?.Model);
 
             CheckHasSelection();
             AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null);
             AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
 
             RemoveAllBeatmaps();
-            AddUntilStep("no drawable selection", getSelectedPanel, () => Is.Null);
+            AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null);
 
             AddBeatmaps(10);
             WaitForDrawablePanels();
 
             CheckHasSelection();
-            AddAssert("no drawable selection", getSelectedPanel, () => Is.Null);
+            AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null);
 
             AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!));
 
-            AddUntilStep("drawable selection restored", () => getSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection));
+            AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
-            AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection));
+            AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection));
-
+            AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True);
-            BeatmapPanel? getSelectedPanel() => Carousel.ChildrenOfType<BeatmapPanel>().SingleOrDefault(p => p.Selected.Value);
         }
 
         [Test]
@@ -130,6 +139,25 @@ namespace osu.Game.Tests.Visual.SongSelect
             WaitForSelection(0, 0);
         }
 
+        [Test]
+        public void TestGroupSelectionOnHeader()
+        {
+            AddBeatmaps(10, 3);
+            WaitForDrawablePanels();
+
+            SelectNextGroup();
+            SelectNextGroup();
+            WaitForSelection(1, 0);
+
+            SelectPrevPanel();
+            SelectPrevGroup();
+            WaitForSelection(1, 0);
+
+            SelectPrevPanel();
+            SelectNextGroup();
+            WaitForSelection(1, 0);
+        }
+
         [Test]
         public void TestKeyboardSelection()
         {

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

@@ -0,0 +1,65 @@
+// 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.Graphics.Primitives;
+using osu.Framework.Testing;
+using osu.Game.Screens.Select;
+using osu.Game.Screens.SelectV2;
+
+namespace osu.Game.Tests.Visual.SongSelect
+{
+    [TestFixture]
+    public partial class TestSceneBeatmapCarouselV2Scrolling : BeatmapCarouselV2TestScene
+    {
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            RemoveAllBeatmaps();
+            CreateCarousel();
+            SortBy(new FilterCriteria());
+
+            AddBeatmaps(10);
+            WaitForDrawablePanels();
+        }
+
+        [Test]
+        public void TestScrollPositionMaintainedOnAddSecondSelected()
+        {
+            Quad positionBefore = default;
+
+            AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First());
+            AddStep("scroll to selected item", () => Scroll.ScrollTo(Scroll.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value)));
+
+            WaitForScrolling();
+
+            AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<BeatmapPanel>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
+
+            RemoveFirstBeatmap();
+            WaitForSorting();
+
+            AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
+                () => Is.EqualTo(positionBefore));
+        }
+
+        [Test]
+        public void TestScrollPositionMaintainedOnAddLastSelected()
+        {
+            Quad positionBefore = default;
+
+            AddStep("scroll to last item", () => Scroll.ScrollToEnd(false));
+
+            AddStep("select last beatmap", () => Carousel.CurrentSelection = BeatmapSets.Last().Beatmaps.Last());
+
+            WaitForScrolling();
+
+            AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<BeatmapPanel>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
+
+            RemoveFirstBeatmap();
+            WaitForSorting();
+            AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<BeatmapPanel>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
+                () => Is.EqualTo(positionBefore));
+        }
+    }
+}

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -106,11 +106,9 @@ namespace osu.Game.Screens.SelectV2
         private GroupDefinition? lastSelectedGroup;
         private BeatmapInfo? lastSelectedBeatmap;
 
-        protected override bool HandleItemSelected(object? model)
+        protected override void HandleItemActivated(CarouselItem item)
         {
-            base.HandleItemSelected(model);
+            switch (item.Model)
-
-            switch (model)
             {
                 case GroupDefinition group:
                     // Special case – collapsing an open group.
@@ -118,37 +116,42 @@ namespace osu.Game.Screens.SelectV2
                     {
                         setExpansionStateOfGroup(lastSelectedGroup, false);
                         lastSelectedGroup = null;
-                        return false;
+                        return;
                     }
 
                     setExpandedGroup(group);
-                    return false;
+                    return;
 
                 case BeatmapSetInfo setInfo:
                     // Selecting a set isn't valid – let's re-select the first difficulty.
                     CurrentSelection = setInfo.Beatmaps.First();
-                    return false;
+                    return;
 
                 case BeatmapInfo beatmapInfo:
-
+                    CurrentSelection = beatmapInfo;
-                    // If we have groups, we need to account for them.
+                    return;
-                    if (Criteria.SplitOutDifficulties)
-                    {
-                        // Find the containing group. There should never be too many groups so iterating is efficient enough.
-                        GroupDefinition? group = grouping.GroupItems.SingleOrDefault(kvp => kvp.Value.Any(i => ReferenceEquals(i.Model, beatmapInfo))).Key;
-
-                        if (group != null)
-                            setExpandedGroup(group);
             }
-                    else
+        }
+
+        protected override void HandleItemSelected(object? model)
         {
+            base.HandleItemSelected(model);
+
+            switch (model)
+            {
+                case BeatmapSetInfo:
+                case GroupDefinition:
+                    throw new InvalidOperationException("Groups should never become selected");
+
+                case BeatmapInfo beatmapInfo:
+                    // Find any containing group. There should never be too many groups so iterating is efficient enough.
+                    GroupDefinition? containingGroup = grouping.GroupItems.SingleOrDefault(kvp => kvp.Value.Any(i => ReferenceEquals(i.Model, beatmapInfo))).Key;
+
+                    if (containingGroup != null)
+                        setExpandedGroup(containingGroup);
                     setExpandedSet(beatmapInfo);
+                    break;
             }
-
-                    return true;
-            }
-
-            return true;
         }
 
         protected override bool CheckValidForGroupSelection(CarouselItem item)
@@ -159,7 +162,7 @@ namespace osu.Game.Screens.SelectV2
                     return true;
 
                 case BeatmapInfo:
-                    return Criteria.SplitOutDifficulties;
+                    return !grouping.BeatmapSetsGroupedTogether;
 
                 case GroupDefinition:
                     return false;
@@ -180,13 +183,47 @@ namespace osu.Game.Screens.SelectV2
         private void setExpansionStateOfGroup(GroupDefinition group, bool expanded)
         {
             if (grouping.GroupItems.TryGetValue(group, out var items))
+            {
+                if (expanded)
                 {
                     foreach (var i in items)
                     {
-                    if (i.Model is GroupDefinition)
+                        switch (i.Model)
-                        i.IsExpanded = expanded;
+                        {
+                            case GroupDefinition:
+                                i.IsExpanded = true;
+                                break;
+
+                            case BeatmapSetInfo set:
+                                // Case where there are set headers, header should be visible
+                                // and items should use the set's expanded state.
+                                i.IsVisible = true;
+                                setExpansionStateOfSetItems(set, i.IsExpanded);
+                                break;
+
+                            default:
+                                // Case where there are no set headers, all items should be visible.
+                                if (!grouping.BeatmapSetsGroupedTogether)
+                                    i.IsVisible = true;
+                                break;
+                        }
+                    }
+                }
                 else
-                        i.IsVisible = expanded;
+                {
+                    foreach (var i in items)
+                    {
+                        switch (i.Model)
+                        {
+                            case GroupDefinition:
+                                i.IsExpanded = false;
+                                break;
+
+                            default:
+                                i.IsVisible = false;
+                                break;
+                        }
+                    }
                 }
             }
         }

osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs

@@ -14,6 +14,8 @@ namespace osu.Game.Screens.SelectV2
 {
     public class BeatmapCarouselFilterGrouping : ICarouselFilter
     {
+        public bool BeatmapSetsGroupedTogether { get; private set; }
+
         /// <summary>
         /// Beatmap sets contain difficulties as related panels. This dictionary holds the relationships between set-difficulties to allow expanding them on selection.
         /// </summary>
@@ -36,8 +38,6 @@ namespace osu.Game.Screens.SelectV2
 
         public async Task<IEnumerable<CarouselItem>> Run(IEnumerable<CarouselItem> items, CancellationToken cancellationToken) => await Task.Run(() =>
         {
-            bool groupSetsTogether;
-
             setItems.Clear();
             groupItems.Clear();
 
@@ -48,12 +48,39 @@ namespace osu.Game.Screens.SelectV2
             switch (criteria.Group)
             {
                 default:
-                    groupSetsTogether = true;
+                    BeatmapSetsGroupedTogether = true;
                     newItems.AddRange(items);
                     break;
 
+                case GroupMode.Artist:
+                    BeatmapSetsGroupedTogether = true;
+                    char groupChar = (char)0;
+
+                    foreach (var item in items)
+                    {
+                        cancellationToken.ThrowIfCancellationRequested();
+
+                        var b = (BeatmapInfo)item.Model;
+
+                        char beatmapFirstChar = char.ToUpperInvariant(b.Metadata.Artist[0]);
+
+                        if (beatmapFirstChar > groupChar)
+                        {
+                            groupChar = beatmapFirstChar;
+                            var groupDefinition = new GroupDefinition($"{groupChar}");
+                            var groupItem = new CarouselItem(groupDefinition) { DrawHeight = GroupPanel.HEIGHT };
+
+                            newItems.Add(groupItem);
+                            groupItems[groupDefinition] = new HashSet<CarouselItem> { groupItem };
+                        }
+
+                        newItems.Add(item);
+                    }
+
+                    break;
+
                 case GroupMode.Difficulty:
-                    groupSetsTogether = false;
+                    BeatmapSetsGroupedTogether = false;
                     int starGroup = int.MinValue;
 
                     foreach (var item in items)
@@ -86,7 +113,7 @@ namespace osu.Game.Screens.SelectV2
             // Add set headers wherever required.
             CarouselItem? lastItem = null;
 
-            if (groupSetsTogether)
+            if (BeatmapSetsGroupedTogether)
             {
                 for (int i = 0; i < newItems.Count; i++)
                 {
@@ -96,7 +123,7 @@ namespace osu.Game.Screens.SelectV2
 
                     if (item.Model is BeatmapInfo beatmap)
                     {
-                        bool newBeatmapSet = lastItem == null || (lastItem.Model is BeatmapInfo lastBeatmap && lastBeatmap.BeatmapSet!.ID != beatmap.BeatmapSet!.ID);
+                        bool newBeatmapSet = lastItem?.Model is not BeatmapInfo lastBeatmap || lastBeatmap.BeatmapSet!.ID != beatmap.BeatmapSet!.ID;
 
                         if (newBeatmapSet)
                         {

osu.Game/Screens/SelectV2/BeatmapPanel.cs

@@ -96,13 +96,7 @@ namespace osu.Game.Screens.SelectV2
 
         protected override bool OnClick(ClickEvent e)
         {
-            if (carousel.CurrentSelection != Item!.Model)
+            carousel.Activate(Item!);
-            {
-                carousel.CurrentSelection = Item!.Model;
-                return true;
-            }
-
-            carousel.TryActivateSelection();
             return true;
         }
 

osu.Game/Screens/SelectV2/BeatmapSetPanel.cs

@@ -1,7 +1,6 @@
 // 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;
 using System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -93,7 +92,7 @@ namespace osu.Game.Screens.SelectV2
 
         protected override bool OnClick(ClickEvent e)
         {
-            carousel.CurrentSelection = Item!.Model;
+            carousel.Activate(Item!);
             return true;
         }
 
@@ -108,8 +107,6 @@ namespace osu.Game.Screens.SelectV2
 
         public void Activated()
         {
-            // sets should never be activated.
-            throw new InvalidOperationException();
         }
 
         #endregion

osu.Game/Screens/SelectV2/Carousel.cs

@@ -16,6 +16,7 @@ using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.Pooling;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Input.Events;
+using osu.Framework.Layout;
 using osu.Framework.Logging;
 using osu.Framework.Utils;
 using osu.Game.Graphics.Containers;
@@ -89,26 +90,39 @@ namespace osu.Game.Screens.SelectV2
         public object? CurrentSelection
         {
             get => currentSelection.Model;
-            set => setSelection(value);
+            set
+            {
+                if (currentSelection.Model != value)
+                {
+                    HandleItemSelected(value);
+
+                    if (currentSelection.Model != null)
+                        HandleItemDeselected(currentSelection.Model);
+
+                    currentKeyboardSelection = new Selection(value);
+                    currentSelection = currentKeyboardSelection;
+                    selectionValid.Invalidate();
+                }
+                else if (currentKeyboardSelection.Model != value)
+                {
+                    // Even if the current selection matches, let's ensure the keyboard selection is reset
+                    // to the newly selected object. This matches user expectations (for now).
+                    currentKeyboardSelection = currentSelection;
+                    selectionValid.Invalidate();
+                }
+            }
         }
 
         /// <summary>
-        /// Activate the current selection, if a selection exists and matches keyboard selection.
+        /// Activate the specified item.
-        /// If keyboard selection does not match selection, this will transfer the selection on first invocation.
         /// </summary>
-        public void TryActivateSelection()
+        /// <param name="item"></param>
+        public void Activate(CarouselItem item)
         {
-            if (currentSelection.CarouselItem != currentKeyboardSelection.CarouselItem)
+            (GetMaterialisedDrawableForItem(item) as ICarouselPanel)?.Activated();
-            {
+            HandleItemActivated(item);
-                CurrentSelection = currentKeyboardSelection.Model;
-                return;
-            }
 
-            if (currentSelection.CarouselItem != null)
+            selectionValid.Invalidate();
-            {
-                (GetMaterialisedDrawableForItem(currentSelection.CarouselItem) as ICarouselPanel)?.Activated();
-                HandleItemActivated(currentSelection.CarouselItem);
-            }
         }
 
         /// <summary>
@@ -176,30 +190,28 @@ namespace osu.Game.Screens.SelectV2
         protected virtual bool CheckValidForGroupSelection(CarouselItem item) => true;
 
         /// <summary>
-        /// Called when an item is "selected".
+        /// Called after an item becomes the <see cref="CurrentSelection"/>.
+        /// Should be used to handle any group expansion, item visibility changes, etc.
         /// </summary>
-        /// <returns>Whether the item should be selected.</returns>
+        protected virtual void HandleItemSelected(object? model) { }
-        protected virtual bool HandleItemSelected(object? model) => true;
 
         /// <summary>
-        /// Called when an item is "deselected".
+        /// Called when the <see cref="CurrentSelection"/> changes to a new selection.
+        /// Should be used to handle any group expansion, item visibility changes, etc.
         /// </summary>
-        protected virtual void HandleItemDeselected(object? model)
+        protected virtual void HandleItemDeselected(object? model) { }
-        {
-        }
 
         /// <summary>
-        /// Called when an item is "activated".
+        /// Called when an item is activated via user input (keyboard traversal or a mouse click).
         /// </summary>
         /// <remarks>
-        /// An activated item should for instance:
+        /// An activated item should decide to perform an action, such as:
-        /// - Open or close a folder
+        /// - Change its expanded state (and show / hide children items).
-        /// - Start gameplay on a beatmap difficulty.
+        /// - Set the item to the <see cref="CurrentSelection"/>.
+        /// - Start gameplay on a beatmap difficulty if already selected.
         /// </remarks>
         /// <param name="item">The carousel item which was activated.</param>
-        protected virtual void HandleItemActivated(CarouselItem item)
+        protected virtual void HandleItemActivated(CarouselItem item) { }
-        {
-        }
 
         #endregion
 
@@ -312,7 +324,8 @@ namespace osu.Game.Screens.SelectV2
             switch (e.Action)
             {
                 case GlobalAction.Select:
-                    TryActivateSelection();
+                    if (currentKeyboardSelection.CarouselItem != null)
+                        Activate(currentKeyboardSelection.CarouselItem);
                     return true;
 
                 case GlobalAction.SelectNext:
@@ -381,32 +394,29 @@ namespace osu.Game.Screens.SelectV2
 
             // If the user has a different keyboard selection and requests
             // group selection, first transfer the keyboard selection to actual selection.
-            if (currentSelection.CarouselItem != currentKeyboardSelection.CarouselItem)
+            if (currentKeyboardSelection.CarouselItem != null && currentSelection.CarouselItem != currentKeyboardSelection.CarouselItem)
             {
-                TryActivateSelection();
+                Activate(currentKeyboardSelection.CarouselItem);
-
-                // There's a chance this couldn't resolve, at which point continue with standard traversal.
-                if (currentSelection.CarouselItem == currentKeyboardSelection.CarouselItem)
                 return;
             }
 
             int originalIndex;
             int newIndex;
 
-            if (currentSelection.Index == null)
+            if (currentKeyboardSelection.Index == null)
             {
                 // If there's no current selection, start from either end of the full list.
                 newIndex = originalIndex = direction > 0 ? carouselItems.Count - 1 : 0;
             }
             else
             {
-                newIndex = originalIndex = currentSelection.Index.Value;
+                newIndex = originalIndex = currentKeyboardSelection.Index.Value;
 
                 // As a second special case, if we're group selecting backwards and the current selection isn't a group,
                 // 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 (direction < 0)
                 {
-                    while (!CheckValidForGroupSelection(carouselItems[newIndex]))
+                    while (newIndex > 0 && !CheckValidForGroupSelection(carouselItems[newIndex]))
                         newIndex--;
                 }
             }
@@ -420,7 +430,7 @@ namespace osu.Game.Screens.SelectV2
 
                 if (CheckValidForGroupSelection(newItem))
                 {
-                    setSelection(newItem.Model);
+                    HandleItemActivated(newItem);
                     return;
                 }
             } while (newIndex != originalIndex);
@@ -435,23 +445,6 @@ namespace osu.Game.Screens.SelectV2
         private Selection currentKeyboardSelection = new Selection();
         private Selection currentSelection = new Selection();
 
-        private void setSelection(object? model)
-        {
-            if (currentSelection.Model == model)
-                return;
-
-            if (HandleItemSelected(model))
-            {
-                if (currentSelection.Model != null)
-                    HandleItemDeselected(currentSelection.Model);
-
-                currentKeyboardSelection = new Selection(model);
-                currentSelection = currentKeyboardSelection;
-            }
-
-            selectionValid.Invalidate();
-        }
-
         private void setKeyboardSelection(object? model)
         {
             currentKeyboardSelection = new Selection(model);
@@ -687,6 +680,15 @@ namespace osu.Game.Screens.SelectV2
             carouselPanel.Expanded.Value = false;
         }
 
+        protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source)
+        {
+            // handles the vertical size of the carousel changing (ie. on window resize when aspect ratio has changed).
+            if (invalidation.HasFlag(Invalidation.DrawSize))
+                selectionValid.Invalidate();
+
+            return base.OnInvalidate(invalidation, source);
+        }
+
         #endregion
 
         #region Internal helper classes

osu.Game/Screens/SelectV2/GroupPanel.cs

@@ -1,7 +1,6 @@
 // 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;
 using System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -106,7 +105,7 @@ namespace osu.Game.Screens.SelectV2
 
         protected override bool OnClick(ClickEvent e)
         {
-            carousel.CurrentSelection = Item!.Model;
+            carousel.Activate(Item!);
             return true;
         }
 
@@ -121,8 +120,6 @@ namespace osu.Game.Screens.SelectV2
 
         public void Activated()
         {
-            // sets should never be activated.
-            throw new InvalidOperationException();
         }
 
         #endregion