From dcd3e5194e3ba41e69fd69c34afb0c0e6243a48f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Sat, 12 Oct 2024 20:31:27 +0200 Subject: [PATCH 1/6] Group `HitResult`s with the same name into one column in beatmap ranking Closes #29911 --- .../Visual/Online/TestSceneScoresContainer.cs | 56 ++++++++++++++++++- .../Overlays/BeatmapSet/Scores/ScoreTable.cs | 34 ++++++----- .../BeatmapSet/Scores/ScoresContainer.cs | 12 ++-- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs b/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs index 33f4d577bd..664eca6e23 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs @@ -8,11 +8,13 @@ using System.Collections.Generic; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Extensions.LocalisationExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Testing; using osu.Framework.Utils; +using osu.Game.Graphics.Sprites; using osu.Game.Online.API; using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays; @@ -90,6 +92,43 @@ namespace osu.Game.Tests.Visual.Online AddAssert("no best score displayed", () => scoresContainer.ChildrenOfType().Count() == 1); } + [Test] + public void TestHitResultsWithSameNameAreGrouped() + { + AddStep("Load scores without user best", () => + { + var allScores = createScores(); + allScores.UserScore = null; + scoresContainer.Scores = allScores; + }); + + AddUntilStep("wait for scores displayed", () => scoresContainer.ChildrenOfType().Any()); + AddAssert("only one column for slider end", () => scoresContainer.ScoreTable.Columns.Count(c => c.Header.Equals("slider end")) == 1); + + AddAssert("all rows show non-zero slider ends", () => + { + int sliderEndColumnIndex = Array.FindIndex(scoresContainer.ScoreTable.Columns, c => c != null && c.Header.Equals("slider end")); + bool sliderEndFilledInEachRow = true; + + for (int i = 0; i < scoresContainer.ScoreTable.Content?.GetLength(0); i++) + { + switch (scoresContainer.ScoreTable.Content[i, sliderEndColumnIndex]) + { + case OsuSpriteText text: + if (text.Text.Equals(0.0d.ToLocalisableString(@"N0"))) + sliderEndFilledInEachRow = false; + break; + + default: + sliderEndFilledInEachRow = false; + break; + } + } + + return sliderEndFilledInEachRow; + }); + } + [Test] public void TestUserBest() { @@ -287,13 +326,17 @@ namespace osu.Game.Tests.Visual.Online const int initial_great_count = 2000; const int initial_tick_count = 100; + const int initial_slider_end_count = 500; int greatCount = initial_great_count; int tickCount = initial_tick_count; + int sliderEndCount = initial_slider_end_count; - foreach (var s in scores.Scores) + foreach (var (score, index) in scores.Scores.Select((s, i) => (s, i))) { - s.Statistics = new Dictionary + HitResult sliderEndResult = index % 2 == 0 ? HitResult.SliderTailHit : HitResult.SmallTickHit; + + score.Statistics = new Dictionary { { HitResult.Great, greatCount }, { HitResult.LargeTickHit, tickCount }, @@ -301,10 +344,19 @@ namespace osu.Game.Tests.Visual.Online { HitResult.Meh, RNG.Next(100) }, { HitResult.Miss, initial_great_count - greatCount }, { HitResult.LargeTickMiss, initial_tick_count - tickCount }, + { sliderEndResult, sliderEndCount }, + }; + + // Some hit results, including SliderTailHit and SmallTickHit, are only displayed + // when the maximum number is known + score.MaximumStatistics = new Dictionary + { + { sliderEndResult, initial_slider_end_count }, }; greatCount -= 100; tickCount -= RNG.Next(1, 5); + sliderEndCount -= 20; } return scores; diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index a6868efb5d..671b5df33b 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -9,7 +9,6 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Extensions; -using osu.Framework.Extensions.EnumExtensions; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; @@ -58,9 +57,11 @@ namespace osu.Game.Overlays.BeatmapSet.Scores } /// - /// The statistics that appear in the table, in order of appearance. + /// The statistics that appear in the table, grouped by their display name. If multiple HitResults have the same + /// DisplayName (for example, "slider end" is the name for both HitResult.SliderTailTick and HitResult.SmallTickHit + /// in osu!std) the name will only be listed once. /// - private readonly List<(HitResult result, LocalisableString displayName)> statisticResultTypes = new List<(HitResult, LocalisableString)>(); + private readonly List<(LocalisableString displayName, IEnumerable results)> statisticResults = new List<(LocalisableString displayName, IEnumerable results)>(); private bool showPerformancePoints; @@ -72,7 +73,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores return; showPerformancePoints = showPerformanceColumn; - statisticResultTypes.Clear(); + statisticResults.Clear(); for (int i = 0; i < scores.Count; i++) backgroundFlow.Add(new ScoreTableRowBackground(i, scores[i], row_height)); @@ -105,20 +106,18 @@ namespace osu.Game.Overlays.BeatmapSet.Scores var ruleset = scores.First().Ruleset.CreateInstance(); - foreach (var result in EnumExtensions.GetValuesInOrder()) + foreach (var resultGroup in ruleset.GetHitResults().GroupBy(r => r.displayName)) { - if (!allScoreStatistics.Contains(result)) + if (!resultGroup.Any(r => allScoreStatistics.Contains(r.result))) continue; // for the time being ignore bonus result types. // this is not being sent from the API and will be empty in all cases. - if (result.IsBonus()) + if (resultGroup.All(r => r.result.IsBonus())) continue; - var displayName = ruleset.GetDisplayNameForHitResult(result); - - columns.Add(new TableColumn(displayName, Anchor.CentreLeft, new Dimension(minSize: 35, maxSize: 60))); - statisticResultTypes.Add((result, displayName)); + columns.Add(new TableColumn(resultGroup.Key, Anchor.CentreLeft, new Dimension(minSize: 35, maxSize: 60))); + statisticResults.Add((resultGroup.Key, resultGroup.Select(r => r.result))); } if (showPerformancePoints) @@ -167,12 +166,17 @@ namespace osu.Game.Overlays.BeatmapSet.Scores #pragma warning restore 618 }; - var availableStatistics = score.GetStatisticsForDisplay().ToDictionary(tuple => tuple.Result); + var availableStatistics = score.GetStatisticsForDisplay().ToLookup(touple => touple.DisplayName); - foreach (var result in statisticResultTypes) + foreach (var (columnName, resultTypes) in statisticResults) { - if (!availableStatistics.TryGetValue(result.result, out var stat)) - stat = new HitResultDisplayStatistic(result.result, 0, null, result.displayName); + HitResultDisplayStatistic stat = new HitResultDisplayStatistic(resultTypes.First(), 0, null, columnName); + + if (availableStatistics.Contains(columnName)) + { + foreach (var s in availableStatistics[columnName]) + stat = s; + } content.Add(new StatisticText(stat.Count, stat.MaxCount, @"N0") { Colour = stat.Count == 0 ? Color4.Gray : Color4.White }); } diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs index b53b7826f3..39491e338e 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs @@ -34,7 +34,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores private readonly IBindable user = new Bindable(); private readonly Box background; - private readonly ScoreTable scoreTable; + public readonly ScoreTable ScoreTable; private readonly FillFlowContainer topScoresContainer; private readonly LoadingLayer loading; private readonly LeaderboardModSelector modSelector; @@ -59,8 +59,8 @@ namespace osu.Game.Overlays.BeatmapSet.Scores loadCancellationSource = new CancellationTokenSource(); topScoresContainer.Clear(); - scoreTable.ClearScores(); - scoreTable.Hide(); + ScoreTable.ClearScores(); + ScoreTable.Hide(); loading.Hide(); loading.FinishTransforms(); @@ -85,8 +85,8 @@ namespace osu.Game.Overlays.BeatmapSet.Scores var scores = value.Scores.Select(s => s.ToScoreInfo(rulesets, beatmapInfo)).OrderByTotalScore().ToArray(); var topScore = scores.First(); - scoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints()); - scoreTable.Show(); + ScoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints()); + ScoreTable.Show(); var userScore = value.UserScore; var userScoreInfo = userScore?.Score.ToScoreInfo(rulesets, beatmapInfo); @@ -175,7 +175,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores Direction = FillDirection.Vertical, Spacing = new Vector2(0, 5), }, - scoreTable = new ScoreTable + ScoreTable = new ScoreTable { Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, From 750e0b29cae455f89c46f9f0c033d9861dc84a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Mon, 14 Oct 2024 16:11:58 +0200 Subject: [PATCH 2/6] Use `ChildrenOfType<>` to get ScoreTable to test --- .../Visual/Online/TestSceneScoresContainer.cs | 13 +++++++++---- .../Overlays/BeatmapSet/Scores/ScoresContainer.cs | 12 ++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs b/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs index 664eca6e23..c17d746232 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneScoresContainer.cs @@ -103,16 +103,21 @@ namespace osu.Game.Tests.Visual.Online }); AddUntilStep("wait for scores displayed", () => scoresContainer.ChildrenOfType().Any()); - AddAssert("only one column for slider end", () => scoresContainer.ScoreTable.Columns.Count(c => c.Header.Equals("slider end")) == 1); + AddAssert("only one column for slider end", () => + { + ScoreTable scoreTable = scoresContainer.ChildrenOfType().First(); + return scoreTable.Columns.Count(c => c.Header.Equals("slider end")) == 1; + }); AddAssert("all rows show non-zero slider ends", () => { - int sliderEndColumnIndex = Array.FindIndex(scoresContainer.ScoreTable.Columns, c => c != null && c.Header.Equals("slider end")); + ScoreTable scoreTable = scoresContainer.ChildrenOfType().First(); + int sliderEndColumnIndex = Array.FindIndex(scoreTable.Columns, c => c != null && c.Header.Equals("slider end")); bool sliderEndFilledInEachRow = true; - for (int i = 0; i < scoresContainer.ScoreTable.Content?.GetLength(0); i++) + for (int i = 0; i < scoreTable.Content?.GetLength(0); i++) { - switch (scoresContainer.ScoreTable.Content[i, sliderEndColumnIndex]) + switch (scoreTable.Content[i, sliderEndColumnIndex]) { case OsuSpriteText text: if (text.Text.Equals(0.0d.ToLocalisableString(@"N0"))) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs index 39491e338e..b53b7826f3 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs @@ -34,7 +34,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores private readonly IBindable user = new Bindable(); private readonly Box background; - public readonly ScoreTable ScoreTable; + private readonly ScoreTable scoreTable; private readonly FillFlowContainer topScoresContainer; private readonly LoadingLayer loading; private readonly LeaderboardModSelector modSelector; @@ -59,8 +59,8 @@ namespace osu.Game.Overlays.BeatmapSet.Scores loadCancellationSource = new CancellationTokenSource(); topScoresContainer.Clear(); - ScoreTable.ClearScores(); - ScoreTable.Hide(); + scoreTable.ClearScores(); + scoreTable.Hide(); loading.Hide(); loading.FinishTransforms(); @@ -85,8 +85,8 @@ namespace osu.Game.Overlays.BeatmapSet.Scores var scores = value.Scores.Select(s => s.ToScoreInfo(rulesets, beatmapInfo)).OrderByTotalScore().ToArray(); var topScore = scores.First(); - ScoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints()); - ScoreTable.Show(); + scoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints()); + scoreTable.Show(); var userScore = value.UserScore; var userScoreInfo = userScore?.Score.ToScoreInfo(rulesets, beatmapInfo); @@ -175,7 +175,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores Direction = FillDirection.Vertical, Spacing = new Vector2(0, 5), }, - ScoreTable = new ScoreTable + scoreTable = new ScoreTable { Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, From 25c0ff4168a6e116faa3e7814a5d75959f9442ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Mon, 14 Oct 2024 16:14:29 +0200 Subject: [PATCH 3/6] Correct reference to hit result and link to them --- osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index 671b5df33b..e6052b0e3b 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -58,8 +58,8 @@ namespace osu.Game.Overlays.BeatmapSet.Scores /// /// The statistics that appear in the table, grouped by their display name. If multiple HitResults have the same - /// DisplayName (for example, "slider end" is the name for both HitResult.SliderTailTick and HitResult.SmallTickHit - /// in osu!std) the name will only be listed once. + /// DisplayName (for example, "slider end" is the name for both and + /// in osu!) the name will only be listed once. /// private readonly List<(LocalisableString displayName, IEnumerable results)> statisticResults = new List<(LocalisableString displayName, IEnumerable results)>(); From 511f0e99b3933459e28c53456566c0c5639f845c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Mon, 14 Oct 2024 16:16:00 +0200 Subject: [PATCH 4/6] Correct typo --- osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index e6052b0e3b..4150316f4b 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -166,7 +166,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores #pragma warning restore 618 }; - var availableStatistics = score.GetStatisticsForDisplay().ToLookup(touple => touple.DisplayName); + var availableStatistics = score.GetStatisticsForDisplay().ToLookup(tuple => tuple.DisplayName); foreach (var (columnName, resultTypes) in statisticResults) { From 285756802cd17710078f9e7576658255c7cced47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Mon, 14 Oct 2024 16:33:30 +0200 Subject: [PATCH 5/6] Sum up totals for hit results with the same name --- osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index 4150316f4b..40055bbda8 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -170,15 +170,20 @@ namespace osu.Game.Overlays.BeatmapSet.Scores foreach (var (columnName, resultTypes) in statisticResults) { - HitResultDisplayStatistic stat = new HitResultDisplayStatistic(resultTypes.First(), 0, null, columnName); + int count = 0; + int? maxCount = null; if (availableStatistics.Contains(columnName)) { + maxCount = 0; foreach (var s in availableStatistics[columnName]) - stat = s; + { + count += s.Count; + maxCount += s.MaxCount; + } } - content.Add(new StatisticText(stat.Count, stat.MaxCount, @"N0") { Colour = stat.Count == 0 ? Color4.Gray : Color4.White }); + content.Add(new StatisticText(count, maxCount, @"N0") { Colour = count == 0 ? Color4.Gray : Color4.White }); } if (showPerformancePoints) From a007a81fe8518215ee5fd40dbfab56a3696cd2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Brandst=C3=B6tter?= Date: Mon, 14 Oct 2024 16:55:07 +0200 Subject: [PATCH 6/6] Only keep track of the names of hit results to display in a `ScoreTable` --- osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index 40055bbda8..c70c41feed 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -57,11 +57,11 @@ namespace osu.Game.Overlays.BeatmapSet.Scores } /// - /// The statistics that appear in the table, grouped by their display name. If multiple HitResults have the same + /// The names of the statistics that appear in the table. If multiple HitResults have the same /// DisplayName (for example, "slider end" is the name for both and /// in osu!) the name will only be listed once. /// - private readonly List<(LocalisableString displayName, IEnumerable results)> statisticResults = new List<(LocalisableString displayName, IEnumerable results)>(); + private readonly List statisticResultNames = new List(); private bool showPerformancePoints; @@ -73,7 +73,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores return; showPerformancePoints = showPerformanceColumn; - statisticResults.Clear(); + statisticResultNames.Clear(); for (int i = 0; i < scores.Count; i++) backgroundFlow.Add(new ScoreTableRowBackground(i, scores[i], row_height)); @@ -117,7 +117,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores continue; columns.Add(new TableColumn(resultGroup.Key, Anchor.CentreLeft, new Dimension(minSize: 35, maxSize: 60))); - statisticResults.Add((resultGroup.Key, resultGroup.Select(r => r.result))); + statisticResultNames.Add(resultGroup.Key); } if (showPerformancePoints) @@ -168,7 +168,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores var availableStatistics = score.GetStatisticsForDisplay().ToLookup(tuple => tuple.DisplayName); - foreach (var (columnName, resultTypes) in statisticResults) + foreach (var columnName in statisticResultNames) { int count = 0; int? maxCount = null; @@ -176,6 +176,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores if (availableStatistics.Contains(columnName)) { maxCount = 0; + foreach (var s in availableStatistics[columnName]) { count += s.Count;