Remove fetch callback logic completely

osu.Game/Online/Leaderboards/Leaderboard.cs

@@ -3,12 +3,11 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using System.Threading;
+using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
-using osu.Framework.Development;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Colour;
@@ -178,9 +177,9 @@ namespace osu.Game.Online.Leaderboards
         /// <summary>
         /// Performs a fetch/refresh of scores to be displayed.
         /// </summary>
-        /// <param name="scoresCallback">A callback which should be called when fetching is completed. Scheduling is not required.</param>
         /// <returns>An <see cref="APIRequest"/> responsible for the fetch operation. This will be queued and performed automatically.</returns>
-        protected abstract APIRequest FetchScores(Action<IEnumerable<TScoreInfo>> scoresCallback);
+        [CanBeNull]
+        protected abstract APIRequest FetchScores();
 
         protected abstract LeaderboardScore CreateDrawableScore(TScoreInfo model, int index);
 
@@ -193,11 +192,7 @@ namespace osu.Game.Online.Leaderboards
             PlaceholderState = PlaceholderState.Retrieving;
             loading.Show();
 
-            getScoresRequest = FetchScores(scores => getScoresRequestCallback = Schedule(() =>
+            getScoresRequest = FetchScores();
-            {
-                Scores = scores.ToArray();
-                PlaceholderState = Scores.Any() ? PlaceholderState.Successful : PlaceholderState.NoScores;
-            }));
 
             if (getScoresRequest == null)
                 return;
@@ -240,11 +235,6 @@ namespace osu.Game.Online.Leaderboards
             get => placeholderState;
             set
             {
-                if (value != PlaceholderState.Successful)
-                {
-                    Reset();
-                }
-
                 if (value == placeholderState)
                     return;
 
@@ -284,10 +274,8 @@ namespace osu.Game.Online.Leaderboards
             }
         }
 
-        private void updateScoresDrawables()
+        private void updateScoresDrawables() => Scheduler.Add(() =>
         {
-            Debug.Assert(ThreadSafety.IsUpdateThread);
-
             scrollFlow?.FadeOut(fade_duration, Easing.OutQuint).Expire();
             scrollFlow = null;
 
@@ -296,6 +284,7 @@ namespace osu.Game.Online.Leaderboards
             if (scores?.Any() != true)
             {
                 loading.Hide();
+                PlaceholderState = PlaceholderState.NoScores;
                 return;
             }
 
@@ -327,7 +316,7 @@ namespace osu.Game.Online.Leaderboards
                 scrollContainer.ScrollToStart(false);
                 loading.Hide();
             }, (showScoresCancellationSource = new CancellationTokenSource()).Token);
-        }
+        }, false);
 
         private void replacePlaceholder(Placeholder placeholder)
         {

osu.Game/Screens/OnlinePlay/Match/Components/MatchLeaderboard.cs

@@ -1,8 +1,6 @@
 // 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 osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Game.Online.API;
@@ -32,7 +30,7 @@ namespace osu.Game.Screens.OnlinePlay.Match.Components
 
         protected override bool IsOnlineScope => true;
 
-        protected override APIRequest FetchScores(Action<IEnumerable<APIUserScoreAggregate>> scoresCallback)
+        protected override APIRequest FetchScores()
         {
             if (roomId.Value == null)
                 return null;
@@ -41,7 +39,7 @@ namespace osu.Game.Screens.OnlinePlay.Match.Components
 
             req.Success += r =>
             {
-                scoresCallback?.Invoke(r.Leaderboard);
+                Scores = r.Leaderboard;
                 TopScore = r.UserScore;
             };
 

osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs

@@ -25,12 +25,6 @@ namespace osu.Game.Screens.Select.Leaderboards
     {
         public Action<ScoreInfo> ScoreSelected;
 
-        [Resolved]
-        private RulesetStore rulesets { get; set; }
-
-        [Resolved]
-        private RealmAccess realm { get; set; }
-
         private BeatmapInfo beatmapInfo;
 
         public BeatmapInfo BeatmapInfo
@@ -52,13 +46,7 @@ namespace osu.Game.Screens.Select.Leaderboards
                 beatmapInfo = value;
                 Scores = null;
 
-                if (IsOnlineScope)
+                RefetchScores();
-                    RefetchScores();
-                else
-                {
-                    if (IsLoaded)
-                        refreshRealmSubscription();
-                }
             }
         }
 
@@ -93,6 +81,14 @@ namespace osu.Game.Screens.Select.Leaderboards
         [Resolved]
         private IAPIProvider api { get; set; }
 
+        [Resolved]
+        private RulesetStore rulesets { get; set; }
+
+        [Resolved]
+        private RealmAccess realm { get; set; }
+
+        private IDisposable scoreSubscription;
+
         [BackgroundDependencyLoader]
         private void load()
         {
@@ -104,33 +100,6 @@ namespace osu.Game.Screens.Select.Leaderboards
             };
         }
 
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            refreshRealmSubscription();
-        }
-
-        private IDisposable scoreSubscription;
-
-        private void refreshRealmSubscription()
-        {
-            scoreSubscription?.Dispose();
-            scoreSubscription = null;
-
-            if (beatmapInfo == null)
-                return;
-
-            scoreSubscription = realm.RegisterForNotifications(r =>
-                    r.All<ScoreInfo>()
-                     .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID),
-                (_, changes, ___) =>
-                {
-                    if (!IsOnlineScope)
-                        RefetchScores();
-                });
-        }
-
         protected override void Reset()
         {
             base.Reset();
@@ -141,13 +110,11 @@ namespace osu.Game.Screens.Select.Leaderboards
 
         private CancellationTokenSource loadCancellationSource;
 
-        protected override APIRequest FetchScores(Action<IEnumerable<ScoreInfo>> scoresCallback)
+        protected override APIRequest FetchScores()
         {
             loadCancellationSource?.Cancel();
             loadCancellationSource = new CancellationTokenSource();
 
-            var cancellationToken = loadCancellationSource.Token;
-
             var fetchBeatmapInfo = BeatmapInfo;
 
             if (fetchBeatmapInfo == null)
@@ -158,31 +125,7 @@ namespace osu.Game.Screens.Select.Leaderboards
 
             if (Scope == BeatmapLeaderboardScope.Local)
             {
-                realm.Run(r =>
+                subscribeToLocalScores();
-                {
-                    var scores = r.All<ScoreInfo>()
-                                  .AsEnumerable()
-                                  // TODO: update to use a realm filter directly (or at least figure out the beatmap part to reduce scope).
-                                  .Where(s => !s.DeletePending && s.BeatmapInfo.ID == fetchBeatmapInfo.ID && s.Ruleset.ShortName == ruleset.Value.ShortName);
-
-                    if (filterMods && !mods.Value.Any())
-                    {
-                        // we need to filter out all scores that have any mods to get all local nomod scores
-                        scores = scores.Where(s => !s.Mods.Any());
-                    }
-                    else if (filterMods)
-                    {
-                        // otherwise find all the scores that have *any* of the currently selected mods (similar to how web applies mod filters)
-                        // we're creating and using a string list representation of selected mods so that it can be translated into the DB query itself
-                        var selectedMods = mods.Value.Select(m => m.Acronym);
-                        scores = scores.Where(s => s.Mods.Any(m => selectedMods.Contains(m.Acronym)));
-                    }
-
-                    scores = scores.Detach();
-
-                    scoreManager.OrderByTotalScoreAsync(scores.ToArray(), cancellationToken)
-                                .ContinueWith(ordered => scoresCallback?.Invoke(ordered.GetResultSafely()), TaskContinuationOptions.OnlyOnRanToCompletion);
-                });
 
                 return null;
             }
@@ -217,13 +160,13 @@ namespace osu.Game.Screens.Select.Leaderboards
 
             req.Success += r =>
             {
-                scoreManager.OrderByTotalScoreAsync(r.Scores.Select(s => s.CreateScoreInfo(rulesets, fetchBeatmapInfo)).ToArray(), cancellationToken)
+                scoreManager.OrderByTotalScoreAsync(r.Scores.Select(s => s.CreateScoreInfo(rulesets, fetchBeatmapInfo)).ToArray(), loadCancellationSource.Token)
                             .ContinueWith(task => Schedule(() =>
                             {
-                                if (cancellationToken.IsCancellationRequested)
+                                if (loadCancellationSource.IsCancellationRequested)
                                     return;
 
-                                scoresCallback?.Invoke(task.GetResultSafely());
+                                Scores = task.GetResultSafely();
                                 TopScore = r.UserScore?.CreateScoreInfo(rulesets, fetchBeatmapInfo);
                             }), TaskContinuationOptions.OnlyOnRanToCompletion);
             };
@@ -241,10 +184,53 @@ namespace osu.Game.Screens.Select.Leaderboards
             Action = () => ScoreSelected?.Invoke(model)
         };
 
+        private void subscribeToLocalScores()
+        {
+            scoreSubscription?.Dispose();
+            scoreSubscription = null;
+
+            if (beatmapInfo == null)
+                return;
+
+            scoreSubscription = realm.RegisterForNotifications(r =>
+                r.All<ScoreInfo>().Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $0"
+                                          + $" AND {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $1"
+                                          + $" AND {nameof(ScoreInfo.DeletePending)} == false"
+                    , beatmapInfo.ID, ruleset.Value.ShortName), localScoresChanged);
+        }
+
+        private void localScoresChanged(IRealmCollection<ScoreInfo> sender, ChangeSet changes, Exception exception)
+        {
+            if (IsOnlineScope)
+                return;
+
+            var scores = sender.AsEnumerable();
+
+            if (filterMods && !mods.Value.Any())
+            {
+                // we need to filter out all scores that have any mods to get all local nomod scores
+                scores = scores.Where(s => !s.Mods.Any());
+            }
+            else if (filterMods)
+            {
+                // otherwise find all the scores that have *any* of the currently selected mods (similar to how web applies mod filters)
+                // we're creating and using a string list representation of selected mods so that it can be translated into the DB query itself
+                var selectedMods = mods.Value.Select(m => m.Acronym);
+                scores = scores.Where(s => s.Mods.Any(m => selectedMods.Contains(m.Acronym)));
+            }
+
+            scores = scores.Detach();
+
+            scoreManager.OrderByTotalScoreAsync(scores.ToArray(), loadCancellationSource.Token)
+                        .ContinueWith(ordered =>
+                        {
+                            Scores = ordered.GetResultSafely();
+                        }, TaskContinuationOptions.OnlyOnRanToCompletion);
+        }
+
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
-
             scoreSubscription?.Dispose();
         }
     }