From 28f9e734f0d3dbf374d90b72e8380e1021aab98e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 3 Mar 2025 12:23:52 +0100 Subject: [PATCH 1/2] Add failing test case --- .../TestScenePlaylistsResultsScreen.cs | 103 +++++++++++++----- 1 file changed, 76 insertions(+), 27 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index dc5fb20e16..469f7c8b74 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -69,9 +69,11 @@ namespace osu.Game.Tests.Visual.Playlists totalCount = 0; userScore = TestResources.CreateTestScoreInfo(); + userScore.OnlineID = 1; userScore.TotalScore = 0; userScore.Statistics = new Dictionary(); userScore.MaximumStatistics = new Dictionary(); + userScore.Position = real_user_position; // Beatmap is required to be an actual beatmap so the scores can get their scores correctly // calculated for standardised scoring, else the tests that rely on ordering will fall over. @@ -243,6 +245,35 @@ namespace osu.Game.Tests.Visual.Playlists AddAssert("placeholder shown", () => this.ChildrenOfType().Count(), () => Is.EqualTo(1)); } + [Test] + public void TestFetchingAllTheWayToFirstNeverDisplaysNegativePosition() + { + AddStep("set user position", () => userScore.Position = 20); + AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); + + createResultsWithScore(() => userScore); + waitForDisplay(); + + AddStep("bind delayed handler", () => bindHandler(true)); + + for (int i = 0; i < 2; i++) + { + AddStep("simulate user falling down ranking", () => userScore.Position += 2); + AddStep("scroll to left", () => resultsScreen.ChildrenOfType().Single().ChildrenOfType().Single().ScrollToStart(false)); + + AddAssert("left loading spinner shown", () => + resultsScreen.ChildrenOfType().Single(l => l.Anchor == Anchor.CentreLeft).State.Value == Visibility.Visible); + + waitForDisplay(); + + AddAssert("left loading spinner hidden", () => + resultsScreen.ChildrenOfType().Single(l => l.Anchor == Anchor.CentreLeft).State.Value == Visibility.Hidden); + } + + AddAssert("total count is 34", () => this.ChildrenOfType().Count(), () => Is.EqualTo(34)); + AddUntilStep("all panels have non-negative position", () => this.ChildrenOfType().All(p => p.ScorePosition.Value > 0)); + } + private void createResultsWithScore(Func getScore) { AddStep("load results", () => @@ -331,7 +362,7 @@ namespace osu.Game.Tests.Visual.Playlists if (userScore == null) triggerFail(s); else - triggerSuccess(s, createUserResponse(userScore)); + triggerSuccess(s, () => createUserResponse(userScore)); break; @@ -339,12 +370,12 @@ namespace osu.Game.Tests.Visual.Playlists if (userScore == null) triggerFail(u); else - triggerSuccess(u, createUserResponse(userScore)); + triggerSuccess(u, () => createUserResponse(userScore)); break; case IndexPlaylistScoresRequest i: - triggerSuccess(i, createIndexResponse(i, noScores)); + triggerSuccess(i, () => createIndexResponse(i, noScores)); break; } }, delay); @@ -352,11 +383,11 @@ namespace osu.Game.Tests.Visual.Playlists return true; }; - private void triggerSuccess(APIRequest req, T result) + private void triggerSuccess(APIRequest req, Func result) where T : class { requestComplete = true; - req.TriggerSuccess(result); + req.TriggerSuccess(result.Invoke()); } private void triggerFail(APIRequest req) @@ -367,28 +398,13 @@ namespace osu.Game.Tests.Visual.Playlists private MultiplayerScore createUserResponse(ScoreInfo userScore) { - var multiplayerUserScore = new MultiplayerScore - { - ID = highestScoreId, - Accuracy = userScore.Accuracy, - Passed = userScore.Passed, - Rank = userScore.Rank, - Position = real_user_position, - MaxCombo = userScore.MaxCombo, - User = userScore.User, - BeatmapId = RNG.Next(0, 7), - ScoresAround = new MultiplayerScoresAround - { - Higher = new MultiplayerScores(), - Lower = new MultiplayerScores() - } - }; + var multiplayerUserScore = createMultiplayerUserScore(userScore); totalCount++; for (int i = 1; i <= scores_per_result; i++) { - multiplayerUserScore.ScoresAround.Lower.Scores.Add(new MultiplayerScore + multiplayerUserScore.ScoresAround!.Lower!.Scores.Add(new MultiplayerScore { ID = getNextLowestScoreId(), Accuracy = userScore.Accuracy, @@ -404,7 +420,7 @@ namespace osu.Game.Tests.Visual.Playlists }, }); - multiplayerUserScore.ScoresAround.Higher.Scores.Add(new MultiplayerScore + multiplayerUserScore.ScoresAround!.Higher!.Scores.Add(new MultiplayerScore { ID = getNextHighestScoreId(), Accuracy = userScore.Accuracy, @@ -423,12 +439,32 @@ namespace osu.Game.Tests.Visual.Playlists totalCount += 2; } - addCursor(multiplayerUserScore.ScoresAround.Lower); - addCursor(multiplayerUserScore.ScoresAround.Higher); + addCursor(multiplayerUserScore.ScoresAround!.Lower!); + addCursor(multiplayerUserScore.ScoresAround!.Higher!); return multiplayerUserScore; } + private MultiplayerScore createMultiplayerUserScore(ScoreInfo userScore) + { + return new MultiplayerScore + { + ID = highestScoreId, + Accuracy = userScore.Accuracy, + Passed = userScore.Passed, + Rank = userScore.Rank, + Position = userScore.Position, + MaxCombo = userScore.MaxCombo, + User = userScore.User, + BeatmapId = RNG.Next(0, 7), + ScoresAround = new MultiplayerScoresAround + { + Higher = new MultiplayerScores(), + Lower = new MultiplayerScores() + } + }; + } + private IndexedMultiplayerScores createIndexResponse(IndexPlaylistScoresRequest req, bool noScores) { var result = new IndexedMultiplayerScores(); @@ -437,11 +473,21 @@ namespace osu.Game.Tests.Visual.Playlists string sort = req.IndexParams?.Properties["sort"].ToObject() ?? "score_desc"; + bool reachedEnd = false; + for (int i = 1; i <= scores_per_result; i++) { + int nextId = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(); + + if (userScore.OnlineID - nextId >= userScore.Position) + { + reachedEnd = true; + break; + } + result.Scores.Add(new MultiplayerScore { - ID = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(), + ID = nextId, Accuracy = 1, Passed = true, Rank = ScoreRank.X, @@ -458,7 +504,10 @@ namespace osu.Game.Tests.Visual.Playlists totalCount++; } - addCursor(result); + if (!reachedEnd) + addCursor(result); + + result.UserScore = createMultiplayerUserScore(userScore); return result; } From bf4fa58f72c61ff217c2d20a48f86d9aa65a4862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 3 Mar 2025 12:56:28 +0100 Subject: [PATCH 2/2] Fix playlists results screens potentially displaying negative score positions Closes https://github.com/ppy/osu/issues/31434. --- .../Playlists/PlaylistItemResultsScreen.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistItemResultsScreen.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistItemResultsScreen.cs index 184de2f50c..0e539936d8 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistItemResultsScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistItemResultsScreen.cs @@ -185,6 +185,24 @@ namespace osu.Game.Screens.OnlinePlay.Playlists { higherScores = index; setPositions(index, pivot, -1); + + // when paginating the results, it's possible for the user's score to naturally fall down the rankings. + // unmitigated, this can cause scores at the very top of the rankings to have zero or negative positions + // because the positions are counted backwards from the user's score, which has increased in this case during pagination. + // if this happens, just give the top score the first position. + // note that this isn't 100% correct, but it *is* however the most reliable way to mask the problem. + int smallestPosition = index.Scores.Min(s => s.Position ?? 1); + + if (smallestPosition < 1) + { + int offset = 1 - smallestPosition; + + foreach (var scorePanel in ScorePanelList.GetScorePanels()) + scorePanel.ScorePosition.Value += offset; + + foreach (var score in index.Scores) + score.Position += offset; + } } return await transformScores(index.Scores).ConfigureAwait(false);