From 3c219a799b88bdc7c78797a128f78f1c106e6938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 13 May 2025 09:35:19 +0200 Subject: [PATCH 1/2] Add failing test coverage for expected behaviour --- .../Ranking/TestSceneSoloResultsScreen.cs | 121 +++++++++++++++++- 1 file changed, 114 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneSoloResultsScreen.cs b/osu.Game.Tests/Visual/Ranking/TestSceneSoloResultsScreen.cs index b3f01d093f..c9ef508a84 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneSoloResultsScreen.cs +++ b/osu.Game.Tests/Visual/Ranking/TestSceneSoloResultsScreen.cs @@ -167,6 +167,57 @@ namespace osu.Game.Tests.Visual.Ranking AddAssert("local score is #16", () => this.ChildrenOfType().Single().GetPanelForScore(localScore).ScorePosition.Value, () => Is.EqualTo(16)); } + [Test] + public void TestOnlineLeaderboardWithLessThan50Scores_UserWasInTop50() + { + ScoreInfo localScore = null!; + + AddStep("set leaderboard to global", () => leaderboardManager.FetchWithCriteria(new LeaderboardCriteria(importedBeatmap, importedBeatmap.Ruleset, BeatmapLeaderboardScope.Global, null))); + AddStep("set up request handling", () => dummyAPI.HandleRequest = req => + { + switch (req) + { + case GetScoresRequest getScoresRequest: + var scores = new List(); + + for (int i = 0; i < 30; ++i) + { + var score = TestResources.CreateTestScoreInfo(importedBeatmap); + score.TotalScore = 10_000 * (30 - i); + score.Position = i + 1; + scores.Add(SoloScoreInfo.ForSubmission(score)); + } + + scores[^1].ID = 123456; + scores[^1].UserID = API.LocalUser.Value.OnlineID; + + getScoresRequest.TriggerSuccess(new APIScoresCollection + { + Scores = scores, + UserScore = new APIScoreWithPosition + { + Score = scores[^1], + Position = 30 + } + }); + return true; + } + + return false; + }); + + AddStep("show results", () => + { + localScore = TestResources.CreateTestScoreInfo(importedBeatmap); + localScore.TotalScore = 151_000; + localScore.Position = null; + LoadScreen(new SoloResultsScreen(localScore)); + }); + AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded); + AddAssert("local score is #16", () => this.ChildrenOfType().Single().GetPanelForScore(localScore).ScorePosition.Value, () => Is.EqualTo(16)); + AddAssert("previous user best not shown", () => this.ChildrenOfType().All(p => p.Score.OnlineID != 123456)); + } + [Test] public void TestOnlineLeaderboardWithLessThan50Scores_UserIsLast() { @@ -207,7 +258,7 @@ namespace osu.Game.Tests.Visual.Ranking } [Test] - public void TestOnlineLeaderboardWithMoreThan50Scores_UserOutsideOfTop50() + public void TestOnlineLeaderboardWithMoreThan50Scores_UserOutsideOfTop50_DidNotBeatOwnBest() { ScoreInfo localScore = null!; @@ -227,15 +278,69 @@ namespace osu.Game.Tests.Visual.Ranking scores.Add(SoloScoreInfo.ForSubmission(score)); } - var userBest = TestResources.CreateTestScoreInfo(importedBeatmap); + var userBest = SoloScoreInfo.ForSubmission(TestResources.CreateTestScoreInfo(importedBeatmap)); userBest.TotalScore = 50_000; + userBest.ID = 123456; getScoresRequest.TriggerSuccess(new APIScoresCollection { Scores = scores, UserScore = new APIScoreWithPosition { - Score = SoloScoreInfo.ForSubmission(userBest), + Score = userBest, + Position = 133_337, + } + }); + return true; + } + + return false; + }); + + AddStep("show results", () => + { + localScore = TestResources.CreateTestScoreInfo(importedBeatmap); + localScore.TotalScore = 31_000; + localScore.Position = null; + LoadScreen(new SoloResultsScreen(localScore)); + }); + AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded); + AddAssert("local score has no position", () => this.ChildrenOfType().Single().GetPanelForScore(localScore).ScorePosition.Value, () => Is.Null); + AddAssert("previous user best shown at same position", () => this.ChildrenOfType().Any(p => p.Score.OnlineID == 123456 && p.ScorePosition.Value == 133_337)); + } + + [Test] + public void TestOnlineLeaderboardWithMoreThan50Scores_UserOutsideOfTop50_BeatOwnBest() + { + ScoreInfo localScore = null!; + + AddStep("set leaderboard to global", () => leaderboardManager.FetchWithCriteria(new LeaderboardCriteria(importedBeatmap, importedBeatmap.Ruleset, BeatmapLeaderboardScope.Global, null))); + AddStep("set up request handling", () => dummyAPI.HandleRequest = req => + { + switch (req) + { + case GetScoresRequest getScoresRequest: + var scores = new List(); + + for (int i = 0; i < 50; ++i) + { + var score = TestResources.CreateTestScoreInfo(importedBeatmap); + score.TotalScore = 500_000 + 10_000 * (50 - i); + score.Position = i + 1; + scores.Add(SoloScoreInfo.ForSubmission(score)); + } + + var userBest = SoloScoreInfo.ForSubmission(TestResources.CreateTestScoreInfo(importedBeatmap)); + userBest.TotalScore = 50_000; + userBest.ID = 123456; + userBest.UserID = API.LocalUser.Value.OnlineID; + + getScoresRequest.TriggerSuccess(new APIScoresCollection + { + Scores = scores, + UserScore = new APIScoreWithPosition + { + Score = userBest, Position = 133_337, } }); @@ -254,7 +359,7 @@ namespace osu.Game.Tests.Visual.Ranking }); AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded); AddAssert("local score has no position", () => this.ChildrenOfType().Single().GetPanelForScore(localScore).ScorePosition.Value, () => Is.Null); - AddAssert("user best position preserved", () => this.ChildrenOfType().Any(p => p.ScorePosition.Value == 133_337)); + AddAssert("previous user best not shown", () => this.ChildrenOfType().All(p => p.Score.OnlineID != 123456)); } [Test] @@ -278,15 +383,17 @@ namespace osu.Game.Tests.Visual.Ranking scores.Add(SoloScoreInfo.ForSubmission(score)); } - var userBest = TestResources.CreateTestScoreInfo(importedBeatmap); + var userBest = SoloScoreInfo.ForSubmission(TestResources.CreateTestScoreInfo(importedBeatmap)); userBest.TotalScore = 50_000; + userBest.ID = 123456; + userBest.UserID = API.LocalUser.Value.OnlineID; getScoresRequest.TriggerSuccess(new APIScoresCollection { Scores = scores, UserScore = new APIScoreWithPosition { - Score = SoloScoreInfo.ForSubmission(userBest), + Score = userBest, Position = 133_337, } }); @@ -305,7 +412,7 @@ namespace osu.Game.Tests.Visual.Ranking }); AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded); AddAssert("local score is #36", () => this.ChildrenOfType().Single().GetPanelForScore(localScore).ScorePosition.Value, () => Is.EqualTo(36)); - AddAssert("user best position incremented by 1", () => this.ChildrenOfType().Any(p => p.ScorePosition.Value == 133_338)); + AddAssert("previous user best not shown", () => this.ChildrenOfType().All(p => p.Score.OnlineID != 123456)); } [Test] From a5ea24e37b5d0c3ea3c2a4eb8d8bfb6929949db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 13 May 2025 10:17:09 +0200 Subject: [PATCH 2/2] Do not show previous best score on solo results screen if the local user just beat it Closes https://github.com/ppy/osu/issues/33109. --- osu.Game/Screens/Ranking/SoloResultsScreen.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Ranking/SoloResultsScreen.cs b/osu.Game/Screens/Ranking/SoloResultsScreen.cs index 8ef083d287..d11e7db178 100644 --- a/osu.Game/Screens/Ranking/SoloResultsScreen.cs +++ b/osu.Game/Screens/Ranking/SoloResultsScreen.cs @@ -10,6 +10,7 @@ using osu.Framework.Bindables; using osu.Framework.Logging; using osu.Game.Beatmaps; using osu.Game.Extensions; +using osu.Game.Online.API; using osu.Game.Online.Leaderboards; using osu.Game.Scoring; using osu.Game.Screens.Select.Leaderboards; @@ -20,6 +21,9 @@ namespace osu.Game.Screens.Ranking { private readonly IBindable globalScores = new Bindable(); + [Resolved] + private IAPIProvider api { get; set; } = null!; + [Resolved] private LeaderboardManager leaderboardManager { get; set; } = null!; @@ -77,7 +81,7 @@ namespace osu.Game.Screens.Ranking Score.Position = clonedScore.Position; sortedScores.Add(Score); } - else + else if (criteria.Scope == BeatmapLeaderboardScope.Local || clonedScore.UserID != api.LocalUser.Value.OnlineID || clonedScore.TotalScore > Score.TotalScore) sortedScores.Add(clonedScore); }