From ef39b8d9c03a62e177e191def950022d0c3552f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 May 2025 18:04:07 +0900 Subject: [PATCH 1/3] Rename and expose `BeatmapCarousel` selection handling variables For test usage and consistency. --- .../SongSelectV2/BeatmapCarouselTestScene.cs | 3 +++ osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 25 ++++++++++--------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index 1ac36b0cee..889a728d43 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -367,6 +367,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { public IEnumerable PostFilterBeatmaps = null!; + public new BeatmapSetInfo? ExpandedBeatmapSet => base.ExpandedBeatmapSet; + public new GroupDefinition? ExpandedGroup => base.ExpandedGroup; + protected override Task> FilterAsync(bool clearExistingPanels = false) { var filterAsync = base.FilterAsync(clearExistingPanels); diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 5642685b23..f6c5b8e8ef 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -174,8 +174,9 @@ namespace osu.Game.Screens.SelectV2 #region Selection handling - private GroupDefinition? lastSelectedGroup; - private BeatmapInfo? lastSelectedBeatmap; + protected GroupDefinition? ExpandedGroup { get; private set; } + + protected BeatmapSetInfo? ExpandedBeatmapSet { get; private set; } protected override void HandleItemActivated(CarouselItem item) { @@ -185,10 +186,10 @@ namespace osu.Game.Screens.SelectV2 { case GroupDefinition group: // Special case – collapsing an open group. - if (lastSelectedGroup == group) + if (ExpandedGroup == group) { - setExpansionStateOfGroup(lastSelectedGroup, false); - lastSelectedGroup = null; + setExpansionStateOfGroup(ExpandedGroup, false); + ExpandedGroup = null; return; } @@ -263,9 +264,9 @@ namespace osu.Game.Screens.SelectV2 private void setExpandedGroup(GroupDefinition group) { - if (lastSelectedGroup != null) - setExpansionStateOfGroup(lastSelectedGroup, false); - lastSelectedGroup = group; + if (ExpandedGroup != null) + setExpansionStateOfGroup(ExpandedGroup, false); + ExpandedGroup = group; setExpansionStateOfGroup(group, true); } @@ -319,10 +320,10 @@ namespace osu.Game.Screens.SelectV2 private void setExpandedSet(BeatmapInfo beatmapInfo) { - if (lastSelectedBeatmap != null) - setExpansionStateOfSetItems(lastSelectedBeatmap.BeatmapSet!, false); - lastSelectedBeatmap = beatmapInfo; - setExpansionStateOfSetItems(beatmapInfo.BeatmapSet!, true); + if (ExpandedBeatmapSet != null) + setExpansionStateOfSetItems(ExpandedBeatmapSet, false); + ExpandedBeatmapSet = beatmapInfo.BeatmapSet!; + setExpansionStateOfSetItems(ExpandedBeatmapSet, true); } private void setExpansionStateOfSetItems(BeatmapSetInfo set, bool expanded) From c94555388e27871f631ea4bdb3583ce5ab679e3c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 May 2025 18:13:36 +0900 Subject: [PATCH 2/3] Add test coverage showing expanded group getting reset on filter --- ...tSceneBeatmapCarouselDifficultyGrouping.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs index 9d908f94ed..c41cc2b59f 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs @@ -224,5 +224,25 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckDisplayedGroupsCount(3); CheckDisplayedBeatmapsCount(30); } + + [Test] + public void TestExpandedGroupStillExpandedAfterFilter() + { + SelectPrevGroup(); + + WaitForGroupSelection(2, 9); + AddAssert("expanded group is last", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(6)); + + SelectNextPanel(); + Select(); + + WaitForGroupSelection(2, 9); + AddAssert("expanded group is first", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(0)); + + // doesn't actually filter anything away, but triggers a filter. + ApplyToFilter("filter", c => c.SearchText = "Some"); + + AddAssert("expanded group is still first", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(0)); + } } } From df66bab31f7419cb9ce1d2be6e1399f8e9b3479f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 May 2025 17:13:59 +0900 Subject: [PATCH 3/3] SongSelectV2: Fix expanded group not being persisted over filter operations --- osu.Game/Graphics/Carousel/Carousel.cs | 10 ++++++++-- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index cf386307ca..bd53154c51 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -195,6 +195,13 @@ namespace osu.Game.Graphics.Carousel return filterTask; } + /// + /// Fired after a filter operation completed. + /// + protected virtual void HandleFilterCompleted() + { + } + /// /// Check whether two models are the same for display purposes. /// @@ -343,8 +350,7 @@ namespace osu.Game.Graphics.Carousel filterReusesPanels.Validate(); } - // Need to call this to ensure correct post-selection logic is handled on the new items list. - HandleItemSelected(currentSelection.Model); + HandleFilterCompleted(); refreshAfterSelection(); if (!Scroll.UserScrolling) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index f6c5b8e8ef..550a7e7699 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -244,6 +244,22 @@ namespace osu.Game.Screens.SelectV2 } } + protected override void HandleFilterCompleted() + { + base.HandleFilterCompleted(); + + // Store selected group before handling selection (it may implicitly change the expanded group). + var groupForReselection = ExpandedGroup; + + // Ensure correct post-selection logic is handled on the new items list. + // This will update the visual state of the selected item. + HandleItemSelected(CurrentSelection); + + // If a group was selected that is not the one containing the selection, reselect it. + if (groupForReselection != null) + setExpandedGroup(groupForReselection); + } + protected override bool CheckValidForGroupSelection(CarouselItem item) { switch (item.Model)