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..68a07cc899 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -253,6 +253,54 @@ 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 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() { diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 4519e594c6..ba54fd269c 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 @@ -352,12 +351,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 +363,11 @@ namespace osu.Game.Screens.SelectV2 { setExpansionStateOfGroup(ExpandedGroup, false); ExpandedGroup = null; - userCollapsedGroup = true; return; } setExpandedGroup(group); - if (userCollapsedGroup) - { - if (grouping.BeatmapSetsGroupedTogether && CurrentGroupedBeatmap != null && CheckModelEquality(group, CurrentGroupedBeatmap.Group)) - 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 +406,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) @@ -500,38 +482,27 @@ 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) { - // Only update the visual state of the selected item. - HandleItemSelected(currentGroupedBeatmap); - } - 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)); - - // 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) + // Check whether the selection-group mapping is still valid post-filter. + if (!grouping.ItemMap.ContainsKey(selection)) { - CurrentSelection = newSelection; - groupForReselection = newSelection.Group; + // 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 => 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; } } - // 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); + // 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); @@ -684,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) @@ -691,7 +664,7 @@ namespace osu.Game.Screens.SelectV2 if (i.Model is GroupedBeatmapSet) i.IsExpanded = expanded; else - i.IsVisible = expanded; + i.IsVisible = canMakeVisible && expanded; } } } @@ -798,9 +771,6 @@ namespace osu.Game.Screens.SelectV2 Criteria = criteria; - if (criteria.Group == GroupMode.None) - userCollapsedGroup = false; - loadingDebounce ??= Scheduler.AddDelayed(() => { if (loading.State.Value == Visibility.Visible)