From 7f3fde2a25d0d78277a6794cc48ec856691e689a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 14 May 2024 11:13:06 +0200 Subject: [PATCH 1/2] Add failing test case --- .../Visual/Navigation/TestScenePresentScore.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs index 004d1de116..212783d047 100644 --- a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs +++ b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs @@ -145,6 +145,19 @@ namespace osu.Game.Tests.Visual.Navigation presentAndConfirm(secondImport, type); } + [Test] + public void TestPresentTwoImportsWithSameOnlineIDButDifferentHashes([Values] ScorePresentType type) + { + AddStep("enter song select", () => Game.ChildrenOfType().Single().OnSolo?.Invoke()); + AddUntilStep("song select is current", () => Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect && songSelect.BeatmapSetsLoaded); + + var firstImport = importScore(1); + presentAndConfirm(firstImport, type); + + var secondImport = importScore(1); + presentAndConfirm(secondImport, type); + } + private void returnToMenu() { // if we don't pause, there's a chance the track may change at the main menu out of our control (due to reaching the end of the track). From 03a279a48d476b2529d01d975022cb927eb80875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 14 May 2024 11:14:46 +0200 Subject: [PATCH 2/2] Use hash rather than online ID as primary lookup key when presenting score Something I ran into when investigating https://github.com/ppy/osu/issues/28169. If there are two scores with the same online ID available in the database - for instance, one being recorded locally, and one recorded by spectator server, of one single play - the lookup code would use online ID first to find the score and pick any first one that matched. This could lead to the wrong replay being refetched and presented / exported. (In the case of the aforementioned issue, I was confused as to whether after restarting spectator server midway through a play and importing the replay saved by spectator server after the restart, I was seeing a complete replay with no dropped frames, even though there was nothing in the code that prevented the frame drop. It turns out that I was getting presented the locally recorded replay instead all along.) Instead, jiggle the fallback preference to use hash first. --- osu.Game/Scoring/ScoreManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 1ba5c7d4cf..0c707ffa19 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -88,15 +88,15 @@ namespace osu.Game.Scoring { ScoreInfo? databasedScoreInfo = null; + if (originalScoreInfo is ScoreInfo scoreInfo) + databasedScoreInfo = Query(s => s.Hash == scoreInfo.Hash); + if (originalScoreInfo.OnlineID > 0) - databasedScoreInfo = Query(s => s.OnlineID == originalScoreInfo.OnlineID); + databasedScoreInfo ??= Query(s => s.OnlineID == originalScoreInfo.OnlineID); if (originalScoreInfo.LegacyOnlineID > 0) databasedScoreInfo ??= Query(s => s.LegacyOnlineID == originalScoreInfo.LegacyOnlineID); - if (originalScoreInfo is ScoreInfo scoreInfo) - databasedScoreInfo ??= Query(s => s.Hash == scoreInfo.Hash); - if (databasedScoreInfo == null) { Logger.Log("The requested score could not be found locally.", LoggingTarget.Information);