Merge pull request #27866 from Joehuu/fix-online-score-export

Fix replay export not working correctly from online leaderboards

osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs

@@ -170,7 +170,7 @@ namespace osu.Game.Tests.Visual.Navigation
                     BeatmapInfo = beatmap.Beatmaps.First(),
                     Ruleset = ruleset ?? new OsuRuleset().RulesetInfo,
                     User = new GuestUser(),
-                }).Value;
+                })!.Value;
             });
 
             AddAssert($"import {i} succeeded", () => imported != null);

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

@@ -104,7 +104,7 @@ namespace osu.Game.Tests.Visual.UserInterface
                         Files = { new RealmNamedFileUsage(new RealmFile { Hash = $"{i}" }, string.Empty) }
                     };
 
-                    importedScores.Add(scoreManager.Import(score).Value);
+                    importedScores.Add(scoreManager.Import(score)!.Value);
                 }
             });
         });

osu.Game/Database/BackgroundDataStoreProcessor.cs

@@ -291,14 +291,17 @@ namespace osu.Game.Database
                 {
                     var score = scoreManager.Query(s => s.ID == id);
 
-                    scoreManager.PopulateMaximumStatistics(score);
+                    if (score != null)
-
-                    // Can't use async overload because we're not on the update thread.
-                    // ReSharper disable once MethodHasAsyncOverload
-                    realmAccess.Write(r =>
                     {
-                        r.Find<ScoreInfo>(id)!.MaximumStatisticsJson = JsonConvert.SerializeObject(score.MaximumStatistics);
+                        scoreManager.PopulateMaximumStatistics(score);
-                    });
+
+                        // Can't use async overload because we're not on the update thread.
+                        // ReSharper disable once MethodHasAsyncOverload
+                        realmAccess.Write(r =>
+                        {
+                            r.Find<ScoreInfo>(id)!.MaximumStatisticsJson = JsonConvert.SerializeObject(score.MaximumStatistics);
+                        });
+                    }
 
                     ++processedCount;
                 }

osu.Game/OsuGame.cs

@@ -706,26 +706,9 @@ namespace osu.Game
         {
             Logger.Log($"Beginning {nameof(PresentScore)} with score {score}");
 
-            // The given ScoreInfo may have missing properties if it was retrieved from online data. Re-retrieve it from the database
+            var databasedScore = ScoreManager.GetScore(score);
-            // to ensure all the required data for presenting a replay are present.
-            ScoreInfo databasedScoreInfo = null;
 
-            if (score.OnlineID > 0)
+            if (databasedScore == null) return;
-                databasedScoreInfo = ScoreManager.Query(s => s.OnlineID == score.OnlineID);
-
-            if (score.LegacyOnlineID > 0)
-                databasedScoreInfo ??= ScoreManager.Query(s => s.LegacyOnlineID == score.LegacyOnlineID);
-
-            if (score is ScoreInfo scoreInfo)
-                databasedScoreInfo ??= ScoreManager.Query(s => s.Hash == scoreInfo.Hash);
-
-            if (databasedScoreInfo == null)
-            {
-                Logger.Log("The requested score could not be found locally.", LoggingTarget.Information);
-                return;
-            }
-
-            var databasedScore = ScoreManager.GetScore(databasedScoreInfo);
 
             if (databasedScore.Replay == null)
             {
@@ -733,7 +716,7 @@ namespace osu.Game
                 return;
             }
 
-            var databasedBeatmap = BeatmapManager.QueryBeatmap(b => b.ID == databasedScoreInfo.BeatmapInfo.ID);
+            var databasedBeatmap = BeatmapManager.QueryBeatmap(b => b.ID == databasedScore.ScoreInfo.BeatmapInfo.ID);
 
             if (databasedBeatmap == null)
             {

osu.Game/Scoring/ScoreManager.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.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -10,8 +8,8 @@ using System.Linq;
 using System.Linq.Expressions;
 using System.Threading;
 using System.Threading.Tasks;
-using JetBrains.Annotations;
 using osu.Framework.Bindables;
+using osu.Framework.Logging;
 using osu.Framework.Platform;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
@@ -28,7 +26,7 @@ namespace osu.Game.Scoring
     public class ScoreManager : ModelManager<ScoreInfo>, IModelImporter<ScoreInfo>
     {
         private readonly Func<BeatmapManager> beatmaps;
-        private readonly OsuConfigManager configManager;
+        private readonly OsuConfigManager? configManager;
         private readonly ScoreImporter scoreImporter;
         private readonly LegacyScoreExporter scoreExporter;
 
@@ -43,7 +41,7 @@ namespace osu.Game.Scoring
         }
 
         public ScoreManager(RulesetStore rulesets, Func<BeatmapManager> beatmaps, Storage storage, RealmAccess realm, IAPIProvider api,
-                            OsuConfigManager configManager = null)
+                            OsuConfigManager? configManager = null)
             : base(storage, realm)
         {
             this.beatmaps = beatmaps;
@@ -60,18 +58,54 @@ namespace osu.Game.Scoring
             };
         }
 
-        public Score GetScore(ScoreInfo score) => scoreImporter.GetScore(score);
+        /// <summary>
+        /// Retrieve a <see cref="Score"/> from a given <see cref="IScoreInfo"/>.
+        /// </summary>
+        /// <param name="scoreInfo">The <see cref="IScoreInfo"/> to convert.</param>
+        /// <returns>The <see cref="Score"/>. Null if the score cannot be found in the database.</returns>
+        /// <remarks>
+        /// The <see cref="IScoreInfo"/> is re-retrieved from the database to ensure all the required data
+        /// for retrieving a replay are present (may have missing properties if it was retrieved from online data).
+        /// </remarks>
+        public Score? GetScore(IScoreInfo scoreInfo)
+        {
+            ScoreInfo? databasedScoreInfo = getDatabasedScoreInfo(scoreInfo);
+
+            return databasedScoreInfo == null ? null : scoreImporter.GetScore(databasedScoreInfo);
+        }
 
         /// <summary>
         /// Perform a lookup query on available <see cref="ScoreInfo"/>s.
         /// </summary>
         /// <param name="query">The query.</param>
         /// <returns>The first result for the provided query, or null if no results were found.</returns>
-        public ScoreInfo Query(Expression<Func<ScoreInfo, bool>> query)
+        public ScoreInfo? Query(Expression<Func<ScoreInfo, bool>> query)
         {
             return Realm.Run(r => r.All<ScoreInfo>().FirstOrDefault(query)?.Detach());
         }
 
+        private ScoreInfo? getDatabasedScoreInfo(IScoreInfo originalScoreInfo)
+        {
+            ScoreInfo? databasedScoreInfo = null;
+
+            if (originalScoreInfo.OnlineID > 0)
+                databasedScoreInfo = Query(s => s.OnlineID == originalScoreInfo.OnlineID);
+
+            if (originalScoreInfo.LegacyOnlineID > 0)
+                databasedScoreInfo ??= Query(s => s.LegacyOnlineID == originalScoreInfo.LegacyOnlineID);
+
+            if (originalScoreInfo is ScoreInfo scoreInfo)
+                databasedScoreInfo ??= Query(s => s.Hash == scoreInfo.Hash);
+
+            if (databasedScoreInfo == null)
+            {
+                Logger.Log("The requested score could not be found locally.", LoggingTarget.Information);
+                return null;
+            }
+
+            return databasedScoreInfo;
+        }
+
         /// <summary>
         /// Retrieves a bindable that represents the total score of a <see cref="ScoreInfo"/>.
         /// </summary>
@@ -80,7 +114,7 @@ namespace osu.Game.Scoring
         /// </remarks>
         /// <param name="score">The <see cref="ScoreInfo"/> to retrieve the bindable for.</param>
         /// <returns>The bindable containing the total score.</returns>
-        public Bindable<long> GetBindableTotalScore([NotNull] ScoreInfo score) => new TotalScoreBindable(score, configManager);
+        public Bindable<long> GetBindableTotalScore(ScoreInfo score) => new TotalScoreBindable(score, configManager);
 
         /// <summary>
         /// Retrieves a bindable that represents the formatted total score string of a <see cref="ScoreInfo"/>.
@@ -90,7 +124,7 @@ namespace osu.Game.Scoring
         /// </remarks>
         /// <param name="score">The <see cref="ScoreInfo"/> to retrieve the bindable for.</param>
         /// <returns>The bindable containing the formatted total score string.</returns>
-        public Bindable<string> GetBindableTotalScoreString([NotNull] ScoreInfo score) => new TotalScoreStringBindable(GetBindableTotalScore(score));
+        public Bindable<string> GetBindableTotalScoreString(ScoreInfo score) => new TotalScoreStringBindable(GetBindableTotalScore(score));
 
         /// <summary>
         /// Provides the total score of a <see cref="ScoreInfo"/>. Responds to changes in the currently-selected <see cref="ScoringMode"/>.
@@ -104,7 +138,7 @@ namespace osu.Game.Scoring
             /// </summary>
             /// <param name="score">The <see cref="ScoreInfo"/> to provide the total score of.</param>
             /// <param name="configManager">The config.</param>
-            public TotalScoreBindable(ScoreInfo score, OsuConfigManager configManager)
+            public TotalScoreBindable(ScoreInfo score, OsuConfigManager? configManager)
             {
                 configManager?.BindWith(OsuSetting.ScoreDisplayMode, scoringMode);
                 scoringMode.BindValueChanged(mode => Value = score.GetDisplayScore(mode.NewValue), true);
@@ -126,7 +160,7 @@ namespace osu.Game.Scoring
             }
         }
 
-        public void Delete([CanBeNull] Expression<Func<ScoreInfo, bool>> filter = null, bool silent = false)
+        public void Delete(Expression<Func<ScoreInfo, bool>>? filter = null, bool silent = false)
         {
             Realm.Run(r =>
             {
@@ -163,11 +197,25 @@ namespace osu.Game.Scoring
 
         public Task<IEnumerable<Live<ScoreInfo>>> Import(ProgressNotification notification, ImportTask[] tasks, ImportParameters parameters = default) => scoreImporter.Import(notification, tasks);
 
-        public Task Export(ScoreInfo score) => scoreExporter.ExportAsync(score.ToLive(Realm));
+        /// <summary>
+        /// Export a replay from a given <see cref="IScoreInfo"/>.
+        /// </summary>
+        /// <param name="scoreInfo">The <see cref="IScoreInfo"/> to export.</param>
+        /// <returns>The <see cref="Task"/>. Return <see cref="Task.CompletedTask"/> if the score cannot be found in the database.</returns>
+        /// <remarks>
+        /// The <see cref="IScoreInfo"/> is re-retrieved from the database to ensure all the required data
+        /// for exporting a replay are present (may have missing properties if it was retrieved from online data).
+        /// </remarks>
+        public Task Export(ScoreInfo scoreInfo)
+        {
+            ScoreInfo? databasedScoreInfo = getDatabasedScoreInfo(scoreInfo);
 
-        public Task<Live<ScoreInfo>> ImportAsUpdate(ProgressNotification notification, ImportTask task, ScoreInfo original) => scoreImporter.ImportAsUpdate(notification, task, original);
+            return databasedScoreInfo == null ? Task.CompletedTask : scoreExporter.ExportAsync(databasedScoreInfo.ToLive(Realm));
+        }
 
-        public Live<ScoreInfo> Import(ScoreInfo item, ArchiveReader archive = null, ImportParameters parameters = default, CancellationToken cancellationToken = default) =>
+        public Task<Live<ScoreInfo>?> ImportAsUpdate(ProgressNotification notification, ImportTask task, ScoreInfo original) => scoreImporter.ImportAsUpdate(notification, task, original);
+
+        public Live<ScoreInfo>? Import(ScoreInfo item, ArchiveReader? archive = null, ImportParameters parameters = default, CancellationToken cancellationToken = default) =>
             scoreImporter.ImportModel(item, archive, parameters, cancellationToken);
 
         /// <summary>
@@ -182,7 +230,7 @@ namespace osu.Game.Scoring
 
         #region Implementation of IPresentImports<ScoreInfo>
 
-        public Action<IEnumerable<Live<ScoreInfo>>> PresentImport
+        public Action<IEnumerable<Live<ScoreInfo>>>? PresentImport
         {
             set => scoreImporter.PresentImport = value;
         }

osu.Game/Screens/Play/Player.cs

@@ -1200,6 +1200,7 @@ namespace osu.Game.Screens.Play
             var importableScore = score.ScoreInfo.DeepClone();
 
             var imported = scoreManager.Import(importableScore, replayReader);
+            Debug.Assert(imported != null);
 
             imported.PerformRead(s =>
             {

osu.Game/Screens/Play/SaveFailedScoreButton.cs

@@ -137,7 +137,7 @@ namespace osu.Game.Screens.Play
         {
             if (state.NewValue != DownloadState.LocallyAvailable) return;
 
-            scoreManager.Export(importedScore);
+            if (importedScore != null) scoreManager.Export(importedScore);
 
             this.state.ValueChanged -= exportWhenReady;
         }