From 4d9ccdc3b2391463355d297387c12181f72a6fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 3 May 2024 13:23:41 +0200 Subject: [PATCH 1/6] Encode user ID to replays --- osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs index 32c18b3af2..b71d9d916e 100644 --- a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs +++ b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs @@ -43,6 +43,9 @@ namespace osu.Game.Scoring.Legacy [JsonConverter(typeof(StringEnumConverter))] public ScoreRank? Rank; + [JsonProperty("user_id")] + public int UserID = -1; + public static LegacyReplaySoloScoreInfo FromScore(ScoreInfo score) => new LegacyReplaySoloScoreInfo { OnlineID = score.OnlineID, @@ -51,6 +54,7 @@ namespace osu.Game.Scoring.Legacy MaximumStatistics = score.MaximumStatistics.Where(kvp => kvp.Value != 0).ToDictionary(), ClientVersion = score.ClientVersion, Rank = score.Rank, + UserID = score.UserID, }; } } From 554ead0d9dd596a01bf55ab36c72f8eb346f81bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 3 May 2024 13:34:02 +0200 Subject: [PATCH 2/6] Decode user ID from score if available --- .../Beatmaps/Formats/LegacyScoreDecoderTest.cs | 9 +++++++++ osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 2 ++ 2 files changed, 11 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 383c08c10f..cc7b37e6a8 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -14,6 +14,7 @@ using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.Legacy; using osu.Game.IO.Legacy; +using osu.Game.Models; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; @@ -31,6 +32,7 @@ using osu.Game.Rulesets.Taiko; using osu.Game.Scoring; using osu.Game.Scoring.Legacy; using osu.Game.Tests.Resources; +using osu.Game.Users; namespace osu.Game.Tests.Beatmaps.Formats { @@ -224,6 +226,12 @@ namespace osu.Game.Tests.Beatmaps.Formats new OsuModDoubleTime { SpeedChange = { Value = 1.1 } } }; scoreInfo.OnlineID = 123123; + scoreInfo.RealmUser = new RealmUser + { + Username = "spaceman_atlas", + OnlineID = 3035836, + CountryCode = CountryCode.PL + }; scoreInfo.ClientVersion = "2023.1221.0"; var beatmap = new TestBeatmap(ruleset); @@ -248,6 +256,7 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.That(decodedAfterEncode.ScoreInfo.MaximumStatistics, Is.EqualTo(scoreInfo.MaximumStatistics)); Assert.That(decodedAfterEncode.ScoreInfo.Mods, Is.EqualTo(scoreInfo.Mods)); Assert.That(decodedAfterEncode.ScoreInfo.ClientVersion, Is.EqualTo("2023.1221.0")); + Assert.That(decodedAfterEncode.ScoreInfo.RealmUser.OnlineID, Is.EqualTo(3035836)); }); } diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index b0d7087ed1..00e294fdcd 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -131,6 +131,8 @@ namespace osu.Game.Scoring.Legacy score.ScoreInfo.Mods = readScore.Mods.Select(m => m.ToMod(currentRuleset)).ToArray(); score.ScoreInfo.ClientVersion = readScore.ClientVersion; decodedRank = readScore.Rank; + if (readScore.UserID > 1) + score.ScoreInfo.RealmUser.OnlineID = readScore.UserID; }); } } From bd869b6cdcd41c3406022fb784f54fe2fdebf849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 13:24:24 +0200 Subject: [PATCH 3/6] Add failing tests for looking up users by online ID if present when importing scores --- osu.Game.Tests/Scores/IO/ImportScoreTest.cs | 82 ++++++++++++++++++++- 1 file changed, 79 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Scores/IO/ImportScoreTest.cs b/osu.Game.Tests/Scores/IO/ImportScoreTest.cs index eb2c098ab8..9c72804a6b 100644 --- a/osu.Game.Tests/Scores/IO/ImportScoreTest.cs +++ b/osu.Game.Tests/Scores/IO/ImportScoreTest.cs @@ -287,7 +287,7 @@ namespace osu.Game.Tests.Scores.IO } [Test] - public void TestUserLookedUpForOnlineScore() + public void TestUserLookedUpByUsernameForOnlineScoreIfUserIDMissing() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) { @@ -301,6 +301,9 @@ namespace osu.Game.Tests.Scores.IO switch (req) { case GetUserRequest userRequest: + if (userRequest.Lookup != "Test user") + return false; + userRequest.TriggerSuccess(new APIUser { Username = "Test user", @@ -350,7 +353,7 @@ namespace osu.Game.Tests.Scores.IO } [Test] - public void TestUserLookedUpForLegacyOnlineScore() + public void TestUserLookedUpByUsernameForLegacyOnlineScore() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) { @@ -364,6 +367,9 @@ namespace osu.Game.Tests.Scores.IO switch (req) { case GetUserRequest userRequest: + if (userRequest.Lookup != "Test user") + return false; + userRequest.TriggerSuccess(new APIUser { Username = "Test user", @@ -413,7 +419,7 @@ namespace osu.Game.Tests.Scores.IO } [Test] - public void TestUserNotLookedUpForOfflineScore() + public void TestUserNotLookedUpForOfflineScoreIfUserIDMissing() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) { @@ -427,6 +433,9 @@ namespace osu.Game.Tests.Scores.IO switch (req) { case GetUserRequest userRequest: + if (userRequest.Lookup != "Test user") + return false; + userRequest.TriggerSuccess(new APIUser { Username = "Test user", @@ -476,6 +485,73 @@ namespace osu.Game.Tests.Scores.IO } } + [Test] + public void TestUserLookedUpByOnlineIDIfPresent([Values] bool isOnlineScore) + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) + { + try + { + var osu = LoadOsuIntoHost(host, true); + + var api = (DummyAPIAccess)osu.API; + api.HandleRequest = req => + { + switch (req) + { + case GetUserRequest userRequest: + if (userRequest.Lookup != "5555") + return false; + + userRequest.TriggerSuccess(new APIUser + { + Username = "Some other guy", + CountryCode = CountryCode.DE, + Id = 5555 + }); + return true; + + default: + return false; + } + }; + + var beatmap = BeatmapImportHelper.LoadOszIntoOsu(osu, TestResources.GetQuickTestBeatmapForImport()).GetResultSafely(); + + var toImport = new ScoreInfo + { + Rank = ScoreRank.B, + TotalScore = 987654, + Accuracy = 0.8, + MaxCombo = 500, + Combo = 250, + User = new APIUser { Id = 5555 }, + Date = DateTimeOffset.Now, + Ruleset = new OsuRuleset().RulesetInfo, + BeatmapInfo = beatmap.Beatmaps.First() + }; + if (isOnlineScore) + toImport.OnlineID = 12345; + + var imported = LoadScoreIntoOsu(osu, toImport); + + Assert.AreEqual(toImport.Rank, imported.Rank); + Assert.AreEqual(toImport.TotalScore, imported.TotalScore); + Assert.AreEqual(toImport.Accuracy, imported.Accuracy); + Assert.AreEqual(toImport.MaxCombo, imported.MaxCombo); + Assert.AreEqual(toImport.Date, imported.Date); + Assert.AreEqual(toImport.OnlineID, imported.OnlineID); + Assert.AreEqual("Some other guy", imported.RealmUser.Username); + Assert.AreEqual(5555, imported.RealmUser.OnlineID); + Assert.AreEqual(CountryCode.DE, imported.RealmUser.CountryCode); + } + finally + { + host.Exit(); + } + } + } + public static ScoreInfo LoadScoreIntoOsu(OsuGameBase osu, ScoreInfo score, ArchiveReader archive = null) { // clone to avoid attaching the input score to realm. From abfb2c00bcaaf071b264f82e5205350ec7a5edf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 13:24:34 +0200 Subject: [PATCH 4/6] Look up users by ID if available when importing scores --- osu.Game/Scoring/ScoreImporter.cs | 71 ++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index 4ae8e51f6d..69c53af16f 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -103,6 +103,14 @@ namespace osu.Game.Scoring } // Very naive local caching to improve performance of large score imports (where the username is usually the same for most or all scores). + + // TODO: `UserLookupCache` cannot currently be used here because of async foibles. + // It only supports lookups by user ID (username would require web changes), and even then the ID lookups cannot be used. + // That is because that component provides an async interface, and async functions cannot be consumed safely here due to the rigid structure of `RealmArchiveModelImporter`. + // The importer has two paths, one async and one sync; the async path runs the sync path in a task. + // This means that sometimes `PostImport()` is called from a sync context, and sometimes from an async one, whilst itself being a sync method. + // That in turn makes `.GetResultSafely()` not callable inside `PostImport()`, as it will throw when called from an async context, + private readonly Dictionary idLookupCache = new Dictionary(); private readonly Dictionary usernameLookupCache = new Dictionary(); protected override void PostImport(ScoreInfo model, Realm realm, ImportParameters parameters) @@ -127,24 +135,34 @@ namespace osu.Game.Scoring if (model.RealmUser.OnlineID == APIUser.SYSTEM_USER_ID) return; - if (model.OnlineID < 0 && model.LegacyOnlineID <= 0) - return; - - string username = model.RealmUser.Username; - - if (usernameLookupCache.TryGetValue(username, out var existing)) + if (model.RealmUser.OnlineID > 1) { - model.User = existing; + model.User = lookupUserById(model.RealmUser.OnlineID) ?? model.User; return; } - var userRequest = new GetUserRequest(username); + if (model.OnlineID < 0 && model.LegacyOnlineID <= 0) + return; + + model.User = lookupUserByName(model.RealmUser.Username) ?? model.User; + } + + private APIUser? lookupUserById(int id) + { + if (idLookupCache.TryGetValue(id, out var existing)) + { + return existing; + } + + var userRequest = new GetUserRequest(id); api.Perform(userRequest); if (userRequest.Response is APIUser user) { - usernameLookupCache.TryAdd(username, new APIUser + APIUser cachedUser; + + idLookupCache.TryAdd(id, cachedUser = new APIUser { // Because this is a permanent cache, let's only store the pieces we're interested in, // rather than the full API response. If we start to store more than these three fields @@ -154,8 +172,41 @@ namespace osu.Game.Scoring CountryCode = user.CountryCode, }); - model.User = user; + return cachedUser; } + + return null; + } + + private APIUser? lookupUserByName(string username) + { + if (usernameLookupCache.TryGetValue(username, out var existing)) + { + return existing; + } + + var userRequest = new GetUserRequest(username); + + api.Perform(userRequest); + + if (userRequest.Response is APIUser user) + { + APIUser cachedUser; + + usernameLookupCache.TryAdd(username, cachedUser = new APIUser + { + // Because this is a permanent cache, let's only store the pieces we're interested in, + // rather than the full API response. If we start to store more than these three fields + // in realm, this should be undone. + Id = user.Id, + Username = user.Username, + CountryCode = user.CountryCode, + }); + + return cachedUser; + } + + return null; } } } From 9c6968c13a2c6a1a0392649dddd1c0cfb7828f1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 6 May 2024 16:29:03 +0200 Subject: [PATCH 5/6] Use `score.User.OnlineID` instead of `score.UserID` You'd hope that they'd be the same thing, but post-https://github.com/ppy/osu-server-spectator/pull/230 it turns out that cannot be guaranteed, so just attempt to use `User` in the encoder consistently everywhere... --- osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs index b71d9d916e..60bec687f4 100644 --- a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs +++ b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs @@ -54,7 +54,7 @@ namespace osu.Game.Scoring.Legacy MaximumStatistics = score.MaximumStatistics.Where(kvp => kvp.Value != 0).ToDictionary(), ClientVersion = score.ClientVersion, Rank = score.Rank, - UserID = score.UserID, + UserID = score.User.OnlineID, }; } } From 7551cf01d18a4e924ae9720417fa1797e8b38db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 8 May 2024 09:45:52 +0200 Subject: [PATCH 6/6] Fix test failure --- osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index cc7b37e6a8..8ca141bb4f 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -14,7 +14,7 @@ using osu.Game.Beatmaps; using osu.Game.Beatmaps.Formats; using osu.Game.Beatmaps.Legacy; using osu.Game.IO.Legacy; -using osu.Game.Models; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Replays; using osu.Game.Rulesets; using osu.Game.Rulesets.Catch; @@ -226,10 +226,10 @@ namespace osu.Game.Tests.Beatmaps.Formats new OsuModDoubleTime { SpeedChange = { Value = 1.1 } } }; scoreInfo.OnlineID = 123123; - scoreInfo.RealmUser = new RealmUser + scoreInfo.User = new APIUser { Username = "spaceman_atlas", - OnlineID = 3035836, + Id = 3035836, CountryCode = CountryCode.PL }; scoreInfo.ClientVersion = "2023.1221.0";