diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 383c08c10f..8ca141bb4f 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.Online.API.Requests.Responses; 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.User = new APIUser + { + Username = "spaceman_atlas", + Id = 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.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. diff --git a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs index 32c18b3af2..60bec687f4 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.User.OnlineID, }; } } 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; }); } } 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; } } }