From c1a817fec607aa2577fab9ebf7223b352020d33e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 1 Sep 2023 07:47:07 +0200 Subject: [PATCH 01/27] Add `LegacyOnlineID` with backwards migration --- osu.Game/Database/RealmAccess.cs | 21 ++++++++++++++++++++- osu.Game/Scoring/ScoreInfo.cs | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index cd97bb6430..12d1753b9b 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -84,8 +84,9 @@ namespace osu.Game.Database /// 32 2023-07-09 Populate legacy scores with the ScoreV2 mod (and restore TotalScore to the legacy total for such scores) using replay files. /// 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-09-01 Add LegacyOnlineID to ScoreInfo. Move osu_scores_*_high IDs stored in OnlineID to LegacyOnlineID. Reset anomalous OnlineIDs. /// - private const int schema_version = 34; + private const int schema_version = 35; /// /// Lock object which is held during sections, blocking realm retrieval during blocking periods. @@ -1031,6 +1032,24 @@ namespace osu.Game.Database break; } + + case 35: + { + foreach (var score in migration.NewRealm.All()) + { + 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"); diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 8531408555..992271d072 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -100,9 +100,25 @@ namespace osu.Game.Scoring public double? PP { get; set; } + /// + /// The online ID of this score. + /// + /// + /// In the osu-web database, this ID (if present) comes from the new solo_scores table. + /// [Indexed] public long OnlineID { get; set; } = -1; + /// + /// The legacy online ID of this score. + /// + /// + /// In the osu-web database, this ID (if present) comes from the legacy osu_scores_*_high tables. + /// This ID is also stored to replays set on osu!stable. + /// + [Indexed] + public long LegacyOnlineID { get; set; } = -1; + [MapTo("User")] public RealmUser RealmUser { get; set; } = null!; From bfcb4f4f2d940835eb8e9f42eb620184d4ed877c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 1 Sep 2023 08:04:35 +0200 Subject: [PATCH 02/27] Add failing test for legacy online ID decoding --- .../Beatmaps/Formats/LegacyScoreDecoderTest.cs | 14 ++++++++++++++ .../taiko-replay-with-legacy-online-id.osr | Bin 0 -> 238 bytes 2 files changed, 14 insertions(+) create mode 100644 osu.Game.Tests/Resources/Replays/taiko-replay-with-legacy-online-id.osr diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index 93cda34ef7..c7fd3ba098 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -87,6 +87,20 @@ 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)); + } + } + [TestCase(3, true)] [TestCase(6, false)] [TestCase(LegacyBeatmapDecoder.LATEST_VERSION, false)] diff --git a/osu.Game.Tests/Resources/Replays/taiko-replay-with-legacy-online-id.osr b/osu.Game.Tests/Resources/Replays/taiko-replay-with-legacy-online-id.osr new file mode 100644 index 0000000000000000000000000000000000000000..85ad28c7ca3f9e32146cfff8994cb9fbcf584a6e GIT binary patch literal 238 zcmZSN=rUpCRxmU*PqQ>jN;EJ_HcqxoN;FS4NKLXxNl8jJvox|WwB+W>P0h|uOvx`U zRpeGMNi{Y$H#9XgPc=7Av^2LcO*A!6v@}XiG&3+yPE9gmVE_RpF!@23fdRzlU|`@D zG%_?ct})OvHr6pUG%%?#)JZt7C`od{EsiJ#28LJ$1_cHyh%N>Ou^y+m=K9*~8{Wl< z!Pg>+HZhB?;7QxF-|^+%RL`SD5uY3~Zah$WyZl?W# Date: Fri, 1 Sep 2023 08:10:26 +0200 Subject: [PATCH 03/27] Move legacy online ID encode/decode to legacy property --- osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 4 ++-- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 8c00110909..1441a14aa0 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/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; diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index a95964ac52..872f09dda6 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/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()); } From c53f4c144cc109d0b0f710adf39ea31be12a29fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 1 Sep 2023 08:23:29 +0200 Subject: [PATCH 04/27] Encode/decode new `OnlineID` into/from `LegacyReplaySoloScoreInfo` --- osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs | 8 ++++++++ osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs | 1 + 2 files changed, 9 insertions(+) diff --git a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs index f2e8cf141b..d34edf7bdf 100644 --- a/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs +++ b/osu.Game/Scoring/Legacy/LegacyReplaySoloScoreInfo.cs @@ -19,6 +19,13 @@ namespace osu.Game.Scoring.Legacy [JsonObject(MemberSerialization.OptIn)] public class LegacyReplaySoloScoreInfo { + /// + /// The value of this property should correspond to + /// (i.e. come from the `solo_scores` ID scheme). + /// + [JsonProperty("online_id")] + public long OnlineID { get; set; } = -1; + [JsonProperty("mods")] public APIMod[] Mods { get; set; } = Array.Empty(); @@ -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), diff --git a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs index 1441a14aa0..c5e6e3bcce 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreDecoder.cs @@ -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(); From b144cfd55c1a5546a42223d3e2de44aecc47c56e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 4 Sep 2023 12:21:23 +0200 Subject: [PATCH 05/27] Add `LegacyOnlineID` handling to places that definitely need it Mostly places that can interact with imported replays. There are other places that use the online ID as a sort tiebreaker, or to check presence of a score on results screens, but they should probably still continue to only use `OnlineID`, since all scores with a legacy online ID should have an online ID, but the converse is not generally true. --- osu.Game/Extensions/ModelExtensions.cs | 20 ++++++++++++++++++-- osu.Game/Online/ScoreDownloadTracker.cs | 4 +++- osu.Game/OsuGame.cs | 3 +++ osu.Game/Scoring/IScoreInfo.cs | 2 ++ osu.Game/Scoring/ScoreManager.cs | 6 +++++- 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/osu.Game/Extensions/ModelExtensions.cs b/osu.Game/Extensions/ModelExtensions.cs index efb3c4d633..eef9b63b62 100644 --- a/osu.Game/Extensions/ModelExtensions.cs +++ b/osu.Game/Extensions/ModelExtensions.cs @@ -114,8 +114,24 @@ namespace osu.Game.Extensions /// /// The instance to compare. /// The other instance to compare against. - /// Whether online IDs match. If either instance is missing an online ID, this will return false. - public static bool MatchesOnlineID(this IScoreInfo? instance, IScoreInfo? other) => matchesOnlineID(instance, other); + /// + /// Whether online IDs match. + /// Both and are checked, in that order. + /// If either instance is missing an online ID, this will return false. + /// + 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? instance, IHasOnlineID? other) { diff --git a/osu.Game/Online/ScoreDownloadTracker.cs b/osu.Game/Online/ScoreDownloadTracker.cs index de42292372..dfdac24d19 100644 --- a/osu.Game/Online/ScoreDownloadTracker.cs +++ b/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().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, _) => { diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 885077a8e8..2f11964f6a 100644 --- a/osu.Game/OsuGame.cs +++ b/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); diff --git a/osu.Game/Scoring/IScoreInfo.cs b/osu.Game/Scoring/IScoreInfo.cs index d17558f800..4083d57fa0 100644 --- a/osu.Game/Scoring/IScoreInfo.cs +++ b/osu.Game/Scoring/IScoreInfo.cs @@ -24,6 +24,8 @@ namespace osu.Game.Scoring bool HasReplay { get; } + long LegacyOnlineID { get; } + DateTimeOffset Date { get; } double? PP { get; } diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 31b5bd8365..02d9e0a280 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/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().Any(s => s.OnlineID == model.OnlineID)); + public override bool IsAvailableLocally(ScoreInfo model) + => Realm.Run(realm => realm.All() + // 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 HandledExtensions => scoreImporter.HandledExtensions; From d5a733d244ba5d5915f3b6370c7dfebb383fc600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 7 Sep 2023 12:32:28 +0200 Subject: [PATCH 06/27] Use solo score ID as `OnlineID` when converting from `MultiplayerScore` --- .../Playlists/TestScenePlaylistsResultsScreen.cs | 13 ++++++++++--- osu.Game/Online/Rooms/MultiplayerScore.cs | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index cb422d8c06..3f230099da 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -245,6 +245,7 @@ namespace osu.Game.Tests.Visual.Playlists var multiplayerUserScore = new MultiplayerScore { ID = highestScoreId, + SoloScoreID = (ulong?)highestScoreId, Accuracy = userScore.Accuracy, Passed = userScore.Passed, Rank = userScore.Rank, @@ -262,9 +263,11 @@ namespace osu.Game.Tests.Visual.Playlists for (int i = 1; i <= scores_per_result; i++) { + int nextLowest = getNextLowestScoreId(); multiplayerUserScore.ScoresAround.Lower.Scores.Add(new MultiplayerScore { - ID = getNextLowestScoreId(), + ID = nextLowest, + SoloScoreID = (ulong?)nextLowest, Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -277,9 +280,11 @@ namespace osu.Game.Tests.Visual.Playlists }, }); + int nextHighest = getNextHighestScoreId(); multiplayerUserScore.ScoresAround.Higher.Scores.Add(new MultiplayerScore { - ID = getNextHighestScoreId(), + ID = nextHighest, + SoloScoreID = (ulong?)nextHighest, Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -309,9 +314,11 @@ namespace osu.Game.Tests.Visual.Playlists for (int i = 1; i <= scores_per_result; i++) { + int id = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(); result.Scores.Add(new MultiplayerScore { - ID = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(), + ID = id, + SoloScoreID = (ulong?)id, Accuracy = 1, Passed = true, Rank = ScoreRank.X, diff --git a/osu.Game/Online/Rooms/MultiplayerScore.cs b/osu.Game/Online/Rooms/MultiplayerScore.cs index d5e0c7a970..21aa8df54b 100644 --- a/osu.Game/Online/Rooms/MultiplayerScore.cs +++ b/osu.Game/Online/Rooms/MultiplayerScore.cs @@ -75,7 +75,7 @@ namespace osu.Game.Online.Rooms var scoreInfo = new ScoreInfo { - OnlineID = ID, + OnlineID = (long?)SoloScoreID ?? -1, TotalScore = TotalScore, MaxCombo = MaxCombo, BeatmapInfo = beatmap, From c4af8591a5952fc760ec967b37153178017d7e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 7 Sep 2023 09:39:10 +0200 Subject: [PATCH 07/27] Preserve `OnlineID` when importing scores Previously, for lazer scores, the ID returned from `osu-web` was discarded and replaced with -1, due to the fact that the appropriate structures for unification with stable, as well as unification across solo and multiplayer, were not in place yet. Now we're at the point where scores from all the aforementioned sources receive a `solo_scores` DB row, and as such, we can start treating `solo_scores`-scheme IDs as canonical "online IDs" for a score. --- osu.Game/Screens/Play/Player.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 97bfa35d49..845615742d 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1166,13 +1166,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 => From fb2293821ab009304c11be3d4d352ee8aa92debe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 7 Sep 2023 11:26:51 +0200 Subject: [PATCH 08/27] Allow watching replay from multi/playlist results screens --- osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs | 2 +- osu.Game/Online/Rooms/MultiplayerScore.cs | 5 ++++- osu.Game/Scoring/ScoreInfo.cs | 5 ++++- .../OnlinePlay/Multiplayer/MultiplayerResultsScreen.cs | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs b/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs index 15f4bace96..783522220b 100644 --- a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs +++ b/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs @@ -189,7 +189,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, }; diff --git a/osu.Game/Online/Rooms/MultiplayerScore.cs b/osu.Game/Online/Rooms/MultiplayerScore.cs index 21aa8df54b..e6307ab7f0 100644 --- a/osu.Game/Online/Rooms/MultiplayerScore.cs +++ b/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; } + /// /// Any scores in the room around this score. /// @@ -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(), Position = Position, diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 992271d072..6b03e876c4 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -94,7 +94,10 @@ namespace osu.Game.Scoring public double Accuracy { get; set; } - public bool HasReplay => !string.IsNullOrEmpty(Hash); + public bool HasReplay => !string.IsNullOrEmpty(Hash) || HasOnlineReplay; + + [Ignored] + public bool HasOnlineReplay { get; set; } public DateTimeOffset Date { get; set; } diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerResultsScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerResultsScreen.cs index 9708a94cd7..f665ed2d41 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerResultsScreen.cs +++ b/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) { } } From e2192806e456a90f252f7265fff941b1cabf4f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 16 Oct 2023 12:00:33 +0200 Subject: [PATCH 09/27] Use `ID` rather than `SoloScoreID` The latter is apparently not going to be a thing anymore. --- .../Visual/Playlists/TestScenePlaylistsResultsScreen.cs | 4 ---- osu.Game/Online/Rooms/MultiplayerScore.cs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index 3f230099da..4d82631fc9 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -245,7 +245,6 @@ namespace osu.Game.Tests.Visual.Playlists var multiplayerUserScore = new MultiplayerScore { ID = highestScoreId, - SoloScoreID = (ulong?)highestScoreId, Accuracy = userScore.Accuracy, Passed = userScore.Passed, Rank = userScore.Rank, @@ -267,7 +266,6 @@ namespace osu.Game.Tests.Visual.Playlists multiplayerUserScore.ScoresAround.Lower.Scores.Add(new MultiplayerScore { ID = nextLowest, - SoloScoreID = (ulong?)nextLowest, Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -284,7 +282,6 @@ namespace osu.Game.Tests.Visual.Playlists multiplayerUserScore.ScoresAround.Higher.Scores.Add(new MultiplayerScore { ID = nextHighest, - SoloScoreID = (ulong?)nextHighest, Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -318,7 +315,6 @@ namespace osu.Game.Tests.Visual.Playlists result.Scores.Add(new MultiplayerScore { ID = id, - SoloScoreID = (ulong?)id, Accuracy = 1, Passed = true, Rank = ScoreRank.X, diff --git a/osu.Game/Online/Rooms/MultiplayerScore.cs b/osu.Game/Online/Rooms/MultiplayerScore.cs index e6307ab7f0..f1b9584d57 100644 --- a/osu.Game/Online/Rooms/MultiplayerScore.cs +++ b/osu.Game/Online/Rooms/MultiplayerScore.cs @@ -78,7 +78,7 @@ namespace osu.Game.Online.Rooms var scoreInfo = new ScoreInfo { - OnlineID = (long?)SoloScoreID ?? -1, + OnlineID = ID, TotalScore = TotalScore, MaxCombo = MaxCombo, BeatmapInfo = beatmap, From 2ba02416c22b9bcd0283b79e28dcb65a41f414e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 09:14:59 +0200 Subject: [PATCH 10/27] Revert test changes fully to clean up diff --- .../Visual/Playlists/TestScenePlaylistsResultsScreen.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index 4d82631fc9..cb422d8c06 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -262,10 +262,9 @@ namespace osu.Game.Tests.Visual.Playlists for (int i = 1; i <= scores_per_result; i++) { - int nextLowest = getNextLowestScoreId(); multiplayerUserScore.ScoresAround.Lower.Scores.Add(new MultiplayerScore { - ID = nextLowest, + ID = getNextLowestScoreId(), Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -278,10 +277,9 @@ namespace osu.Game.Tests.Visual.Playlists }, }); - int nextHighest = getNextHighestScoreId(); multiplayerUserScore.ScoresAround.Higher.Scores.Add(new MultiplayerScore { - ID = nextHighest, + ID = getNextHighestScoreId(), Accuracy = userScore.Accuracy, Passed = true, Rank = userScore.Rank, @@ -311,10 +309,9 @@ namespace osu.Game.Tests.Visual.Playlists for (int i = 1; i <= scores_per_result; i++) { - int id = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(); result.Scores.Add(new MultiplayerScore { - ID = id, + ID = sort == "score_asc" ? getNextHighestScoreId() : getNextLowestScoreId(), Accuracy = 1, Passed = true, Rank = ScoreRank.X, From dbb69419e6d49e6661d31eb22f2445e499e5e742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 12:39:07 +0200 Subject: [PATCH 11/27] Add test coverage for parsing new online ID --- .../Beatmaps/Formats/LegacyScoreDecoderTest.cs | 14 ++++++++++++++ .../Replays/taiko-replay-with-new-online-id.osr | Bin 0 -> 1531 bytes 2 files changed, 14 insertions(+) create mode 100644 osu.Game.Tests/Resources/Replays/taiko-replay-with-new-online-id.osr diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs index c7fd3ba098..ab88be1511 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyScoreDecoderTest.cs @@ -101,6 +101,20 @@ namespace osu.Game.Tests.Beatmaps.Formats } } + [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)] diff --git a/osu.Game.Tests/Resources/Replays/taiko-replay-with-new-online-id.osr b/osu.Game.Tests/Resources/Replays/taiko-replay-with-new-online-id.osr new file mode 100644 index 0000000000000000000000000000000000000000..63e05f5fcdbda51e8f7c3322def87f4fc74f8da4 GIT binary patch literal 1531 zcmVvTrHZVD6F=Go4b8ul} zWo=<@Utx4?VRJGIAU8K+FfnCiFg9f~Ic8-uIb%6xWnyJxW-~W5FgY}2Hc0>o00001 z0000000e#v08sz|000003jk|tWgGt0+X(9f003P803ZOTO#lD@000007K}rXlJKRA z7M`&HOhZfu53FrP?1?N(Ce<_OIe53onYYKWQKjB!yy__GNTqRxRs4t|7d^=w?^a2; z=;p=RN*NtHSfdZ7KM{`CJ^MjsRn>7amAkGdpDK+x=PDo#tk4aBL?k%f)$Ebh0t%Nn zphxu;)QK_xDZ_`29#Slz$-BsTX6oS*tEpL9Ev7g9I5ScQ++9#K`PS+Bg7YLtZ zZhi6@^BMY%^-@>8#b8diGT>rK8W$eYWM@f+i3@MI*x2jwMoDv7i9dcH+Blva=xLAQ zQ+tf$l}n5c>1hLZYL!1qq`!()(I1Bi(teRO5k6sX_aTq*mVFs>aNFfP7QeOX;b~w1IAj)Te-(%eMdm^#T`gS4`h= z`(YRBu5x52OkrvAE;$$5Ka1rR+V@w)I!3+SveB0rxD~l$s=FEk0DPJXI01( zX<*T#vp20$0MeFEDmFU3$xU;pcASo(q2) zN#UlWQX+|aTArDrD&&we-Q#@5I>P@U#aZ7E_WVu}QZ z{u?U)bPE60zy8;&*@>H#nKF^riT4w%Ra_eUgCD(pFv%H+A(z{%SB~Z zD*JNyOplwu9x#_rabI#%4~0{9EZmm)IZ%cF!fh>HiAX@d}Izf*u8yw}?8+ zNnZ9YLWQcYNC*QlO6tVSziS_G!(qvFc@44`+vV{%9koFdUqvujoNp;)_jO1RN`{^0 zg=z?MuejP`10NKa5$Cxp-Se8FTJyl!tovh4wVKlspX4~-i8~TINGZ^#);*P#i)SE8 zzW1JsdM+A;?~>4&l@303`MAkXiv{_WIWc4$HYYGgMxnMFNGc{X2NX~R?2b@ZRs9cq z$!to>PZGzdmXRl&AW$UR-r`TGqkjmloZEFshw0Jkawz&1Ok4MmYz+~Jhh=obE&;f{ z+ZS(WELeZ_C2m3BkbxZ&DAUVh89w}bvS8+#qHjecQMK`PKw!@~opfX|DAuPffrNhq zDizxF1oWO4uo1ML<2%wPOFapwVrJdCB~~#Tt?D*NdN!;|NsC0|Ahbm z09^n8AOPI}000000000zf`AtsFAk^B)WLFNAyG7WV$N8-!j+hPu8$we_e;|#*+iNK z$kRN=q&Sl0(d`|*oEU1|*~5eG@dH3Miy0i*F=j=X+_a<6T3%2U@VpmXv8`-JhNX1! hacf^hACxf}U(1HP^l0fR;XoK|ePzq5Xz7uby&!FJ#7qDH literal 0 HcmV?d00001 From 526ee6e14070a2ba0d09ad544353be52796e698c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 14:46:24 +0200 Subject: [PATCH 12/27] Remove `IScoreInfo : IHasNamedFiles` inheritance --- osu.Game/Database/IHasRealmFiles.cs | 6 ++++-- osu.Game/Scoring/IScoreInfo.cs | 2 +- osu.Game/Scoring/ScoreInfo.cs | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/IHasRealmFiles.cs b/osu.Game/Database/IHasRealmFiles.cs index 79ea719583..b301bb04de 100644 --- a/osu.Game/Database/IHasRealmFiles.cs +++ b/osu.Game/Database/IHasRealmFiles.cs @@ -10,13 +10,15 @@ namespace osu.Game.Database /// /// A model that contains a list of files it is responsible for. /// - public interface IHasRealmFiles + public interface IHasRealmFiles : IHasNamedFiles { /// /// Available files in this model, with locally filenames. /// When performing lookups, consider using or to do case-insensitive lookups. /// - IList Files { get; } + new IList Files { get; } + + IEnumerable IHasNamedFiles.Files => Files; /// /// A combined hash representing the model, based on the files it contains. diff --git a/osu.Game/Scoring/IScoreInfo.cs b/osu.Game/Scoring/IScoreInfo.cs index 4083d57fa0..cde48c3be3 100644 --- a/osu.Game/Scoring/IScoreInfo.cs +++ b/osu.Game/Scoring/IScoreInfo.cs @@ -9,7 +9,7 @@ using osu.Game.Users; namespace osu.Game.Scoring { - public interface IScoreInfo : IHasOnlineID, IHasNamedFiles + public interface IScoreInfo : IHasOnlineID { IUser User { get; } diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 6b03e876c4..722d83cac8 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -187,7 +187,6 @@ namespace osu.Game.Scoring IRulesetInfo IScoreInfo.Ruleset => Ruleset; IBeatmapInfo? IScoreInfo.Beatmap => BeatmapInfo; IUser IScoreInfo.User => User; - IEnumerable IHasNamedFiles.Files => Files; #region Properties required to make things work with existing usages From 900530080ff7f4c33a19b6107214c80ff4f0f997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 14:56:22 +0200 Subject: [PATCH 13/27] Make `SoloScoreInfo` implement `IScoreInfo` --- .../API/Requests/Responses/SoloScoreInfo.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs b/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs index 783522220b..0e31f11dc1 100644 --- a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs +++ b/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 + 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}"; /// @@ -223,7 +235,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; } } From c3e9f5184f504f119867c3745af730c6fafe02de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 15:09:34 +0200 Subject: [PATCH 14/27] Fix `SoloScoreInfo` not copying over legacy score ID when converting to `ScoreInfo` --- osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs b/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs index 0e31f11dc1..ac2d8152b1 100644 --- a/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs +++ b/osu.Game/Online/API/Requests/Responses/SoloScoreInfo.cs @@ -190,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 }, From cbb2a0dd70ddce70e2e156be7bac632bbc9b6480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 15:09:59 +0200 Subject: [PATCH 15/27] Use both score ID types to deduplicate score on solo results screen --- osu.Game/Screens/Ranking/SoloResultsScreen.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Ranking/SoloResultsScreen.cs b/osu.Game/Screens/Ranking/SoloResultsScreen.cs index f187b8a302..da08a26a58 100644 --- a/osu.Game/Screens/Ranking/SoloResultsScreen.cs +++ b/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; } From f931f4c324f96063a3964fc660daf06d99e657d2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 27 Oct 2023 18:19:44 +0900 Subject: [PATCH 16/27] Remove reundant interface specification --- osu.Game/Skinning/SkinInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index c2b80b7ead..9763d3b57e 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/osu.Game/Skinning/SkinInfo.cs @@ -14,7 +14,7 @@ namespace osu.Game.Skinning { [MapTo("Skin")] [JsonObject(MemberSerialization.OptIn)] - public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable, IHasGuidPrimaryKey, ISoftDelete, IHasNamedFiles + public class SkinInfo : RealmObject, IHasRealmFiles, IEquatable, 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"); From 7140eee870bab0d86ab25c15f3d56dce1b68d627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 11:38:10 +0200 Subject: [PATCH 17/27] Add failing test coverage for quick retry after completion not changing rank --- osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index a6d4fb0b52..2f378917e6 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -259,6 +259,7 @@ namespace osu.Game.Tests.Visual.Navigation var getOriginalPlayer = playToCompletion(); AddStep("attempt to retry", () => getOriginalPlayer().ChildrenOfType().First().Action()); + AddAssert("original play isn't failed", () => getOriginalPlayer().Score.ScoreInfo.Rank, () => Is.Not.EqualTo(ScoreRank.F)); AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); } From 86a8ab6db6548d211e5f0e5ff1a4feb4b7a506c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 11:39:43 +0200 Subject: [PATCH 18/27] Fix quick retry immediately after completion marking score as failed Closes https://github.com/ppy/osu/issues/25247. The scenario involved here is as follows: 1. User completes beatmap by hitting all notes. 2. `checkScoreCompleted()` determines completion, and calls `progressToResults(withDelay: true)`. 3. `progressToResults()` schedules `resultsDisplayDelegate`, which includes a call to `prepareAndImportScoreAsync()`, a second in the future. 4. User presses quick retry hotkey. This calls `Player.Restart(quickRestart: true)`, which invokes `Player.RestartRequested`, which in turn calls `PlayerLoader.restartRequested(true)`, which in turn causes `PlayerLoader` to make itself current, which means that `Player.OnExiting()` will get called. 5. `Player.OnExiting()` sees that `prepareScoreForDisplayTask` is null (because `prepareAndImportScoreAsync()` - which sets it - is scheduled to happen in the future), and as such assumes that the score did not complete. Thus, it marks the score as failed. 6. `Player.Restart()` after invoking `RestartRequested` calls `PerformExit(false)`, which then will unconditionally call `prepareAndImportScoreAsync()`. But the score has already been marked as failed. The flow above can be described as "convoluted", but I'm not sure I have it in me right now to try and refactor it again. Therefore, to fix, switch the `prepareScoreForDisplayTask` null check in `Player.OnExiting()` to check `GameplayState.HasPassed` instead, as it is not susceptible to the same out-of-order read issue. --- osu.Game/Screens/Play/Player.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 97bfa35d49..18d0a65d7a 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1115,8 +1115,7 @@ namespace osu.Game.Screens.Play if (!GameplayState.HasPassed && !GameplayState.HasFailed) GameplayState.HasQuit = true; - // if arriving here and the results screen preparation task hasn't run, it's safe to say the user has not completed the beatmap. - if (prepareScoreForDisplayTask == null && DrawableRuleset.ReplayScore == null) + if (!GameplayState.HasPassed && DrawableRuleset.ReplayScore == null) ScoreProcessor.FailScore(Score.ScoreInfo); } From 3944b045ed4582b233066dc714be6075b47482f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 11:58:02 +0200 Subject: [PATCH 19/27] Add extra test coverage for marking score as failed --- osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 2f378917e6..7fa4f8c836 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -247,6 +247,7 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("end spectator before retry", () => Game.SpectatorClient.EndPlaying(player.GameplayState)); AddStep("attempt to retry", () => player.ChildrenOfType().First().Action()); + AddAssert("old player score marked failed", () => player.Score.ScoreInfo.Rank, () => Is.EqualTo(ScoreRank.F)); AddUntilStep("wait for old player gone", () => Game.ScreenStack.CurrentScreen != player); AddUntilStep("get new player", () => (player = Game.ScreenStack.CurrentScreen as Player) != null); From 96d784e06bd6068f09620eaaa45c4766d0da900d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 12:39:54 +0200 Subject: [PATCH 20/27] Delete `ScoreInfo.HasReplay` as no longer needed --- osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs | 2 +- osu.Game/Scoring/IScoreInfo.cs | 2 -- osu.Game/Scoring/ScoreInfo.cs | 2 -- osu.Game/Screens/Ranking/ReplayDownloadButton.cs | 2 +- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs index 6ccf73d8ff..5b32f380b9 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneReplayDownloadButton.cs +++ b/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, diff --git a/osu.Game/Scoring/IScoreInfo.cs b/osu.Game/Scoring/IScoreInfo.cs index cde48c3be3..a1d076b8c2 100644 --- a/osu.Game/Scoring/IScoreInfo.cs +++ b/osu.Game/Scoring/IScoreInfo.cs @@ -22,8 +22,6 @@ namespace osu.Game.Scoring double Accuracy { get; } - bool HasReplay { get; } - long LegacyOnlineID { get; } DateTimeOffset Date { get; } diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 722d83cac8..d712702331 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -94,8 +94,6 @@ namespace osu.Game.Scoring public double Accuracy { get; set; } - public bool HasReplay => !string.IsNullOrEmpty(Hash) || HasOnlineReplay; - [Ignored] public bool HasOnlineReplay { get; set; } diff --git a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs b/osu.Game/Screens/Ranking/ReplayDownloadButton.cs index b6166e97f6..df5f9c7a8a 100644 --- a/osu.Game/Screens/Ranking/ReplayDownloadButton.cs +++ b/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; From 32fc19ea0d2515aeab4e84248f6cf67ec7d7de98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 13:22:17 +0200 Subject: [PATCH 21/27] Fix results screen test failure --- osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs b/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs index 146482e6fb..ab2e867255 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs +++ b/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); } From 2d5b1711f6ffc233ca308153a665d18dcbbccea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 13:27:48 +0200 Subject: [PATCH 22/27] Share `!HasPassed` condition --- osu.Game/Screens/Play/Player.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 18d0a65d7a..a1ec0b3167 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1110,12 +1110,12 @@ namespace osu.Game.Screens.Play failAnimationContainer?.Stop(); PauseOverlay?.StopAllSamples(); - if (LoadedBeatmapSuccessfully) + if (LoadedBeatmapSuccessfully && !GameplayState.HasPassed) { - if (!GameplayState.HasPassed && !GameplayState.HasFailed) + if (!GameplayState.HasFailed) GameplayState.HasQuit = true; - if (!GameplayState.HasPassed && DrawableRuleset.ReplayScore == null) + if (DrawableRuleset.ReplayScore == null) ScoreProcessor.FailScore(Score.ScoreInfo); } From dc7f5cd6edc94275a56b3b5cdb5766c76c84181c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 13:30:51 +0200 Subject: [PATCH 23/27] Add preventive assertions concerning submission flow state --- osu.Game/Screens/Play/Player.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index a1ec0b3167..58c8c3389a 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1112,6 +1112,8 @@ namespace osu.Game.Screens.Play if (LoadedBeatmapSuccessfully && !GameplayState.HasPassed) { + Debug.Assert(resultsDisplayDelegate == null && prepareScoreForDisplayTask == null); + if (!GameplayState.HasFailed) GameplayState.HasQuit = true; From 6789a522d6d79d6fd1d4f8c15b9c83b119ab61b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 14:15:30 +0200 Subject: [PATCH 24/27] Rename test to distinguish it from test-to-come --- .../Visual/Navigation/TestSceneSkinEditorNavigation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs index 88904bf85b..d08d53f747 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs @@ -31,7 +31,7 @@ namespace osu.Game.Tests.Visual.Navigation private SkinEditor skinEditor => Game.ChildrenOfType().FirstOrDefault(); [Test] - public void TestEditComponentDuringGameplay() + public void TestEditComponentFromGameplayScene() { advanceToSongSelect(); openSkinEditor(); From b5cb5380045b0d8be6031cbdffb4a799ac402bd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 14:23:41 +0200 Subject: [PATCH 25/27] Add failing test case for skin editor freeze --- .../TestSceneSkinEditorNavigation.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs index d08d53f747..c17a9ddf5f 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneSkinEditorNavigation.cs @@ -5,6 +5,7 @@ using System.Linq; using NUnit.Framework; +using osu.Framework.Allocation; using osu.Framework.Extensions; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.UserInterface; @@ -18,6 +19,7 @@ using osu.Game.Rulesets.Osu.Mods; using osu.Game.Screens.Edit.Components; using osu.Game.Screens.Play; using osu.Game.Screens.Play.HUD.HitErrorMeters; +using osu.Game.Skinning; using osu.Game.Tests.Beatmaps.IO; using osuTK; using osuTK.Input; @@ -69,6 +71,28 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("value is less than default", () => hitErrorMeter.JudgementLineThickness.Value < hitErrorMeter.JudgementLineThickness.Default); } + [Test] + public void TestMutateProtectedSkinDuringGameplay() + { + advanceToSongSelect(); + AddStep("set default skin", () => Game.Dependencies.Get().CurrentSkinInfo.SetDefault()); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("enable NF", () => Game.SelectedMods.Value = new[] { new OsuModNoFail() }); + AddStep("enter gameplay", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => + { + DismissAnyNotifications(); + return Game.ScreenStack.CurrentScreen is Player; + }); + + openSkinEditor(); + AddUntilStep("current skin is mutable", () => !Game.Dependencies.Get().CurrentSkin.Value.SkinInfo.Value.Protected); + } + [Test] public void TestComponentsDeselectedOnSkinEditorHide() { From 5ad962070c0448b81c8457b8816818b6e6041502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 14:34:30 +0200 Subject: [PATCH 26/27] Fix skin editor freezing game if opened during active gameplay --- osu.Game/Database/ImportParameters.cs | 6 ++++++ osu.Game/Database/RealmArchiveModelImporter.cs | 6 +++--- osu.Game/Skinning/SkinManager.cs | 5 ++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/osu.Game/Database/ImportParameters.cs b/osu.Game/Database/ImportParameters.cs index 83ca0ac694..8d37597afc 100644 --- a/osu.Game/Database/ImportParameters.cs +++ b/osu.Game/Database/ImportParameters.cs @@ -21,5 +21,11 @@ namespace osu.Game.Database /// Whether this import should use hard links rather than file copy operations if available. /// public bool PreferHardLinks { get; set; } + + /// + /// If set to , this import will not respect . + /// This is useful for cases where an import must complete even if gameplay is in progress. + /// + public bool ImportImmediately { get; set; } } } diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index 730465e1b0..5383040eb4 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -261,7 +261,7 @@ namespace osu.Game.Database /// An optional cancellation token. public virtual Live? ImportModel(TModel item, ArchiveReader? archive = null, ImportParameters parameters = default, CancellationToken cancellationToken = default) => Realm.Run(realm => { - pauseIfNecessary(cancellationToken); + pauseIfNecessary(parameters, cancellationToken); TModel? existing; @@ -560,9 +560,9 @@ namespace osu.Game.Database /// Whether to perform deletion. protected virtual bool ShouldDeleteArchive(string path) => false; - private void pauseIfNecessary(CancellationToken cancellationToken) + private void pauseIfNecessary(ImportParameters importParameters, CancellationToken cancellationToken) { - if (!PauseImports) + if (!PauseImports || importParameters.ImportImmediately) return; Logger.Log($@"{GetType().Name} is being paused."); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index ca46d3af0c..59c2a8bca0 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -182,7 +182,10 @@ namespace osu.Game.Skinning Name = NamingUtils.GetNextBestName(existingSkinNames, $@"{s.Name} (modified)") }; - var result = skinImporter.ImportModel(skinInfo); + var result = skinImporter.ImportModel(skinInfo, parameters: new ImportParameters + { + ImportImmediately = true // to avoid possible deadlocks when editing skin during gameplay. + }); if (result != null) { From 35f30d6135b29b73b68d861bb68564cc951f1a27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 27 Oct 2023 14:39:58 +0200 Subject: [PATCH 27/27] Scale back debug assertion The import preparation task can actually be non-null when exiting even if the player hasn't passed: - fail beatmap - click import button to import the failed replay --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 58c8c3389a..2392fd9f76 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -1112,7 +1112,7 @@ namespace osu.Game.Screens.Play if (LoadedBeatmapSuccessfully && !GameplayState.HasPassed) { - Debug.Assert(resultsDisplayDelegate == null && prepareScoreForDisplayTask == null); + Debug.Assert(resultsDisplayDelegate == null); if (!GameplayState.HasFailed) GameplayState.HasQuit = true;