Merge pull request #24794 from bdach/score-encoding-cleanup

Correctly handle multiple online score ID types

osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs

@@ -87,6 +87,34 @@ namespace osu.Game.Tests.Beatmaps.Formats
             }
         }
 
+        [Test]
+        public void TestDecodeLegacyOnlineID()
+        {
+            var decoder = new TestLegacyScoreDecoder();
+
+            using (var resourceStream = TestResources.OpenResource("Replays/taiko-replay-with-legacy-online-id.osr"))
+            {
+                var score = decoder.Parse(resourceStream);
+
+                Assert.That(score.ScoreInfo.OnlineID, Is.EqualTo(-1));
+                Assert.That(score.ScoreInfo.LegacyOnlineID, Is.EqualTo(255));
+            }
+        }
+
+        [Test]
+        public void TestDecodeNewOnlineID()
+        {
+            var decoder = new TestLegacyScoreDecoder();
+
+            using (var resourceStream = TestResources.OpenResource("Replays/taiko-replay-with-new-online-id.osr"))
+            {
+                var score = decoder.Parse(resourceStream);
+
+                Assert.That(score.ScoreInfo.OnlineID, Is.EqualTo(258));
+                Assert.That(score.ScoreInfo.LegacyOnlineID, Is.EqualTo(-1));
+            }
+        }
+
         [TestCase(3, true)]
         [TestCase(6, false)]
         [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)]

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

@@ -213,7 +213,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             OnlineID = hasOnlineId ? online_score_id : 0,
             Ruleset = new OsuRuleset().RulesetInfo,
             BeatmapInfo = beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps.First(),
-            Hash = replayAvailable ? "online" : string.Empty,
+            HasOnlineReplay = replayAvailable,
             User = new APIUser
             {
                 Id = 39828,

osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs

@@ -362,7 +362,7 @@ namespace osu.Game.Tests.Visual.Ranking
                 {
                     var score = TestResources.CreateTestScoreInfo();
                     score.TotalScore += 10 - i;
-                    score.Hash = $"test{i}";
+                    score.HasOnlineReplay = true;
                     scores.Add(score);
                 }
 

osu.Game/Database/IHasRealmFiles.cs

@@ -10,13 +10,15 @@ namespace osu.Game.Database
     /// <summary>
     /// A model that contains a list of files it is responsible for.
     /// </summary>
-    public interface IHasRealmFiles
+    public interface IHasRealmFiles : IHasNamedFiles
     {
         /// <summary>
         /// Available files in this model, with locally filenames.
         /// When performing lookups, consider using <see cref="BeatmapSetInfoExtensions.GetFile"/> or <see cref="BeatmapSetInfoExtensions.GetPathForFile"/> to do case-insensitive lookups.
         /// </summary>
-        IList<RealmNamedFileUsage> Files { get; }
+        new IList<RealmNamedFileUsage> Files { get; }
+
+        IEnumerable<INamedFileUsage> IHasNamedFiles.Files => Files;
 
         /// <summary>
         /// A combined hash representing the model, based on the files it contains.

osu.Game/Database/RealmAccess.cs

@@ -87,8 +87,9 @@ namespace osu.Game.Database
         /// 33   2023-08-16    Reset default chat toggle key binding to avoid conflict with newly added leaderboard toggle key binding.
         /// 34   2023-08-21    Add BackgroundReprocessingFailed flag to ScoreInfo to track upgrade failures.
         /// 35   2023-10-16    Clear key combinations of keybindings that are assigned to more than one action in a given settings section.
+        /// 36   2023-10-26    Add LegacyOnlineID to ScoreInfo. Move osu_scores_*_high IDs stored in OnlineID to LegacyOnlineID. Reset anomalous OnlineIDs.
         /// </summary>
-        private const int schema_version = 35;
+        private const int schema_version = 36;
 
         /// <summary>
         /// Lock object which is held during <see cref="BlockAllOperations"/> sections, blocking realm retrieval during blocking periods.
@@ -1075,6 +1076,24 @@ namespace osu.Game.Database
 
                     break;
                 }
+
+                case 36:
+                {
+                    foreach (var score in migration.NewRealm.All<ScoreInfo>())
+                    {
+                        if (score.OnlineID > 0)
+                        {
+                            score.LegacyOnlineID = score.OnlineID;
+                            score.OnlineID = -1;
+                        }
+                        else
+                        {
+                            score.LegacyOnlineID = score.OnlineID = -1;
+                        }
+                    }
+
+                    break;
+                }
             }
 
             Logger.Log($"Migration completed in {stopwatch.ElapsedMilliseconds}ms");

osu.Game/Extensions/ModelExtensions.cs

@@ -114,8 +114,24 @@ namespace osu.Game.Extensions
         /// </summary>
         /// <param name="instance">The instance to compare.</param>
         /// <param name="other">The other instance to compare against.</param>
-        /// <returns>Whether online IDs match. If either instance is missing an online ID, this will return false.</returns>
+        /// <returns>
-        public static bool MatchesOnlineID(this IScoreInfo? instance, IScoreInfo? other) => matchesOnlineID(instance, other);
+        /// Whether online IDs match.
+        /// Both <see cref="IHasOnlineID{T}.OnlineID"/> and <see cref="IScoreInfo.LegacyOnlineID"/> are checked, in that order.
+        /// If either instance is missing an online ID, this will return false.
+        /// </returns>
+        public static bool MatchesOnlineID(this IScoreInfo? instance, IScoreInfo? other)
+        {
+            if (matchesOnlineID(instance, other))
+                return true;
+
+            if (instance == null || other == null)
+                return false;
+
+            if (instance.LegacyOnlineID < 0 || other.LegacyOnlineID < 0)
+                return false;
+
+            return instance.LegacyOnlineID.Equals(other.LegacyOnlineID);
+        }
 
         private static bool matchesOnlineID(this IHasOnlineID<long>? instance, IHasOnlineID<long>? other)
         {

osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs

@@ -7,16 +7,16 @@ using System.Linq;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Converters;
 using osu.Game.Beatmaps;
-using osu.Game.Database;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Scoring;
+using osu.Game.Users;
 
 namespace osu.Game.Online.API.Requests.Responses
 {
     [Serializable]
-    public class SoloScoreInfo : IHasOnlineID<long>
+    public class SoloScoreInfo : IScoreInfo
     {
         [JsonProperty("beatmap_id")]
         public int BeatmapID { get; set; }
@@ -138,6 +138,18 @@ namespace osu.Game.Online.API.Requests.Responses
 
         #endregion
 
+        #region IScoreInfo
+
+        public long OnlineID => (long?)ID ?? -1;
+
+        IUser IScoreInfo.User => User!;
+        DateTimeOffset IScoreInfo.Date => EndedAt;
+        long IScoreInfo.LegacyOnlineID => (long?)LegacyScoreId ?? -1;
+        IBeatmapInfo IScoreInfo.Beatmap => Beatmap!;
+        IRulesetInfo IScoreInfo.Ruleset => Beatmap!.Ruleset;
+
+        #endregion
+
         public override string ToString() => $"score_id: {ID} user_id: {UserID}";
 
         /// <summary>
@@ -178,6 +190,7 @@ namespace osu.Game.Online.API.Requests.Responses
             var score = new ScoreInfo
             {
                 OnlineID = OnlineID,
+                LegacyOnlineID = (long?)LegacyScoreId ?? -1,
                 User = User ?? new APIUser { Id = UserID },
                 BeatmapInfo = new BeatmapInfo { OnlineID = BeatmapID },
                 Ruleset = new RulesetInfo { OnlineID = RulesetID },
@@ -189,7 +202,7 @@ namespace osu.Game.Online.API.Requests.Responses
                 Statistics = Statistics,
                 MaximumStatistics = MaximumStatistics,
                 Date = EndedAt,
-                Hash = HasReplay ? "online" : string.Empty, // TODO: temporary?
+                HasOnlineReplay = HasReplay,
                 Mods = mods,
                 PP = PP,
             };
@@ -223,7 +236,5 @@ namespace osu.Game.Online.API.Requests.Responses
             Statistics = score.Statistics.Where(kvp => kvp.Value != 0).ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
             MaximumStatistics = score.MaximumStatistics.Where(kvp => kvp.Value != 0).ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
         };
-
-        public long OnlineID => (long?)ID ?? -1;
     }
 }

osu.Game/Online/Rooms/MultiplayerScore.cs

@@ -58,6 +58,9 @@ namespace osu.Game.Online.Rooms
         [JsonProperty("position")]
         public int? Position { get; set; }
 
+        [JsonProperty("has_replay")]
+        public bool HasReplay { get; set; }
+
         /// <summary>
         /// Any scores in the room around this score.
         /// </summary>
@@ -84,7 +87,7 @@ namespace osu.Game.Online.Rooms
                 User = User,
                 Accuracy = Accuracy,
                 Date = EndedAt,
-                Hash = string.Empty, // todo: temporary?
+                HasOnlineReplay = HasReplay,
                 Rank = Rank,
                 Mods = Mods?.Select(m => m.ToMod(rulesetInstance)).ToArray() ?? Array.Empty<Mod>(),
                 Position = Position,

osu.Game/Online/ScoreDownloadTracker.cs

@@ -39,7 +39,8 @@ namespace osu.Game.Online
             var scoreInfo = new ScoreInfo
             {
                 ID = TrackedItem.ID,
-                OnlineID = TrackedItem.OnlineID
+                OnlineID = TrackedItem.OnlineID,
+                LegacyOnlineID = TrackedItem.LegacyOnlineID
             };
 
             Downloader.DownloadBegan += downloadBegan;
@@ -47,6 +48,7 @@ namespace osu.Game.Online
 
             realmSubscription = realm.RegisterForNotifications(r => r.All<ScoreInfo>().Where(s =>
                 ((s.OnlineID > 0 && s.OnlineID == TrackedItem.OnlineID)
+                 || (s.LegacyOnlineID > 0 && s.LegacyOnlineID == TrackedItem.LegacyOnlineID)
                  || (!string.IsNullOrEmpty(s.Hash) && s.Hash == TrackedItem.Hash))
                 && !s.DeletePending), (items, _) =>
             {

osu.Game/OsuGame.cs

@@ -678,6 +678,9 @@ namespace osu.Game
             if (score.OnlineID > 0)
                 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);
 

osu.Game/Scoring/IScoreInfo.cs

@@ -9,7 +9,7 @@ using osu.Game.Users;
 
 namespace osu.Game.Scoring
 {
-    public interface IScoreInfo : IHasOnlineID<long>, IHasNamedFiles
+    public interface IScoreInfo : IHasOnlineID<long>
     {
         IUser User { get; }
 
@@ -22,7 +22,7 @@ namespace osu.Game.Scoring
 
         double Accuracy { get; }
 
-        bool HasReplay { get; }
+        long LegacyOnlineID { get; }
 
         DateTimeOffset Date { get; }
 

osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs

@@ -19,6 +19,13 @@ namespace osu.Game.Scoring.Legacy
     [JsonObject(MemberSerialization.OptIn)]
     public class LegacyReplaySoloScoreInfo
     {
+        /// <remarks>
+        /// The value of this property should correspond to <see cref="ScoreInfo.OnlineID"/>
+        /// (i.e. come from the `solo_scores` ID scheme).
+        /// </remarks>
+        [JsonProperty("online_id")]
+        public long OnlineID { get; set; } = -1;
+
         [JsonProperty("mods")]
         public APIMod[] Mods { get; set; } = Array.Empty<APIMod>();
 
@@ -30,6 +37,7 @@ namespace osu.Game.Scoring.Legacy
 
         public static LegacyReplaySoloScoreInfo FromScore(ScoreInfo score) => new LegacyReplaySoloScoreInfo
         {
+            OnlineID = score.OnlineID,
             Mods = score.APIMods,
             Statistics = score.Statistics.Where(kvp => kvp.Value != 0).ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
             MaximumStatistics = score.MaximumStatistics.Where(kvp => kvp.Value != 0).ToDictionary(kvp => kvp.Key, kvp => kvp.Value),

osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs

@@ -101,9 +101,9 @@ namespace osu.Game.Scoring.Legacy
                 byte[] compressedReplay = sr.ReadByteArray();
 
                 if (version >= 20140721)
-                    scoreInfo.OnlineID = sr.ReadInt64();
+                    scoreInfo.LegacyOnlineID = sr.ReadInt64();
                 else if (version >= 20121008)
-                    scoreInfo.OnlineID = sr.ReadInt32();
+                    scoreInfo.LegacyOnlineID = sr.ReadInt32();
 
                 byte[] compressedScoreInfo = null;
 
@@ -121,6 +121,7 @@ namespace osu.Game.Scoring.Legacy
 
                         Debug.Assert(readScore != null);
 
+                        score.ScoreInfo.OnlineID = readScore.OnlineID;
                         score.ScoreInfo.Statistics = readScore.Statistics;
                         score.ScoreInfo.MaximumStatistics = readScore.MaximumStatistics;
                         score.ScoreInfo.Mods = readScore.Mods.Select(m => m.ToMod(currentRuleset)).ToArray();

osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs

@@ -84,7 +84,7 @@ namespace osu.Game.Scoring.Legacy
                 sw.Write(getHpGraphFormatted());
                 sw.Write(score.ScoreInfo.Date.DateTime);
                 sw.WriteByteArray(createReplayData());
-                sw.Write((long)0);
+                sw.Write(score.ScoreInfo.LegacyOnlineID);
                 writeModSpecificData(score.ScoreInfo, sw);
                 sw.WriteByteArray(createScoreInfoData());
             }

osu.Game/Scoring/ScoreInfo.cs

@@ -94,15 +94,32 @@ namespace osu.Game.Scoring
 
         public double Accuracy { get; set; }
 
-        public bool HasReplay => !string.IsNullOrEmpty(Hash);
+        [Ignored]
+        public bool HasOnlineReplay { get; set; }
 
         public DateTimeOffset Date { get; set; }
 
         public double? PP { get; set; }
 
+        /// <summary>
+        /// The online ID of this score.
+        /// </summary>
+        /// <remarks>
+        /// In the osu-web database, this ID (if present) comes from the new <c>solo_scores</c> table.
+        /// </remarks>
         [Indexed]
         public long OnlineID { get; set; } = -1;
 
+        /// <summary>
+        /// The legacy online ID of this score.
+        /// </summary>
+        /// <remarks>
+        /// In the osu-web database, this ID (if present) comes from the legacy <c>osu_scores_*_high</c> tables.
+        /// This ID is also stored to replays set on osu!stable.
+        /// </remarks>
+        [Indexed]
+        public long LegacyOnlineID { get; set; } = -1;
+
         [MapTo("User")]
         public RealmUser RealmUser { get; set; } = null!;
 
@@ -168,7 +185,6 @@ namespace osu.Game.Scoring
         IRulesetInfo IScoreInfo.Ruleset => Ruleset;
         IBeatmapInfo? IScoreInfo.Beatmap => BeatmapInfo;
         IUser IScoreInfo.User => User;
-        IEnumerable<INamedFileUsage> IHasNamedFiles.Files => Files;
 
         #region Properties required to make things work with existing usages
 

osu.Game/Scoring/ScoreManager.cs

@@ -150,7 +150,11 @@ namespace osu.Game.Scoring
 
         public Task Import(ImportTask[] imports, ImportParameters parameters = default) => scoreImporter.Import(imports, parameters);
 
-        public override bool IsAvailableLocally(ScoreInfo model) => Realm.Run(realm => realm.All<ScoreInfo>().Any(s => s.OnlineID == model.OnlineID));
+        public override bool IsAvailableLocally(ScoreInfo model)
+            => Realm.Run(realm => realm.All<ScoreInfo>()
+                                       // this basically inlines `ModelExtension.MatchesOnlineID(IScoreInfo, IScoreInfo)`,
+                                       // because that method can't be used here, as realm can't translate it to its query language.
+                                       .Any(s => s.OnlineID == model.OnlineID || s.LegacyOnlineID == model.LegacyOnlineID));
 
         public IEnumerable<string> HandledExtensions => scoreImporter.HandledExtensions;
 

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerResultsScreen.cs

@@ -10,7 +10,7 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer
     public partial class MultiplayerResultsScreen : PlaylistsResultsScreen
     {
         public MultiplayerResultsScreen(ScoreInfo score, long roomId, PlaylistItem playlistItem)
-            : base(score, roomId, playlistItem, false, false)
+            : base(score, roomId, playlistItem, false)
         {
         }
     }

osu.Game/Screens/Play/Player.cs

@@ -1167,13 +1167,6 @@ namespace osu.Game.Screens.Play
             // the import process will re-attach managed beatmap/rulesets to this score. we don't want this for now, so create a temporary copy to import.
             var importableScore = score.ScoreInfo.DeepClone();
 
-            // For the time being, online ID responses are not really useful for anything.
-            // In addition, the IDs provided via new (lazer) endpoints are based on a different autoincrement from legacy (stable) scores.
-            //
-            // Until we better define the server-side logic behind this, let's not store the online ID to avoid potential unique constraint
-            // conflicts across various systems (ie. solo and multiplayer).
-            importableScore.OnlineID = -1;
-
             var imported = scoreManager.Import(importableScore, replayReader);
 
             imported.PerformRead(s =>

osu.Game/Screens/Ranking/ReplayDownloadButton.cs

@@ -37,7 +37,7 @@ namespace osu.Game.Screens.Ranking
                 if (State.Value == DownloadState.LocallyAvailable)
                     return ReplayAvailability.Local;
 
-                if (Score.Value?.HasReplay == true)
+                if (Score.Value?.HasOnlineReplay == true)
                     return ReplayAvailability.Online;
 
                 return ReplayAvailability.NotAvailable;

osu.Game/Screens/Ranking/SoloResultsScreen.cs

@@ -7,6 +7,7 @@ using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Game.Beatmaps;
+using osu.Game.Extensions;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests;
 using osu.Game.Online.Solo;
@@ -67,7 +68,7 @@ namespace osu.Game.Screens.Ranking
                 return null;
 
             getScoreRequest = new GetScoresRequest(Score.BeatmapInfo, Score.Ruleset);
-            getScoreRequest.Success += r => scoresCallback?.Invoke(r.Scores.Where(s => s.OnlineID != Score.OnlineID).Select(s => s.ToScoreInfo(rulesets, Beatmap.Value.BeatmapInfo)));
+            getScoreRequest.Success += r => scoresCallback?.Invoke(r.Scores.Where(s => !s.MatchesOnlineID(Score)).Select(s => s.ToScoreInfo(rulesets, Beatmap.Value.BeatmapInfo)));
             return getScoreRequest;
         }
 

osu.Game/Skinning/SkinInfo.cs

@@ -14,7 +14,7 @@ namespace osu.Game.Skinning
 {
     [MapTo("Skin")]
     [JsonObject(MemberSerialization.OptIn)]
-    public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable<SkinInfo>, IHasGuidPrimaryKey, ISoftDelete, IHasNamedFiles
+    public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable<SkinInfo>, IHasGuidPrimaryKey, ISoftDelete
     {
         internal static readonly Guid TRIANGLES_SKIN = new Guid("2991CFD8-2140-469A-BCB9-2EC23FBCE4AD");
         internal static readonly Guid ARGON_SKIN = new Guid("CFFA69DE-B3E3-4DEE-8563-3C4F425C05D0");