From da4ebd0681e05318d7328fc5be587151233e8f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 9 Feb 2024 10:41:36 +0100 Subject: [PATCH] Refactor `SoloStatisticsWatcher` to no longer require explicit subscription callbacks --- .../Online/TestSceneSoloStatisticsWatcher.cs | 41 ++++--------- osu.Game/Online/Solo/SoloStatisticsWatcher.cs | 59 +++++-------------- .../TransientUserStatisticsUpdateDisplay.cs | 10 ++++ osu.Game/Screens/Play/SubmittingPlayer.cs | 5 ++ osu.Game/Screens/Ranking/SoloResultsScreen.cs | 24 ++++---- 5 files changed, 56 insertions(+), 83 deletions(-) create mode 100644 osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs diff --git a/osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs b/osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs index 3607b37c7e..19121b7f58 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs @@ -35,8 +35,6 @@ namespace osu.Game.Tests.Visual.Online private Action? handleGetUsersRequest; private Action? handleGetUserRequest; - private IDisposable? subscription; - private readonly Dictionary<(int userId, string rulesetName), UserStatistics> serverSideStatistics = new Dictionary<(int userId, string rulesetName), UserStatistics>(); [SetUpSteps] @@ -252,26 +250,6 @@ namespace osu.Game.Tests.Visual.Online AddAssert("values after are correct", () => update!.After.TotalScore, () => Is.EqualTo(6_000_000)); } - [Test] - public void TestStatisticsUpdateNotFiredAfterSubscriptionDisposal() - { - int userId = getUserId(); - setUpUser(userId); - - long scoreId = getScoreId(); - var ruleset = new OsuRuleset().RulesetInfo; - - SoloStatisticsUpdate? update = null; - registerForUpdates(scoreId, ruleset, receivedUpdate => update = receivedUpdate); - AddStep("unsubscribe", () => subscription!.Dispose()); - - feignScoreProcessing(userId, ruleset, 5_000_000); - - AddStep("signal score processed", () => ((ISpectatorClient)spectatorClient).UserScoreProcessed(userId, scoreId)); - AddWaitStep("wait a bit", 5); - AddAssert("update not received", () => update == null); - } - [Test] public void TestGlobalStatisticsUpdatedAfterRegistrationAddedAndScoreProcessed() { @@ -312,13 +290,20 @@ namespace osu.Game.Tests.Visual.Online } private void registerForUpdates(long scoreId, RulesetInfo rulesetInfo, Action onUpdateReady) => - AddStep("register for updates", () => subscription = watcher.RegisterForStatisticsUpdateAfter( - new ScoreInfo(Beatmap.Value.BeatmapInfo, new OsuRuleset().RulesetInfo, new RealmUser()) + AddStep("register for updates", () => + { + watcher.RegisterForStatisticsUpdateAfter( + new ScoreInfo(Beatmap.Value.BeatmapInfo, new OsuRuleset().RulesetInfo, new RealmUser()) + { + Ruleset = rulesetInfo, + OnlineID = scoreId + }); + watcher.LatestUpdate.BindValueChanged(update => { - Ruleset = rulesetInfo, - OnlineID = scoreId - }, - onUpdateReady)); + if (update.NewValue?.Score.OnlineID == scoreId) + onUpdateReady.Invoke(update.NewValue); + }); + }); private void feignScoreProcessing(int userId, RulesetInfo rulesetInfo, long newTotalScore) => AddStep("feign score processing", () => serverSideStatistics[(userId, rulesetInfo.ShortName)] = new UserStatistics { TotalScore = newTotalScore }); diff --git a/osu.Game/Online/Solo/SoloStatisticsWatcher.cs b/osu.Game/Online/Solo/SoloStatisticsWatcher.cs index 55b27fb364..2072e8633f 100644 --- a/osu.Game/Online/Solo/SoloStatisticsWatcher.cs +++ b/osu.Game/Online/Solo/SoloStatisticsWatcher.cs @@ -1,10 +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; using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Game.Extensions; @@ -22,14 +22,16 @@ namespace osu.Game.Online.Solo /// public partial class SoloStatisticsWatcher : Component { + public IBindable LatestUpdate => latestUpdate; + private readonly Bindable latestUpdate = new Bindable(); + [Resolved] private SpectatorClient spectatorClient { get; set; } = null!; [Resolved] private IAPIProvider api { get; set; } = null!; - private readonly Dictionary callbacks = new Dictionary(); - private long? lastProcessedScoreId; + private readonly Dictionary watchedScores = new Dictionary(); private Dictionary? latestStatistics; @@ -45,9 +47,7 @@ namespace osu.Game.Online.Solo /// Registers for a user statistics update after the given has been processed server-side. /// /// The score to listen for the statistics update for. - /// The callback to be invoked once the statistics update has been prepared. - /// An representing the subscription. Disposing it is equivalent to unsubscribing from future notifications. - public IDisposable RegisterForStatisticsUpdateAfter(ScoreInfo score, Action onUpdateReady) + public void RegisterForStatisticsUpdateAfter(ScoreInfo score) { Schedule(() => { @@ -57,24 +57,12 @@ namespace osu.Game.Online.Solo if (!score.Ruleset.IsLegacyRuleset() || score.OnlineID <= 0) return; - var callback = new StatisticsUpdateCallback(score, onUpdateReady); - - if (lastProcessedScoreId == score.OnlineID) - { - requestStatisticsUpdate(api.LocalUser.Value.Id, callback); - return; - } - - callbacks.Add(score.OnlineID, callback); + watchedScores.Add(score.OnlineID, score); }); - - return new InvokeOnDisposal(() => Schedule(() => callbacks.Remove(score.OnlineID))); } private void onUserChanged(APIUser? localUser) => Schedule(() => { - callbacks.Clear(); - lastProcessedScoreId = null; latestStatistics = null; if (localUser == null || localUser.OnlineID <= 1) @@ -107,25 +95,22 @@ namespace osu.Game.Online.Solo if (userId != api.LocalUser.Value?.OnlineID) return; - lastProcessedScoreId = scoreId; - - if (!callbacks.TryGetValue(scoreId, out var callback)) + if (!watchedScores.Remove(scoreId, out var scoreInfo)) return; - requestStatisticsUpdate(userId, callback); - callbacks.Remove(scoreId); + requestStatisticsUpdate(userId, scoreInfo); } - private void requestStatisticsUpdate(int userId, StatisticsUpdateCallback callback) + private void requestStatisticsUpdate(int userId, ScoreInfo scoreInfo) { - var request = new GetUserRequest(userId, callback.Score.Ruleset); - request.Success += user => Schedule(() => dispatchStatisticsUpdate(callback, user.Statistics)); + var request = new GetUserRequest(userId, scoreInfo.Ruleset); + request.Success += user => Schedule(() => dispatchStatisticsUpdate(scoreInfo, user.Statistics)); api.Queue(request); } - private void dispatchStatisticsUpdate(StatisticsUpdateCallback callback, UserStatistics updatedStatistics) + private void dispatchStatisticsUpdate(ScoreInfo scoreInfo, UserStatistics updatedStatistics) { - string rulesetName = callback.Score.Ruleset.ShortName; + string rulesetName = scoreInfo.Ruleset.ShortName; api.UpdateStatistics(updatedStatistics); @@ -135,9 +120,7 @@ namespace osu.Game.Online.Solo latestStatistics.TryGetValue(rulesetName, out UserStatistics? latestRulesetStatistics); latestRulesetStatistics ??= new UserStatistics(); - var update = new SoloStatisticsUpdate(callback.Score, latestRulesetStatistics, updatedStatistics); - callback.OnUpdateReady.Invoke(update); - + latestUpdate.Value = new SoloStatisticsUpdate(scoreInfo, latestRulesetStatistics, updatedStatistics); latestStatistics[rulesetName] = updatedStatistics; } @@ -148,17 +131,5 @@ namespace osu.Game.Online.Solo base.Dispose(isDisposing); } - - private class StatisticsUpdateCallback - { - public ScoreInfo Score { get; } - public Action OnUpdateReady { get; } - - public StatisticsUpdateCallback(ScoreInfo score, Action onUpdateReady) - { - Score = score; - OnUpdateReady = onUpdateReady; - } - } } } diff --git a/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs new file mode 100644 index 0000000000..3917933958 --- /dev/null +++ b/osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs @@ -0,0 +1,10 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Game.Overlays.Toolbar +{ + public class TransientUserStatisticsUpdateDisplay + { + + } +} diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs index c8e84f1961..bbd36c05d8 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -17,6 +17,7 @@ using osu.Game.Database; using osu.Game.Online.API; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; +using osu.Game.Online.Solo; using osu.Game.Online.Spectator; using osu.Game.Rulesets.Scoring; using osu.Game.Scoring; @@ -42,6 +43,9 @@ namespace osu.Game.Screens.Play [Resolved] private SessionStatics statics { get; set; } + [Resolved] + private SoloStatisticsWatcher soloStatisticsWatcher { get; set; } + private readonly object scoreSubmissionLock = new object(); private TaskCompletionSource scoreSubmissionSource; @@ -175,6 +179,7 @@ namespace osu.Game.Screens.Play await submitScore(score).ConfigureAwait(false); spectatorClient.EndPlaying(GameplayState); + soloStatisticsWatcher.RegisterForStatisticsUpdateAfter(score.ScoreInfo); } [Resolved] diff --git a/osu.Game/Screens/Ranking/SoloResultsScreen.cs b/osu.Game/Screens/Ranking/SoloResultsScreen.cs index 22d631e137..cba2fa18c0 100644 --- a/osu.Game/Screens/Ranking/SoloResultsScreen.cs +++ b/osu.Game/Screens/Ranking/SoloResultsScreen.cs @@ -31,10 +31,7 @@ namespace osu.Game.Screens.Ranking [Resolved] private RulesetStore rulesets { get; set; } = null!; - [Resolved] - private SoloStatisticsWatcher soloStatisticsWatcher { get; set; } = null!; - - private IDisposable? statisticsSubscription; + private IBindable latestUpdate = null!; private readonly Bindable statisticsUpdate = new Bindable(); public SoloResultsScreen(ScoreInfo score, bool allowRetry) @@ -42,14 +39,20 @@ namespace osu.Game.Screens.Ranking { } - protected override void LoadComplete() + [BackgroundDependencyLoader] + private void load(SoloStatisticsWatcher soloStatisticsWatcher) { - base.LoadComplete(); - - Debug.Assert(Score != null); - if (ShowUserStatistics) - statisticsSubscription = soloStatisticsWatcher.RegisterForStatisticsUpdateAfter(Score, update => statisticsUpdate.Value = update); + { + Debug.Assert(Score != null); + + latestUpdate = soloStatisticsWatcher.LatestUpdate.GetBoundCopy(); + latestUpdate.BindValueChanged(update => + { + if (update.NewValue?.Score.MatchesOnlineID(Score) == true) + statisticsUpdate.Value = update.NewValue; + }); + } } protected override StatisticsPanel CreateStatisticsPanel() @@ -84,7 +87,6 @@ namespace osu.Game.Screens.Ranking base.Dispose(isDisposing); getScoreRequest?.Cancel(); - statisticsSubscription?.Dispose(); } } }