Merge pull request #17574 from peppy/fix-tournament-bracket-invalid-r…

…uleset-id Fix tournament bracket parsing's ruleset refetch logic not working correctly
ppy · Mar 31, 2022 · 46be6bc · 46be6bc
2 parents bc4d6a3 + a06b0a4
commit 46be6bc
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 13 deletions.
diff --git a/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs b/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs
@@ -1,9 +1,13 @@
 // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.IO;
+using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions;
 using osu.Framework.Platform;
 using osu.Game.Rulesets;
 using osu.Game.Tests;
@@ -12,14 +16,53 @@ namespace osu.Game.Tournament.Tests.NonVisual
 {
     public class DataLoadTest : TournamentHostTest
     {
+        [Test]
+        public void TestRulesetGetsValidOnlineID()
+        {
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
+            {
+                try
+                {
+                    var osu = new TestTournament(runOnLoadComplete: () =>
+                    {
+                        // ReSharper disable once AccessToDisposedClosure
+                        var storage = host.Storage.GetStorageForDirectory(Path.Combine("tournaments", "default"));
+
+                        using (var stream = storage.GetStream("bracket.json", FileAccess.Write, FileMode.Create))
+                        using (var writer = new StreamWriter(stream))
+                        {
+                            writer.Write(@"{
+                        ""Ruleset"": {
+                            ""ShortName"": ""taiko"",
+                            ""OnlineID"": -1,
+                            ""Name"": ""osu!taiko"",
+                            ""InstantiationInfo"": ""osu.Game.Rulesets.OsuTaiko.TaikoRuleset, osu.Game.Rulesets.Taiko"",
+                            ""Available"": true
+                        } }");
+                        }
+                    });
+
+                    LoadTournament(host, osu);
+
+                    osu.BracketLoadTask.WaitSafely();
+
+                    Assert.That(osu.Dependencies.Get<IBindable<RulesetInfo>>().Value.OnlineID, Is.EqualTo(1));
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
         [Test]
         public void TestUnavailableRuleset()
         {
             using (HeadlessGameHost host = new CleanRunHeadlessGameHost())
             {
                 try
                 {
-                    var osu = new TestTournament();
+                    var osu = new TestTournament(true);
 
                     LoadTournament(host, osu);
                     var storage = osu.Dependencies.Get<Storage>();
@@ -35,10 +78,23 @@ public void TestUnavailableRuleset()
 
         public class TestTournament : TournamentGameBase
         {
+            private readonly bool resetRuleset;
+            private readonly Action runOnLoadComplete;
+
+            public new Task BracketLoadTask => base.BracketLoadTask;
+
+            public TestTournament(bool resetRuleset = false, Action runOnLoadComplete = null)
+            {
+                this.resetRuleset = resetRuleset;
+                this.runOnLoadComplete = runOnLoadComplete;
+            }
+
             protected override void LoadComplete()
             {
+                runOnLoadComplete?.Invoke();
                 base.LoadComplete();
-                Ruleset.Value = new RulesetInfo(); // not available
+                if (resetRuleset)
+                    Ruleset.Value = new RulesetInfo(); // not available
             }
         }
     }

diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs
@@ -64,6 +64,16 @@ private void load(Storage baseStorage)
             Textures.AddStore(new TextureLoaderStore(new StorageBackedResourceStore(storage)));
 
             dependencies.CacheAs(new StableInfo(storage));
+        }
+
+        protected override void LoadComplete()
+        {
+            MenuCursorContainer.Cursor.AlwaysPresent = true; // required for tooltip display
+
+            // we don't want to show the menu cursor as it would appear on stream output.
+            MenuCursorContainer.Cursor.Alpha = 0;
+
+            base.LoadComplete();
 
             Task.Run(readBracket);
         }
@@ -81,10 +91,14 @@ private void readBracket()
 
                 ladder ??= new LadderInfo();
 
-                ladder.Ruleset.Value = ladder.Ruleset.Value != null
+                var resolvedRuleset = ladder.Ruleset.Value != null
                     ? RulesetStore.GetRuleset(ladder.Ruleset.Value.ShortName)
                     : RulesetStore.AvailableRulesets.First();
 
+                // Must set to null initially to avoid the following re-fetch hitting `ShortName` based equality check.
+                ladder.Ruleset.Value = null;
+                ladder.Ruleset.Value = resolvedRuleset;
+
                 bool addedInfo = false;
 
                 // assign teams
@@ -282,16 +296,6 @@ void populate()
             }
         }
 
-        protected override void LoadComplete()
-        {
-            MenuCursorContainer.Cursor.AlwaysPresent = true; // required for tooltip display
-
-            // we don't want to show the menu cursor as it would appear on stream output.
-            MenuCursorContainer.Cursor.Alpha = 0;
-
-            base.LoadComplete();
-        }
-
         protected virtual void SaveChanges()
         {
             if (!bracketLoadTaskCompletionSource.Task.IsCompletedSuccessfully)