From b1fae2bc6a766a6301b8ebdabd3573697ef3020f Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 13 Jan 2024 16:24:48 +0300 Subject: [PATCH 1/8] Add failing test case --- .../TestScenePlayerScoreSubmission.cs | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs index f75a2656ef..833cb24f41 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs @@ -15,11 +15,13 @@ using osu.Game.Online.Rooms; using osu.Game.Online.Solo; using osu.Game.Rulesets; using osu.Game.Rulesets.Mania; +using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Judgements; using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.Taiko; +using osu.Game.Rulesets.Taiko.Mods; using osu.Game.Scoring; using osu.Game.Screens.Ranking; using osu.Game.Tests.Beatmaps; @@ -34,12 +36,19 @@ namespace osu.Game.Tests.Visual.Gameplay private Func createCustomBeatmap; private Func createCustomRuleset; + private Func createCustomMods; private DummyAPIAccess dummyAPI => (DummyAPIAccess)API; protected override bool HasCustomSteps => true; - protected override TestPlayer CreatePlayer(Ruleset ruleset) => new FakeImportingPlayer(false); + protected override TestPlayer CreatePlayer(Ruleset ruleset) + { + if (createCustomMods != null) + SelectedMods.Value = SelectedMods.Value.Concat(createCustomMods()).ToList(); + + return new FakeImportingPlayer(false); + } protected new FakeImportingPlayer Player => (FakeImportingPlayer)base.Player; @@ -277,13 +286,27 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("ensure no submission", () => Player.SubmittedScore == null); } - private void createPlayerTest(bool allowFail = false, Func createBeatmap = null, Func createRuleset = null) + [Test] + public void TestNoSubmissionWithModsOfDifferentRuleset() + { + prepareTestAPI(true); + + createPlayerTest(createRuleset: () => new OsuRuleset(), createMods: () => new Mod[] { new TaikoModHidden() }); + + AddUntilStep("wait for token request", () => Player.TokenCreationRequested); + + AddStep("exit", () => Player.Exit()); + AddAssert("ensure no submission", () => Player.SubmittedScore == null); + } + + private void createPlayerTest(bool allowFail = false, Func createBeatmap = null, Func createRuleset = null, Func createMods = null) { CreateTest(() => AddStep("set up requirements", () => { this.allowFail = allowFail; createCustomBeatmap = createBeatmap; createCustomRuleset = createRuleset; + createCustomMods = createMods; })); } From 0c02062780312b56dd2f79301e097b1b21de146a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 13 Jan 2024 15:52:57 +0300 Subject: [PATCH 2/8] Add guard against starting gameplay with invalid mod instances Move guard to `Player` instead --- osu.Game/Screens/Play/Player.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index b87306b9a2..ffd585148b 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -213,6 +213,12 @@ namespace osu.Game.Screens.Play if (playableBeatmap == null) return; + if (gameplayMods.Any(m => ruleset.AllMods.All(rulesetMod => m.GetType() != rulesetMod.GetType()))) + { + Logger.Log($@"Gameplay was started with a mod belonging to a ruleset different than '{ruleset.Description}'.", level: LogLevel.Important); + return; + } + mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); if (game != null) From 494c1be6553ed9b891b8b05dc37bbe6d50d3f3a7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 15 Jan 2024 13:30:34 +0300 Subject: [PATCH 3/8] Fix blatant error in `TestSceneModAccuracyChallenge` --- osu.Game.Tests/Visual/Mods/TestSceneModAccuracyChallenge.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Mods/TestSceneModAccuracyChallenge.cs b/osu.Game.Tests/Visual/Mods/TestSceneModAccuracyChallenge.cs index 6bdb9132e1..c5e56c6453 100644 --- a/osu.Game.Tests/Visual/Mods/TestSceneModAccuracyChallenge.cs +++ b/osu.Game.Tests/Visual/Mods/TestSceneModAccuracyChallenge.cs @@ -8,6 +8,7 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Mods; using osu.Game.Rulesets.Osu.Objects; using osuTK; @@ -29,7 +30,7 @@ namespace osu.Game.Tests.Visual.Mods public void TestMaximumAchievableAccuracy() => CreateModTest(new ModTestData { - Mod = new ModAccuracyChallenge + Mod = new OsuModAccuracyChallenge { MinimumAccuracy = { Value = 0.6 } }, @@ -49,7 +50,7 @@ namespace osu.Game.Tests.Visual.Mods public void TestStandardAccuracy() => CreateModTest(new ModTestData { - Mod = new ModAccuracyChallenge + Mod = new OsuModAccuracyChallenge { MinimumAccuracy = { Value = 0.6 }, AccuracyJudgeMode = { Value = ModAccuracyChallenge.AccuracyMode.Standard } From 1d7b63e204bfac6a5350b3221b3c951f2e4e5510 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 15 Jan 2024 14:57:17 +0300 Subject: [PATCH 4/8] Move checking logic inside `ModUtils` and somewhat optimise --- osu.Game/Screens/Play/Player.cs | 3 ++- osu.Game/Utils/ModUtils.cs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index ffd585148b..5f52f59ad2 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -38,6 +38,7 @@ using osu.Game.Screens.Play.HUD; using osu.Game.Screens.Ranking; using osu.Game.Skinning; using osu.Game.Users; +using osu.Game.Utils; using osuTK.Graphics; namespace osu.Game.Screens.Play @@ -213,7 +214,7 @@ namespace osu.Game.Screens.Play if (playableBeatmap == null) return; - if (gameplayMods.Any(m => ruleset.AllMods.All(rulesetMod => m.GetType() != rulesetMod.GetType()))) + if (!ModUtils.CheckModsBelongToRuleset(ruleset, gameplayMods)) { Logger.Log($@"Gameplay was started with a mod belonging to a ruleset different than '{ruleset.Description}'.", level: LogLevel.Important); return; diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 252579a186..827b199b16 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -229,6 +229,38 @@ namespace osu.Game.Utils return proposedWereValid; } + /// + /// Verifies all mods provided belong to the given ruleset. + /// + /// The ruleset to check the proposed mods against. + /// The mods proposed for checking. + /// Whether all belong to the given . + public static bool CheckModsBelongToRuleset(Ruleset ruleset, IEnumerable proposedMods) + { + var rulesetModsTypes = ruleset.AllMods.Select(m => m.GetType()).ToList(); + + foreach (var proposedMod in proposedMods) + { + bool found = false; + + var proposedModType = proposedMod.GetType(); + + foreach (var rulesetModType in rulesetModsTypes) + { + if (rulesetModType == proposedModType) + { + found = true; + break; + } + } + + if (!found) + return true; + } + + return true; + } + /// /// Given a value of a score multiplier, returns a string version with special handling for a value near 1.00x. /// From d346dd065027d08d374d76f045302c356d05fb9a Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 15 Jan 2024 15:01:13 +0300 Subject: [PATCH 5/8] Fix `TestModReinstantiation` failing due to custom mod being used --- osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs | 8 +------- osu.Game/Utils/ModUtils.cs | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs index dbd1ce1f6e..f97372e9b6 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs @@ -13,7 +13,6 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Localisation; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Framework.Utils; @@ -487,13 +486,8 @@ namespace osu.Game.Tests.Visual.Gameplay } } - private class TestMod : Mod, IApplicableToScoreProcessor + private class TestMod : OsuModDoubleTime, IApplicableToScoreProcessor { - public override string Name => string.Empty; - public override string Acronym => string.Empty; - public override double ScoreMultiplier => 1; - public override LocalisableString Description => string.Empty; - public bool Applied { get; private set; } public void ApplyToScoreProcessor(ScoreProcessor scoreProcessor) diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 827b199b16..7a9f93e06f 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -247,7 +247,7 @@ namespace osu.Game.Utils foreach (var rulesetModType in rulesetModsTypes) { - if (rulesetModType == proposedModType) + if (rulesetModType.IsAssignableFrom(proposedModType)) { found = true; break; From 2935bd4809d1e071d8feb93604629fa94fb2cbbc Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 16 Jan 2024 14:27:24 +0300 Subject: [PATCH 6/8] Add test cases for helper method --- osu.Game.Tests/Mods/ModUtilsTest.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/osu.Game.Tests/Mods/ModUtilsTest.cs b/osu.Game.Tests/Mods/ModUtilsTest.cs index 13da69871e..decb0a31ac 100644 --- a/osu.Game.Tests/Mods/ModUtilsTest.cs +++ b/osu.Game.Tests/Mods/ModUtilsTest.cs @@ -7,7 +7,9 @@ using Moq; using NUnit.Framework; using osu.Framework.Localisation; using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Rulesets.Taiko.Mods; using osu.Game.Utils; namespace osu.Game.Tests.Mods @@ -310,6 +312,16 @@ namespace osu.Game.Tests.Mods Assert.That(invalid?.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); } + [Test] + public void TestModBelongsToRuleset() + { + Assert.That(ModUtils.CheckModsBelongToRuleset(new OsuRuleset(), Array.Empty())); + Assert.That(ModUtils.CheckModsBelongToRuleset(new OsuRuleset(), new Mod[] { new OsuModDoubleTime() })); + Assert.That(ModUtils.CheckModsBelongToRuleset(new OsuRuleset(), new Mod[] { new OsuModDoubleTime(), new OsuModAccuracyChallenge() })); + Assert.That(ModUtils.CheckModsBelongToRuleset(new OsuRuleset(), new Mod[] { new OsuModDoubleTime(), new ModAccuracyChallenge() }), Is.False); + Assert.That(ModUtils.CheckModsBelongToRuleset(new OsuRuleset(), new Mod[] { new OsuModDoubleTime(), new TaikoModFlashlight() }), Is.False); + } + [Test] public void TestFormatScoreMultiplier() { From 90da39c65d252e157b8a550429e18c944cd65380 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 16 Jan 2024 14:27:30 +0300 Subject: [PATCH 7/8] Fix helper method returning incorrect result --- osu.Game/Utils/ModUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 7a9f93e06f..2c9eef41e3 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -255,7 +255,7 @@ namespace osu.Game.Utils } if (!found) - return true; + return false; } return true; From 485195d4c25d3b8f13f52f6da7faf72472a21b0e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 16 Jan 2024 14:27:40 +0300 Subject: [PATCH 8/8] Fix submission test not asserting properly --- osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs index 833cb24f41..e2ce3a014c 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs @@ -294,6 +294,7 @@ namespace osu.Game.Tests.Visual.Gameplay createPlayerTest(createRuleset: () => new OsuRuleset(), createMods: () => new Mod[] { new TaikoModHidden() }); AddUntilStep("wait for token request", () => Player.TokenCreationRequested); + AddAssert("gameplay not loaded", () => Player.DrawableRuleset == null); AddStep("exit", () => Player.Exit()); AddAssert("ensure no submission", () => Player.SubmittedScore == null);