1
0
mirror of https://github.com/ppy/osu.git synced 2026-05-17 09:22:34 +08:00

Merge pull request #36404 from peppy/fix-carousel-thing

Fix random selection not showing selection when all groups are collapsed
This commit is contained in:
Bartłomiej Dach
2026-01-22 08:24:03 +01:00
committed by GitHub
Unverified
3 changed files with 68 additions and 49 deletions
@@ -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));
@@ -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()
{
+19 -49
View File
@@ -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
}
}
/// <summary>
/// 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.
/// </summary>
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<GroupedBeatmap>().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<GroupedBeatmap>()
.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<PanelBeatmapSet>().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)