Merge pull request #32199 from bdach/negative-leaderboard-position

Fix playlists results screens potentially displaying negative score positions

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<HitResult, int>();
                 userScore.MaximumStatistics = new Dictionary<HitResult, int>();
+                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<MessagePlaceholder>().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<ScorePanelList>().Single().ChildrenOfType<OsuScrollContainer>().Single().ScrollToStart(false));
+
+                AddAssert("left loading spinner shown", () =>
+                    resultsScreen.ChildrenOfType<LoadingSpinner>().Single(l => l.Anchor == Anchor.CentreLeft).State.Value == Visibility.Visible);
+
+                waitForDisplay();
+
+                AddAssert("left loading spinner hidden", () =>
+                    resultsScreen.ChildrenOfType<LoadingSpinner>().Single(l => l.Anchor == Anchor.CentreLeft).State.Value == Visibility.Hidden);
+            }
+
+            AddAssert("total count is 34", () => this.ChildrenOfType<ScorePanel>().Count(), () => Is.EqualTo(34));
+            AddUntilStep("all panels have non-negative position", () => this.ChildrenOfType<ScorePanel>().All(p => p.ScorePosition.Value > 0));
+        }
+
         private void createResultsWithScore(Func<ScoreInfo> 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<T>(APIRequest<T> req, T result)
+        private void triggerSuccess<T>(APIRequest<T> req, Func<T> 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
+            var multiplayerUserScore = createMultiplayerUserScore(userScore);
-            {
-                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()
-                }
-            };
 
             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!.Lower!);
-            addCursor(multiplayerUserScore.ScoresAround.Higher);
+            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<string>() ?? "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,8 +504,11 @@ namespace osu.Game.Tests.Visual.Playlists
                 totalCount++;
             }
 
+            if (!reachedEnd)
                 addCursor(result);
 
+            result.UserScore = createMultiplayerUserScore(userScore);
+
             return result;
         }
 

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);