From 64aafa4e4c7c2e4f32ec2c646bd50f34cfb6338d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 17 Apr 2025 02:19:31 +0900 Subject: [PATCH] Apply changes from reviews --- osu.Game.Tests/Mods/ModUtilsTest.cs | 301 +++++++++------------------- osu.Game/Rulesets/Mods/IMod.cs | 6 + osu.Game/Utils/ModUtils.cs | 11 +- 3 files changed, 105 insertions(+), 213 deletions(-) diff --git a/osu.Game.Tests/Mods/ModUtilsTest.cs b/osu.Game.Tests/Mods/ModUtilsTest.cs index 074d9438d4..a006a13477 100644 --- a/osu.Game.Tests/Mods/ModUtilsTest.cs +++ b/osu.Game.Tests/Mods/ModUtilsTest.cs @@ -188,148 +188,6 @@ namespace osu.Game.Tests.Mods }, }; - private static readonly object[] invalid_multiplayer_mod_test_scenarios = - { - // incompatible pair. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModApproachDifferent() }, - new[] { typeof(OsuModHidden), typeof(OsuModApproachDifferent) } - }, - // incompatible pair with derived class. - new object[] - { - new Mod[] { new OsuModDeflate(), new OsuModApproachDifferent() }, - new[] { typeof(OsuModDeflate), typeof(OsuModApproachDifferent) } - }, - // system mod. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModTouchDevice() }, - new[] { typeof(OsuModTouchDevice) } - }, - // multi mod. - new object[] - { - new Mod[] { new MultiMod(new OsuModSuddenDeath(), new OsuModPerfect()) }, - new[] { typeof(MultiMod) } - }, - // invalid multiplayer mod. - new object[] - { - new Mod[] { new OsuModHidden(), new InvalidMultiplayerMod() }, - new[] { typeof(InvalidMultiplayerMod) } - }, - // invalid free mod is valid for multiplayer global. - new object[] - { - new Mod[] { new OsuModHidden(), new InvalidMultiplayerFreeMod() }, - Array.Empty() - }, - // valid pair. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModHardRock() }, - Array.Empty() - }, - }; - - private static readonly object[] invalid_free_mod_test_scenarios = - { - // system mod. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModTouchDevice() }, - new[] { typeof(OsuModTouchDevice) } - }, - // multi mod. - new object[] - { - new Mod[] { new MultiMod(new OsuModSuddenDeath(), new OsuModPerfect()) }, - new[] { typeof(MultiMod) } - }, - // invalid multiplayer mod. - new object[] - { - new Mod[] { new OsuModHidden(), new InvalidMultiplayerMod() }, - new[] { typeof(InvalidMultiplayerMod) } - }, - // invalid free mod. - new object[] - { - new Mod[] { new OsuModHidden(), new InvalidMultiplayerFreeMod() }, - new[] { typeof(InvalidMultiplayerFreeMod) } - }, - // incompatible pair is valid for free mods. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModApproachDifferent() }, - Array.Empty(), - }, - // incompatible pair with derived class is valid for free mods. - new object[] - { - new Mod[] { new OsuModDeflate(), new OsuModSpinIn() }, - Array.Empty(), - }, - // valid pair. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModHardRock() }, - Array.Empty() - }, - }; - - private static readonly object[] invalid_freestyle_required_mod_test_scenarios = - { - // system mod. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModTouchDevice() }, - new[] { typeof(OsuModTouchDevice) } - }, - // multi mod. - new object[] - { - new Mod[] { new MultiMod(new OsuModSuddenDeath(), new OsuModPerfect()) }, - new[] { typeof(MultiMod) } - }, - // invalid freestyle mod. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModNoScope(), new InvalidFreestyleMod() }, - new[] { typeof(OsuModNoScope), typeof(InvalidFreestyleMod) } - }, - // valid pair. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModHardRock() }, - Array.Empty() - }, - }; - - private static readonly object[] invalid_freestyle_allowed_mod_test_scenarios = - { - // system mod. - new object[] - { - new Mod[] { new OsuModHidden(), new OsuModTouchDevice() }, - new[] { typeof(OsuModHidden), typeof(OsuModTouchDevice) } - }, - // multi mod. - new object[] - { - new Mod[] { new MultiMod(new OsuModSuddenDeath(), new OsuModPerfect()) }, - new[] { typeof(MultiMod) } - }, - // invalid freestyle mod. - new object[] - { - new Mod[] { new OsuModHidden() }, - new[] { typeof(OsuModHidden) } - }, - }; - [TestCaseSource(nameof(invalid_mod_test_scenarios))] public void TestInvalidModScenarios(Mod[] inputMods, Type[] expectedInvalid) { @@ -343,32 +201,6 @@ namespace osu.Game.Tests.Mods Assert.That(invalid?.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); } - [TestCaseSource(nameof(invalid_multiplayer_mod_test_scenarios))] - public void TestInvalidMultiplayerModScenarios(Mod[] inputMods, Type[] expectedInvalid) - { - bool isValid = ModUtils.CheckValidRequiredModsForMultiplayer(inputMods, false, out var invalid); - - Assert.That(isValid, Is.EqualTo(expectedInvalid.Length == 0)); - - if (isValid) - Assert.IsNull(invalid); - else - Assert.That(invalid?.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); - } - - [TestCaseSource(nameof(invalid_freestyle_required_mod_test_scenarios))] - public void TestInvalidFreestyleRequiredModScenarios(Mod[] inputMods, Type[] expectedInvalid) - { - bool isValid = ModUtils.CheckValidRequiredModsForMultiplayer(inputMods, true, out var invalid); - - Assert.That(isValid, Is.EqualTo(expectedInvalid.Length == 0)); - - if (isValid) - Assert.IsNull(invalid); - else - Assert.That(invalid?.Select(t => t.GetType()), Is.EquivalentTo(expectedInvalid)); - } - [Test] public void TestModBelongsToRuleset() { @@ -399,53 +231,102 @@ namespace osu.Game.Tests.Mods Assert.AreEqual(ModUtils.FormatScoreMultiplier(1.055).ToString(), "1.06x"); } - [Test] - public void TestRoomModValidity() + private static readonly object[] multiplayer_mod_test_scenarios = { - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), MatchType.Playlists, true, false)); - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModDoubleTime(), MatchType.Playlists, true, false)); - Assert.IsTrue(ModUtils.IsValidModForMatch(new ModAdaptiveSpeed(), MatchType.Playlists, true, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModAutoplay(), MatchType.Playlists, true, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModTouchDevice(), MatchType.Playlists, true, false)); + // valid - as allowed mod. + new MultiplayerTestScenario(false, false, [new OsuModBarrelRoll()], []), + new MultiplayerTestScenario(false, true, [new OsuModBarrelRoll()], []), + // valid - as allowed mod (incompatible pair). + new MultiplayerTestScenario(false, false, [new OsuModHardRock(), new OsuModEasy()], []), + new MultiplayerTestScenario(false, true, [new OsuModHardRock(), new OsuModEasy()], []), + // valid - as allowed mod (incompatible pair with derived classes). + new MultiplayerTestScenario(false, false, [new OsuModDeflate(), new OsuModApproachDifferent()], []), + new MultiplayerTestScenario(false, true, [new OsuModDeflate(), new OsuModApproachDifferent()], []), + // valid - as allowed mod (not implemented in all rulesets). + new MultiplayerTestScenario(false, false, [new OsuModBarrelRoll()], []), + new MultiplayerTestScenario(false, true, [new OsuModBarrelRoll()], []), + // valid - as required mod. + new MultiplayerTestScenario(true, false, [new OsuModStrictTracking()], []), + // valid - as required mod when not freestyle. + new MultiplayerTestScenario(true, false, [new InvalidFreestyleRequiredMod()], []), + // valid - as required mod when freestyle (implemented in all rulesets). + new MultiplayerTestScenario(true, true, [new OsuModEasy()], []), + new MultiplayerTestScenario(true, true, [new OsuModNoFail()], []), + new MultiplayerTestScenario(true, true, [new OsuModHalfTime()], []), + new MultiplayerTestScenario(true, true, [new OsuModDaycore()], []), + new MultiplayerTestScenario(true, true, [new OsuModHardRock()], []), + new MultiplayerTestScenario(true, true, [new OsuModSuddenDeath()], []), + new MultiplayerTestScenario(true, true, [new OsuModPerfect()], []), + new MultiplayerTestScenario(true, true, [new OsuModDoubleTime()], []), + new MultiplayerTestScenario(true, true, [new OsuModNightcore()], []), + new MultiplayerTestScenario(true, true, [new OsuModHidden()], []), + new MultiplayerTestScenario(true, true, [new OsuModFlashlight()], []), + new MultiplayerTestScenario(true, true, [new OsuModAccuracyChallenge()], []), + new MultiplayerTestScenario(true, true, [new OsuModDifficultyAdjust()], []), + new MultiplayerTestScenario(true, true, [new ModWindUp()], []), + new MultiplayerTestScenario(true, true, [new ModWindDown()], []), + new MultiplayerTestScenario(true, true, [new OsuModMuted()], []), - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), MatchType.HeadToHead, true, false)); - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModDoubleTime(), MatchType.HeadToHead, true, false)); - // For now, adaptive speed isn't allowed in multiplayer because it's a per-user rate adjustment. - Assert.IsFalse(ModUtils.IsValidModForMatch(new ModAdaptiveSpeed(), MatchType.HeadToHead, true, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModAutoplay(), MatchType.HeadToHead, true, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModTouchDevice(), MatchType.HeadToHead, true, false)); + // invalid - always (system mod) + new MultiplayerTestScenario(false, false, [new OsuModTouchDevice()], [typeof(OsuModTouchDevice)]), + new MultiplayerTestScenario(true, false, [new OsuModTouchDevice()], [typeof(OsuModTouchDevice)]), + // invalid - always (multi mod). + new MultiplayerTestScenario(false, false, [new MultiMod()], [typeof(MultiMod)]), + new MultiplayerTestScenario(true, false, [new MultiMod()], [typeof(MultiMod)]), + // invalid - always (disallowed by mod) + new MultiplayerTestScenario(false, false, [new InvalidMultiplayerMod()], [typeof(InvalidMultiplayerMod)]), + new MultiplayerTestScenario(true, false, [new InvalidMultiplayerMod()], [typeof(InvalidMultiplayerMod)]), + new MultiplayerTestScenario(false, false, [new OsuModAutoplay()], [typeof(OsuModAutoplay)]), + new MultiplayerTestScenario(true, false, [new OsuModAutoplay()], [typeof(OsuModAutoplay)]), + // invalid - always (changes play length - for now not allowed in multiplayer). + new MultiplayerTestScenario(false, false, [new ModAdaptiveSpeed()], [typeof(ModAdaptiveSpeed)]), + new MultiplayerTestScenario(true, false, [new ModAdaptiveSpeed()], [typeof(ModAdaptiveSpeed)]), + // invalid - as allowed mod (disallowed by mod). + new MultiplayerTestScenario(false, false, [new InvalidMultiplayerFreeMod()], [typeof(InvalidMultiplayerFreeMod)]), + new MultiplayerTestScenario(false, true, [new InvalidMultiplayerFreeMod()], [typeof(InvalidMultiplayerFreeMod)]), + // invalid - as allowed mod (changes play length - for now not allowed in multiplayer). + new MultiplayerTestScenario(false, false, [new OsuModHalfTime()], [typeof(OsuModHalfTime)]), + new MultiplayerTestScenario(false, false, [new OsuModDaycore()], [typeof(OsuModDaycore)]), + new MultiplayerTestScenario(false, false, [new OsuModDoubleTime()], [typeof(OsuModDoubleTime)]), + new MultiplayerTestScenario(false, false, [new OsuModNightcore()], [typeof(OsuModNightcore)]), + // invalid - as required mod (incompatible pair) + new MultiplayerTestScenario(true, false, [new OsuModHidden(), new OsuModApproachDifferent()], [typeof(OsuModHidden), typeof(OsuModApproachDifferent)]), + new MultiplayerTestScenario(true, true, [new OsuModHidden(), new OsuModApproachDifferent()], [typeof(OsuModHidden), typeof(OsuModApproachDifferent)]), + new MultiplayerTestScenario(true, false, [new OsuModDeflate(), new OsuModApproachDifferent()], [typeof(OsuModDeflate), typeof(OsuModApproachDifferent)]), + new MultiplayerTestScenario(true, true, [new OsuModDeflate(), new OsuModApproachDifferent()], [typeof(OsuModDeflate), typeof(OsuModApproachDifferent)]), + // invalid - as required mod when freestyle (disallowed by mod). + new MultiplayerTestScenario(true, true, [new InvalidFreestyleRequiredMod()], [typeof(InvalidFreestyleRequiredMod)]), + // invalid - as required mod when freestyle (not implemented in all rulesets). + new MultiplayerTestScenario(true, true, [new OsuModStrictTracking()], [typeof(OsuModStrictTracking)]), + new MultiplayerTestScenario(true, true, [new OsuModBarrelRoll()], [typeof(OsuModBarrelRoll)]), + }; + + [TestCaseSource(nameof(multiplayer_mod_test_scenarios))] + public void TestMultiplayerModScenarios(MultiplayerTestScenario scenario) + { + List? invalidMods; + bool isValid = scenario.IsRequired + ? ModUtils.CheckValidRequiredModsForMultiplayer(scenario.Mods, scenario.IsFreestyle, out invalidMods) + : ModUtils.CheckValidAllowedModsForMultiplayer(scenario.Mods, scenario.IsFreestyle, out invalidMods); + + Assert.That(isValid, Is.EqualTo(scenario.InvalidTypes.Length == 0)); + + if (isValid) + Assert.IsNull(invalidMods); + else + Assert.That(invalidMods?.Select(t => t.GetType()), Is.EquivalentTo(scenario.InvalidTypes)); } [Test] - public void TestRoomFreeModValidity() + public void TestPlaylistsModScenarios() { + // The rest are tested by TestMultiplayerModScenarios. Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), MatchType.Playlists, false, false)); + Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), MatchType.Playlists, true, false)); Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModDoubleTime(), MatchType.Playlists, false, false)); + Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModDoubleTime(), MatchType.Playlists, true, false)); Assert.IsTrue(ModUtils.IsValidModForMatch(new ModAdaptiveSpeed(), MatchType.Playlists, false, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModAutoplay(), MatchType.Playlists, false, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModTouchDevice(), MatchType.Playlists, false, false)); - - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), MatchType.HeadToHead, false, false)); - // For now, all rate adjustment mods aren't allowed as free mods in multiplayer. - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModDoubleTime(), MatchType.HeadToHead, false, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new ModAdaptiveSpeed(), MatchType.HeadToHead, false, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModAutoplay(), MatchType.HeadToHead, false, false)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModTouchDevice(), MatchType.HeadToHead, false, false)); - } - - [Test] - public void TestFreestyleModValidity() - { - foreach (MatchType type in new[] { MatchType.Playlists, MatchType.HeadToHead }) - { - // Required mods - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), type, true, true)); - Assert.IsFalse(ModUtils.IsValidModForMatch(new OsuModBarrelRoll(), type, true, true)); - - // Allowed mods - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModHardRock(), type, false, true)); - Assert.IsTrue(ModUtils.IsValidModForMatch(new OsuModBarrelRoll(), type, false, true)); - } + Assert.IsTrue(ModUtils.IsValidModForMatch(new ModAdaptiveSpeed(), MatchType.Playlists, true, false)); } [Test] @@ -502,7 +383,7 @@ namespace osu.Game.Tests.Mods public override bool ValidForMultiplayerAsFreeMod => false; } - public class InvalidFreestyleMod : Mod + public class InvalidFreestyleRequiredMod : Mod { public override string Name => string.Empty; public override LocalisableString Description => string.Empty; @@ -512,8 +393,12 @@ namespace osu.Game.Tests.Mods public override bool ValidForFreestyleAsRequiredMod => false; } - public interface IModCompatibilitySpecification + public interface IModCompatibilitySpecification; + + public readonly record struct MultiplayerTestScenario(bool IsRequired, bool IsFreestyle, Mod[] Mods, Type[] InvalidTypes, MatchType Type = MatchType.HeadToHead) { + public override string ToString() + => $"{IsRequired}, {IsFreestyle}, [{string.Join(',', Mods.Select(m => m.GetType().ReadableName()))}], [{string.Join(',', InvalidTypes.Select(t => t.ReadableName()))}]"; } } } diff --git a/osu.Game/Rulesets/Mods/IMod.cs b/osu.Game/Rulesets/Mods/IMod.cs index 3a33d14835..75d2d699ad 100644 --- a/osu.Game/Rulesets/Mods/IMod.cs +++ b/osu.Game/Rulesets/Mods/IMod.cs @@ -53,6 +53,12 @@ namespace osu.Game.Rulesets.Mods /// bool ValidForMultiplayer { get; } + /// + /// Whether this mod is valid as a required mod on freestyle online play items. + /// Should be true for mods that are guaranteed to be implemented across all rulesets. + /// + bool ValidForFreestyleAsRequiredMod { get; } + /// /// Whether this mod is valid as a free mod in multiplayer matches. /// Should be false for mods that affect the gameplay duration (e.g. and ). diff --git a/osu.Game/Utils/ModUtils.cs b/osu.Game/Utils/ModUtils.cs index 27396f95e4..e483811dab 100644 --- a/osu.Game/Utils/ModUtils.cs +++ b/osu.Game/Utils/ModUtils.cs @@ -128,7 +128,7 @@ namespace osu.Game.Utils } /// - /// Checks that all s in a combination are valid as "required mods" in a multiplayer match session. + /// Checks whether the given combination of mods may be set as the required mods of a multiplayer playlist item. /// /// The mods to check. /// Whether freestyle is enabled for the playlist item. @@ -146,11 +146,11 @@ namespace osu.Game.Utils if (!CheckCompatibleSet(mods, out invalidMods)) return false; - return checkValid(mods, m => m.Type != ModType.System && m.HasImplementation && m.ValidForMultiplayer && (!freestyle || m.ValidForFreestyleAsRequiredMod), out invalidMods); + return checkValid(mods, m => IsValidModForMatch(m, MatchType.HeadToHead, true, freestyle), out invalidMods); } /// - /// Checks that all s in a combination are valid as "free mods" in a multiplayer match session. + /// Checks whether the given mods are valid to appear as allowed mods in a multiplayer playlist item. /// /// /// Note that this does not check compatibility between mods, @@ -158,10 +158,11 @@ namespace osu.Game.Utils /// not to be confused with the list of mods the user currently has selected for the multiplayer match. /// /// The mods to check. + /// Whether freestyle is enabled for the playlist item. /// Invalid mods, if any were found. Will be null if all mods were valid. /// Whether the input mods were all valid. If false, will contain all invalid entries. - public static bool CheckValidFreeModsForMultiplayer(IEnumerable mods, [NotNullWhen(false)] out List? invalidMods) - => checkValid(mods, m => m.Type != ModType.System && m.HasImplementation && m.ValidForMultiplayerAsFreeMod && !(m is MultiMod), out invalidMods); + public static bool CheckValidAllowedModsForMultiplayer(IEnumerable mods, bool freestyle, [NotNullWhen(false)] out List? invalidMods) + => checkValid(mods, m => IsValidModForMatch(m, MatchType.HeadToHead, false, freestyle), out invalidMods); private static bool checkValid(IEnumerable mods, Predicate valid, [NotNullWhen(false)] out List? invalidMods) {