diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs index 2c3013af12..2390261cdb 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -4,9 +4,12 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Testing; +using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Graphics.Carousel; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; +using osu.Game.Tests.Resources; using osuTK; namespace osu.Game.Tests.Visual.SongSelectV2 @@ -322,5 +325,38 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); AddUntilStep("no beatmap panels visible", () => GetVisiblePanels().Count(), () => Is.Zero); } + + [Test] + public void TestGroupChangedAfterEngagingArtistGrouping() + { + RemoveAllBeatmaps(); + AddStep("add test beatmaps", () => + { + for (int i = 0; i < 5; ++i) + { + var baseTestBeatmap = TestResources.CreateTestBeatmapSetInfo(3); + + var metadata = new BeatmapMetadata + { + Artist = $"{(char)('A' + i)} artist", + Title = $"{(char)('A' + 4 - i)} title", + }; + + foreach (var b in baseTestBeatmap.Beatmaps) + b.Metadata = metadata; + + Realm.Write(r => r.Add(baseTestBeatmap, update: true)); + BeatmapSets.Add(baseTestBeatmap.Detach()); + } + + SortAndGroupBy(SortMode.Title, GroupMode.Title); + SelectNextSet(); + SelectNextSet(); + WaitForExpandedGroup(1); + + SortAndGroupBy(SortMode.Artist, GroupMode.Artist); + WaitForExpandedGroup(3); + }); + } } } diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 36b066a308..edee63c0fa 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -497,25 +497,35 @@ namespace osu.Game.Screens.SelectV2 // 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 groupStillExists = currentGroupedBeatmap?.Group != null && grouping.GroupItems.ContainsKey(currentGroupedBeatmap.Group); - if (groupingRemainsOff || groupStillExists) + bool groupStillValid = false; + + if (currentGroupedBeatmap?.Group != null) + { + groupStillValid = grouping.GroupItems.TryGetValue(currentGroupedBeatmap.Group, out var items) + && items.Any(i => CheckModelEquality(i.Model, currentGroupedBeatmap)); + } + + if (groupingRemainsOff || groupStillValid) { // Only update the visual state of the selected item. HandleItemSelected(currentGroupedBeatmap); } else if (currentGroupedBeatmap != null) { - // If the group no longer exists, grab an arbitrary other instance of the beatmap under the first group encountered. + // 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) + { CurrentSelection = newSelection; + groupForReselection = newSelection.Group; + } } // If a group was selected that is not the one containing the selection, attempt to reselect it. - // If the original group was not found, ExpandedGroup will already have been updated to a valid value in `HandleItemSelected` above. if (groupForReselection != null && grouping.GroupItems.TryGetValue(groupForReselection, out _)) setExpandedGroup(groupForReselection); }