Merge pull request #16497 from peppy/top-local-rank-optimisation

Rewrite `TopLocalRank` to use realm subscriptions

osu.Game.Tests/Visual/SongSelect/TestSceneTopLocalRank.cs

@@ -0,0 +1,143 @@
+// 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.Linq;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Audio;
+using osu.Framework.Extensions;
+using osu.Framework.Graphics;
+using osu.Framework.Platform;
+using osu.Framework.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Rulesets;
+using osu.Game.Scoring;
+using osu.Game.Screens.Select.Carousel;
+using osu.Game.Tests.Resources;
+using osuTK;
+
+namespace osu.Game.Tests.Visual.SongSelect
+{
+    public class TestSceneTopLocalRank : OsuTestScene
+    {
+        private RulesetStore rulesets;
+        private BeatmapManager beatmapManager;
+        private ScoreManager scoreManager;
+        private TopLocalRank topLocalRank;
+
+        [BackgroundDependencyLoader]
+        private void load(GameHost host, AudioManager audio)
+        {
+            Dependencies.Cache(rulesets = new RulesetStore(ContextFactory));
+            Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default));
+            Dependencies.Cache(scoreManager = new ScoreManager(rulesets, () => beatmapManager, LocalStorage, ContextFactory, Scheduler));
+            Dependencies.Cache(ContextFactory);
+
+            beatmapManager.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely();
+        }
+
+        private BeatmapInfo importedBeatmap => beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.Ruleset.ShortName == "osu");
+
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("Delete all scores", () => scoreManager.Delete());
+
+            AddStep("Create local rank", () =>
+            {
+                Add(topLocalRank = new TopLocalRank(importedBeatmap)
+                {
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                    Scale = new Vector2(10),
+                });
+            });
+        }
+
+        [Test]
+        public void TestBasicImportDelete()
+        {
+            ScoreInfo testScoreInfo = null;
+
+            AddAssert("Initially not present", () => !topLocalRank.IsPresent);
+
+            AddStep("Add score for current user", () =>
+            {
+                testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
+
+                testScoreInfo.User = API.LocalUser.Value;
+                testScoreInfo.Rank = ScoreRank.B;
+
+                scoreManager.Import(testScoreInfo);
+            });
+
+            AddUntilStep("Became present", () => topLocalRank.IsPresent);
+            AddAssert("Correct rank", () => topLocalRank.Rank == ScoreRank.B);
+
+            AddStep("Delete score", () =>
+            {
+                scoreManager.Delete(testScoreInfo);
+            });
+
+            AddUntilStep("Became not present", () => !topLocalRank.IsPresent);
+        }
+
+        [Test]
+        public void TestRulesetChange()
+        {
+            ScoreInfo testScoreInfo;
+
+            AddStep("Add score for current user", () =>
+            {
+                testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
+
+                testScoreInfo.User = API.LocalUser.Value;
+                testScoreInfo.Rank = ScoreRank.B;
+
+                scoreManager.Import(testScoreInfo);
+            });
+
+            AddUntilStep("Wait for initial presence", () => topLocalRank.IsPresent);
+
+            AddStep("Change ruleset", () => Ruleset.Value = rulesets.GetRuleset("fruits"));
+            AddUntilStep("Became not present", () => !topLocalRank.IsPresent);
+
+            AddStep("Change ruleset back", () => Ruleset.Value = rulesets.GetRuleset("osu"));
+            AddUntilStep("Became present", () => topLocalRank.IsPresent);
+        }
+
+        [Test]
+        public void TestHigherScoreSet()
+        {
+            ScoreInfo testScoreInfo = null;
+
+            AddAssert("Initially not present", () => !topLocalRank.IsPresent);
+
+            AddStep("Add score for current user", () =>
+            {
+                testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
+
+                testScoreInfo.User = API.LocalUser.Value;
+                testScoreInfo.Rank = ScoreRank.B;
+
+                scoreManager.Import(testScoreInfo);
+            });
+
+            AddUntilStep("Became present", () => topLocalRank.IsPresent);
+            AddAssert("Correct rank", () => topLocalRank.Rank == ScoreRank.B);
+
+            AddStep("Add higher score for current user", () =>
+            {
+                var testScoreInfo2 = TestResources.CreateTestScoreInfo(importedBeatmap);
+
+                testScoreInfo2.User = API.LocalUser.Value;
+                testScoreInfo2.Rank = ScoreRank.S;
+                testScoreInfo2.TotalScore = testScoreInfo.TotalScore + 1;
+
+                scoreManager.Import(testScoreInfo2);
+            });
+
+            AddAssert("Correct rank", () => topLocalRank.Rank == ScoreRank.S);
+        }
+    }
+}

osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs

@@ -159,7 +159,6 @@ namespace osu.Game.Screens.Select.Carousel
                                         new TopLocalRank(beatmapInfo)
                                         {
                                             Scale = new Vector2(0.8f),
-                                            Size = new Vector2(40, 20)
                                         },
                                         starCounter = new StarCounter
                                         {

osu.Game/Screens/Select/Carousel/TopLocalRank.cs

@@ -6,13 +6,14 @@ using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Threading;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
+using osu.Game.Models;
 using osu.Game.Online.API;
 using osu.Game.Online.Leaderboards;
 using osu.Game.Rulesets;
 using osu.Game.Scoring;
+using osuTK;
 using Realms;
 
 namespace osu.Game.Screens.Select.Carousel
@@ -30,72 +31,39 @@ namespace osu.Game.Screens.Select.Carousel
         [Resolved]
         private IAPIProvider api { get; set; }
 
+        private IDisposable scoreSubscription;
+
         public TopLocalRank(BeatmapInfo beatmapInfo)
             : base(null)
         {
             this.beatmapInfo = beatmapInfo;
-        }
 
-        [BackgroundDependencyLoader]
-        private void load()
-        {
-            ruleset.ValueChanged += _ => fetchAndLoadTopScore();
-
-            fetchAndLoadTopScore();
+            Size = new Vector2(40, 20);
         }
 
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
-            scoreSubscription = realmFactory.Context.All<ScoreInfo>()
-                                            .Filter($"{nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} = $0", beatmapInfo.ID)
-                                            .QueryAsyncWithNotifications((_, changes, ___) =>
-                                            {
-                                                if (changes == null)
-                                                    return;
-
-                                                fetchTopScoreRank();
-                                            });
-        }
-
-        private IDisposable scoreSubscription;
-
-        private ScheduledDelegate scheduledRankUpdate;
-
-        private void fetchAndLoadTopScore()
-        {
-            // TODO: this lookup likely isn't required, we can use the results of the subscription directly.
-            var rank = fetchTopScoreRank();
-
-            scheduledRankUpdate = Scheduler.Add(() =>
+            ruleset.BindValueChanged(_ =>
             {
-                Rank = rank;
-
-                // Required since presence is changed via IsPresent override
-                Invalidate(Invalidation.Presence);
-            });
+                scoreSubscription?.Dispose();
+                scoreSubscription = realmFactory.Context.All<ScoreInfo>()
+                                                .Filter($"{nameof(ScoreInfo.User)}.{nameof(RealmUser.OnlineID)} == $0"
+                                                        + $" && {nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $1"
+                                                        + $" && {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $2"
+                                                        + $" && {nameof(ScoreInfo.DeletePending)} == false", api.LocalUser.Value.Id, beatmapInfo.ID, ruleset.Value.ShortName)
+                                                .OrderByDescending(s => s.TotalScore)
+                                                .QueryAsyncWithNotifications((items, changes, ___) =>
+                                                {
+                                                    Rank = items.FirstOrDefault()?.Rank;
+                                                    // Required since presence is changed via IsPresent override
+                                                    Invalidate(Invalidation.Presence);
+                                                });
+            }, true);
         }
 
-        // We're present if a rank is set, or if there is a pending rank update (IsPresent = true is required for the scheduler to run).
-        public override bool IsPresent => base.IsPresent && (Rank != null || scheduledRankUpdate?.Completed == false);
-
-        private ScoreRank? fetchTopScoreRank()
-        {
-            if (realmFactory == null || beatmapInfo == null || ruleset?.Value == null || api?.LocalUser.Value == null)
-                return null;
-
-            using (var realm = realmFactory.CreateContext())
-            {
-                return realm.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.UserID == api.LocalUser.Value.Id && s.BeatmapInfoID == beatmapInfo.ID && s.RulesetID == ruleset.Value.ID && !s.DeletePending)
-                            .OrderByDescending(s => s.TotalScore)
-                            .FirstOrDefault()
-                            ?.Rank;
-            }
-        }
+        public override bool IsPresent => base.IsPresent && Rank != null;
 
         protected override void Dispose(bool isDisposing)
         {