From a7a7584d3ec5077c98fa7c8bb02aa7beb5bda09e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 31 Mar 2022 18:31:18 +0900 Subject: [PATCH 1/2] Add test coverage ensuring ruleset ID is correct after bracket read Historically, tournament client may have written incorrect `OnlineID` values. We wanted to use `ShortName` to re-fetch the ruleset. This test ensures this flow is working correctly. --- .../NonVisual/DataLoadTest.cs | 60 ++++++++++++++++++- osu.Game.Tournament/TournamentGameBase.cs | 4 +- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs b/osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs index 65753bfe00ec..4c1256df2ec2 100644 --- 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 . 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,6 +16,45 @@ 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>().Value.OnlineID, Is.EqualTo(1)); + } + finally + { + host.Exit(); + } + } + } + [Test] public void TestUnavailableRuleset() { @@ -19,7 +62,7 @@ public void TestUnavailableRuleset() { try { - var osu = new TestTournament(); + var osu = new TestTournament(true); LoadTournament(host, osu); var storage = osu.Dependencies.Get(); @@ -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 index f318c8bd85fa..b88f4edaa690 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -64,8 +64,6 @@ private void load(Storage baseStorage) Textures.AddStore(new TextureLoaderStore(new StorageBackedResourceStore(storage))); dependencies.CacheAs(new StableInfo(storage)); - - Task.Run(readBracket); } private void readBracket() @@ -290,6 +288,8 @@ protected override void LoadComplete() MenuCursorContainer.Cursor.Alpha = 0; base.LoadComplete(); + + Task.Run(readBracket); } protected virtual void SaveChanges() From a06b0a49662c94b53c05d1c482d6457cf1c3bb98 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 31 Mar 2022 18:40:58 +0900 Subject: [PATCH 2/2] Fix tournament bracket parsing's ruleset refetch logic not working correctly Due to equality being based on `ShortName`, it was feasible that the re-fetch exited early (in bindable shortcutting logic) causing the ruleset's `OnlineID` to remain `-1` or something equally wrong. Resolves issue pointed out at https://github.com/ppy/osu/discussions/17538#discussioncomment-2471746. --- osu.Game.Tournament/TournamentGameBase.cs | 30 +++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index b88f4edaa690..a251a043f721 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -66,6 +66,18 @@ private void load(Storage baseStorage) 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); + } + private void readBracket() { try @@ -79,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 @@ -280,18 +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(); - - Task.Run(readBracket); - } - protected virtual void SaveChanges() { if (!bracketLoadTaskCompletionSource.Task.IsCompletedSuccessfully)