Merge pull request #20011 from smoogipoo/scoring-refactor

Refactor scoring to remove async methods / simplify

osu.Game.Tests/Visual/Gameplay/TestScenePlayerLocalScoreImport.cs

@@ -37,7 +37,7 @@ private void load(GameHost host, AudioManager audio)
         {
             Dependencies.Cache(rulesets = new RealmRulesetStore(Realm));
             Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default));
-            Dependencies.Cache(new ScoreManager(rulesets, () => beatmaps, LocalStorage, Realm, Scheduler, API));
+            Dependencies.Cache(new ScoreManager(rulesets, () => beatmaps, LocalStorage, Realm, API));
             Dependencies.Cache(Realm);
         }
 

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

@@ -47,7 +47,7 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl
 
             dependencies.Cache(rulesetStore = new RealmRulesetStore(Realm));
             dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, Realm, null, dependencies.Get<AudioManager>(), Resources, dependencies.Get<GameHost>(), Beatmap.Default));
-            dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, Realm, Scheduler, API));
+            dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, Realm, API));
             Dependencies.Cache(Realm);
 
             return dependencies;

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

@@ -32,7 +32,7 @@ private void load(GameHost host, AudioManager audio)
         {
             Dependencies.Cache(rulesets = new RealmRulesetStore(Realm));
             Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default));
-            Dependencies.Cache(scoreManager = new ScoreManager(rulesets, () => beatmapManager, LocalStorage, Realm, Scheduler, API));
+            Dependencies.Cache(scoreManager = new ScoreManager(rulesets, () => beatmapManager, LocalStorage, Realm, API));
             Dependencies.Cache(Realm);
 
             beatmapManager.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely();

osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs

@@ -73,7 +73,7 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl
 
             dependencies.Cache(new RealmRulesetStore(Realm));
             dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, Realm, null, dependencies.Get<AudioManager>(), Resources, dependencies.Get<GameHost>(), Beatmap.Default));
-            dependencies.Cache(scoreManager = new ScoreManager(dependencies.Get<RulesetStore>(), () => beatmapManager, LocalStorage, Realm, Scheduler, API));
+            dependencies.Cache(scoreManager = new ScoreManager(dependencies.Get<RulesetStore>(), () => beatmapManager, LocalStorage, Realm, API));
             Dependencies.Cache(Realm);
 
             return dependencies;

osu.Game/OsuGameBase.cs

@@ -273,7 +273,7 @@ private void load(ReadableKeyCombinationProvider keyCombinationProvider)
             dependencies.Cache(difficultyCache = new BeatmapDifficultyCache());
 
             // ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup()
-            dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, realm, Scheduler, API, difficultyCache, LocalConfig));
+            dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, realm, API, LocalConfig));
 
             dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, realm, API, Audio, Resources, Host, defaultBeatmap, difficultyCache, performOnlineLookups: true));
 

osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs

@@ -6,10 +6,8 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Threading;
-using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
-using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
@@ -87,27 +85,19 @@ protected APIScoresCollection Scores
                     MD5Hash = apiBeatmap.MD5Hash
                 };
 
-                scoreManager.OrderByTotalScoreAsync(value.Scores.Select(s => s.ToScoreInfo(rulesets, beatmapInfo)).ToArray(), loadCancellationSource.Token)
-                            .ContinueWith(task => Schedule(() =>
-                            {
-                                if (loadCancellationSource.IsCancellationRequested)
-                                    return;
-
-                                var scores = task.GetResultSafely();
-
-                                var topScore = scores.First();
+                var scores = scoreManager.OrderByTotalScore(value.Scores.Select(s => s.ToScoreInfo(rulesets, beatmapInfo))).ToArray();
+                var topScore = scores.First();
 
-                                scoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints());
-                                scoreTable.Show();
+                scoreTable.DisplayScores(scores, apiBeatmap.Status.GrantsPerformancePoints());
+                scoreTable.Show();
 
-                                var userScore = value.UserScore;
-                                var userScoreInfo = userScore?.Score.ToScoreInfo(rulesets, beatmapInfo);
+                var userScore = value.UserScore;
+                var userScoreInfo = userScore?.Score.ToScoreInfo(rulesets, beatmapInfo);
 
-                                topScoresContainer.Add(new DrawableTopScore(topScore));
+                topScoresContainer.Add(new DrawableTopScore(topScore));
 
-                                if (userScoreInfo != null && userScoreInfo.OnlineID != topScore.OnlineID)
-                                    topScoresContainer.Add(new DrawableTopScore(userScoreInfo, userScore.Position));
-                            }), TaskContinuationOptions.OnlyOnRanToCompletion);
+                if (userScoreInfo != null && userScoreInfo.OnlineID != topScore.OnlineID)
+                    topScoresContainer.Add(new DrawableTopScore(userScoreInfo, userScore.Position));
             });
         }
 

osu.Game/Scoring/ScoreManager.cs

@@ -11,10 +11,7 @@
 using System.Threading.Tasks;
 using JetBrains.Annotations;
 using osu.Framework.Bindables;
-using osu.Framework.Extensions;
-using osu.Framework.Logging;
 using osu.Framework.Platform;
-using osu.Framework.Threading;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Database;
@@ -28,17 +25,12 @@ namespace osu.Game.Scoring
 {
     public class ScoreManager : ModelManager<ScoreInfo>, IModelImporter<ScoreInfo>
     {
-        private readonly Scheduler scheduler;
-        private readonly BeatmapDifficultyCache difficultyCache;
         private readonly OsuConfigManager configManager;
         private readonly ScoreImporter scoreImporter;
 
-        public ScoreManager(RulesetStore rulesets, Func<BeatmapManager> beatmaps, Storage storage, RealmAccess realm, Scheduler scheduler, IAPIProvider api,
-                            BeatmapDifficultyCache difficultyCache = null, OsuConfigManager configManager = null)
+        public ScoreManager(RulesetStore rulesets, Func<BeatmapManager> beatmaps, Storage storage, RealmAccess realm, IAPIProvider api, OsuConfigManager configManager = null)
             : base(storage, realm)
         {
-            this.scheduler = scheduler;
-            this.difficultyCache = difficultyCache;
             this.configManager = configManager;
 
             scoreImporter = new ScoreImporter(rulesets, beatmaps, storage, realm, api)
@@ -63,28 +55,9 @@ public ScoreInfo Query(Expression<Func<ScoreInfo, bool>> query)
         /// Orders an array of <see cref="ScoreInfo"/>s by total score.
         /// </summary>
         /// <param name="scores">The array of <see cref="ScoreInfo"/>s to reorder.</param>
-        /// <param name="cancellationToken">A <see cref="CancellationToken"/> to cancel the process.</param>
         /// <returns>The given <paramref name="scores"/> ordered by decreasing total score.</returns>
-        public async Task<ScoreInfo[]> OrderByTotalScoreAsync(ScoreInfo[] scores, CancellationToken cancellationToken = default)
-        {
-            if (difficultyCache != null)
-            {
-                // Compute difficulties asynchronously first to prevent blocking via the GetTotalScore() call below.
-                foreach (var s in scores)
-                {
-                    await difficultyCache.GetDifficultyAsync(s.BeatmapInfo, s.Ruleset, s.Mods, cancellationToken).ConfigureAwait(false);
-                    cancellationToken.ThrowIfCancellationRequested();
-                }
-            }
-
-            long[] totalScores = await Task.WhenAll(scores.Select(s => GetTotalScoreAsync(s, cancellationToken: cancellationToken))).ConfigureAwait(false);
-
-            return scores.Select((score, index) => (score, totalScore: totalScores[index]))
-                         .OrderByDescending(g => g.totalScore)
-                         .ThenBy(g => g.score.OnlineID)
-                         .Select(g => g.score)
-                         .ToArray();
-        }
+        public IEnumerable<ScoreInfo> OrderByTotalScore(IEnumerable<ScoreInfo> scores)
+            => scores.OrderByDescending(s => GetTotalScore(s)).ThenBy(s => s.OnlineID);
 
         /// <summary>
         /// Retrieves a bindable that represents the total score of a <see cref="ScoreInfo"/>.
@@ -106,44 +79,18 @@ public async Task<ScoreInfo[]> OrderByTotalScoreAsync(ScoreInfo[] scores, Cancel
         /// <returns>The bindable containing the formatted total score string.</returns>
         public Bindable<string> GetBindableTotalScoreString([NotNull] ScoreInfo score) => new TotalScoreStringBindable(GetBindableTotalScore(score));
 
-        /// <summary>
-        /// Retrieves the total score of a <see cref="ScoreInfo"/> in the given <see cref="ScoringMode"/>.
-        /// The score is returned in a callback that is run on the update thread.
-        /// </summary>
-        /// <param name="score">The <see cref="ScoreInfo"/> to calculate the total score of.</param>
-        /// <param name="callback">The callback to be invoked with the total score.</param>
-        /// <param name="mode">The <see cref="ScoringMode"/> to return the total score as.</param>
-        /// <param name="cancellationToken">A <see cref="CancellationToken"/> to cancel the process.</param>
-        public void GetTotalScore([NotNull] ScoreInfo score, [NotNull] Action<long> callback, ScoringMode mode = ScoringMode.Standardised, CancellationToken cancellationToken = default)
-        {
-            GetTotalScoreAsync(score, mode, cancellationToken)
-                .ContinueWith(task => scheduler.Add(() =>
-                {
-                    if (!cancellationToken.IsCancellationRequested)
-                        callback(task.GetResultSafely());
-                }), TaskContinuationOptions.OnlyOnRanToCompletion);
-        }
-
         /// <summary>
         /// Retrieves the total score of a <see cref="ScoreInfo"/> in the given <see cref="ScoringMode"/>.
         /// </summary>
         /// <param name="score">The <see cref="ScoreInfo"/> to calculate the total score of.</param>
         /// <param name="mode">The <see cref="ScoringMode"/> to return the total score as.</param>
-        /// <param name="cancellationToken">A <see cref="CancellationToken"/> to cancel the process.</param>
         /// <returns>The total score.</returns>
-        public async Task<long> GetTotalScoreAsync([NotNull] ScoreInfo score, ScoringMode mode = ScoringMode.Standardised, CancellationToken cancellationToken = default)
+        public long GetTotalScore([NotNull] ScoreInfo score, ScoringMode mode = ScoringMode.Standardised)
         {
             // TODO: This is required for playlist aggregate scores. They should likely not be getting here in the first place.
             if (string.IsNullOrEmpty(score.BeatmapInfo.MD5Hash))
                 return score.TotalScore;
 
-            int? beatmapMaxCombo = await GetMaximumAchievableComboAsync(score, cancellationToken).ConfigureAwait(false);
-            if (beatmapMaxCombo == null)
-                return score.TotalScore;
-
-            if (beatmapMaxCombo == 0)
-                return 0;
-
             var ruleset = score.Ruleset.CreateInstance();
             var scoreProcessor = ruleset.CreateScoreProcessor();
             scoreProcessor.Mods.Value = score.Mods;
@@ -155,46 +102,15 @@ public async Task<long> GetTotalScoreAsync([NotNull] ScoreInfo score, ScoringMod
         /// Retrieves the maximum achievable combo for the provided score.
         /// </summary>
         /// <param name="score">The <see cref="ScoreInfo"/> to compute the maximum achievable combo for.</param>
-        /// <param name="cancellationToken">A <see cref="CancellationToken"/> to cancel the process.</param>
-        /// <returns>The maximum achievable combo. A <see langword="null"/> return value indicates the difficulty cache has failed to retrieve the combo.</returns>
-        public async Task<int?> GetMaximumAchievableComboAsync([NotNull] ScoreInfo score, CancellationToken cancellationToken = default)
-        {
-            if (score.IsLegacyScore)
-            {
-                // This score is guaranteed to be an osu!stable score.
-                // The combo must be determined through either the beatmap's max combo value or the difficulty calculator, as lazer's scoring has changed and the score statistics cannot be used.
-#pragma warning disable CS0618
-                if (score.BeatmapInfo.MaxCombo != null)
-                    return score.BeatmapInfo.MaxCombo.Value;
-#pragma warning restore CS0618
-
-                if (difficultyCache == null)
-                    return null;
-
-                // We can compute the max combo locally after the async beatmap difficulty computation.
-                var difficulty = await difficultyCache.GetDifficultyAsync(score.BeatmapInfo, score.Ruleset, score.Mods, cancellationToken).ConfigureAwait(false);
-
-                if (difficulty == null)
-                    Logger.Log($"Couldn't get beatmap difficulty for beatmap {score.BeatmapInfo.OnlineID}");
-
-                return difficulty?.MaxCombo;
-            }
-
-            // This is guaranteed to be a non-legacy score.
-            // The combo must be determined through the score's statistics, as both the beatmap's max combo and the difficulty calculator will provide osu!stable combo values.
-            return Enum.GetValues(typeof(HitResult)).OfType<HitResult>().Where(r => r.AffectsCombo()).Select(r => score.Statistics.GetValueOrDefault(r)).Sum();
-        }
+        /// <returns>The maximum achievable combo.</returns>
+        public int GetMaximumAchievableCombo([NotNull] ScoreInfo score) => score.MaximumStatistics.Where(kvp => kvp.Key.AffectsCombo()).Sum(kvp => kvp.Value);
 
         /// <summary>
         /// Provides the total score of a <see cref="ScoreInfo"/>. Responds to changes in the currently-selected <see cref="ScoringMode"/>.
         /// </summary>
         private class TotalScoreBindable : Bindable<long>
         {
             private readonly Bindable<ScoringMode> scoringMode = new Bindable<ScoringMode>();
-            private readonly ScoreInfo score;
-            private readonly ScoreManager scoreManager;
-
-            private CancellationTokenSource difficultyCalculationCancellationSource;
 
             /// <summary>
             /// Creates a new <see cref="TotalScoreBindable"/>.
@@ -204,19 +120,8 @@ private class TotalScoreBindable : Bindable<long>
             /// <param name="configManager">The config.</param>
             public TotalScoreBindable(ScoreInfo score, ScoreManager scoreManager, OsuConfigManager configManager)
             {
-                this.score = score;
-                this.scoreManager = scoreManager;
-
                 configManager?.BindWith(OsuSetting.ScoreDisplayMode, scoringMode);
-                scoringMode.BindValueChanged(onScoringModeChanged, true);
-            }
-
-            private void onScoringModeChanged(ValueChangedEvent<ScoringMode> mode)
-            {
-                difficultyCalculationCancellationSource?.Cancel();
-                difficultyCalculationCancellationSource = new CancellationTokenSource();
-
-                scoreManager.GetTotalScore(score, s => Value = s, mode.NewValue, difficultyCalculationCancellationSource.Token);
+                scoringMode.BindValueChanged(mode => Value = scoreManager.GetTotalScore(score, mode.NewValue), true);
             }
         }
 

osu.Game/Screens/OnlinePlay/Playlists/PlaylistsResultsScreen.cs

@@ -180,31 +180,26 @@ private APIRequest createIndexRequest(Action<IEnumerable<ScoreInfo>> scoresCallb
         /// <param name="callback">The callback to invoke with the final <see cref="ScoreInfo"/>s.</param>
         /// <param name="scores">The <see cref="MultiplayerScore"/>s that were retrieved from <see cref="APIRequest"/>s.</param>
         /// <param name="pivot">An optional pivot around which the scores were retrieved.</param>
-        private void performSuccessCallback([NotNull] Action<IEnumerable<ScoreInfo>> callback, [NotNull] List<MultiplayerScore> scores, [CanBeNull] MultiplayerScores pivot = null)
+        private void performSuccessCallback([NotNull] Action<IEnumerable<ScoreInfo>> callback, [NotNull] List<MultiplayerScore> scores, [CanBeNull] MultiplayerScores pivot = null) => Schedule(() =>
         {
-            var scoreInfos = scores.Select(s => s.CreateScoreInfo(rulesets, playlistItem, Beatmap.Value.BeatmapInfo)).ToArray();
+            var scoreInfos = scoreManager.OrderByTotalScore(scores.Select(s => s.CreateScoreInfo(rulesets, playlistItem, Beatmap.Value.BeatmapInfo))).ToArray();
 
-            // Score panels calculate total score before displaying, which can take some time. In order to count that calculation as part of the loading spinner display duration,
-            // calculate the total scores locally before invoking the success callback.
-            scoreManager.OrderByTotalScoreAsync(scoreInfos).ContinueWith(_ => Schedule(() =>
+            // Select a score if we don't already have one selected.
+            // Note: This is done before the callback so that the panel list centres on the selected score before panels are added (eliminating initial scroll).
+            if (SelectedScore.Value == null)
             {
-                // Select a score if we don't already have one selected.
-                // Note: This is done before the callback so that the panel list centres on the selected score before panels are added (eliminating initial scroll).
-                if (SelectedScore.Value == null)
+                Schedule(() =>
                 {
-                    Schedule(() =>
-                    {
-                        // Prefer selecting the local user's score, or otherwise default to the first visible score.
-                        SelectedScore.Value = scoreInfos.FirstOrDefault(s => s.User.OnlineID == api.LocalUser.Value.Id) ?? scoreInfos.FirstOrDefault();
-                    });
-                }
+                    // Prefer selecting the local user's score, or otherwise default to the first visible score.
+                    SelectedScore.Value = scoreInfos.FirstOrDefault(s => s.User.OnlineID == api.LocalUser.Value.Id) ?? scoreInfos.FirstOrDefault();
+                });
+            }
 
-                // Invoke callback to add the scores. Exclude the user's current score which was added previously.
-                callback.Invoke(scoreInfos.Where(s => s.OnlineID != Score?.OnlineID));
+            // Invoke callback to add the scores. Exclude the user's current score which was added previously.
+            callback.Invoke(scoreInfos.Where(s => s.OnlineID != Score?.OnlineID));
 
-                hideLoadingSpinners(pivot);
-            }));
-        }
+            hideLoadingSpinners(pivot);
+        });
 
         private void hideLoadingSpinners([CanBeNull] MultiplayerScores pivot = null)
         {