From 3e240cbadec001ea77de2272ccbffe895491b898 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 May 2025 17:11:40 +0900 Subject: [PATCH 1/4] Refactor sorting filter to better handle cases of beatmap set grouping Previously the logic would fall over under various scenarios due to applying aggregate logic where it shouldn't be, or vice-versa. Now the sorting filter explicitly checks the grouping mode and reacts accordingly. I was hoping we could avoid the sorting filter having any knowledge of grouping, but I don't see a way around this. --- .../SelectV2/BeatmapCarouselFilterSorting.cs | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterSorting.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterSorting.cs index eb39b499de..0ebfc084bd 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterSorting.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterSorting.cs @@ -27,19 +27,27 @@ namespace osu.Game.Screens.SelectV2 { var criteria = getCriteria(); + bool groupedSets = BeatmapCarouselFilterGrouping.ShouldGroupBeatmapsTogether(criteria); + return items.Order(Comparer.Create((a, b) => { var ab = (BeatmapInfo)a.Model; var bb = (BeatmapInfo)b.Model; - if (ab.BeatmapSet!.Equals(bb.BeatmapSet)) - return compareDifficulty(ab, bb); + if (groupedSets) + { + if (ab.BeatmapSet!.Equals(bb.BeatmapSet)) + return compareDifficulty(ab, bb, criteria.Sort); - return compare(ab, bb, criteria.Sort); + // If we're grouping by sets, all fallback sorts need to be aggregates for the set. + return compare(ab, bb, criteria.Sort, aggregate: true); + } + + return compare(ab, bb, criteria.Sort, aggregate: false); })).ToList(); }, cancellationToken).ConfigureAwait(false); - private static int compare(BeatmapInfo a, BeatmapInfo b, SortMode sort) + private static int compare(BeatmapInfo a, BeatmapInfo b, SortMode sort, bool aggregate) { int comparison; @@ -80,15 +88,24 @@ namespace osu.Game.Screens.SelectV2 break; case SortMode.LastPlayed: - comparison = -compareUsingAggregateMax(a, b, static b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds()); + if (aggregate) + comparison = compareUsingAggregateMax(b, a, static b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds()); + else + comparison = Nullable.Compare(b.LastPlayed, a.LastPlayed); break; case SortMode.BPM: - comparison = compareUsingAggregateMax(a, b, static b => b.BPM); + if (aggregate) + comparison = compareUsingAggregateMax(a, b, static b => b.BPM); + else + comparison = a.BPM.CompareTo(b.BPM); break; case SortMode.Length: - comparison = compareUsingAggregateMax(a, b, static b => b.Length); + if (aggregate) + comparison = compareUsingAggregateMax(a, b, static b => b.Length); + else + comparison = a.Length.CompareTo(b.Length); break; default: @@ -108,7 +125,7 @@ namespace osu.Game.Screens.SelectV2 return comparison; } - private static int compareDifficulty(BeatmapInfo a, BeatmapInfo b) + private static int compareDifficulty(BeatmapInfo a, BeatmapInfo b, SortMode sort) { int comparison = a.Ruleset.CompareTo(b.Ruleset); From be688fd35090ec7fc362404dceb5105a15b381ac Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 May 2025 19:21:53 +0900 Subject: [PATCH 2/4] Refactor grouping filter to also handle aggregate cases correctly This also fixes *another* inefficient case of full items iteration for no reason. --- .../SelectV2/BeatmapCarouselFilterGrouping.cs | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs index 86256ad99e..27f3dcf8ff 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs @@ -152,12 +152,15 @@ namespace osu.Game.Screens.SelectV2 case GroupMode.LastPlayed: return getGroupsBy(b => { - DateTimeOffset? maxLastPlayed = aggregateMax(b, items, bb => bb.LastPlayed); + var date = b.LastPlayed; - if (maxLastPlayed == null) + if (BeatmapSetsGroupedTogether) + date = aggregateMax(b, static b => (b.LastPlayed ?? DateTimeOffset.MinValue)); + + if (date == null) return new GroupDefinition(int.MaxValue, "Never"); - return defineGroupByDate(maxLastPlayed.Value); + return defineGroupByDate(date.Value); }, items); case GroupMode.RankedStatus: @@ -166,8 +169,12 @@ namespace osu.Game.Screens.SelectV2 case GroupMode.BPM: return getGroupsBy(b => { - double maxBPM = aggregateMax(b, items, bb => bb.BPM); - return defineGroupByBPM(maxBPM); + double bpm = b.BPM; + + if (BeatmapSetsGroupedTogether) + bpm = aggregateMax(b, bb => bb.BPM); + + return defineGroupByBPM(bpm); }, items); case GroupMode.Difficulty: @@ -176,8 +183,12 @@ namespace osu.Game.Screens.SelectV2 case GroupMode.Length: return getGroupsBy(b => { - double maxLength = aggregateMax(b, items, bb => bb.Length); - return defineGroupByLength(maxLength); + double length = b.Length; + + if (BeatmapSetsGroupedTogether) + length = aggregateMax(b, bb => bb.Length); + + return defineGroupByLength(length); }, items); case GroupMode.Collections: @@ -334,10 +345,10 @@ namespace osu.Game.Screens.SelectV2 return new GroupDefinition(11, "Over 10 minutes"); } - private static T? aggregateMax(BeatmapInfo b, IEnumerable items, Func func) + private static T? aggregateMax(BeatmapInfo b, Func func) { - var matchedBeatmaps = items.Select(i => i.Model).Cast().Where(beatmap => beatmap.BeatmapSet!.Equals(b.BeatmapSet)); - return matchedBeatmaps.Max(func); + var beatmaps = b.BeatmapSet!.Beatmaps.Where(bb => !bb.Hidden); + return beatmaps.Max(func); } private record GroupMapping(GroupDefinition? Group, List ItemsInGroup); From a8eff6b0964f92e7fdabba70ed5919fe5b7afbd2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 May 2025 17:13:00 +0900 Subject: [PATCH 3/4] Split out difficulties when sorting and grouping by "last played" --- .../Screens/SelectV2/BeatmapCarouselFilterGrouping.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs index 27f3dcf8ff..1174e172e1 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs @@ -124,7 +124,15 @@ namespace osu.Game.Screens.SelectV2 public static bool ShouldGroupBeatmapsTogether(FilterCriteria criteria) { - return criteria.Sort != SortMode.Difficulty && criteria.Group != GroupMode.Difficulty; + // In certain cases, we intentionally split out difficulties + // where it's more relevant or convenient to view them as individual items. + if (criteria.Sort == SortMode.Difficulty || criteria.Group == GroupMode.Difficulty) + return false; + if (criteria.Sort == SortMode.LastPlayed && criteria.Group == GroupMode.LastPlayed) + return false; + + // In the majority case we group sets together for display. + return true; } private List getGroups(List items, FilterCriteria criteria) From 66e2c5c287ea90b291bcce4ba0729ff39e622ab9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 May 2025 20:50:50 +0900 Subject: [PATCH 4/4] Fix oversight in date handling --- osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs index 1174e172e1..7d449661ab 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarouselFilterGrouping.cs @@ -165,7 +165,7 @@ namespace osu.Game.Screens.SelectV2 if (BeatmapSetsGroupedTogether) date = aggregateMax(b, static b => (b.LastPlayed ?? DateTimeOffset.MinValue)); - if (date == null) + if (date == null || date == DateTimeOffset.MinValue) return new GroupDefinition(int.MaxValue, "Never"); return defineGroupByDate(date.Value);