From 76f05a4837e3da5d912d629735d1adb6f755fcc8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jan 2026 19:15:57 +0900 Subject: [PATCH 1/7] Add failing test showing incorrect behaviour on random select with collapsed groups --- .../Visual/SongSelectV2/BeatmapCarouselTestScene.cs | 1 + .../TestSceneBeatmapCarouselArtistGrouping.cs | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index 02c017f570..d961946c04 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -222,6 +222,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 protected void SelectPrevPanel() => AddStep("select prev panel", () => InputManager.Key(Key.Up)); protected void SelectNextSet() => AddStep("select next set", () => InputManager.Key(Key.Right)); protected void SelectPrevSet() => AddStep("select prev set", () => InputManager.Key(Key.Left)); + protected void SelectRandomSet() => AddStep("select random set", () => Carousel.NextRandom()); protected void Select() => AddStep("select", () => InputManager.Key(Key.Enter)); diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs index 2390261cdb..2829364c68 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -253,6 +253,19 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckDisplayedBeatmapsCount(30); } + [Test] + public void TestGroupDoesExpandAfterRandomTraversal() + { + SelectNextSet(); + + ToggleGroupCollapse(); + AddAssert("group not expanded", () => Carousel.ExpandedGroup, () => Is.Null); + + SelectRandomSet(); + + AddAssert("group expanded", () => Carousel.ExpandedGroup, () => Is.Not.Null); + } + [Test] public void TestGroupDoesNotExpandAgainOnRefilterIfManuallyCollapsed() { From fdf9092f4143270964eb08ff9de182480b4fc167 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jan 2026 19:14:59 +0900 Subject: [PATCH 2/7] Revert "Fix current beatmap set being incorrectly expanded after collapsing group with current selection" This reverts commit 9ba99660786a2c25e73c866d70a492576ca02cac. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 4519e594c6..79acfb5f9e 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -378,7 +378,7 @@ namespace osu.Game.Screens.SelectV2 if (userCollapsedGroup) { - if (grouping.BeatmapSetsGroupedTogether && CurrentGroupedBeatmap != null && CheckModelEquality(group, CurrentGroupedBeatmap.Group)) + if (grouping.BeatmapSetsGroupedTogether && CurrentGroupedBeatmap != null) setExpandedSet(new GroupedBeatmapSet(CurrentGroupedBeatmap.Group, CurrentGroupedBeatmap.Beatmap.BeatmapSet!)); userCollapsedGroup = false; } From 8c86ff36a26523f38cfab74c0606aaa8550dc240 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jan 2026 19:15:21 +0900 Subject: [PATCH 3/7] Revert "Do not forcibly re-expand carousel groups on refilters if the user manually collapsed them" This reverts commit bc4d5d07d7b70a793c2d3fc32c3cef3f645d3676. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 79acfb5f9e..e17fbafd64 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -352,12 +352,6 @@ namespace osu.Game.Screens.SelectV2 } } - /// - /// Tracks whether the user has manually requested to collapse an open group. - /// In this case, refilters should not forcibly expand groups until the user expands a group again themselves. - /// - private bool userCollapsedGroup; - protected override void HandleItemActivated(CarouselItem item) { try @@ -370,19 +364,11 @@ namespace osu.Game.Screens.SelectV2 { setExpansionStateOfGroup(ExpandedGroup, false); ExpandedGroup = null; - userCollapsedGroup = true; return; } setExpandedGroup(group); - if (userCollapsedGroup) - { - if (grouping.BeatmapSetsGroupedTogether && CurrentGroupedBeatmap != null) - setExpandedSet(new GroupedBeatmapSet(CurrentGroupedBeatmap.Group, CurrentGroupedBeatmap.Beatmap.BeatmapSet!)); - userCollapsedGroup = false; - } - // If the active selection is within this group, it should get keyboard focus immediately. if (CurrentSelectionItem?.IsVisible == true && CurrentSelection is GroupedBeatmap gb) RequestSelection(gb); @@ -421,9 +407,6 @@ namespace osu.Game.Screens.SelectV2 throw new InvalidOperationException("Groups should never become selected"); case GroupedBeatmap groupedBeatmap: - if (userCollapsedGroup) - break; - setExpandedGroup(groupedBeatmap.Group); if (grouping.BeatmapSetsGroupedTogether) @@ -798,9 +781,6 @@ namespace osu.Game.Screens.SelectV2 Criteria = criteria; - if (criteria.Group == GroupMode.None) - userCollapsedGroup = false; - loadingDebounce ??= Scheduler.AddDelayed(() => { if (loading.State.Value == Visibility.Visible) From 8860eec9f9bf9972f1cacd005df64ba02f1a72ea Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jan 2026 19:20:44 +0900 Subject: [PATCH 4/7] Handle expand set logic locally rather than calling `HandleItemSelected` This avoids the implicit group expansion happening that the added code was attempting to fix. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index e17fbafd64..e0f46af409 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -32,7 +32,6 @@ using osu.Game.Online.API; using osu.Game.Rulesets; using osu.Game.Scoring; using osu.Game.Screens.Select; -using osu.Game.Screens.Select.Filter; using Realms; namespace osu.Game.Screens.SelectV2 @@ -495,8 +494,9 @@ namespace osu.Game.Screens.SelectV2 if (groupingRemainsOff || groupStillValid) { - // Only update the visual state of the selected item. - HandleItemSelected(currentGroupedBeatmap); + // Update the visual state of the selected item if it should still be expanded post filter. + if (currentGroupedBeatmap != null && currentGroupedBeatmap.Group == groupForReselection) + setExpandedSet(new GroupedBeatmapSet(currentGroupedBeatmap.Group, currentGroupedBeatmap.Beatmap.BeatmapSet!)); } else if (currentGroupedBeatmap != null) { From 46b2d5374eceedfde35bc172af0137495189f4fc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jan 2026 19:36:21 +0900 Subject: [PATCH 5/7] Refactor `HandleFilterCompleted` to (hopefully) be easier to parse This is not required to fix the issue, but it is required for my brain to follow the method to some degress. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 53 +++++++++----------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index e0f46af409..72d59c62f7 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -482,40 +482,37 @@ namespace osu.Game.Screens.SelectV2 attemptSelectSingleFilteredResult(); - // Store selected group before handling selection (it may implicitly change the expanded group). - var groupForReselection = ExpandedGroup; - - var currentGroupedBeatmap = CurrentSelection as GroupedBeatmap; - - // The filter might have changed the set of available groups, which means that the current selection may point to a stale group. - // Check whether that is the case. - bool groupingRemainsOff = currentGroupedBeatmap?.Group == null && grouping.GroupItems.Count == 0; - bool groupStillValid = currentGroupedBeatmap?.Group != null && grouping.ItemMap.ContainsKey(currentGroupedBeatmap); - - if (groupingRemainsOff || groupStillValid) + if (CurrentSelection is GroupedBeatmap selection) { - // Update the visual state of the selected item if it should still be expanded post filter. - if (currentGroupedBeatmap != null && currentGroupedBeatmap.Group == groupForReselection) - setExpandedSet(new GroupedBeatmapSet(currentGroupedBeatmap.Group, currentGroupedBeatmap.Beatmap.BeatmapSet!)); - } - else if (currentGroupedBeatmap != null) - { - // If the group no longer exists (or the item no longer exists in the previous group), grab an arbitrary other instance of the beatmap under the first group encountered. - var newSelection = GetCarouselItems()?.Select(i => i.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(currentGroupedBeatmap.Beatmap)); + // The filter might have changed the set of available groups, which means that the current selection may point to a stale group. + // Check whether that is the case. + bool groupingRemainsOff = selection.Group == null && grouping.GroupItems.Count == 0; + bool groupStillValid = selection.Group != null && grouping.ItemMap.ContainsKey(selection); - // Only change the selection if we actually got a positive hit. - // This is necessary so that selection isn't lost if the panel reappears later due to e.g. unapplying some filter criteria that made it disappear in the first place. - if (newSelection != null) + if (groupingRemainsOff || groupStillValid) { - CurrentSelection = newSelection; - groupForReselection = newSelection.Group; + if (selection.Group == ExpandedGroup) + { + // Update the visual state of the selected item if it should still be expanded post filter. + HandleItemSelected(CurrentSelection); + } + } + else + { + // If the group no longer exists (or the item no longer exists in the previous group), grab an arbitrary other instance of the beatmap under the first group encountered. + var newSelection = GetCarouselItems()?.Select(i => i.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(selection.Beatmap)); + + // Only change the selection if we actually got a positive hit. + // This is necessary so that selection isn't lost if the panel reappears later due to e.g. unapplying some filter criteria that made it disappear in the first place. + if (newSelection != null) + { + CurrentSelection = newSelection; + // have to run this here as the above operation may noop on model equality. + HandleItemSelected(CurrentSelection); + } } } - // If a group was selected that is not the one containing the selection, attempt to reselect it. - if (groupForReselection != null && grouping.GroupItems.TryGetValue(groupForReselection, out _)) - setExpandedGroup(groupForReselection); - foreach (var item in Scroll.Panels.OfType().Where(p => p.Item != null)) updateVisibleBeatmaps((GroupedBeatmapSet)item.Item!.Model, item); } From 6f146a5ce9eb903912895083f7e9a17222928ebf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 22 Jan 2026 14:59:37 +0900 Subject: [PATCH 6/7] Add failing test case showing visual state desync --- .../TestSceneBeatmapCarouselArtistGrouping.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs index 2829364c68..68a07cc899 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -266,6 +266,41 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert("group expanded", () => Carousel.ExpandedGroup, () => Is.Not.Null); } + [Test] + public void TestFilterWhileCollapsedUpdatesVisualStateConnectly() + { + SelectNextSet(); + + CheckHasSelection(); + AddAssert("group expanded", () => Carousel.ExpandedGroup, () => Is.Not.Null); + AddAssert("has expanded set", () => Carousel.ExpandedBeatmapSet != null); + + AddAssert("has visible beatmaps", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmap && item.IsVisible), () => Is.EqualTo(3)); + AddAssert("has visually expanded set", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmapSet && item.IsExpanded && item.IsVisible), () => Is.EqualTo(1)); + + ToggleGroupCollapse(); + + CheckHasSelection(); + AddAssert("group not expanded", () => Carousel.ExpandedGroup, () => Is.Null); + AddAssert("has expanded set", () => Carousel.ExpandedBeatmapSet != null); + + AddAssert("has no visible beatmaps", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmap && item.IsVisible), () => Is.Zero); + AddAssert("has no visually expanded set", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmapSet && item.IsExpanded && item.IsVisible), () => Is.Zero); + + // filter while collapsed. + ApplyToFilterAndWaitForFilter("filter", c => c.SearchText = Carousel.SelectedBeatmapSet!.Metadata.Title); + + // then expand. + ToggleGroupCollapse(); + + CheckHasSelection(); + AddAssert("group expanded", () => Carousel.ExpandedGroup, () => Is.Not.Null); + AddAssert("has expanded set", () => Carousel.ExpandedBeatmapSet != null); + + AddAssert("has visible beatmaps", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmap && item.IsVisible), () => Is.EqualTo(3)); + AddAssert("has visually expanded set", () => Carousel.GetCarouselItems()!.Count(item => item.Model is GroupedBeatmapSet && item.IsExpanded && item.IsVisible), () => Is.EqualTo(1)); + } + [Test] public void TestGroupDoesNotExpandAgainOnRefilterIfManuallyCollapsed() { From 6354286452d1238005a3cf8cbc99f3969b88a79b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 22 Jan 2026 03:04:44 +0900 Subject: [PATCH 7/7] Simplify logic further and fix regressing case --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 33 ++++++++------------ 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 72d59c62f7..ba54fd269c 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -484,35 +484,26 @@ namespace osu.Game.Screens.SelectV2 if (CurrentSelection is GroupedBeatmap selection) { - // The filter might have changed the set of available groups, which means that the current selection may point to a stale group. - // Check whether that is the case. - bool groupingRemainsOff = selection.Group == null && grouping.GroupItems.Count == 0; - bool groupStillValid = selection.Group != null && grouping.ItemMap.ContainsKey(selection); - - if (groupingRemainsOff || groupStillValid) - { - if (selection.Group == ExpandedGroup) - { - // Update the visual state of the selected item if it should still be expanded post filter. - HandleItemSelected(CurrentSelection); - } - } - else + // Check whether the selection-group mapping is still valid post-filter. + if (!grouping.ItemMap.ContainsKey(selection)) { // If the group no longer exists (or the item no longer exists in the previous group), grab an arbitrary other instance of the beatmap under the first group encountered. - var newSelection = GetCarouselItems()?.Select(i => i.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(selection.Beatmap)); + var newSelection = GetCarouselItems()? + .Select(i => i.Model) + .OfType() + .FirstOrDefault(gb => CheckModelEquality(gb.Beatmap, selection.Beatmap)); // Only change the selection if we actually got a positive hit. // This is necessary so that selection isn't lost if the panel reappears later due to e.g. unapplying some filter criteria that made it disappear in the first place. if (newSelection != null) - { CurrentSelection = newSelection; - // have to run this here as the above operation may noop on model equality. - HandleItemSelected(CurrentSelection); - } } } + // Transfer the previous flag states across to the new models. + if (ExpandedBeatmapSet != null) setExpandedSet(ExpandedBeatmapSet); + if (ExpandedGroup != null) setExpandedGroup(ExpandedGroup); + foreach (var item in Scroll.Panels.OfType().Where(p => p.Item != null)) updateVisibleBeatmaps((GroupedBeatmapSet)item.Item!.Model, item); } @@ -664,6 +655,8 @@ namespace osu.Game.Screens.SelectV2 private void setExpansionStateOfSetItems(GroupedBeatmapSet set, bool expanded) { + bool canMakeVisible = !grouping.GroupItems.Any() || ExpandedGroup == set.Group; + if (grouping.SetItems.TryGetValue(set, out var items)) { foreach (var i in items) @@ -671,7 +664,7 @@ namespace osu.Game.Screens.SelectV2 if (i.Model is GroupedBeatmapSet) i.IsExpanded = expanded; else - i.IsVisible = expanded; + i.IsVisible = canMakeVisible && expanded; } } }