Merge pull request #28262 from bdach/fix-present-confusion-on-empty-hash

Do not attempt to match score by equality of empty hash when presenting it
ppy · May 21, 2024 · 20a539b · 20a539b
2 parents d7d569c + ed498f6
commit 20a539b
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 10 deletions.
diff --git 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 <contact@ppy.sh>. 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 @@ public partial class TestScenePresentScore : OsuGameTestScene
                             Ruleset = new OsuRuleset().RulesetInfo
                         },
                     }
-                })?.Value;
+                })!.Value;
             });
         }
 
@@ -158,6 +156,27 @@ public void TestPresentTwoImportsWithSameOnlineIDButDifferentHashes([Values] Sco
             presentAndConfirm(secondImport, type);
         }
 
+        [Test]
+        public void TestScoreRefetchIgnoresEmptyHash()
+        {
+            AddStep("enter song select", () => Game.ChildrenOfType<ButtonSystem>().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).
@@ -171,14 +190,14 @@ private void returnToMenu()
             AddUntilStep("wait for menu", () => Game.ScreenStack.CurrentScreen is MainMenu);
         }
 
-        private Func<ScoreInfo> importScore(int i, RulesetInfo ruleset = null)
+        private Func<ScoreInfo> importScore(int i, RulesetInfo? ruleset = null, string? hash = null)
         {
-            ScoreInfo imported = 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,
@@ -188,14 +207,14 @@ private Func<ScoreInfo> importScore(int i, RulesetInfo ruleset = null)
 
             AddAssert($"import {i} succeeded", () => imported != null);
 
-            return () => imported;
+            return () => imported!;
         }
 
         /// <summary>
         /// 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.
         /// </summary>
-        private IScreen lastWaitedScreen;
+        private IScreen lastWaitedScreen = null!;
 
         private void presentAndConfirm(Func<ScoreInfo> getImport, ScorePresentType type)
         {

diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs
@@ -88,7 +88,7 @@ public ScoreManager(RulesetStore rulesets, Func<BeatmapManager> beatmaps, Storag
         {
             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)