From d304a31757956f11e1624ce054645bf4d5972626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 28 Aug 2025 11:36:22 +0200 Subject: [PATCH 01/22] Make grouping by collections and rank achieved testable without involving realm --- .../Visual/SongSelectV2/BeatmapCarouselTestScene.cs | 8 ++++++++ osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index f18e1e9b52..7178dc014d 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -16,11 +16,13 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Collections; using osu.Game.Database; using osu.Game.Graphics; using osu.Game.Graphics.Carousel; using osu.Game.Graphics.Containers; using osu.Game.Overlays; +using osu.Game.Scoring; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; @@ -443,6 +445,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2 public new GroupedBeatmapSet? ExpandedBeatmapSet => base.ExpandedBeatmapSet; public new GroupDefinition? ExpandedGroup => base.ExpandedGroup; + public Func> AllCollections { get; set; } = () => []; + public Func> BeatmapInfoGuidToTopRankMapping { get; set; } = _ => new Dictionary(); + public TestBeatmapCarousel() { RequestPresentBeatmap = _ => RequestPresentBeatmapCount++; @@ -464,6 +469,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2 PostFilterBeatmaps = items.Select(i => i.Model).OfType(); return items; } + + protected override List GetAllCollections() => AllCollections.Invoke(); + protected override Dictionary GetBeatmapInfoGuidToTopRankMapping(FilterCriteria criteria) => BeatmapInfoGuidToTopRankMapping.Invoke(criteria); } } } diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index c2711ceef0..18cf005ed3 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -103,14 +103,14 @@ namespace osu.Game.Screens.SelectV2 { new BeatmapCarouselFilterMatching(() => Criteria!), new BeatmapCarouselFilterSorting(() => Criteria!), - grouping = new BeatmapCarouselFilterGrouping(() => Criteria!, getDetachedCollections, getTopRanksMapping) + grouping = new BeatmapCarouselFilterGrouping(() => Criteria!, GetAllCollections, GetBeatmapInfoGuidToTopRankMapping) }; AddInternal(loading = new LoadingLayer()); } [BackgroundDependencyLoader] - private void load(BeatmapStore beatmapStore, RealmAccess realm, AudioManager audio, OsuConfigManager config, CancellationToken? cancellationToken) + private void load(BeatmapStore beatmapStore, AudioManager audio, OsuConfigManager config, CancellationToken? cancellationToken) { setupPools(); detachedBeatmaps = beatmapStore.GetBeatmapSets(cancellationToken); @@ -687,9 +687,9 @@ namespace osu.Game.Screens.SelectV2 [Resolved] private RealmAccess realm { get; set; } = null!; - private List getDetachedCollections() => realm.Run(r => r.All().AsEnumerable().Detach()); + protected virtual List GetAllCollections() => realm.Run(r => r.All().AsEnumerable().Detach()); - private Dictionary getTopRanksMapping(FilterCriteria criteria) => realm.Run(r => + protected virtual Dictionary GetBeatmapInfoGuidToTopRankMapping(FilterCriteria criteria) => realm.Run(r => { var topRankMapping = new Dictionary(); From 2ed79d354c8827d2b7a3c3f3e521309bb821af5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 28 Aug 2025 11:50:39 +0200 Subject: [PATCH 02/22] Add baseline test exercising desired duplicated display --- ...tSceneBeatmapCarouselCollectionGrouping.cs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselCollectionGrouping.cs diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselCollectionGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselCollectionGrouping.cs new file mode 100644 index 0000000000..e410d66ce8 --- /dev/null +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselCollectionGrouping.cs @@ -0,0 +1,66 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Testing; +using osu.Game.Collections; +using osu.Game.Screens.Select.Filter; + +namespace osu.Game.Tests.Visual.SongSelectV2 +{ + [TestFixture] + public partial class TestSceneBeatmapCarouselCollectionGrouping : BeatmapCarouselTestScene + { + [SetUpSteps] + public void SetUpSteps() + { + RemoveAllBeatmaps(); + CreateCarousel(); + + AddBeatmaps(10, 3); + + AddStep("set up collections", () => + { + List collections = + [ + new BeatmapCollection("collection one", [ + ..BeatmapSets[0].Beatmaps.Select(b => b.MD5Hash), + ..BeatmapSets[1].Beatmaps.Select(b => b.MD5Hash), + ..BeatmapSets[2].Beatmaps.Select(b => b.MD5Hash), + BeatmapSets[5].Beatmaps[1].MD5Hash, + BeatmapSets[8].Beatmaps[0].MD5Hash, + ]), + new BeatmapCollection("collection two", [ + BeatmapSets[0].Beatmaps[0].MD5Hash, + ..BeatmapSets[1].Beatmaps.Select(b => b.MD5Hash), + ..BeatmapSets[2].Beatmaps.Select(b => b.MD5Hash), + BeatmapSets[6].Beatmaps[2].MD5Hash, + BeatmapSets[8].Beatmaps[2].MD5Hash, + ]), + new BeatmapCollection("collection one copy", [ + ..BeatmapSets[0].Beatmaps.Select(b => b.MD5Hash), + ..BeatmapSets[1].Beatmaps.Select(b => b.MD5Hash), + ..BeatmapSets[2].Beatmaps.Select(b => b.MD5Hash), + BeatmapSets[5].Beatmaps[1].MD5Hash, + BeatmapSets[8].Beatmaps[0].MD5Hash, + ]), + ]; + Carousel.AllCollections = () => collections; + }); + + SortAndGroupBy(SortMode.Title, GroupMode.Collections); + WaitForDrawablePanels(); + } + + [Test] + public void TestMultipleCopiesOfBeatmapsPresent() + { + CheckDisplayedGroupsCount(4); // one for each collection, plus no collections + // all three collections have beatmaps from 5 beatmap sets + // 7 beatmap sets have beatmaps which belong to no collection + CheckDisplayedBeatmapSetsCount(5 + 5 + 5 + 7); + } + } +} From a84c364e44d1e1f89c209da4f29e0ab524b3e2ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 28 Aug 2025 12:42:54 +0200 Subject: [PATCH 03/22] Introduce new model for "beatmaps under grouping" & allow beatmaps to appear in multiple groups This bypasses the immediate first issue of not being able to display multiple instances of a beatmap on the carousel because of model equality being baked into the structure. It inevitably poses a bunch of *other* problems, but it's a start. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 56 +++---- .../SelectV2/BeatmapCarouselFilterGrouping.cs | 137 ++++++++++-------- osu.Game/Screens/SelectV2/PanelBeatmap.cs | 18 ++- .../SelectV2/PanelBeatmapStandalone.cs | 19 ++- 4 files changed, 136 insertions(+), 94 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 18cf005ed3..36264223ea 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -74,11 +74,11 @@ namespace osu.Game.Screens.SelectV2 return SPACING * 2; // ..and the bottom. - if (top.Model is BeatmapInfo && bottom.Model is GroupedBeatmapSet) + if (top.Model is GroupedBeatmap && bottom.Model is GroupedBeatmapSet) return SPACING * 2; // Beatmap difficulty panels do not overlap with themselves or any other panel. - if (top.Model is BeatmapInfo || bottom.Model is BeatmapInfo) + if (top.Model is GroupedBeatmap || bottom.Model is GroupedBeatmap) return SPACING; } else @@ -200,13 +200,13 @@ namespace osu.Game.Screens.SelectV2 { if (CheckValidForSetSelection(item)) { - if (item.Model is BeatmapInfo beatmapInfo) + if (item.Model is GroupedBeatmap groupedBeatmap) { // check the new selection wasn't deleted above - if (!Items.Contains(beatmapInfo)) + if (!Items.Contains(groupedBeatmap.Beatmap)) return false; - RequestSelection(beatmapInfo); + RequestSelection(groupedBeatmap.Beatmap); return true; } @@ -289,7 +289,7 @@ namespace osu.Game.Screens.SelectV2 protected GroupedBeatmapSet? ExpandedBeatmapSet { get; private set; } protected override bool ShouldActivateOnKeyboardSelection(CarouselItem item) => - grouping.BeatmapSetsGroupedTogether && item.Model is BeatmapInfo; + grouping.BeatmapSetsGroupedTogether && item.Model is GroupedBeatmap; protected override void HandleItemActivated(CarouselItem item) { @@ -318,14 +318,14 @@ namespace osu.Game.Screens.SelectV2 selectRecommendedDifficultyForBeatmapSet(groupedSet); return; - case BeatmapInfo beatmapInfo: - if (CurrentSelection != null && CheckModelEquality(CurrentSelection, beatmapInfo)) + case GroupedBeatmap groupedBeatmap: + if (CurrentSelection != null && CheckModelEquality(CurrentSelection, groupedBeatmap)) { - RequestPresentBeatmap?.Invoke(beatmapInfo); + RequestPresentBeatmap?.Invoke(groupedBeatmap.Beatmap); return; } - RequestSelection(beatmapInfo); + RequestSelection(groupedBeatmap.Beatmap); return; } } @@ -345,14 +345,11 @@ namespace osu.Game.Screens.SelectV2 case GroupDefinition: throw new InvalidOperationException("Groups should never become selected"); - case BeatmapInfo beatmapInfo: - // Find any containing group. There should never be too many groups so iterating is efficient enough. - GroupDefinition? containingGroup = grouping.GroupItems.SingleOrDefault(kvp => kvp.Value.Any(i => CheckModelEquality(i.Model, beatmapInfo))).Key; - - setExpandedGroup(containingGroup); + case GroupedBeatmap groupedBeatmap: + setExpandedGroup(groupedBeatmap.Group); if (grouping.BeatmapSetsGroupedTogether) - setExpandedSet(new GroupedBeatmapSet(containingGroup, beatmapInfo.BeatmapSet!)); + setExpandedSet(new GroupedBeatmapSet(groupedBeatmap.Group, groupedBeatmap.Beatmap.BeatmapSet!)); break; } } @@ -432,7 +429,7 @@ namespace osu.Game.Screens.SelectV2 // Selecting a set isn't valid – let's re-select the first visible difficulty. if (grouping.SetItems.TryGetValue(set, out var items)) { - var beatmaps = items.Select(i => i.Model).OfType(); + var beatmaps = items.Select(i => i.Model).OfType().Select(b => b.Beatmap); RequestRecommendedSelection(beatmaps); } } @@ -450,8 +447,10 @@ namespace osu.Game.Screens.SelectV2 foreach (var item in items) { - if (item.Model is BeatmapInfo beatmapInfo) + if (item.Model is GroupedBeatmap groupedBeatmap) { + var beatmapInfo = groupedBeatmap.Beatmap; + if (beatmapSetInfo == null) { beatmapSetInfo = beatmapInfo.BeatmapSet!; @@ -481,7 +480,7 @@ namespace osu.Game.Screens.SelectV2 case GroupedBeatmapSet: return true; - case BeatmapInfo: + case GroupedBeatmap: return !grouping.BeatmapSetsGroupedTogether; case GroupDefinition: @@ -492,7 +491,7 @@ namespace osu.Game.Screens.SelectV2 } } - private void setExpandedGroup(GroupDefinition group) + private void setExpandedGroup(GroupDefinition? group) { if (ExpandedGroup != null) setExpansionStateOfGroup(ExpandedGroup, false); @@ -500,7 +499,7 @@ namespace osu.Game.Screens.SelectV2 ExpandedGroup = group; if (ExpandedGroup != null) - setExpansionStateOfGroup(group, true); + setExpansionStateOfGroup(ExpandedGroup, true); } private void setExpansionStateOfGroup(GroupDefinition group, bool expanded) @@ -607,7 +606,7 @@ namespace osu.Game.Screens.SelectV2 sampleChangeSet?.Play(); return; - case BeatmapInfo: + case GroupedBeatmap: sampleChangeDifficulty?.Play(); return; } @@ -745,8 +744,8 @@ namespace osu.Game.Screens.SelectV2 if (x is GroupedBeatmapSet groupedSetX && y is GroupedBeatmapSet groupedSetY) return groupedSetX.Equals(groupedSetY); - if (x is BeatmapInfo beatmapX && y is BeatmapInfo beatmapY) - return beatmapX.Equals(beatmapY); + if (x is GroupedBeatmap groupedBeatmapX && y is GroupedBeatmap groupedBeatmapY) + return groupedBeatmapX.Equals(groupedBeatmapY); if (x is GroupDefinition groupX && y is GroupDefinition groupY) return groupX.Equals(groupY); @@ -767,7 +766,7 @@ namespace osu.Game.Screens.SelectV2 case GroupDefinition: return groupPanelPool.Get(); - case BeatmapInfo: + case GroupedBeatmap: if (!grouping.BeatmapSetsGroupedTogether) return standalonePanelPool.Get(); @@ -1021,4 +1020,11 @@ namespace osu.Game.Screens.SelectV2 /// The purpose of this model is to support splitting beatmap sets apart when the active grouping mode demands it. /// public record GroupedBeatmapSet([UsedImplicitly] GroupDefinition? Group, BeatmapSetInfo BeatmapSet); + + /// + /// Used to represent a under a . + /// The purpose of this model is to support showing multiple copies of a beatmap, which can occur if a beatmap appears in multiple groups + /// (most prominently, collections group mode). + /// + public record GroupedBeatmap(GroupDefinition? Group, BeatmapInfo Beatmap); } diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs index 0d2489c304..9fa4e28e93 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; using osu.Framework.Extensions; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; using osu.Game.Collections; using osu.Game.Graphics.Carousel; @@ -124,7 +125,7 @@ namespace osu.Game.Screens.SelectV2 item.DrawHeight = PanelBeatmapStandalone.HEIGHT; } - addItem(item); + addItem(new CarouselItem(new GroupedBeatmap(group, beatmap))); lastBeatmap = beatmap; displayedBeatmapsCount++; } @@ -192,7 +193,7 @@ namespace osu.Game.Screens.SelectV2 var date = b.LastPlayed; if (date == null || date == DateTimeOffset.MinValue) - return new GroupDefinition(int.MaxValue, "Never"); + return new GroupDefinition(int.MaxValue, "Never").Yield(); return defineGroupByDate(date.Value); }, items); @@ -236,184 +237,204 @@ namespace osu.Game.Screens.SelectV2 } } - private List getGroupsBy(Func getGroup, List items) + private List getGroupsBy(Func> defineGroups, List items) { - return items.GroupBy(i => getGroup((BeatmapInfo)i.Model)) - .Where(g => g.Key != null) - .OrderBy(g => g.Key!.Order) - .ThenBy(g => g.Key!.Title) - .Select(g => new GroupMapping(g.Key, g.ToList())) - .ToList(); + var groups = new Dictionary(); + + foreach (var item in items) + { + foreach (var groupDefinition in defineGroups((BeatmapInfo)item.Model)) + { + if (!groups.TryGetValue(groupDefinition, out var group)) + group = groups[groupDefinition] = new GroupMapping(groupDefinition, []); + + group.ItemsInGroup.Add(item); + } + } + + return groups.Values + .OrderBy(g => g.Group!.Order) + .ThenBy(g => g.Group!.Title) + .ToList(); } - private GroupDefinition defineGroupAlphabetically(string name) + private IEnumerable defineGroupAlphabetically(string name) { char firstChar = name.FirstOrDefault(); if (char.IsAsciiDigit(firstChar)) - return new GroupDefinition(int.MinValue, "0-9"); + return new GroupDefinition(int.MinValue, "0-9").Yield(); if (char.IsAsciiLetter(firstChar)) - return new GroupDefinition(char.ToUpperInvariant(firstChar) - 'A', char.ToUpperInvariant(firstChar).ToString()); + return new GroupDefinition(char.ToUpperInvariant(firstChar) - 'A', char.ToUpperInvariant(firstChar).ToString()).Yield(); - return new GroupDefinition(int.MaxValue, "Other"); + return new GroupDefinition(int.MaxValue, "Other").Yield(); } - private GroupDefinition defineGroupByDate(DateTimeOffset date) + private IEnumerable defineGroupByDate(DateTimeOffset date) { var now = DateTimeOffset.Now; var elapsed = now - date; if (elapsed.TotalDays < 1) - return new GroupDefinition(0, "Today"); + return new GroupDefinition(0, "Today").Yield(); if (elapsed.TotalDays < 2) - return new GroupDefinition(1, "Yesterday"); + return new GroupDefinition(1, "Yesterday").Yield(); if (elapsed.TotalDays < 7) - return new GroupDefinition(2, "Last week"); + return new GroupDefinition(2, "Last week").Yield(); if (elapsed.TotalDays < 30) - return new GroupDefinition(3, "Last month"); + return new GroupDefinition(3, "Last month").Yield(); if (elapsed.TotalDays < 60) - return new GroupDefinition(4, "1 month ago"); + return new GroupDefinition(4, "1 month ago").Yield(); for (int i = 90; i <= 150; i += 30) { if (elapsed.TotalDays < i) - return new GroupDefinition(i, $"{i / 30 - 1} months ago"); + return new GroupDefinition(i, $"{i / 30 - 1} months ago").Yield(); } - return new GroupDefinition(151, "Over 5 months ago"); + return new GroupDefinition(151, "Over 5 months ago").Yield(); } - private GroupDefinition defineGroupByRankedDate(DateTimeOffset? date) + private IEnumerable defineGroupByRankedDate(DateTimeOffset? date) { if (date == null) - return new GroupDefinition(0, "Unranked"); + return new GroupDefinition(0, "Unranked").Yield(); - return new GroupDefinition(-date.Value.Year, $"{date.Value.Year}"); + return new GroupDefinition(-date.Value.Year, $"{date.Value.Year}").Yield(); } - private GroupDefinition defineGroupByStatus(BeatmapOnlineStatus status) + private IEnumerable defineGroupByStatus(BeatmapOnlineStatus status) { switch (status) { case BeatmapOnlineStatus.Ranked: case BeatmapOnlineStatus.Approved: - return new GroupDefinition(0, BeatmapOnlineStatus.Ranked.GetDescription()); + return new GroupDefinition(0, BeatmapOnlineStatus.Ranked.GetDescription()).Yield(); case BeatmapOnlineStatus.Qualified: - return new GroupDefinition(1, status.GetDescription()); + return new GroupDefinition(1, status.GetDescription()).Yield(); case BeatmapOnlineStatus.WIP: - return new GroupDefinition(2, status.GetDescription()); + return new GroupDefinition(2, status.GetDescription()).Yield(); case BeatmapOnlineStatus.Pending: - return new GroupDefinition(3, status.GetDescription()); + return new GroupDefinition(3, status.GetDescription()).Yield(); case BeatmapOnlineStatus.Graveyard: - return new GroupDefinition(4, status.GetDescription()); + return new GroupDefinition(4, status.GetDescription()).Yield(); case BeatmapOnlineStatus.LocallyModified: - return new GroupDefinition(5, status.GetDescription()); + return new GroupDefinition(5, status.GetDescription()).Yield(); case BeatmapOnlineStatus.None: - return new GroupDefinition(6, status.GetDescription()); + return new GroupDefinition(6, status.GetDescription()).Yield(); case BeatmapOnlineStatus.Loved: - return new GroupDefinition(7, status.GetDescription()); + return new GroupDefinition(7, status.GetDescription()).Yield(); default: throw new ArgumentOutOfRangeException(nameof(status), status, null); } } - private GroupDefinition defineGroupByBPM(double bpm) + private IEnumerable defineGroupByBPM(double bpm) { if (bpm < 60) - return new GroupDefinition(60, "Under 60 BPM"); + return new GroupDefinition(60, "Under 60 BPM").Yield(); for (int i = 70; i <= 300; i += 10) { if (bpm < i) - return new GroupDefinition(i, $"{i - 10} - {i} BPM"); + return new GroupDefinition(i, $"{i - 10} - {i} BPM").Yield(); } - return new GroupDefinition(301, "Over 300 BPM"); + return new GroupDefinition(301, "Over 300 BPM").Yield(); } - private GroupDefinition defineGroupByStars(double stars) + private IEnumerable defineGroupByStars(double stars) { // truncation is intentional - compare `FormatUtils.FormatStarRating()` int starInt = (int)stars; var starDifficulty = new StarDifficulty(starInt, 0); if (starInt == 0) - return new StarDifficultyGroupDefinition(0, "Below 1 Star", starDifficulty); + return new StarDifficultyGroupDefinition(0, "Below 1 Star", starDifficulty).Yield(); if (starInt == 1) - return new StarDifficultyGroupDefinition(1, "1 Star", starDifficulty); + return new StarDifficultyGroupDefinition(1, "1 Star", starDifficulty).Yield(); - return new StarDifficultyGroupDefinition(starInt, $"{starInt} Stars", starDifficulty); + return new StarDifficultyGroupDefinition(starInt, $"{starInt} Stars", starDifficulty).Yield(); } - private GroupDefinition defineGroupByLength(double length) + private IEnumerable defineGroupByLength(double length) { for (int i = 1; i < 6; i++) { if (length <= i * 60_000) { if (i == 1) - return new GroupDefinition(1, "1 minute or less"); + return new GroupDefinition(1, "1 minute or less").Yield(); - return new GroupDefinition(i, $"{i} minutes or less"); + return new GroupDefinition(i, $"{i} minutes or less").Yield(); } } if (length <= 10 * 60_000) - return new GroupDefinition(10, "10 minutes or less"); + return new GroupDefinition(10, "10 minutes or less").Yield(); - return new GroupDefinition(11, "Over 10 minutes"); + return new GroupDefinition(11, "Over 10 minutes").Yield(); } - private GroupDefinition defineGroupBySource(string source) + private IEnumerable defineGroupBySource(string source) { if (string.IsNullOrEmpty(source)) - return new GroupDefinition(1, "Unsourced"); + return new GroupDefinition(1, "Unsourced").Yield(); - return new GroupDefinition(0, source); + return new GroupDefinition(0, source).Yield(); } - private GroupDefinition defineGroupByCollection(BeatmapInfo beatmap, IEnumerable collections) + private IEnumerable defineGroupByCollection(BeatmapInfo beatmap, IEnumerable collections) { + bool anyCollections = false; + foreach (var collection in collections) { if (collection.BeatmapMD5Hashes.Contains(beatmap.MD5Hash)) - return new GroupDefinition(0, collection.Name); + { + yield return new GroupDefinition(0, collection.Name); + + anyCollections = true; + } } - return new GroupDefinition(1, "Not in collection"); + if (anyCollections) + yield break; + + yield return new GroupDefinition(1, "Not in collection"); } - private GroupDefinition? defineGroupByOwnMaps(BeatmapInfo beatmap, int? localUserId, string? localUserUsername) + private IEnumerable defineGroupByOwnMaps(BeatmapInfo beatmap, int? localUserId, string? localUserUsername) { var author = beatmap.BeatmapSet!.Metadata.Author; if (author.OnlineID == localUserId || (author.OnlineID <= 1 && author.Username == localUserUsername)) - return new GroupDefinition(0, "My maps"); + return new GroupDefinition(0, "My maps").Yield(); // discard beatmaps not owned by the user. - return null; + return []; } - private GroupDefinition defineGroupByRankAchieved(BeatmapInfo beatmap, IReadOnlyDictionary topRankMapping) + private IEnumerable defineGroupByRankAchieved(BeatmapInfo beatmap, IReadOnlyDictionary topRankMapping) { if (topRankMapping.TryGetValue(beatmap.ID, out var rank)) - return new GroupDefinition(-(int)rank, rank.GetDescription()); + return new GroupDefinition(-(int)rank, rank.GetDescription()).Yield(); - return new GroupDefinition(int.MaxValue, "Unplayed"); + return new GroupDefinition(int.MaxValue, "Unplayed").Yield(); } private record GroupMapping(GroupDefinition? Group, List ItemsInGroup); diff --git a/osu.Game/Screens/SelectV2/PanelBeatmap.cs b/osu.Game/Screens/SelectV2/PanelBeatmap.cs index 106b911606..545439684b 100644 --- a/osu.Game/Screens/SelectV2/PanelBeatmap.cs +++ b/osu.Game/Screens/SelectV2/PanelBeatmap.cs @@ -72,6 +72,15 @@ namespace osu.Game.Screens.SelectV2 [Resolved] private ISongSelect? songSelect { get; set; } + private GroupedBeatmap groupedBeatmap + { + get + { + Debug.Assert(Item != null); + return (GroupedBeatmap)Item!.Model; + } + } + public PanelBeatmap() { PanelXOffset = 60; @@ -207,8 +216,7 @@ namespace osu.Game.Screens.SelectV2 { base.PrepareForUse(); - Debug.Assert(Item != null); - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; difficultyIcon.Icon = getRulesetIcon(beatmap.Ruleset); @@ -248,7 +256,7 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; starDifficultyBindable = difficultyCache.GetBindableDifficulty(beatmap, starDifficultyCancellationSource.Token, SongSelect.SELECTION_DEBOUNCE); starDifficultyBindable.BindValueChanged(starDifficulty => @@ -293,7 +301,7 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; if (ruleset.Value.OnlineID == 3) { @@ -319,7 +327,7 @@ namespace osu.Game.Screens.SelectV2 List items = new List(); if (songSelect != null) - items.AddRange(songSelect.GetForwardActions((BeatmapInfo)Item.Model)); + items.AddRange(songSelect.GetForwardActions(groupedBeatmap.Beatmap)); return items.ToArray(); } diff --git a/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs b/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs index 87a35facbd..226a1b1d06 100644 --- a/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs +++ b/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs @@ -73,6 +73,15 @@ namespace osu.Game.Screens.SelectV2 private Box backgroundBorder = null!; + private GroupedBeatmap groupedBeatmap + { + get + { + Debug.Assert(Item != null); + return (GroupedBeatmap)Item!.Model; + } + } + public PanelBeatmapStandalone() { PanelXOffset = 20; @@ -219,9 +228,7 @@ namespace osu.Game.Screens.SelectV2 { base.PrepareForUse(); - Debug.Assert(Item != null); - - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; var beatmapSet = beatmap.BeatmapSet!; beatmapBackground.Beatmap = beatmaps.GetWorkingBeatmap(beatmap); @@ -262,7 +269,7 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; starDifficultyBindable = difficultyCache.GetBindableDifficulty(beatmap, starDifficultyCancellationSource.Token, SongSelect.SELECTION_DEBOUNCE); starDifficultyBindable.BindValueChanged(starDifficulty => @@ -300,7 +307,7 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = (BeatmapInfo)Item.Model; + var beatmap = groupedBeatmap.Beatmap; if (ruleset.Value.OnlineID == 3) { @@ -326,7 +333,7 @@ namespace osu.Game.Screens.SelectV2 List items = new List(); if (songSelect != null) - items.AddRange(songSelect.GetForwardActions((BeatmapInfo)Item.Model)); + items.AddRange(songSelect.GetForwardActions(groupedBeatmap.Beatmap)); return items.ToArray(); } From e98579d3afbb4eb7df72f6e41cad4f57470b93e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 28 Aug 2025 12:50:47 +0200 Subject: [PATCH 04/22] Apply most trivial adjustments to tests after beatmap model replacement --- .../BeatmapCarouselFilterGroupingTest.cs | 4 ++-- .../SongSelectV2/BeatmapCarouselTestScene.cs | 8 +++---- .../TestSceneBeatmapCarouselArtistGrouping.cs | 5 ++--- ...tSceneBeatmapCarouselDifficultyGrouping.cs | 5 ++--- .../TestSceneBeatmapCarouselFiltering.cs | 22 +++++++++---------- .../TestSceneBeatmapCarouselNoGrouping.cs | 3 +-- .../SongSelectV2/TestScenePanelBeatmap.cs | 8 +++---- .../TestScenePanelBeatmapStandalone.cs | 8 +++---- .../TestSceneSongSelectGrouping.cs | 2 +- 9 files changed, 31 insertions(+), 34 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselFilterGroupingTest.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselFilterGroupingTest.cs index 32a7b89424..5f3cd26d55 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselFilterGroupingTest.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselFilterGroupingTest.cs @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 var results = await runGrouping(GroupMode.None, beatmapSets); Assert.That(results.Select(r => r.Model).OfType().Select(groupedSet => groupedSet.BeatmapSet), Is.EquivalentTo(beatmapSets)); - Assert.That(results.Select(r => r.Model).OfType(), Is.EquivalentTo(allBeatmaps)); + Assert.That(results.Select(r => r.Model).OfType().Select(groupedBeatmap => groupedBeatmap.Beatmap), Is.EquivalentTo(allBeatmaps)); assertTotal(results, beatmapSets.Count + allBeatmaps.Length); } @@ -391,7 +391,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 var groupModel = (GroupDefinition)groupItem.Model; Assert.That(groupModel.Title, Is.EqualTo(expectedTitle)); - Assert.That(itemsInGroup.Select(i => i.Model).OfType(), Is.EquivalentTo(expectedBeatmaps)); + Assert.That(itemsInGroup.Select(i => i.Model).OfType().Select(gb => gb.Beatmap), Is.EquivalentTo(expectedBeatmaps)); totalItems += itemsInGroup.Count() + 1; } diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index 7178dc014d..06d2a42e0d 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -283,8 +283,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 // offset by one because the group itself is included in the items list. CarouselItem item = groupingFilter.GroupItems[groupDefinition].ElementAt(panel + 1); - return (Carousel.CurrentSelection as BeatmapInfo)? - .Equals(item.Model as BeatmapInfo) == true; + return (Carousel.CurrentSelection as GroupedBeatmap)? + .Equals(item.Model as GroupedBeatmap) == true; }); } @@ -293,7 +293,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 if (diff != null) { AddUntilStep($"selected is set{set} diff{diff.Value}", - () => (Carousel.CurrentSelection as BeatmapInfo), + () => (Carousel.CurrentSelection as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(BeatmapSets[set].Beatmaps[diff.Value])); } else @@ -466,7 +466,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 if (FilterDelay != 0) await Task.Delay(FilterDelay).ConfigureAwait(true); - PostFilterBeatmaps = items.Select(i => i.Model).OfType(); + PostFilterBeatmaps = items.Select(i => i.Model).OfType().Select(i => i.Beatmap); return items; } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs index 521221f0c7..78b6985fdb 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Testing; -using osu.Game.Beatmaps; using osu.Game.Graphics.Carousel; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; @@ -92,7 +91,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null); - AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!)); + AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); @@ -132,7 +131,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForBeatmapSelection(0, 1); // Expanding a group will move keyboard selection to the selected beatmap if contained. AddAssert("keyboard selected panel is expanded", () => groupPanel?.Expanded.Value, () => Is.True); - AddAssert("keyboard selected panel is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, Is.TypeOf); + AddAssert("keyboard selected panel is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, Is.TypeOf); } [Test] diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs index 2ab0eda172..c28860e368 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Testing; -using osu.Game.Beatmaps; using osu.Game.Graphics.Carousel; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; @@ -82,7 +81,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null); - AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!)); + AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); @@ -121,7 +120,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForBeatmapSelection(0, 0); // Expanding a group will move keyboard selection to the selected beatmap if contained. AddAssert("keyboard selected panel is expanded", () => groupPanel?.Expanded.Value, () => Is.True); - AddAssert("keyboard selected panel is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, Is.TypeOf); + AddAssert("keyboard selected panel is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, Is.TypeOf); } [Test] diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs index 687c4c23be..70af48069a 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs @@ -130,14 +130,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextPanel(); Select(); - AddStep("record selection", () => selectedID = ((BeatmapInfo)Carousel.CurrentSelection!).ID); + AddStep("record selection", () => selectedID = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID); for (int i = 0; i < 5; i++) { ApplyToFilterAndWaitForFilter("filter all", c => c.SearchText = Guid.NewGuid().ToString()); - AddAssert("selection not changed", () => ((BeatmapInfo)Carousel.CurrentSelection!).ID == selectedID); + AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); ApplyToFilterAndWaitForFilter("remove filter", c => c.SearchText = string.Empty); - AddAssert("selection not changed", () => ((BeatmapInfo)Carousel.CurrentSelection!).ID == selectedID); + AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); } } @@ -177,14 +177,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); - AddStep("record selection", () => selectedID = ((BeatmapInfo)Carousel.CurrentSelection!).ID); + AddStep("record selection", () => selectedID = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID); for (int i = 0; i < 5; i++) { ApplyToFilterAndWaitForFilter("filter all", c => c.SearchText = Guid.NewGuid().ToString()); - AddAssert("selection not changed", () => ((BeatmapInfo)Carousel.CurrentSelection!).ID == selectedID); + AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); ApplyToFilterAndWaitForFilter("remove filter", c => c.SearchText = string.Empty); - AddAssert("selection not changed", () => ((BeatmapInfo)Carousel.CurrentSelection!).ID == selectedID); + AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); } } @@ -239,7 +239,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 var visibleBeatmapPanels = GetVisiblePanels(); return visibleBeatmapPanels.Count() == 1 - && visibleBeatmapPanels.Count(p => ((BeatmapInfo)p.Item!.Model).Ruleset.OnlineID == 0) == 1; + && visibleBeatmapPanels.Count(p => ((GroupedBeatmap)p.Item!.Model).Beatmap.Ruleset.OnlineID == 0) == 1; }); ApplyToFilterAndWaitForFilter("filter to taiko", c => c.Ruleset = rulesets.AvailableRulesets.ElementAt(1)); @@ -249,8 +249,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 var visibleBeatmapPanels = GetVisiblePanels(); return visibleBeatmapPanels.Count() == 2 - && visibleBeatmapPanels.Count(p => ((BeatmapInfo)p.Item!.Model).Ruleset.OnlineID == 0) == 1 - && visibleBeatmapPanels.Count(p => ((BeatmapInfo)p.Item!.Model).Ruleset.OnlineID == 1) == 1; + && visibleBeatmapPanels.Count(p => ((GroupedBeatmap)p.Item!.Model).Beatmap.Ruleset.OnlineID == 0) == 1 + && visibleBeatmapPanels.Count(p => ((GroupedBeatmap)p.Item!.Model).Beatmap.Ruleset.OnlineID == 1) == 1; }); ApplyToFilterAndWaitForFilter("filter to catch", c => c.Ruleset = rulesets.AvailableRulesets.ElementAt(2)); @@ -260,8 +260,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 var visibleBeatmapPanels = GetVisiblePanels(); return visibleBeatmapPanels.Count() == 2 - && visibleBeatmapPanels.Count(p => ((BeatmapInfo)p.Item!.Model).Ruleset.OnlineID == 0) == 1 - && visibleBeatmapPanels.Count(p => ((BeatmapInfo)p.Item!.Model).Ruleset.OnlineID == 2) == 1; + && visibleBeatmapPanels.Count(p => ((GroupedBeatmap)p.Item!.Model).Beatmap.Ruleset.OnlineID == 0) == 1 + && visibleBeatmapPanels.Count(p => ((GroupedBeatmap)p.Item!.Model).Beatmap.Ruleset.OnlineID == 2) == 1; }); } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs index 648f531a6e..0b0f93b3bc 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Testing; -using osu.Game.Beatmaps; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; using osuTK; @@ -101,7 +100,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("no drawable selection", GetSelectedPanel, () => Is.Null); - AddStep("add previous selection", () => BeatmapSets.Add(((BeatmapInfo)selection!).BeatmapSet!)); + AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmap.cs b/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmap.cs index 09f8c68951..618b9e0d48 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmap.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmap.cs @@ -104,21 +104,21 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { new PanelBeatmap { - Item = new CarouselItem(beatmap) + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)) }, new PanelBeatmap { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), KeyboardSelected = { Value = true } }, new PanelBeatmap { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), Selected = { Value = true } }, new PanelBeatmap { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), KeyboardSelected = { Value = true }, Selected = { Value = true } }, diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmapStandalone.cs b/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmapStandalone.cs index e9361b3d7f..67a9f54f1a 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmapStandalone.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestScenePanelBeatmapStandalone.cs @@ -104,21 +104,21 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { new PanelBeatmapStandalone { - Item = new CarouselItem(beatmap) + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)) }, new PanelBeatmapStandalone { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), KeyboardSelected = { Value = true } }, new PanelBeatmapStandalone { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), Selected = { Value = true } }, new PanelBeatmapStandalone { - Item = new CarouselItem(beatmap), + Item = new CarouselItem(new GroupedBeatmap(null, beatmap)), KeyboardSelected = { Value = true }, Selected = { Value = true } }, diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectGrouping.cs index 0772607a57..aa80321033 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectGrouping.cs @@ -317,7 +317,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert($"\"{name}\" present", () => { var group = grouping.GroupItems.Single(g => g.Key.Title == name); - var actualBeatmaps = group.Value.Select(i => i.Model).OfType().OrderBy(b => b.ID); + var actualBeatmaps = group.Value.Select(i => i.Model).OfType().Select(gb => gb.Beatmap).OrderBy(b => b.ID); var expectedBeatmaps = getBeatmaps().SelectMany(s => s.Beatmaps).OrderBy(b => b.ID); return actualBeatmaps.SequenceEqual(expectedBeatmaps); }); From d4b357dfa0133d24083aa08372644f86c799a14b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 28 Aug 2025 13:08:31 +0200 Subject: [PATCH 05/22] Fix carousel selection not working Basically, `BeatmapCarousel.CurrentSelection`, which is magic-object-typed, can no longer use `BeatmapInfo` directly, it now must also use `GroupedBeatmap`. This spills out all the way into song select because of beatmap selection flows that require hookup from song select. --- .../SongSelectV2/BeatmapCarouselTestScene.cs | 12 +-- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 74 ++++++++++--------- osu.Game/Screens/SelectV2/SongSelect.cs | 20 +++-- 3 files changed, 58 insertions(+), 48 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index 06d2a42e0d..c60ee55110 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -117,13 +117,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 NewItemsPresented = _ => NewItemsPresentedInvocationCount++, RequestSelection = b => { - BeatmapRequestedSelections.Push(b); + BeatmapRequestedSelections.Push(b.Beatmap); Carousel.CurrentSelection = b; }, - RequestRecommendedSelection = beatmaps => + RequestRecommendedSelection = groupedBeatmaps => { - BeatmapSetRequestedSelections.Push(beatmaps.First().BeatmapSet!); - Carousel.CurrentSelection = BeatmapRecommendationFunction?.Invoke(beatmaps) ?? beatmaps.First(); + var recommendedBeatmap = BeatmapRecommendationFunction?.Invoke(groupedBeatmaps.Select(gb => gb.Beatmap)) ?? groupedBeatmaps.First().Beatmap; + var recommendedGroupedBeatmap = groupedBeatmaps.First(gb => gb.Beatmap.Equals(recommendedBeatmap)); + BeatmapSetRequestedSelections.Push(recommendedBeatmap.BeatmapSet!); + Carousel.CurrentSelection = recommendedGroupedBeatmap; }, BleedTop = 50, BleedBottom = 50, @@ -439,7 +441,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 public IEnumerable PostFilterBeatmaps = null!; - public BeatmapInfo? SelectedBeatmapInfo => CurrentSelection as BeatmapInfo; + public BeatmapInfo? SelectedBeatmapInfo => (CurrentSelection as GroupedBeatmap)?.Beatmap; public BeatmapSetInfo? SelectedBeatmapSet => SelectedBeatmapInfo?.BeatmapSet; public new GroupedBeatmapSet? ExpandedBeatmapSet => base.ExpandedBeatmapSet; diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 36264223ea..46ea61ca9d 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -41,12 +41,12 @@ namespace osu.Game.Screens.SelectV2 /// /// From the provided beatmaps, select the most appropriate one for the user's skill. /// - public required Action> RequestRecommendedSelection { private get; init; } + public required Action> RequestRecommendedSelection { private get; init; } /// /// Selection requested for the provided beatmap. /// - public required Action RequestSelection { private get; init; } + public required Action RequestSelection { private get; init; } public const float SPACING = 3f; @@ -158,7 +158,7 @@ namespace osu.Game.Screens.SelectV2 foreach (var beatmap in set.Beatmaps) { Items.RemoveAll(i => i is BeatmapInfo bi && beatmap.Equals(bi)); - selectedSetDeleted |= CheckModelEquality(CurrentSelection, beatmap); + selectedSetDeleted |= CheckModelEquality((CurrentSelection as GroupedBeatmap)?.Beatmap, beatmap); } } @@ -206,7 +206,7 @@ namespace osu.Game.Screens.SelectV2 if (!Items.Contains(groupedBeatmap.Beatmap)) return false; - RequestSelection(groupedBeatmap.Beatmap); + RequestSelection(groupedBeatmap); return true; } @@ -215,7 +215,7 @@ namespace osu.Game.Screens.SelectV2 if (oldItems.Contains(groupedSet.BeatmapSet)) return false; - RequestRecommendedSelection(groupedSet.BeatmapSet.Beatmaps); + selectRecommendedDifficultyForBeatmapSet(groupedSet); return true; } } @@ -256,8 +256,12 @@ namespace osu.Game.Screens.SelectV2 { // TODO: should this exist in song select instead of here? // we need to ensure the global beatmap is also updated alongside changes. - if (CurrentSelection != null && CheckModelEquality(beatmap, CurrentSelection)) - RequestSelection(matchingNewBeatmap); + if (CurrentSelection is GroupedBeatmap currentBeatmapUnderGrouping) + { + var candidateSelection = currentBeatmapUnderGrouping with { Beatmap = beatmap }; + if (CheckModelEquality(candidateSelection, CurrentSelection)) + RequestSelection(candidateSelection); + } Items.ReplaceRange(previousIndex, 1, [matchingNewBeatmap]); newSetBeatmaps.Remove(matchingNewBeatmap); @@ -309,8 +313,8 @@ namespace osu.Game.Screens.SelectV2 setExpandedGroup(group); // If the active selection is within this group, it should get keyboard focus immediately. - if (CurrentSelectionItem?.IsVisible == true && CurrentSelection is BeatmapInfo info) - RequestSelection(info); + if (CurrentSelectionItem?.IsVisible == true && CurrentSelection is GroupedBeatmap gb) + RequestSelection(gb); return; @@ -325,7 +329,7 @@ namespace osu.Game.Screens.SelectV2 return; } - RequestSelection(groupedBeatmap.Beatmap); + RequestSelection(groupedBeatmap); return; } } @@ -429,7 +433,7 @@ namespace osu.Game.Screens.SelectV2 // Selecting a set isn't valid – let's re-select the first visible difficulty. if (grouping.SetItems.TryGetValue(set, out var items)) { - var beatmaps = items.Select(i => i.Model).OfType().Select(b => b.Beatmap); + var beatmaps = items.Select(i => i.Model).OfType(); RequestRecommendedSelection(beatmaps); } } @@ -463,9 +467,9 @@ namespace osu.Game.Screens.SelectV2 } } - var beatmaps = items.Select(i => i.Model).OfType(); + var beatmaps = items.Select(i => i.Model).OfType(); - if (beatmaps.Any(b => b.Equals(CurrentSelection as BeatmapInfo))) + if (beatmaps.Any(b => b.Equals(CurrentSelection as GroupedBeatmap))) return; RequestRecommendedSelection(beatmaps); @@ -784,8 +788,8 @@ namespace osu.Game.Screens.SelectV2 #region Random selection handling private readonly Bindable randomAlgorithm = new Bindable(); - private readonly List previouslyVisitedRandomBeatmaps = new List(); - private readonly List randomHistory = new List(); + private readonly HashSet previouslyVisitedRandomBeatmaps = new HashSet(); + private readonly List randomHistory = new List(); private Sample? spinSample; private Sample? randomSelectSample; @@ -798,7 +802,7 @@ namespace osu.Game.Screens.SelectV2 return false; var selectionBefore = CurrentSelectionItem; - var beatmapBefore = selectionBefore?.Model as BeatmapInfo; + var beatmapBefore = selectionBefore?.Model as GroupedBeatmap; bool success; @@ -808,7 +812,7 @@ namespace osu.Game.Screens.SelectV2 randomHistory.Add(beatmapBefore); // keep track of visited beatmaps for "RandomPermutation" random tracking. // note that this is reset when we run out of beatmaps, while `randomHistory` is not. - previouslyVisitedRandomBeatmaps.Add(beatmapBefore); + previouslyVisitedRandomBeatmaps.Add(beatmapBefore.Beatmap); } if (grouping.BeatmapSetsGroupedTogether) @@ -836,29 +840,29 @@ namespace osu.Game.Screens.SelectV2 private bool nextRandomBeatmap() { - ICollection visibleBeatmaps = ExpandedGroup != null + ICollection visibleBeatmaps = ExpandedGroup != null // In the case of grouping, users expect random to only operate on the expanded group. // This is going to incur some overhead as we don't have a group-beatmapset mapping currently. // // If this becomes an issue, we could either store a mapping, or run the random algorithm many times // using the `SetItems` method until we get a group HIT. - ? grouping.GroupItems[ExpandedGroup].Select(i => i.Model).OfType().ToArray() - : GetCarouselItems()!.Select(i => i.Model).OfType().ToArray(); + ? grouping.GroupItems[ExpandedGroup].Select(i => i.Model).OfType().ToArray() + : GetCarouselItems()!.Select(i => i.Model).OfType().ToArray(); - BeatmapInfo beatmap; + GroupedBeatmap beatmap; switch (randomAlgorithm.Value) { case RandomSelectAlgorithm.RandomPermutation: { - ICollection notYetVisitedBeatmaps = visibleBeatmaps.Except(previouslyVisitedRandomBeatmaps).ToList(); + ICollection notYetVisitedBeatmaps = visibleBeatmaps.ExceptBy(previouslyVisitedRandomBeatmaps, gb => gb.Beatmap).ToList(); if (!notYetVisitedBeatmaps.Any()) { - previouslyVisitedRandomBeatmaps.RemoveAll(b => visibleBeatmaps.Contains(b)); + previouslyVisitedRandomBeatmaps.ExceptWith(visibleBeatmaps.Select(b => b.Beatmap)); notYetVisitedBeatmaps = visibleBeatmaps; - if (CurrentSelection is BeatmapInfo beatmapInfo) - notYetVisitedBeatmaps = notYetVisitedBeatmaps.Except([beatmapInfo]).ToList(); + if (CurrentSelection is GroupedBeatmap groupedBeatmap) + notYetVisitedBeatmaps = notYetVisitedBeatmaps.Except([groupedBeatmap]).ToList(); } if (notYetVisitedBeatmaps.Count == 0) @@ -882,7 +886,7 @@ namespace osu.Game.Screens.SelectV2 private bool nextRandomSet() { - ICollection visibleSetsUnderGrouping = ExpandedGroup != null + ICollection visibleGroupedSets = ExpandedGroup != null // In the case of grouping, users expect random to only operate on the expanded group. // This is going to incur some overhead as we don't have a group-beatmapset mapping currently. // @@ -899,14 +903,14 @@ namespace osu.Game.Screens.SelectV2 case RandomSelectAlgorithm.RandomPermutation: { ICollection notYetVisitedSets = - visibleSetsUnderGrouping.ExceptBy(previouslyVisitedRandomBeatmaps.Select(b => b.BeatmapSet!), groupedSet => groupedSet.BeatmapSet).ToList(); + visibleGroupedSets.ExceptBy(previouslyVisitedRandomBeatmaps.Select(b => b.BeatmapSet!), groupedSet => groupedSet.BeatmapSet).ToList(); if (!notYetVisitedSets.Any()) { - previouslyVisitedRandomBeatmaps.RemoveAll(b => visibleSetsUnderGrouping.Any(groupedSet => groupedSet.BeatmapSet.Equals(b.BeatmapSet!))); - notYetVisitedSets = visibleSetsUnderGrouping; - if (CurrentSelection is BeatmapInfo beatmapInfo) - notYetVisitedSets = notYetVisitedSets.ExceptBy([beatmapInfo.BeatmapSet!], groupedSet => groupedSet.BeatmapSet).ToList(); + previouslyVisitedRandomBeatmaps.ExceptWith(visibleGroupedSets.SelectMany(setUnderGrouping => setUnderGrouping.BeatmapSet.Beatmaps)); + notYetVisitedSets = visibleGroupedSets; + if (CurrentSelection is GroupedBeatmap groupedBeatmap) + notYetVisitedSets = notYetVisitedSets.ExceptBy([groupedBeatmap.Beatmap.BeatmapSet!], groupedSet => groupedSet.BeatmapSet).ToList(); } if (notYetVisitedSets.Count == 0) @@ -917,7 +921,7 @@ namespace osu.Game.Screens.SelectV2 } case RandomSelectAlgorithm.Random: - set = visibleSetsUnderGrouping.ElementAt(RNG.Next(visibleSetsUnderGrouping.Count)); + set = visibleGroupedSets.ElementAt(RNG.Next(visibleGroupedSets.Count)); break; default: @@ -940,15 +944,15 @@ namespace osu.Game.Screens.SelectV2 var previousBeatmap = randomHistory[^1]; randomHistory.RemoveAt(randomHistory.Count - 1); - var previousBeatmapItem = carouselItems.FirstOrDefault(i => i.Model is BeatmapInfo b && b.Equals(previousBeatmap)); + var previousBeatmapItem = carouselItems.FirstOrDefault(i => i.Model is GroupedBeatmap gb && gb.Equals(previousBeatmap)); if (previousBeatmapItem == null) return false; - if (CurrentSelection is BeatmapInfo beatmapInfo) + if (CurrentSelection is GroupedBeatmap groupedBeatmap) { if (randomAlgorithm.Value == RandomSelectAlgorithm.RandomPermutation) - previouslyVisitedRandomBeatmaps.Remove(beatmapInfo); + previouslyVisitedRandomBeatmaps.Remove(groupedBeatmap.Beatmap); if (CurrentSelectionItem == null) playSpinSample(0); diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index 947b8f9c7c..5cca9467c2 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -289,9 +289,10 @@ namespace osu.Game.Screens.SelectV2 }); } - private void requestRecommendedSelection(IEnumerable b) + private void requestRecommendedSelection(IEnumerable groupedBeatmaps) { - queueBeatmapSelection(difficultyRecommender?.GetRecommendedBeatmap(b) ?? b.First()); + var recommendedBeatmap = difficultyRecommender?.GetRecommendedBeatmap(groupedBeatmaps.Select(gb => gb.Beatmap)) ?? groupedBeatmaps.First().Beatmap; + queueBeatmapSelection(groupedBeatmaps.First(bug => bug.Beatmap.Equals(recommendedBeatmap))); } /// @@ -472,22 +473,24 @@ namespace osu.Game.Screens.SelectV2 /// - After , update the global beatmap. This in turn causes song select visuals (title, details, leaderboard) to update. /// This debounce is intended to avoid high overheads from churning lookups while a user is changing selection via rapid keyboard operations. /// - /// The beatmap to be selected. - private void queueBeatmapSelection(BeatmapInfo beatmap) + /// The beatmap to be selected. + private void queueBeatmapSelection(GroupedBeatmap groupedBeatmap) { if (!this.IsCurrentScreen()) return; - carousel.CurrentSelection = beatmap; + carousel.CurrentSelection = groupedBeatmap; // Debounce consideration is to avoid beatmap churn on key repeat selection. selectionDebounce?.Cancel(); selectionDebounce = Scheduler.AddDelayed(() => { - if (Beatmap.Value.BeatmapInfo.Equals(beatmap)) + var beatmapInfo = groupedBeatmap.Beatmap; + + if (Beatmap.Value.BeatmapInfo.Equals(beatmapInfo)) return; - Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap); + Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmapInfo); }, SELECTION_DEBOUNCE); } @@ -532,7 +535,8 @@ namespace osu.Game.Screens.SelectV2 if (validBeatmaps.Any()) { - requestRecommendedSelection(validBeatmaps); + // TODO: this needs a primitive that tells the carousel "I need this beatmap to be selected, you figure out the grouping". + //requestRecommendedSelection(validBeatmaps); return true; } } From 3f637db39162f7f1d6693a9ef22434cf64c4f5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 10:40:36 +0200 Subject: [PATCH 06/22] Fix obvious test failures from using `GroupedBeatmap` in `BeatmapCarousel.CurrentSelection` --- .../SongSelectV2/BeatmapCarouselTestScene.cs | 2 +- .../TestSceneBeatmapCarouselFiltering.cs | 6 +-- .../TestSceneBeatmapCarouselRandom.cs | 18 ++++----- .../TestSceneBeatmapCarouselScrolling.cs | 10 ++--- .../TestSceneBeatmapCarouselUpdateHandling.cs | 40 +++++++++---------- ...neSongSelectCurrentSelectionInvalidated.cs | 2 +- 6 files changed, 39 insertions(+), 39 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index c60ee55110..b616055157 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -300,7 +300,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 } else { - AddUntilStep($"selected is set{set}", () => BeatmapSets[set].Beatmaps.Contains(Carousel.CurrentSelection)); + AddUntilStep($"selected is set{set}", () => BeatmapSets[set].Beatmaps.Contains(((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap)); } } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs index 70af48069a..b232d12e46 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs @@ -201,13 +201,13 @@ namespace osu.Game.Tests.Visual.SongSelectV2 int diff = i; AddStep($"select diff {diff}", () => Carousel.CurrentSelection = chosenBeatmap = BeatmapSets[20].Beatmaps[diff]); - AddUntilStep("selection changed", () => Carousel.CurrentSelection, () => Is.EqualTo(chosenBeatmap)); + AddUntilStep("selection changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); SortBy(SortMode.Difficulty); - AddAssert("selection retained", () => Carousel.CurrentSelection, () => Is.EqualTo(chosenBeatmap)); + AddAssert("selection retained", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); SortBy(SortMode.Title); - AddAssert("selection retained", () => Carousel.CurrentSelection, () => Is.EqualTo(chosenBeatmap)); + AddAssert("selection retained", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); } } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs index ed694c9e3d..0e35f5e45d 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs @@ -50,12 +50,12 @@ namespace osu.Game.Tests.Visual.SongSelectV2 nextRandom(); ensureRandomDidNotRepeat(); - AddStep("store selection", () => originalSelected = (BeatmapInfo)Carousel.CurrentSelection!); + AddStep("store selection", () => originalSelected = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap); SortAndGroupBy(SortMode.Artist, GroupMode.Difficulty); WaitForFiltering(); - AddAssert("selection not changed", () => Carousel.CurrentSelection, () => Is.EqualTo(originalSelected)); + AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(originalSelected)); storeExpandedGroup(); @@ -253,12 +253,12 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddBeatmaps(10, 3, true); WaitForDrawablePanels(); - BeatmapInfo? originalSelected = null; + GroupedBeatmap? originalSelected = null; nextRandom(); CheckHasSelection(); - AddStep("store selection", () => originalSelected = (BeatmapInfo)Carousel.CurrentSelection!); + AddStep("store selection", () => originalSelected = ((GroupedBeatmap)Carousel.CurrentSelection!)); AddStep("random then rewind", () => { @@ -275,20 +275,20 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddBeatmaps(10, 3, true); WaitForDrawablePanels(); - BeatmapInfo? originalSelected = null; - BeatmapInfo? postRandomSelection = null; + GroupedBeatmap? originalSelected = null; + GroupedBeatmap? postRandomSelection = null; nextRandom(); CheckHasSelection(); - AddStep("store selection", () => originalSelected = (BeatmapInfo)Carousel.CurrentSelection!); + AddStep("store selection", () => originalSelected = (GroupedBeatmap)Carousel.CurrentSelection!); nextRandom(); - AddStep("store selection", () => postRandomSelection = (BeatmapInfo)Carousel.CurrentSelection!); + AddStep("store selection", () => postRandomSelection = (GroupedBeatmap)Carousel.CurrentSelection!); AddAssert("selection changed", () => originalSelected, () => Is.Not.SameAs(postRandomSelection)); - AddStep("delete previous selection beatmaps", () => BeatmapSets.Remove(originalSelected!.BeatmapSet!)); + AddStep("delete previous selection beatmaps", () => BeatmapSets.Remove(originalSelected!.Beatmap.BeatmapSet!)); WaitForFiltering(); AddAssert("selection not changed", () => Carousel.CurrentSelection, () => Is.EqualTo(postRandomSelection)); diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs index 29aa976fe3..d05c874641 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First()); + AddStep("select middle beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); WaitForScrolling(); @@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First()); + AddStep("select middle beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); WaitForScrolling(); AddStep("override scroll with user scroll", () => @@ -71,7 +71,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("scroll to end", () => Scroll.ScrollToEnd(false)); - AddStep("select last beatmap", () => Carousel.CurrentSelection = BeatmapSets.Last().Beatmaps.Last()); + AddStep("select last beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.Last().Beatmaps.Last())); WaitForScrolling(); @@ -88,7 +88,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select first beatmap", () => Carousel.CurrentSelection = BeatmapSets.First().Beatmaps.First()); + AddStep("select first beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); WaitForScrolling(); @@ -108,7 +108,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select first beatmap", () => Carousel.CurrentSelection = BeatmapSets.First().Beatmaps.First()); + AddStep("select first beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); WaitForScrolling(); AddStep("override scroll with user scroll", () => diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs index 3638c8eeec..a331879684 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs @@ -179,8 +179,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => { @@ -195,8 +195,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } [Test] // Checks that we keep selection based on online ID where possible. @@ -205,15 +205,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.DifficultyName = "new name"); assertDidFilter(); WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } [Test] // Checks that we fallback to keeping selection based on difficulty name. @@ -222,15 +222,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.OnlineID = b.OnlineID + 1); assertDidFilter(); WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } [Test] // Checks that we don't crash if there exists a difficulty with the same online ID as the selected difficulty. @@ -239,8 +239,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); // Add another difficulty with same online ID. updateBeatmap(null, bs => @@ -252,8 +252,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } [Test] // Checks that we don't crash if there exists a difficulty with the same name as the selected difficulty. @@ -262,8 +262,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); // Remove original selected difficulty, and add two difficulties with same name as selection. updateBeatmap(null, bs => @@ -284,8 +284,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } /// diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs index 0736925584..c480d6ca7e 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 /// public partial class TestSceneSongSelectCurrentSelectionInvalidated : SongSelectTestScene { - private BeatmapInfo? selectedBeatmap => (BeatmapInfo?)Carousel.CurrentSelection; + private BeatmapInfo? selectedBeatmap => (Carousel.CurrentSelection as GroupedBeatmap)?.Beatmap; private BeatmapSetInfo? selectedBeatmapSet => selectedBeatmap?.BeatmapSet; [SetUpSteps] From 107e103825cc64c7ebc66fa65e415f93639a9c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 10:59:04 +0200 Subject: [PATCH 07/22] Fix changing group mode causing `CurrentSelection` to retain a stale `GroupDefinition` --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 46ea61ca9d..f58d0bbf1d 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -418,9 +418,21 @@ namespace osu.Game.Screens.SelectV2 // 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); + 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 the current selection's group still exists. + if (currentGroupedBeatmap?.Group != null && grouping.GroupItems.ContainsKey(currentGroupedBeatmap.Group)) + { + // If the group still exists, then 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. + var newSelection = GetCarouselItems()?.Select(i => i.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(currentGroupedBeatmap.Beatmap)); + CurrentSelection = newSelection; + } // 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. From 3cf0a9b9c02621118720087942ecc4cf674dab53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 11:25:15 +0200 Subject: [PATCH 08/22] Fix standalone beatmap panels not having the correct height --- osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs index 9fa4e28e93..69f5596578 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs @@ -121,11 +121,12 @@ namespace osu.Game.Screens.SelectV2 { if (groupItem != null) groupItem.NestedItemCount++; - - item.DrawHeight = PanelBeatmapStandalone.HEIGHT; } - addItem(new CarouselItem(new GroupedBeatmap(group, beatmap))); + addItem(new CarouselItem(new GroupedBeatmap(group, beatmap)) + { + DrawHeight = BeatmapSetsGroupedTogether ? PanelBeatmap.HEIGHT : PanelBeatmapStandalone.HEIGHT, + }); lastBeatmap = beatmap; displayedBeatmapsCount++; } From dfed564bda7408ec4fd1c82ab15f90f410cdd444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 11:45:48 +0200 Subject: [PATCH 09/22] Allow `BeatmapCarousel.CurrentSelection` to accept raw `BeatmapInfo`s (and redirect to appropriate grouped beatmap) This is probably where things get a little controversial. There are some song select flows wherein song select just wants to ensure sanity by authoritatively setting the global beatmap. The goal is to change the beatmap immediately and instantly. Therefore it should kind of be the carousel's job to figure out its grouping complications. To that end, `CurrentSelection` is made virtual, and overridden in `BeatmapCarousel` to perform a sort of reconciliation logic. If an external component sets `CurrentSelection` to a `BeatmapInfo`, one of the two following things happen: - Nothing, if the current `GroupedBeatmap` is already a copy of the beatmap that needs to be selected, or - The carousel looks at its items, finds any first copy which matches the beatmap that the external consumer wanted selected, and changes selection to that instead. --- osu.Game/Graphics/Carousel/Carousel.cs | 2 +- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 20 ++++++++++++++++++++ osu.Game/Screens/SelectV2/SongSelect.cs | 3 +-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index b81df0a7eb..5adc37ea40 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -107,7 +107,7 @@ namespace osu.Game.Graphics.Carousel /// The selection is never reset due to not existing. It can be set to anything. /// If no matching carousel item exists, there will be no visually selected item while waiting for potential new item which matches. /// - public object? CurrentSelection + public virtual object? CurrentSelection { get => currentSelection.Model; set diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index f58d0bbf1d..55751718b1 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -295,6 +295,26 @@ namespace osu.Game.Screens.SelectV2 protected override bool ShouldActivateOnKeyboardSelection(CarouselItem item) => grouping.BeatmapSetsGroupedTogether && item.Model is GroupedBeatmap; + public override object? CurrentSelection + { + get => base.CurrentSelection; + set + { + // this is a special pathway for external consumers who only care about showing some particular copy of a beatmap + // (there could be multiple panels for one beatmap due to grouping). + // in this pathway we basically figure out what group to use internally, and continue working with `GroupedBeatmap` all the way after that. + if (value is BeatmapInfo beatmapInfo) + { + if (CurrentSelection is GroupedBeatmap groupedBeatmap && beatmapInfo.Equals(groupedBeatmap.Beatmap)) + return; + + value = GetCarouselItems()?.Select(item => item.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(beatmapInfo)); + } + + base.CurrentSelection = value; + } + } + protected override void HandleItemActivated(CarouselItem item) { try diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index 5cca9467c2..64c85dd31a 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -535,8 +535,7 @@ namespace osu.Game.Screens.SelectV2 if (validBeatmaps.Any()) { - // TODO: this needs a primitive that tells the carousel "I need this beatmap to be selected, you figure out the grouping". - //requestRecommendedSelection(validBeatmaps); + carousel.CurrentSelection = difficultyRecommender?.GetRecommendedBeatmap(validBeatmaps) ?? validBeatmaps.First(); return true; } } From 1bb24c923dca4943e7d19cdd0ffde9959896500d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 12:23:51 +0200 Subject: [PATCH 10/22] Fix stale group refresh logic inadvertently losing selection entirely sometimes --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 55751718b1..b5f2a3aa25 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -451,7 +451,10 @@ namespace osu.Game.Screens.SelectV2 { // If the group no longer exists, 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)); - CurrentSelection = newSelection; + // 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. From 89492cbd81e5e211286ca4b06c7de35d58e3c446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 12:48:28 +0200 Subject: [PATCH 11/22] Do not attempt to refresh group in current selection if grouping is not relevant --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index b5f2a3aa25..20a3516613 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -441,10 +441,13 @@ namespace osu.Game.Screens.SelectV2 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 the current selection's group still exists. - if (currentGroupedBeatmap?.Group != null && grouping.GroupItems.ContainsKey(currentGroupedBeatmap.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) { - // If the group still exists, then only update the visual state of the selected item. + // Only update the visual state of the selected item. HandleItemSelected(currentGroupedBeatmap); } else if (currentGroupedBeatmap != null) From 2b52c1de0b123a7e7699de913a0f7d7d0bae03d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 13:45:27 +0200 Subject: [PATCH 12/22] Fix presenting individual beatmaps from main menu breaking In this case `CurrentSelection` is being set on the song select screen's `OnEntering()`, at which point grouping is not yet known. --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 20a3516613..1cd9396e5a 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -308,7 +308,12 @@ namespace osu.Game.Screens.SelectV2 if (CurrentSelection is GroupedBeatmap groupedBeatmap && beatmapInfo.Equals(groupedBeatmap.Beatmap)) return; - value = GetCarouselItems()?.Select(item => item.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(beatmapInfo)); + // it is not universally guaranteed that the carousel items will be materialised at the time this is set. + // therefore, in cases where it is known that they will not be, default to a null group. + // even if grouping is active, this will be rectified to a correct group on the next invocation of `HandleFilterCompleted()`. + value = IsLoaded && !IsFiltering + ? GetCarouselItems()?.Select(item => item.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(beatmapInfo)) + : new GroupedBeatmap(null, beatmapInfo); } base.CurrentSelection = value; From a27fef243780ab4b4915633b1b3cac15123be70d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 14:06:53 +0200 Subject: [PATCH 13/22] Add failing test for rewind not working over grouping mode changes --- .../TestSceneBeatmapCarouselRandom.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs index 0e35f5e45d..9f31c875b6 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs @@ -247,6 +247,30 @@ namespace osu.Game.Tests.Visual.SongSelectV2 } } + [Test] + public void TestRewindOverGroupingModeChange() + { + const int local_set_count = 3; + + SortAndGroupBy(SortMode.Artist, GroupMode.Artist); + AddBeatmaps(local_set_count, 3); + WaitForDrawablePanels(); + + SelectNextSet(); + + for (int i = 0; i < local_set_count; i++) + nextRandom(); + + SortAndGroupBy(SortMode.Title, GroupMode.LastPlayed); + WaitForDrawablePanels(); + + for (int i = 0; i < local_set_count; i++) + { + prevRandomSet(); + checkRewindCorrectSet(); + } + } + [Test] public void TestRandomThenRewindSameFrame() { From 4ffc262073804e5fdad0e62a86832e81618635a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 29 Aug 2025 14:11:40 +0200 Subject: [PATCH 14/22] Fix rewind not working over grouping mode changes --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 1cd9396e5a..6c98630274 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -987,7 +987,11 @@ namespace osu.Game.Screens.SelectV2 var previousBeatmap = randomHistory[^1]; randomHistory.RemoveAt(randomHistory.Count - 1); - var previousBeatmapItem = carouselItems.FirstOrDefault(i => i.Model is GroupedBeatmap gb && gb.Equals(previousBeatmap)); + // when going back through rewind history, we may no longer be in the same grouping mode. + // the user wants to go back to the beatmap first and foremost, so the most important thing is to find a panel that corresponds to the beatmap. + // going back to the same group is a nice-to-have, but a secondary concern. + var previousBeatmapItem = carouselItems.Where(i => i.Model is GroupedBeatmap gb && gb.Beatmap.Equals(previousBeatmap.Beatmap)) + .MaxBy(i => ((GroupedBeatmap)i.Model).Group == previousBeatmap.Group); if (previousBeatmapItem == null) return false; @@ -1003,7 +1007,7 @@ namespace osu.Game.Screens.SelectV2 playSpinSample(visiblePanelCountBetweenItems(previousBeatmapItem, CurrentSelectionItem)); } - RequestSelection(previousBeatmap); + RequestSelection((GroupedBeatmap)previousBeatmapItem.Model); return true; } From 6399f7e3db1645d6d9ea4a2cf3c083a77169d1c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 3 Sep 2025 08:59:33 +0200 Subject: [PATCH 15/22] Add failing test case --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 62e7a80435..66e286cf4f 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -294,6 +294,39 @@ namespace osu.Game.Tests.Skins.IO #endregion + /// + /// Note that this test passing / failing is platform / OS-specific (if it is to fail, it'll fail on windows). + /// + [Test] + public async Task TestExternallyMountingImportWithInvalidFilename() + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) + { + try + { + var osu = LoadOsuIntoHost(host); + + var zipStream = new MemoryStream(); + using var zip = ZipArchive.Create(); + zip.AddEntry("test?.png", new MemoryStream(new byte[] { 0xDE, 0xAD, 0xBE, 0xEF })); + zip.SaveTo(zipStream); + + var import = await loadSkinIntoOsu(osu, new ImportTask(zipStream, "test skin.osk")); + + var skinManager = osu.Dependencies.Get(); + var externalEdit = await skinManager.BeginExternalEditing(import.PerformRead(s => s.Detach())); // should not fail + + Task finishTask = Task.CompletedTask; + host.UpdateThread.Scheduler.Add(() => finishTask = externalEdit.Finish()); + await finishTask; + } + finally + { + host.Exit(); + } + } + } + private void assertCorrectMetadata(Live import1, string name, string creator, decimal version, OsuGameBase osu) { import1.PerformRead(i => From 51e75934462d78bb83122d67b54b6e2d258c2a66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 3 Sep 2025 08:59:57 +0200 Subject: [PATCH 16/22] Fix external edit operations failing due to invalid filenames --- osu.Game/Database/RealmArchiveModelImporter.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index a3cdc2dc77..0f9832578b 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -208,7 +208,16 @@ namespace osu.Game.Database foreach (var realmFile in model.Files) { string sourcePath = Files.Storage.GetFullPath(realmFile.File.GetStoragePath()); - string destinationPath = Path.Join(mountedPath, realmFile.Filename); + // there are edge cases where externalising an imported model to the filesystem could fail due to invalid filenames. + // one scenario where this happens goes something like this: + // - stable user exports an archive, which contains filenames that get mangled by stable's default zip encoding codepage (Shift-JIS) + // - said archive is imported to lazer, but the invalid filename is not actually an issue due to lazer file store structure + // (the file is stored under a filename correspondent to its SHA instead, and its real filename is only stored in realm) + // - however attempts to externally edit the model fail as the external edit attempts and fails to produce the file's "real" filename in the mounted path + // to prevent this bricking external edit, strip invalid characters on external edit. + // the presumption here is that whatever produced the mangled archive is primarily at fault here, and we're just trying to trudge on locally as best as possible. + // if there are further troubles related to similar issues, reevaluate moving this sort of check to the import side instead (sanitising filenames on import from archive). + string destinationPath = Path.Join(mountedPath, realmFile.Filename.GetValidFilename()); Directory.CreateDirectory(Path.GetDirectoryName(destinationPath)!); From 5c66998c57850993b671ab1f17185908f91721ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 3 Sep 2025 09:32:43 +0200 Subject: [PATCH 17/22] Replace local copy of `GetValidFilename()` with direct usage Noticed in passing. --- osu.Game/Beatmaps/WorkingBeatmapCache.cs | 26 +++++------------------- osu.Game/Extensions/ModelExtensions.cs | 1 + 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/osu.Game/Beatmaps/WorkingBeatmapCache.cs b/osu.Game/Beatmaps/WorkingBeatmapCache.cs index 30bbbbc1fe..9957935977 100644 --- a/osu.Game/Beatmaps/WorkingBeatmapCache.cs +++ b/osu.Game/Beatmaps/WorkingBeatmapCache.cs @@ -20,6 +20,7 @@ using osu.Framework.Platform; using osu.Framework.Statistics; using osu.Game.Beatmaps.Formats; using osu.Game.Database; +using osu.Game.Extensions; using osu.Game.IO; using osu.Game.Skinning; using osu.Game.Storyboards; @@ -341,27 +342,10 @@ namespace osu.Game.Beatmaps { // Matches stable implementation, because it's probably simpler than trying to do anything else. // This may need to be reconsidered after we begin storing storyboards in the new editor. - return windowsFilenameStrip( - (metadata.Artist.Length > 0 ? metadata.Artist + @" - " + metadata.Title : Path.GetFileNameWithoutExtension(metadata.AudioFile)) - + (metadata.Author.Username.Length > 0 ? @" (" + metadata.Author.Username + @")" : string.Empty) - + @".osb"); - - string windowsFilenameStrip(string entry) - { - // Inlined from Path.GetInvalidFilenameChars() to ensure the windows characters are used (to match stable). - char[] invalidCharacters = - { - '\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', - '\x08', '\x09', '\x0A', '\x0B', '\x0C', '\x0D', '\x0E', '\x0F', '\x10', '\x11', '\x12', - '\x13', '\x14', '\x15', '\x16', '\x17', '\x18', '\x19', '\x1A', '\x1B', '\x1C', '\x1D', - '\x1E', '\x1F', '\x22', '\x3C', '\x3E', '\x7C', ':', '*', '?', '\\', '/' - }; - - foreach (char c in invalidCharacters) - entry = entry.Replace(c.ToString(), string.Empty); - - return entry; - } + string baseFilename = (metadata.Artist.Length > 0 ? metadata.Artist + @" - " + metadata.Title : Path.GetFileNameWithoutExtension(metadata.AudioFile)) + + (metadata.Author.Username.Length > 0 ? @" (" + metadata.Author.Username + @")" : string.Empty) + + @".osb"; + return baseFilename.GetValidFilename(); } } } diff --git a/osu.Game/Extensions/ModelExtensions.cs b/osu.Game/Extensions/ModelExtensions.cs index 18c991297a..7c9d929999 100644 --- a/osu.Game/Extensions/ModelExtensions.cs +++ b/osu.Game/Extensions/ModelExtensions.cs @@ -175,6 +175,7 @@ namespace osu.Game.Extensions /// DO NOT CHANGE THE SEMANTICS OF THIS METHOD unless you know well what you are doing. /// /// + /// public static string GetValidFilename(this string filename) { foreach (char c in invalid_filename_chars) From 315eea8d9ade4e2b2f23a4d1cfa4214670fe45c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 3 Sep 2025 09:52:37 +0200 Subject: [PATCH 18/22] Simplify beatmap access in carousel panels --- osu.Game/Screens/SelectV2/PanelBeatmap.cs | 18 ++---------------- .../Screens/SelectV2/PanelBeatmapStandalone.cs | 17 ++--------------- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/osu.Game/Screens/SelectV2/PanelBeatmap.cs b/osu.Game/Screens/SelectV2/PanelBeatmap.cs index 545439684b..7dfd82cd1f 100644 --- a/osu.Game/Screens/SelectV2/PanelBeatmap.cs +++ b/osu.Game/Screens/SelectV2/PanelBeatmap.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -72,14 +71,7 @@ namespace osu.Game.Screens.SelectV2 [Resolved] private ISongSelect? songSelect { get; set; } - private GroupedBeatmap groupedBeatmap - { - get - { - Debug.Assert(Item != null); - return (GroupedBeatmap)Item!.Model; - } - } + private BeatmapInfo beatmap => ((GroupedBeatmap)Item!.Model).Beatmap; public PanelBeatmap() { @@ -216,8 +208,6 @@ namespace osu.Game.Screens.SelectV2 { base.PrepareForUse(); - var beatmap = groupedBeatmap.Beatmap; - difficultyIcon.Icon = getRulesetIcon(beatmap.Ruleset); localRank.Beatmap = beatmap; @@ -256,8 +246,6 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = groupedBeatmap.Beatmap; - starDifficultyBindable = difficultyCache.GetBindableDifficulty(beatmap, starDifficultyCancellationSource.Token, SongSelect.SELECTION_DEBOUNCE); starDifficultyBindable.BindValueChanged(starDifficulty => { @@ -301,8 +289,6 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = groupedBeatmap.Beatmap; - if (ruleset.Value.OnlineID == 3) { // Account for mania differences locally for now. @@ -327,7 +313,7 @@ namespace osu.Game.Screens.SelectV2 List items = new List(); if (songSelect != null) - items.AddRange(songSelect.GetForwardActions(groupedBeatmap.Beatmap)); + items.AddRange(songSelect.GetForwardActions(beatmap)); return items.ToArray(); } diff --git a/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs b/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs index 226a1b1d06..b54eecd548 100644 --- a/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs +++ b/osu.Game/Screens/SelectV2/PanelBeatmapStandalone.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; @@ -73,14 +72,7 @@ namespace osu.Game.Screens.SelectV2 private Box backgroundBorder = null!; - private GroupedBeatmap groupedBeatmap - { - get - { - Debug.Assert(Item != null); - return (GroupedBeatmap)Item!.Model; - } - } + private BeatmapInfo beatmap => ((GroupedBeatmap)Item!.Model).Beatmap; public PanelBeatmapStandalone() { @@ -228,7 +220,6 @@ namespace osu.Game.Screens.SelectV2 { base.PrepareForUse(); - var beatmap = groupedBeatmap.Beatmap; var beatmapSet = beatmap.BeatmapSet!; beatmapBackground.Beatmap = beatmaps.GetWorkingBeatmap(beatmap); @@ -269,8 +260,6 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = groupedBeatmap.Beatmap; - starDifficultyBindable = difficultyCache.GetBindableDifficulty(beatmap, starDifficultyCancellationSource.Token, SongSelect.SELECTION_DEBOUNCE); starDifficultyBindable.BindValueChanged(starDifficulty => { @@ -307,8 +296,6 @@ namespace osu.Game.Screens.SelectV2 if (Item == null) return; - var beatmap = groupedBeatmap.Beatmap; - if (ruleset.Value.OnlineID == 3) { // Account for mania differences locally for now. @@ -333,7 +320,7 @@ namespace osu.Game.Screens.SelectV2 List items = new List(); if (songSelect != null) - items.AddRange(songSelect.GetForwardActions(groupedBeatmap.Beatmap)); + items.AddRange(songSelect.GetForwardActions(beatmap)); return items.ToArray(); } From a840e55977aed7510ee8610035b34b670a87f7f4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 3 Sep 2025 17:14:45 +0900 Subject: [PATCH 19/22] Check exported filenames for added safety --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 66e286cf4f..f909638333 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -316,6 +316,13 @@ namespace osu.Game.Tests.Skins.IO var skinManager = osu.Dependencies.Get(); var externalEdit = await skinManager.BeginExternalEditing(import.PerformRead(s => s.Detach())); // should not fail + Assert.That(Directory.Exists(externalEdit.MountedPath)); + Assert.That(new DirectoryInfo(externalEdit.MountedPath).GetFiles().Select(f => f.Name), Is.EquivalentTo(new[] + { + "skin.ini", + "test.png" + })); + Task finishTask = Task.CompletedTask; host.UpdateThread.Scheduler.Add(() => finishTask = externalEdit.Finish()); await finishTask; From eb1263aa32cc399d0c860178ac4f07757cc44377 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 3 Sep 2025 17:23:41 +0900 Subject: [PATCH 20/22] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index 46d558354e..5d9158a45a 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -10,7 +10,7 @@ true - + diff --git a/osu.iOS.props b/osu.iOS.props index 3768550c21..8e269d292d 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -17,6 +17,6 @@ -all - + From 4a7ee6fafc75240a36a810e79f01eced48ecda5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 3 Sep 2025 10:24:25 +0200 Subject: [PATCH 21/22] Hide object-typed `CurrentSelection` and expose strongly-typed alternatives instead --- .../Background/TestSceneUserDimBackgrounds.cs | 2 +- .../SongSelectV2/BeatmapCarouselTestScene.cs | 17 +++---- .../TestSceneBeatmapCarouselArtistGrouping.cs | 4 +- ...tSceneBeatmapCarouselDifficultyGrouping.cs | 6 +-- .../TestSceneBeatmapCarouselFiltering.cs | 20 ++++---- .../TestSceneBeatmapCarouselNoGrouping.cs | 12 ++--- .../TestSceneBeatmapCarouselRandom.cs | 16 +++--- .../TestSceneBeatmapCarouselScrolling.cs | 10 ++-- .../TestSceneBeatmapCarouselUpdateHandling.cs | 20 ++++---- ...neSongSelectCurrentSelectionInvalidated.cs | 2 +- osu.Game/Graphics/Carousel/Carousel.cs | 2 +- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 51 +++++++++++++------ osu.Game/Screens/SelectV2/SongSelect.cs | 6 +-- 13 files changed, 93 insertions(+), 75 deletions(-) diff --git a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs index 58fb02c90c..3021589cdb 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs @@ -325,7 +325,7 @@ namespace osu.Game.Tests.Visual.Background private void setupUserSettings() { AddUntilStep("Song select is current", () => songSelect.IsCurrentScreen()); - AddUntilStep("Song select has selection", () => songSelect.Carousel?.CurrentSelection != null); + AddUntilStep("Song select has selection", () => songSelect.Carousel?.CurrentGroupedBeatmap != null); AddStep("Set default user settings", () => { SelectedMods.Value = new[] { new OsuModNoFail() }; diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index b616055157..a180097863 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -118,14 +118,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 RequestSelection = b => { BeatmapRequestedSelections.Push(b.Beatmap); - Carousel.CurrentSelection = b; + Carousel.CurrentGroupedBeatmap = b; }, RequestRecommendedSelection = groupedBeatmaps => { var recommendedBeatmap = BeatmapRecommendationFunction?.Invoke(groupedBeatmaps.Select(gb => gb.Beatmap)) ?? groupedBeatmaps.First().Beatmap; var recommendedGroupedBeatmap = groupedBeatmaps.First(gb => gb.Beatmap.Equals(recommendedBeatmap)); BeatmapSetRequestedSelections.Push(recommendedBeatmap.BeatmapSet!); - Carousel.CurrentSelection = recommendedGroupedBeatmap; + Carousel.CurrentGroupedBeatmap = recommendedGroupedBeatmap; }, BleedTop = 50, BleedBottom = 50, @@ -219,8 +219,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 protected void Select() => AddStep("select", () => InputManager.Key(Key.Enter)); - protected void CheckNoSelection() => AddAssert("has no selection", () => Carousel.CurrentSelection, () => Is.Null); - protected void CheckHasSelection() => AddAssert("has selection", () => Carousel.CurrentSelection, () => Is.Not.Null); + protected void CheckNoSelection() => AddAssert("has no selection", () => Carousel.CurrentGroupedBeatmap, () => Is.Null); + protected void CheckHasSelection() => AddAssert("has selection", () => Carousel.CurrentGroupedBeatmap, () => Is.Not.Null); protected void CheckRequestPresentCount(int expected) => AddAssert($"check present count is {expected}", () => Carousel.RequestPresentBeatmapCount, () => Is.EqualTo(expected)); @@ -285,8 +285,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 // offset by one because the group itself is included in the items list. CarouselItem item = groupingFilter.GroupItems[groupDefinition].ElementAt(panel + 1); - return (Carousel.CurrentSelection as GroupedBeatmap)? - .Equals(item.Model as GroupedBeatmap) == true; + return Carousel.CurrentGroupedBeatmap?.Equals(item.Model as GroupedBeatmap) == true; }); } @@ -295,12 +294,12 @@ namespace osu.Game.Tests.Visual.SongSelectV2 if (diff != null) { AddUntilStep($"selected is set{set} diff{diff.Value}", - () => (Carousel.CurrentSelection as GroupedBeatmap)?.Beatmap, + () => Carousel.CurrentBeatmap, () => Is.EqualTo(BeatmapSets[set].Beatmaps[diff.Value])); } else { - AddUntilStep($"selected is set{set}", () => BeatmapSets[set].Beatmaps.Contains(((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap)); + AddUntilStep($"selected is set{set}", () => BeatmapSets[set].Beatmaps.Contains(Carousel.CurrentBeatmap!)); } } @@ -419,7 +418,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 tracked: {Carousel.ItemsTracked} displayable: {Carousel.DisplayableItems} displayed: {Carousel.VisibleItems} - selected: {Carousel.CurrentSelection} + selected: {Carousel.CurrentGroupedBeatmap} """); void createHeader(string text) diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs index 78b6985fdb..c34077889d 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselArtistGrouping.cs @@ -80,7 +80,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null); - AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); RemoveAllBeatmaps(); AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null); @@ -93,7 +93,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); - AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True); diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs index c28860e368..58ecfcbf3b 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs @@ -70,7 +70,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null); - AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); RemoveAllBeatmaps(); AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null); @@ -83,7 +83,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); - AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True); @@ -198,7 +198,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 } private void checkBeatmapIsKeyboardSelected() => - AddUntilStep("check keyboard selected group is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, () => Is.EqualTo(Carousel.CurrentSelection)); + AddUntilStep("check keyboard selected group is beatmap", () => GetKeyboardSelectedPanel()?.Item?.Model, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); private void checkGroupKeyboardSelected(int index) => AddUntilStep($"check keyboard selected group is {index}", () => GetKeyboardSelectedPanel()?.Item?.Model, () => { diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs index b232d12e46..b1bd9fd3ed 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselFiltering.cs @@ -130,14 +130,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextPanel(); Select(); - AddStep("record selection", () => selectedID = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID); + AddStep("record selection", () => selectedID = Carousel.CurrentBeatmap!.ID); for (int i = 0; i < 5; i++) { ApplyToFilterAndWaitForFilter("filter all", c => c.SearchText = Guid.NewGuid().ToString()); - AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); + AddAssert("selection not changed", () => Carousel.CurrentBeatmap!.ID == selectedID); ApplyToFilterAndWaitForFilter("remove filter", c => c.SearchText = string.Empty); - AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); + AddAssert("selection not changed", () => Carousel.CurrentBeatmap!.ID == selectedID); } } @@ -177,14 +177,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); - AddStep("record selection", () => selectedID = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID); + AddStep("record selection", () => selectedID = Carousel.CurrentBeatmap!.ID); for (int i = 0; i < 5; i++) { ApplyToFilterAndWaitForFilter("filter all", c => c.SearchText = Guid.NewGuid().ToString()); - AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); + AddAssert("selection not changed", () => Carousel.CurrentBeatmap!.ID == selectedID); ApplyToFilterAndWaitForFilter("remove filter", c => c.SearchText = string.Empty); - AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap.ID == selectedID); + AddAssert("selection not changed", () => Carousel.CurrentBeatmap!.ID == selectedID); } } @@ -200,14 +200,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { int diff = i; - AddStep($"select diff {diff}", () => Carousel.CurrentSelection = chosenBeatmap = BeatmapSets[20].Beatmaps[diff]); - AddUntilStep("selection changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); + AddStep($"select diff {diff}", () => Carousel.CurrentBeatmap = chosenBeatmap = BeatmapSets[20].Beatmaps[diff]); + AddUntilStep("selection changed", () => Carousel.CurrentBeatmap, () => Is.EqualTo(chosenBeatmap)); SortBy(SortMode.Difficulty); - AddAssert("selection retained", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); + AddAssert("selection retained", () => Carousel.CurrentBeatmap, () => Is.EqualTo(chosenBeatmap)); SortBy(SortMode.Title); - AddAssert("selection retained", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(chosenBeatmap)); + AddAssert("selection retained", () => Carousel.CurrentBeatmap, () => Is.EqualTo(chosenBeatmap)); } } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs index 0b0f93b3bc..c839a28055 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselNoGrouping.cs @@ -89,7 +89,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 CheckHasSelection(); AddAssert("drawable selection non-null", () => selection, () => Is.Not.Null); - AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("drawable selection matches carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); RemoveAllBeatmaps(); AddUntilStep("no drawable selection", GetSelectedPanel, () => Is.Null); @@ -102,7 +102,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("add previous selection", () => BeatmapSets.Add(((GroupedBeatmap)selection!).Beatmap.BeatmapSet!)); - AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentSelection)); + AddAssert("selection matches original carousel selection", () => selection, () => Is.EqualTo(Carousel.CurrentGroupedBeatmap)); AddUntilStep("drawable selection restored", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(selection)); AddAssert("carousel item is visible", () => GetSelectedPanel()?.Item?.IsVisible, () => Is.True); } @@ -389,15 +389,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 private void checkSelectionIterating(bool isIterating) { - object? selection = null; + GroupedBeatmap? selection = null; for (int i = 0; i < 3; i++) { - AddStep("store selection", () => selection = Carousel.CurrentSelection); + AddStep("store selection", () => selection = Carousel.CurrentGroupedBeatmap); if (isIterating) - AddUntilStep("selection changed", () => Carousel.CurrentSelection != selection); + AddUntilStep("selection changed", () => Carousel.CurrentGroupedBeatmap != selection); else - AddUntilStep("selection not changed", () => Carousel.CurrentSelection == selection); + AddUntilStep("selection not changed", () => Carousel.CurrentGroupedBeatmap == selection); } } } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs index 9f31c875b6..ce68d587c8 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselRandom.cs @@ -50,12 +50,12 @@ namespace osu.Game.Tests.Visual.SongSelectV2 nextRandom(); ensureRandomDidNotRepeat(); - AddStep("store selection", () => originalSelected = ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap); + AddStep("store selection", () => originalSelected = Carousel.CurrentBeatmap!); SortAndGroupBy(SortMode.Artist, GroupMode.Difficulty); WaitForFiltering(); - AddAssert("selection not changed", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(originalSelected)); + AddAssert("selection not changed", () => Carousel.CurrentBeatmap, () => Is.EqualTo(originalSelected)); storeExpandedGroup(); @@ -282,7 +282,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 nextRandom(); CheckHasSelection(); - AddStep("store selection", () => originalSelected = ((GroupedBeatmap)Carousel.CurrentSelection!)); + AddStep("store selection", () => originalSelected = Carousel.CurrentGroupedBeatmap!); AddStep("random then rewind", () => { @@ -290,7 +290,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 Carousel.PreviousRandom(); }); - AddAssert("selection not changed", () => Carousel.CurrentSelection, () => Is.EqualTo(originalSelected)); + AddAssert("selection not changed", () => Carousel.CurrentGroupedBeatmap, () => Is.EqualTo(originalSelected)); } [Test] @@ -305,20 +305,20 @@ namespace osu.Game.Tests.Visual.SongSelectV2 nextRandom(); CheckHasSelection(); - AddStep("store selection", () => originalSelected = (GroupedBeatmap)Carousel.CurrentSelection!); + AddStep("store selection", () => originalSelected = Carousel.CurrentGroupedBeatmap!); nextRandom(); - AddStep("store selection", () => postRandomSelection = (GroupedBeatmap)Carousel.CurrentSelection!); + AddStep("store selection", () => postRandomSelection = Carousel.CurrentGroupedBeatmap!); AddAssert("selection changed", () => originalSelected, () => Is.Not.SameAs(postRandomSelection)); AddStep("delete previous selection beatmaps", () => BeatmapSets.Remove(originalSelected!.Beatmap.BeatmapSet!)); WaitForFiltering(); - AddAssert("selection not changed", () => Carousel.CurrentSelection, () => Is.EqualTo(postRandomSelection)); + AddAssert("selection not changed", () => Carousel.CurrentGroupedBeatmap, () => Is.EqualTo(postRandomSelection)); prevRandomSet(); - AddAssert("selection not changed", () => Carousel.CurrentSelection, () => Is.EqualTo(postRandomSelection)); + AddAssert("selection not changed", () => Carousel.CurrentGroupedBeatmap, () => Is.EqualTo(postRandomSelection)); } private void nextRandom() => diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs index d05c874641..c1cee4e398 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselScrolling.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select middle beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); + AddStep("select middle beatmap", () => Carousel.CurrentGroupedBeatmap = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); WaitForScrolling(); @@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select middle beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); + AddStep("select middle beatmap", () => Carousel.CurrentGroupedBeatmap = new GroupedBeatmap(null, BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First())); WaitForScrolling(); AddStep("override scroll with user scroll", () => @@ -71,7 +71,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("scroll to end", () => Scroll.ScrollToEnd(false)); - AddStep("select last beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.Last().Beatmaps.Last())); + AddStep("select last beatmap", () => Carousel.CurrentGroupedBeatmap = new GroupedBeatmap(null, BeatmapSets.Last().Beatmaps.Last())); WaitForScrolling(); @@ -88,7 +88,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select first beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); + AddStep("select first beatmap", () => Carousel.CurrentGroupedBeatmap = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); WaitForScrolling(); @@ -108,7 +108,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { Quad positionBefore = default; - AddStep("select first beatmap", () => Carousel.CurrentSelection = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); + AddStep("select first beatmap", () => Carousel.CurrentGroupedBeatmap = new GroupedBeatmap(null, BeatmapSets.First().Beatmaps.First())); WaitForScrolling(); AddStep("override scroll with user scroll", () => diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs index a331879684..d1cef3420a 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs @@ -179,7 +179,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => @@ -195,7 +195,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } @@ -205,14 +205,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.DifficultyName = "new name"); assertDidFilter(); WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } @@ -222,14 +222,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.OnlineID = b.OnlineID + 1); assertDidFilter(); WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } @@ -239,7 +239,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); // Add another difficulty with same online ID. @@ -252,7 +252,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } @@ -262,7 +262,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 SelectNextSet(); WaitForSetSelection(1, 0); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); // Remove original selected difficulty, and add two difficulties with same name as selection. @@ -284,7 +284,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 WaitForFiltering(); - AddAssert("selection is updateable beatmap", () => ((GroupedBeatmap)Carousel.CurrentSelection!).Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); + AddAssert("selection is updateable beatmap", () => Carousel.CurrentBeatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => (GetSelectedPanel()?.Item?.Model as GroupedBeatmap)?.Beatmap, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); } diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs index c480d6ca7e..7c604eb37b 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectCurrentSelectionInvalidated.cs @@ -27,7 +27,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 /// public partial class TestSceneSongSelectCurrentSelectionInvalidated : SongSelectTestScene { - private BeatmapInfo? selectedBeatmap => (Carousel.CurrentSelection as GroupedBeatmap)?.Beatmap; + private BeatmapInfo? selectedBeatmap => Carousel.CurrentBeatmap; private BeatmapSetInfo? selectedBeatmapSet => selectedBeatmap?.BeatmapSet; [SetUpSteps] diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index 5adc37ea40..0df183bb71 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -107,7 +107,7 @@ namespace osu.Game.Graphics.Carousel /// The selection is never reset due to not existing. It can be set to anything. /// If no matching carousel item exists, there will be no visually selected item while waiting for potential new item which matches. /// - public virtual object? CurrentSelection + protected object? CurrentSelection { get => currentSelection.Model; set diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 6c98630274..ab520525a5 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -295,28 +295,47 @@ namespace osu.Game.Screens.SelectV2 protected override bool ShouldActivateOnKeyboardSelection(CarouselItem item) => grouping.BeatmapSetsGroupedTogether && item.Model is GroupedBeatmap; - public override object? CurrentSelection + /// + /// The currently selected . + /// + /// + /// The selection is never reset due to not existing. It can be set to anything. + /// If no matching carousel item exists, there will be no visually selected item while waiting for potential new item which matches. + /// + public GroupedBeatmap? CurrentGroupedBeatmap { - get => base.CurrentSelection; + get => CurrentSelection as GroupedBeatmap; + set => CurrentSelection = value; + } + + /// + /// The currently selected . + /// + /// + /// This is a property mostly dedicated to external consumers who only care about showing some particular copy of a beatmap + /// (there could be multiple panels for one beatmap due to grouping). + /// Through this property, the carousel basically figures out what group to use internally. + /// + public BeatmapInfo? CurrentBeatmap + { + get => CurrentGroupedBeatmap?.Beatmap; set { - // this is a special pathway for external consumers who only care about showing some particular copy of a beatmap - // (there could be multiple panels for one beatmap due to grouping). - // in this pathway we basically figure out what group to use internally, and continue working with `GroupedBeatmap` all the way after that. - if (value is BeatmapInfo beatmapInfo) + if (value == null) { - if (CurrentSelection is GroupedBeatmap groupedBeatmap && beatmapInfo.Equals(groupedBeatmap.Beatmap)) - return; - - // it is not universally guaranteed that the carousel items will be materialised at the time this is set. - // therefore, in cases where it is known that they will not be, default to a null group. - // even if grouping is active, this will be rectified to a correct group on the next invocation of `HandleFilterCompleted()`. - value = IsLoaded && !IsFiltering - ? GetCarouselItems()?.Select(item => item.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(beatmapInfo)) - : new GroupedBeatmap(null, beatmapInfo); + CurrentGroupedBeatmap = null; + return; } - base.CurrentSelection = value; + if (CurrentGroupedBeatmap != null && value.Equals(CurrentGroupedBeatmap.Beatmap)) + return; + + // it is not universally guaranteed that the carousel items will be materialised at the time this is set. + // therefore, in cases where it is known that they will not be, default to a null group. + // even if grouping is active, this will be rectified to a correct group on the next invocation of `HandleFilterCompleted()`. + CurrentGroupedBeatmap = IsLoaded && !IsFiltering + ? GetCarouselItems()?.Select(item => item.Model).OfType().FirstOrDefault(gb => gb.Beatmap.Equals(value)) + : new GroupedBeatmap(null, value); } } diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index 7597912ae6..9949f86808 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -479,7 +479,7 @@ namespace osu.Game.Screens.SelectV2 if (!this.IsCurrentScreen()) return; - carousel.CurrentSelection = groupedBeatmap; + carousel.CurrentGroupedBeatmap = groupedBeatmap; // Debounce consideration is to avoid beatmap churn on key repeat selection. selectionDebounce?.Cancel(); @@ -512,7 +512,7 @@ namespace osu.Game.Screens.SelectV2 if (validSelection) { - carousel.CurrentSelection = currentBeatmap.BeatmapInfo; + carousel.CurrentBeatmap = currentBeatmap.BeatmapInfo; return true; } @@ -535,7 +535,7 @@ namespace osu.Game.Screens.SelectV2 if (validBeatmaps.Any()) { - carousel.CurrentSelection = difficultyRecommender?.GetRecommendedBeatmap(validBeatmaps) ?? validBeatmaps.First(); + carousel.CurrentBeatmap = difficultyRecommender?.GetRecommendedBeatmap(validBeatmaps) ?? validBeatmaps.First(); return true; } } From 71d0afd4c2b3c7fe6e92558b5ecd849f67cb6384 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 3 Sep 2025 18:49:19 +0900 Subject: [PATCH 22/22] Attempt to fix flaky test `TestFilteringRunsAfterReturningFromGameplay` The double escape key press was dodgy from the get-go. --- osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs index 895f148965..553205d400 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs @@ -320,9 +320,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); AddUntilStep("wait for fail", () => ((Player)Stack.CurrentScreen).GameplayState.HasFailed); - AddStep("exit gameplay", () => InputManager.Key(Key.Escape)); - AddStep("exit gameplay", () => InputManager.Key(Key.Escape)); + AddStep("exit gameplay", () => Stack.CurrentScreen.Exit()); + AddUntilStep("wait for song select", () => Stack.CurrentScreen is Screens.SelectV2.SongSelect); AddUntilStep("wait for filtered", () => SongSelect.ChildrenOfType().Single().FilterCount, () => Is.EqualTo(2)); }