diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs index bef43b3108..f0b2f710c6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs @@ -183,30 +183,6 @@ namespace osu.Game.Tests.Visual.Gameplay () => Does.Contain("#FF549A")); } - [Test] - public void TestTrackedScorePosition([Values] bool partial) - { - createLeaderboard(partial); - - AddStep("add many scores in one go", () => - { - for (int i = 0; i < 49; i++) - createRandomScore(new APIUser { Username = $"Player {i + 1}" }); - - // Add player at end to force an animation down the whole list. - playerScore.Value = 0; - createLeaderboardScore(playerScore, new APIUser { Username = "You", Id = 3 }, true); - }); - - if (partial) - AddUntilStep("tracked player has null position", () => leaderboard.TrackedScore?.ScorePosition, () => Is.Null); - else - AddUntilStep("tracked player is #50", () => leaderboard.TrackedScore?.ScorePosition, () => Is.EqualTo(50)); - - AddStep("move tracked player to top", () => leaderboard.TrackedScore!.TotalScore.Value = 8_000_000); - AddUntilStep("all players have non-null position", () => leaderboard.AllScores.Select(s => s.ScorePosition), () => Does.Not.Contain(null)); - } - private void addLocalPlayer() { AddStep("add local player", () => @@ -216,12 +192,11 @@ namespace osu.Game.Tests.Visual.Gameplay }); } - private void createLeaderboard(bool partial = false) + private void createLeaderboard() { AddStep("create leaderboard", () => { leaderboardProvider.Scores.Clear(); - leaderboardProvider.IsPartial = partial; Child = leaderboard = new TestDrawableGameplayLeaderboard { Anchor = Anchor.Centre, @@ -247,7 +222,7 @@ namespace osu.Game.Tests.Visual.Gameplay { var scoreItem = Flow.FirstOrDefault(i => i.User?.Username == username); - return scoreItem != null && scoreItem.ScorePosition == expectedPosition; + return scoreItem != null && scoreItem.ScorePosition.Value == expectedPosition; } public IEnumerable GetAllScoresForUsername(string username) @@ -260,7 +235,6 @@ namespace osu.Game.Tests.Visual.Gameplay { IBindableList IGameplayLeaderboardProvider.Scores => Scores; public BindableList Scores { get; } = new BindableList(); - public bool IsPartial { get; set; } } } } diff --git a/osu.Game/Online/Leaderboards/LeaderboardManager.cs b/osu.Game/Online/Leaderboards/LeaderboardManager.cs index dd68085103..4aca3b1a4a 100644 --- a/osu.Game/Online/Leaderboards/LeaderboardManager.cs +++ b/osu.Game/Online/Leaderboards/LeaderboardManager.cs @@ -125,7 +125,14 @@ namespace osu.Game.Online.Leaderboards var result = LeaderboardScores.Success ( - response.Scores.Select(s => s.ToScoreInfo(rulesets, newCriteria.Beatmap)).OrderByTotalScore().ToArray(), + response.Scores.Select(s => s.ToScoreInfo(rulesets, newCriteria.Beatmap)) + .OrderByTotalScore() + .Select((s, idx) => + { + s.Position = idx + 1; + return s; + }) + .ToArray(), response.UserScore?.CreateScoreInfo(rulesets, newCriteria.Beatmap) ); inFlightOnlineRequest = null; diff --git a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs index 005cd784c4..af286731aa 100644 --- a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs +++ b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs @@ -5,7 +5,6 @@ using System; using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; -using osu.Framework.Caching; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; @@ -20,8 +19,6 @@ namespace osu.Game.Screens.Play.HUD { public partial class DrawableGameplayLeaderboard : CompositeDrawable { - private readonly Cached sorting = new Cached(); - public Bindable Expanded = new Bindable(); protected readonly FillFlowContainer Flow; @@ -87,7 +84,6 @@ namespace osu.Game.Screens.Play.HUD }, true); } - Scheduler.AddDelayed(sort, 1000, true); configVisibility.BindValueChanged(_ => this.FadeTo(configVisibility.Value ? 1 : 0, 100, Easing.OutQuint), true); } @@ -109,8 +105,8 @@ namespace osu.Game.Screens.Play.HUD drawable.Expanded.BindTo(Expanded); Flow.Add(drawable); - drawable.TotalScore.BindValueChanged(_ => sorting.Invalidate(), true); - drawable.DisplayOrder.BindValueChanged(_ => sorting.Invalidate(), true); + drawable.ScorePosition.BindValueChanged(_ => Scheduler.AddOnce(sort)); + drawable.DisplayOrder.BindValueChanged(_ => Scheduler.AddOnce(sort), true); int displayCount = Math.Min(Flow.Count, max_panels); Height = displayCount * (DrawableGameplayLeaderboardScore.PANEL_HEIGHT + Flow.Spacing.Y); @@ -179,22 +175,8 @@ namespace osu.Game.Screens.Play.HUD private void sort() { - if (sorting.IsValid) - return; - - var orderedByScore = Flow - .OrderByDescending(i => i.TotalScore.Value) - .ThenBy(i => i.DisplayOrder.Value) - .ToList(); - - for (int i = 0; i < Flow.Count; i++) - { - var score = orderedByScore[i]; - Flow.SetLayoutPosition(score, i); - score.ScorePosition = i + 1 == Flow.Count && leaderboardProvider?.IsPartial == true && score.Tracked ? null : i + 1; - } - - sorting.Validate(); + foreach (var score in Flow.ToArray()) + Flow.SetLayoutPosition(score, score.DisplayOrder.Value); } private partial class InputDisabledScrollContainer : OsuScrollContainer diff --git a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboardScore.cs b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboardScore.cs index b14e31983c..e4f2cc0d68 100644 --- a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboardScore.cs +++ b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboardScore.cs @@ -56,6 +56,7 @@ namespace osu.Game.Screens.Play.HUD public BindableDouble Accuracy { get; } = new BindableDouble(1); public BindableInt Combo { get; } = new BindableInt(); public BindableBool HasQuit { get; } = new BindableBool(); + public Bindable ScorePosition { get; } = new Bindable(); public Bindable DisplayOrder { get; } = new Bindable(); private Func? getDisplayScoreFunction; @@ -69,28 +70,6 @@ namespace osu.Game.Screens.Play.HUD public Color4? TextColour { get; set; } - private int? scorePosition; - - private bool scorePositionIsSet; - - public int? ScorePosition - { - get => scorePosition; - set - { - // We always want to run once, as the incoming value may be null and require a visual update to "-". - if (value == scorePosition && scorePositionIsSet) - return; - - scorePosition = value; - - positionText.Text = scorePosition.HasValue ? $"#{scorePosition.Value.FormatRank()}" : "-"; - scorePositionIsSet = true; - - updateState(); - } - } - public IUser? User { get; } /// @@ -123,6 +102,7 @@ namespace osu.Game.Screens.Play.HUD Accuracy.BindTo(score.Accuracy); Combo.BindTo(score.Combo); HasQuit.BindTo(score.HasQuit); + ScorePosition.BindTo(score.Position); DisplayOrder.BindTo(score.DisplayOrder); GetDisplayScore = score.GetDisplayScore; @@ -334,6 +314,7 @@ namespace osu.Game.Screens.Play.HUD updateState(); Expanded.BindValueChanged(changeExpandedState, true); + ScorePosition.BindValueChanged(_ => updateState(), true); FinishTransforms(true); } @@ -392,7 +373,9 @@ namespace osu.Game.Screens.Play.HUD return; } - if (scorePosition == 1) + positionText.Text = ScorePosition.Value.HasValue ? $"#{ScorePosition.Value.Value.FormatRank()}" : "-"; + + if (ScorePosition.Value == 1) { widthExtension = true; panelColour = BackgroundColour ?? Color4Extensions.FromHex("7fcc33"); diff --git a/osu.Game/Screens/Select/Leaderboards/GameplayLeaderboardScore.cs b/osu.Game/Screens/Select/Leaderboards/GameplayLeaderboardScore.cs index 2655fd8dba..b681306053 100644 --- a/osu.Game/Screens/Select/Leaderboards/GameplayLeaderboardScore.cs +++ b/osu.Game/Screens/Select/Leaderboards/GameplayLeaderboardScore.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Select.Leaderboards /// An optional value to guarantee stable ordering. /// Lower numbers will appear higher in cases of ties. /// - public Bindable DisplayOrder { get; } = new BindableLong(); + public long TotalScoreTiebreaker { get; init; } /// /// A custom function which handles converting a score to a display score using a provided . @@ -68,6 +68,25 @@ namespace osu.Game.Screens.Select.Leaderboards /// public Colour4? TeamColour { get; init; } + /// + /// The initial position of the score on the leaderboard. + /// Mostly used for cases like the local user's best score on the global leaderboard (which will not be contiguous with the other scores). + /// + public int? InitialPosition { get; init; } = null; + + /// + /// The displayed rank of the score on the leaderboard. + /// + public Bindable Position { get; } = new Bindable(); + + /// + /// The index of the score on the leaderboard. + /// This differs from in that it is required (must always be known) + /// and that it doesn't represent the score's position on global leaderboards. + /// It's a property completely local to and relative to all scores provided by the managing . + /// + public Bindable DisplayOrder { get; } = new BindableLong(); + public GameplayLeaderboardScore(IUser user, ScoreProcessor scoreProcessor, bool tracked) { User = user; @@ -95,8 +114,9 @@ namespace osu.Game.Screens.Select.Leaderboards TotalScore.Value = scoreInfo.TotalScore; Accuracy.Value = scoreInfo.Accuracy; Combo.Value = scoreInfo.Combo; - DisplayOrder.Value = scoreInfo.OnlineID > 0 ? scoreInfo.OnlineID : scoreInfo.Date.ToUnixTimeSeconds(); + TotalScoreTiebreaker = scoreInfo.OnlineID > 0 ? scoreInfo.OnlineID : scoreInfo.Date.ToUnixTimeSeconds(); GetDisplayScore = scoreInfo.GetDisplayScore; + InitialPosition = scoreInfo.Position; } /// diff --git a/osu.Game/Screens/Select/Leaderboards/IGameplayLeaderboardProvider.cs b/osu.Game/Screens/Select/Leaderboards/IGameplayLeaderboardProvider.cs index 4399c422b4..468a5cbf9c 100644 --- a/osu.Game/Screens/Select/Leaderboards/IGameplayLeaderboardProvider.cs +++ b/osu.Game/Screens/Select/Leaderboards/IGameplayLeaderboardProvider.cs @@ -14,14 +14,5 @@ namespace osu.Game.Screens.Select.Leaderboards /// List of all scores to display on the leaderboard. /// public IBindableList Scores { get; } - - /// - /// Whether this leaderboard is a partial leaderboard (e.g. contains only the top 50 of all scores), - /// or is a full leaderboard (contains all scores that there will ever be). - /// - /// - /// If this is and a tracked score is last on the leaderboard, it will show an "unknown" score position. - /// - bool IsPartial { get; } } } diff --git a/osu.Game/Screens/Select/Leaderboards/MultiplayerLeaderboardProvider.cs b/osu.Game/Screens/Select/Leaderboards/MultiplayerLeaderboardProvider.cs index edfccd0e7e..80a5692841 100644 --- a/osu.Game/Screens/Select/Leaderboards/MultiplayerLeaderboardProvider.cs +++ b/osu.Game/Screens/Select/Leaderboards/MultiplayerLeaderboardProvider.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Caching; using osu.Framework.Extensions; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Extensions.ObjectExtensions; @@ -55,6 +56,8 @@ namespace osu.Game.Screens.Select.Leaderboards [Resolved] private OsuColour colours { get; set; } = null!; + private readonly Cached sorting = new Cached(); + public MultiplayerLeaderboardProvider(MultiplayerRoomUser[] users) { this.users = users; @@ -101,6 +104,8 @@ namespace osu.Game.Screens.Select.Leaderboards HasQuit = { BindTarget = trackedUser.UserQuit }, TeamColour = UserScores[user.OnlineID].Team is int team ? getTeamColour(team) : null, }; + leaderboardScore.TotalScore.BindValueChanged(_ => sorting.Invalidate()); + leaderboardScore.DisplayOrder.BindValueChanged(_ => sorting.Invalidate(), true); scores.Add(leaderboardScore); } }); @@ -124,6 +129,8 @@ namespace osu.Game.Screens.Select.Leaderboards // new players are not supported. playingUserIds.BindTo(multiplayerClient.CurrentMatchPlayingUserIds); playingUserIds.BindCollectionChanged(playingUsersChanged); + + Scheduler.AddDelayed(sort, 1000, true); } private void playingUsersChanged(object? sender, NotifyCollectionChangedEventArgs e) @@ -174,6 +181,26 @@ namespace osu.Game.Screens.Select.Leaderboards } } + private void sort() + { + if (sorting.IsValid) + return; + + var orderedByScore = scores + .OrderByDescending(i => i.TotalScore.Value) + .ThenBy(i => i.TotalScoreTiebreaker) + .ToList(); + + for (int i = 0; i < orderedByScore.Count; i++) + { + var score = orderedByScore[i]; + score.DisplayOrder.Value = i; + score.Position.Value = i + 1; + } + + sorting.Validate(); + } + protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); diff --git a/osu.Game/Screens/Select/Leaderboards/SoloGameplayLeaderboardProvider.cs b/osu.Game/Screens/Select/Leaderboards/SoloGameplayLeaderboardProvider.cs index 5cbbb3f3b0..41d57f7d24 100644 --- a/osu.Game/Screens/Select/Leaderboards/SoloGameplayLeaderboardProvider.cs +++ b/osu.Game/Screens/Select/Leaderboards/SoloGameplayLeaderboardProvider.cs @@ -1,8 +1,10 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Linq; using osu.Framework.Allocation; using osu.Framework.Bindables; +using osu.Framework.Caching; using osu.Framework.Graphics; using osu.Game.Online.Leaderboards; using osu.Game.Scoring; @@ -12,8 +14,6 @@ namespace osu.Game.Screens.Select.Leaderboards { public partial class SoloGameplayLeaderboardProvider : Component, IGameplayLeaderboardProvider { - public bool IsPartial { get; private set; } - public IBindableList Scores => scores; private readonly BindableList scores = new BindableList(); @@ -23,13 +23,16 @@ namespace osu.Game.Screens.Select.Leaderboards [Resolved] private GameplayState? gameplayState { get; set; } + private readonly Cached sorting = new Cached(); + private bool isPartial; + protected override void LoadComplete() { base.LoadComplete(); var globalScores = leaderboardManager?.Scores.Value; - IsPartial = leaderboardManager?.CurrentCriteria?.Scope != BeatmapLeaderboardScope.Local && globalScores?.TopScores.Count >= 50; + isPartial = leaderboardManager?.CurrentCriteria?.Scope != BeatmapLeaderboardScope.Local && globalScores?.TopScores.Count >= 50; if (globalScores != null) { @@ -39,12 +42,70 @@ namespace osu.Game.Screens.Select.Leaderboards if (gameplayState != null) { - scores.Add(new GameplayLeaderboardScore(gameplayState.Score.ScoreInfo.User, gameplayState.ScoreProcessor, true) + var localScore = new GameplayLeaderboardScore(gameplayState.Score.ScoreInfo.User, gameplayState.ScoreProcessor, true) { // Local score should always show lower than any existing scores in cases of ties. - DisplayOrder = { Value = long.MaxValue } - }); + TotalScoreTiebreaker = long.MaxValue + }; + localScore.TotalScore.BindValueChanged(_ => sorting.Invalidate()); + scores.Add(localScore); } + + Scheduler.AddDelayed(sort, 1000, true); + } + + private void sort() + { + if (sorting.IsValid) + return; + + var orderedByScore = scores + .OrderByDescending(i => i.TotalScore.Value) + .ThenBy(i => i.TotalScoreTiebreaker) + .ToList(); + + int delta = 0; + + for (int i = 0; i < orderedByScore.Count; i++) + { + var score = orderedByScore[i]; + + score.DisplayOrder.Value = i + 1; + + // if we know we have all scores there can ever be, we can do the simple and obvious thing. + if (!isPartial) + score.Position.Value = i + 1; + else + { + // we have a partial leaderboard, with potential gaps. + // we have initial score positions which were valid at the point of starting play. + // the assumption here is that non-tracked scores here cannot move around, only tracked ones can. + if (score.Tracked) + { + int? previousScorePosition = i > 0 ? orderedByScore[i - 1].InitialPosition : 0; + int? nextScorePosition = i < orderedByScore.Count - 1 ? orderedByScore[i + 1].InitialPosition : null; + + // if the tracked score is perfectly between two scores which have known neighbouring initial positions, + // we can assign it the position of the previous score plus one... + if (previousScorePosition != null && nextScorePosition != null && previousScorePosition + 1 == nextScorePosition) + { + score.Position.Value = previousScorePosition + 1; + // but we also need to ensure all subsequent scores get shifted down one position, too. + delta++; + } + // conversely, if the tracked score is not between neighbouring two scores and the leaderboard is partial, + // we can't really assign a valid position at all. it could be any number between the two neighbours. + else + score.Position.Value = null; + } + // for non-tracked scores, we just need to apply any delta that might have come from the tracked scores + // which might have been encountered and assigned a position earlier. + else + score.Position.Value = score.InitialPosition + delta; + } + } + + sorting.Validate(); } } }