From 85f85dee9ef28b4b65678d58f6e3f54abf0fe2f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 May 2024 14:46:28 +0200 Subject: [PATCH 1/3] Enable NRT in `TestScenePresentScore` --- .../Visual/Navigation/TestScenePresentScore.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs index 212783d047..2d4302c0df 100644 --- a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs +++ b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Linq; using NUnit.Framework; @@ -26,7 +24,7 @@ namespace osu.Game.Tests.Visual.Navigation { public partial class TestScenePresentScore : OsuGameTestScene { - private BeatmapSetInfo beatmap; + private BeatmapSetInfo beatmap = null!; [SetUpSteps] public new void SetUpSteps() @@ -64,7 +62,7 @@ namespace osu.Game.Tests.Visual.Navigation Ruleset = new OsuRuleset().RulesetInfo }, } - })?.Value; + })!.Value; }); } @@ -171,9 +169,9 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for menu", () => Game.ScreenStack.CurrentScreen is MainMenu); } - private Func importScore(int i, RulesetInfo ruleset = null) + private Func importScore(int i, RulesetInfo? ruleset = null) { - ScoreInfo imported = null; + ScoreInfo? imported = null; AddStep($"import score {i}", () => { imported = Game.ScoreManager.Import(new ScoreInfo @@ -188,14 +186,14 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert($"import {i} succeeded", () => imported != null); - return () => imported; + return () => imported!; } /// /// Some tests test waiting for a particular screen twice in a row, but expect a new instance each time. /// There's a case where they may succeed incorrectly if we don't compare against the previous instance. /// - private IScreen lastWaitedScreen; + private IScreen lastWaitedScreen = null!; private void presentAndConfirm(Func getImport, ScorePresentType type) { From 3b15c223be4a38c624f68abd30561638d46ea14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 May 2024 14:48:02 +0200 Subject: [PATCH 2/3] Add failing test case --- .../Navigation/TestScenePresentScore.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs index 2d4302c0df..2c2335de13 100644 --- a/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs +++ b/osu.Game.Tests/Visual/Navigation/TestScenePresentScore.cs @@ -156,6 +156,27 @@ namespace osu.Game.Tests.Visual.Navigation presentAndConfirm(secondImport, type); } + [Test] + public void TestScoreRefetchIgnoresEmptyHash() + { + AddStep("enter song select", () => Game.ChildrenOfType().Single().OnSolo?.Invoke()); + AddUntilStep("song select is current", () => Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect && songSelect.BeatmapSetsLoaded); + + importScore(-1, hash: string.Empty); + importScore(3, hash: @"deadbeef"); + + // oftentimes a `PresentScore()` call will be given a `ScoreInfo` which is converted from an online score, + // in which cases the hash will generally not be available. + AddStep("present score", () => Game.PresentScore(new ScoreInfo { OnlineID = 3, Hash = string.Empty })); + + AddUntilStep("wait for results", () => lastWaitedScreen != Game.ScreenStack.CurrentScreen && Game.ScreenStack.CurrentScreen is ResultsScreen); + AddUntilStep("correct score displayed", () => + { + var score = ((ResultsScreen)Game.ScreenStack.CurrentScreen).Score!; + return score.OnlineID == 3 && score.Hash == "deadbeef"; + }); + } + 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). @@ -169,14 +190,14 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for menu", () => Game.ScreenStack.CurrentScreen is MainMenu); } - private Func importScore(int i, RulesetInfo? ruleset = null) + private Func importScore(int i, RulesetInfo? ruleset = null, string? hash = null) { ScoreInfo? imported = null; AddStep($"import score {i}", () => { imported = Game.ScoreManager.Import(new ScoreInfo { - Hash = Guid.NewGuid().ToString(), + Hash = hash ?? Guid.NewGuid().ToString(), OnlineID = i, BeatmapInfo = beatmap.Beatmaps.First(), Ruleset = ruleset ?? new OsuRuleset().RulesetInfo, From ed498f6eed66a4c801604484dbcbea883b229cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 20 May 2024 14:48:36 +0200 Subject: [PATCH 3/3] Do not attempt to match score by equality of hash if it's empty Closes https://github.com/ppy/osu/issues/28216. The affected user's database contained six sentakki scores with an empty hash. When an online score is being imported, an online model (which does not have a hash) will be transmogrified into a `ScoreInfo` with an empty hash, which would end up accidentally matching those scores and basically breaking everything at that point. To fix, avoid attempting to match anything on empty hash. This does not break online score matching because for those cases the actual online ID of the score will be used. --- osu.Game/Scoring/ScoreManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 0c707ffa19..df4735b5e6 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -88,7 +88,7 @@ namespace osu.Game.Scoring { ScoreInfo? databasedScoreInfo = null; - if (originalScoreInfo is ScoreInfo scoreInfo) + if (originalScoreInfo is ScoreInfo scoreInfo && !string.IsNullOrEmpty(scoreInfo.Hash)) databasedScoreInfo = Query(s => s.Hash == scoreInfo.Hash); if (originalScoreInfo.OnlineID > 0)