Refactor SoloStatisticsWatcher to no longer require explicit subscription callbacks

osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs

@@ -35,8 +35,6 @@ namespace osu.Game.Tests.Visual.Online
         private Action<GetUsersRequest>? handleGetUsersRequest;
         private Action<GetUserRequest>? 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<SoloStatisticsUpdate> onUpdateReady) =>
-            AddStep("register for updates", () => subscription = watcher.RegisterForStatisticsUpdateAfter(
+            AddStep("register for updates", () =>
-                new ScoreInfo(Beatmap.Value.BeatmapInfo, new OsuRuleset().RulesetInfo, new RealmUser())
+            {
+                watcher.RegisterForStatisticsUpdateAfter(
+                    new ScoreInfo(Beatmap.Value.BeatmapInfo, new OsuRuleset().RulesetInfo, new RealmUser())
+                    {
+                        Ruleset = rulesetInfo,
+                        OnlineID = scoreId
+                    });
+                watcher.LatestUpdate.BindValueChanged(update =>
                 {
-                    Ruleset = rulesetInfo,
+                    if (update.NewValue?.Score.OnlineID == scoreId)
-                    OnlineID = scoreId
+                        onUpdateReady.Invoke(update.NewValue);
-                },
+                });
-                onUpdateReady));
+            });
 
         private void feignScoreProcessing(int userId, RulesetInfo rulesetInfo, long newTotalScore)
             => AddStep("feign score processing", () => serverSideStatistics[(userId, rulesetInfo.ShortName)] = new UserStatistics { TotalScore = newTotalScore });

osu.Game/Online/Solo/SoloStatisticsWatcher.cs

@@ -1,10 +1,10 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
     /// </summary>
     public partial class SoloStatisticsWatcher : Component
     {
+        public IBindable<SoloStatisticsUpdate?> LatestUpdate => latestUpdate;
+        private readonly Bindable<SoloStatisticsUpdate?> latestUpdate = new Bindable<SoloStatisticsUpdate?>();
+
         [Resolved]
         private SpectatorClient spectatorClient { get; set; } = null!;
 
         [Resolved]
         private IAPIProvider api { get; set; } = null!;
 
-        private readonly Dictionary<long, StatisticsUpdateCallback> callbacks = new Dictionary<long, StatisticsUpdateCallback>();
+        private readonly Dictionary<long, ScoreInfo> watchedScores = new Dictionary<long, ScoreInfo>();
-        private long? lastProcessedScoreId;
 
         private Dictionary<string, UserStatistics>? latestStatistics;
 
@@ -45,9 +47,7 @@ namespace osu.Game.Online.Solo
         /// Registers for a user statistics update after the given <paramref name="score"/> has been processed server-side.
         /// </summary>
         /// <param name="score">The score to listen for the statistics update for.</param>
-        /// <param name="onUpdateReady">The callback to be invoked once the statistics update has been prepared.</param>
+        public void RegisterForStatisticsUpdateAfter(ScoreInfo score)
-        /// <returns>An <see cref="IDisposable"/> representing the subscription. Disposing it is equivalent to unsubscribing from future notifications.</returns>
-        public IDisposable RegisterForStatisticsUpdateAfter(ScoreInfo score, Action<SoloStatisticsUpdate> onUpdateReady)
         {
             Schedule(() =>
             {
@@ -57,24 +57,12 @@ namespace osu.Game.Online.Solo
                 if (!score.Ruleset.IsLegacyRuleset() || score.OnlineID <= 0)
                     return;
 
-                var callback = new StatisticsUpdateCallback(score, onUpdateReady);
+                watchedScores.Add(score.OnlineID, score);
-
-                if (lastProcessedScoreId == score.OnlineID)
-                {
-                    requestStatisticsUpdate(api.LocalUser.Value.Id, callback);
-                    return;
-                }
-
-                callbacks.Add(score.OnlineID, callback);
             });
-
-            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 (!watchedScores.Remove(scoreId, out var scoreInfo))
-
-            if (!callbacks.TryGetValue(scoreId, out var callback))
                 return;
 
-            requestStatisticsUpdate(userId, callback);
+            requestStatisticsUpdate(userId, scoreInfo);
-            callbacks.Remove(scoreId);
         }
 
-        private void requestStatisticsUpdate(int userId, StatisticsUpdateCallback callback)
+        private void requestStatisticsUpdate(int userId, ScoreInfo scoreInfo)
         {
-            var request = new GetUserRequest(userId, callback.Score.Ruleset);
+            var request = new GetUserRequest(userId, scoreInfo.Ruleset);
-            request.Success += user => Schedule(() => dispatchStatisticsUpdate(callback, user.Statistics));
+            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);
+            latestUpdate.Value = new SoloStatisticsUpdate(scoreInfo, latestRulesetStatistics, updatedStatistics);
-            callback.OnUpdateReady.Invoke(update);
-
             latestStatistics[rulesetName] = updatedStatistics;
         }
 
@@ -148,17 +131,5 @@ namespace osu.Game.Online.Solo
 
             base.Dispose(isDisposing);
         }
-
-        private class StatisticsUpdateCallback
-        {
-            public ScoreInfo Score { get; }
-            public Action<SoloStatisticsUpdate> OnUpdateReady { get; }
-
-            public StatisticsUpdateCallback(ScoreInfo score, Action<SoloStatisticsUpdate> onUpdateReady)
-            {
-                Score = score;
-                OnUpdateReady = onUpdateReady;
-            }
-        }
     }
 }

osu.Game/Overlays/Toolbar/TransientUserStatisticsUpdateDisplay.cs

@@ -0,0 +1,10 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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
+    {
+        
+    }
+}

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<bool> scoreSubmissionSource;
 
@@ -175,6 +179,7 @@ namespace osu.Game.Screens.Play
 
             await submitScore(score).ConfigureAwait(false);
             spectatorClient.EndPlaying(GameplayState);
+            soloStatisticsWatcher.RegisterForStatisticsUpdateAfter(score.ScoreInfo);
         }
 
         [Resolved]

osu.Game/Screens/Ranking/SoloResultsScreen.cs

@@ -31,10 +31,7 @@ namespace osu.Game.Screens.Ranking
         [Resolved]
         private RulesetStore rulesets { get; set; } = null!;
 
-        [Resolved]
+        private IBindable<SoloStatisticsUpdate?> latestUpdate = null!;
-        private SoloStatisticsWatcher soloStatisticsWatcher { get; set; } = null!;
-
-        private IDisposable? statisticsSubscription;
         private readonly Bindable<SoloStatisticsUpdate?> statisticsUpdate = new Bindable<SoloStatisticsUpdate?>();
 
         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();
         }
     }
 }