h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2026-05-28 01:47:04 +08:00
Code Issues Releases Wiki Activity

Remove LeaderboardManager return value and simplify flow further

Browse Source 
The rationale for this change is that the return value was mostly
useless, and at worst, misleading.

When using `LeaderboardManager`, it's assumed that a consumer will bind
to the global `Scores` list to ensure they receive updates for things
like local score changes via the internal realm subscription. If one
decides to instead use the return value of the task, it will be a static
snapshot that potentially becomes stale in the future.

I fell into this trap when refactoring the new leaderboard component
(while attempting to assert correctness that the values we are
displaying were in fact from the fetch operation we requested).

In the interest of keeping things simple, removing the return value
seems to be the best path forward.
This commit is contained in:
Dean Herbert
2025-04-22 19:00:45 +09:00
Unverified
parent 7e6e082bac
commit f7d1809cb7
3 changed files with 63 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
osu.Game/Online/Leaderboards/LeaderboardManager.cs
+39 -11
View File
@@ -9,6 +9,7 @@ using System.Threading.Tasks;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Logging;
using osu.Game.Beatmaps;
using osu.Game.Database;
using osu.Game.Extensions;
@@ -43,10 +44,14 @@ namespace osu.Game.Online.Leaderboards
[Resolved]
private RulesetStore rulesets { get; set; } = null!;
public Task<LeaderboardScores?> FetchWithCriteriaAsync(LeaderboardCriteria newCriteria)
/// <summary>
/// Fetch leaderboard content with the new criteria specified in the background.
/// On completion, <see cref="Scores"/> will be updated with the results from this call (unless a more recent call with a different criteria has completed).
/// </summary>
public void FetchWithCriteria(LeaderboardCriteria newCriteria)
{
if (CurrentCriteria?.Equals(newCriteria) == true && lastFetchCompletionSource?.Task.IsFaulted == false)
return lastFetchCompletionSource?.Task ?? Task.FromResult(Scores.Value);
return;
CurrentCriteria = newCriteria;
localScoreSubscription?.Dispose();
@@ -55,7 +60,10 @@ namespace osu.Game.Online.Leaderboards
scores.Value = null;
if (newCriteria.Beatmap == null || newCriteria.Ruleset == null)
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NoneSelected));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NoneSelected);
return;
}
switch (newCriteria.Scope)
{
@@ -70,25 +78,40 @@ namespace osu.Game.Online.Leaderboards
+ $" AND {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $1"
+ $" AND {nameof(ScoreInfo.DeletePending)} == false"
, newCriteria.Beatmap.ID, newCriteria.Ruleset.ShortName), localScoresChanged);
return localFetchCompletionSource.Task;
return;
}
default:
{
if (!api.IsLoggedIn)
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NotLoggedIn));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NotLoggedIn);
return;
}
if (!newCriteria.Ruleset.IsLegacyRuleset())
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.RulesetUnavailable));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.RulesetUnavailable);
return;
}
if (newCriteria.Beatmap.OnlineID <= 0 || newCriteria.Beatmap.Status <= BeatmapOnlineStatus.Pending)
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.BeatmapUnavailable));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.BeatmapUnavailable);
return;
}
if ((newCriteria.Scope.RequiresSupporter(newCriteria.ExactMods != null)) && !api.LocalUser.Value.IsSupporter)
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NotSupporter));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NotSupporter);
return;
}
if (newCriteria.Scope == BeatmapLeaderboardScope.Team && api.LocalUser.Value.Team == null)
return Task.FromResult<LeaderboardScores?>(scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NoTeam));
{
scores.Value = LeaderboardScores.Failure(LeaderboardFailState.NoTeam);
return;
}
var onlineFetchCompletionSource = new TaskCompletionSource<LeaderboardScores?>();
lastFetchCompletionSource = onlineFetchCompletionSource;
@@ -119,9 +142,14 @@ namespace osu.Game.Online.Leaderboards
if (onlineFetchCompletionSource.TrySetResult(result))
scores.Value = result;
};
newRequest.Failure += _ => onlineFetchCompletionSource.TrySetResult(LeaderboardScores.Failure(LeaderboardFailState.NetworkFailure));
newRequest.Failure += ex =>
{
Logger.Log($@"Failed to fetch leaderboards when displaying results: {ex}", LoggingTarget.Network);
onlineFetchCompletionSource.TrySetResult(LeaderboardScores.Failure(LeaderboardFailState.NetworkFailure));
};
api.Queue(inFlightOnlineRequest = newRequest);
return onlineFetchCompletionSource.Task;
break;
}
}
}
osu.Game/OsuGame.cs
+1 -6
View File
@@ -801,12 +801,7 @@ namespace osu.Game
var newLeaderboard = currentLeaderboard != null
? currentLeaderboard with { Beatmap = databasedBeatmap, Ruleset = databasedScore.ScoreInfo.Ruleset }
: new LeaderboardCriteria(databasedBeatmap, databasedScore.ScoreInfo.Ruleset, BeatmapLeaderboardScope.Global, null);
LeaderboardManager.FetchWithCriteriaAsync(newLeaderboard)
.ContinueWith(t =>
{
if (t.Exception != null)
Logger.Log($@"Failed to fetch leaderboards when displaying results: {t.Exception}", LoggingTarget.Network);
});
LeaderboardManager.FetchWithCriteria(newLeaderboard);
}
switch (presentType)
osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs
+23 -21
View File
@@ -62,7 +62,7 @@ namespace osu.Game.Screens.Select.Leaderboards
}
}
private readonly Bindable<LeaderboardScores?> fetchedScores = new Bindable<LeaderboardScores?>();
private readonly IBindable<LeaderboardScores?> fetchedScores = new Bindable<LeaderboardScores?>();
[Resolved]
private IBindable<RulesetInfo> ruleset { get; set; } = null!;
@@ -82,9 +82,10 @@ namespace osu.Game.Screens.Select.Leaderboards
if (filterMods)
RefetchScores();
};
((IBindable<LeaderboardScores?>)fetchedScores).BindTo(leaderboardManager.Scores);
}
private bool initialFetchComplete;
protected override bool IsOnlineScope => Scope != BeatmapLeaderboardScope.Local;
protected override APIRequest? FetchScores(CancellationToken cancellationToken)
@@ -96,30 +97,31 @@ namespace osu.Game.Screens.Select.Leaderboards
if (fetchBeatmapInfo == null)
return null;
leaderboardManager.FetchWithCriteriaAsync(new LeaderboardCriteria(fetchBeatmapInfo, fetchRuleset, Scope, filterMods ? mods.Value.ToArray() : null))
.ContinueWith(t =>
{
if (t.Exception != null && !t.IsCanceled)
{
Schedule(() => SetErrorState(LeaderboardState.NetworkFailure));
return;
}
leaderboardManager.FetchWithCriteria(new LeaderboardCriteria(fetchBeatmapInfo, fetchRuleset, Scope, filterMods ? mods.Value.ToArray() : null));
fetchedScores.UnbindEvents();
fetchedScores.BindValueChanged(scores =>
{
if (scores.NewValue == null) return;
if (scores.NewValue.FailState == null)
Schedule(() => SetScores(scores.NewValue.TopScores, scores.NewValue.UserScore));
else
Schedule(() => SetErrorState((LeaderboardState)scores.NewValue.FailState));
}, true);
}, cancellationToken);
if (!initialFetchComplete)
{
// only bind this after the first fetch to avoid reading stale scores.
fetchedScores.BindTo(leaderboardManager.Scores);
fetchedScores.BindValueChanged(_ => updateScores(), true);
initialFetchComplete = true;
}
return null;
}
private void updateScores()
{
var scores = fetchedScores.Value;
if (scores == null) return;
if (scores.FailState == null)
Schedule(() => SetScores(scores.TopScores, scores.UserScore));
else
Schedule(() => SetErrorState((LeaderboardState)scores.FailState));
}
protected override LeaderboardScore CreateDrawableScore(ScoreInfo model, int index) => new LeaderboardScore(model, index, IsOnlineScope, Scope != BeatmapLeaderboardScope.Friend)
{
Action = () => ScoreSelected?.Invoke(model)