From a407e267a238575d1418a94f3d912479a84f0bca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 15 Sep 2019 22:55:25 +0200 Subject: [PATCH] Fix PF/SD legacy mod conversion Upon investigating an user report in #6091 that indicated that viewing replays using the Perfect mod would also display the Sudden Death mod icon despite Perfect being the more restrictive of the two, it turned out that the logic of importing legacy scores was missing that corner case. A similar case of Double Time/Nightcore mutual exclusion was handled, but PF/SD was missed. Add analogous handling of PF/SD legacy mods for all four rulesets, and additionally cover a tiny fraction of all cases with unit tests. The most problematic cases (NC+HD and PF+SD) are covered in all four basic rulesets. --- .../CatchLegacyModConversionTest.cs | 29 +++++++++++++++ osu.Game.Rulesets.Catch/CatchRuleset.cs | 11 +++--- .../ManiaLegacyModConversionTest.cs | 30 ++++++++++++++++ osu.Game.Rulesets.Mania/ManiaRuleset.cs | 11 +++--- .../OsuLegacyModConversionTest.cs | 30 ++++++++++++++++ osu.Game.Rulesets.Osu/OsuRuleset.cs | 11 +++--- .../TaikoLegacyModConversionTest.cs | 29 +++++++++++++++ osu.Game.Rulesets.Taiko/TaikoRuleset.cs | 11 +++--- .../Tests/Beatmaps/LegacyModConversionTest.cs | 35 +++++++++++++++++++ 9 files changed, 173 insertions(+), 24 deletions(-) create mode 100644 osu.Game.Rulesets.Catch.Tests/CatchLegacyModConversionTest.cs create mode 100644 osu.Game.Rulesets.Mania.Tests/ManiaLegacyModConversionTest.cs create mode 100644 osu.Game.Rulesets.Osu.Tests/OsuLegacyModConversionTest.cs create mode 100644 osu.Game.Rulesets.Taiko.Tests/TaikoLegacyModConversionTest.cs create mode 100644 osu.Game/Tests/Beatmaps/LegacyModConversionTest.cs diff --git a/osu.Game.Rulesets.Catch.Tests/CatchLegacyModConversionTest.cs b/osu.Game.Rulesets.Catch.Tests/CatchLegacyModConversionTest.cs new file mode 100644 index 0000000000..04e6dea376 --- /dev/null +++ b/osu.Game.Rulesets.Catch.Tests/CatchLegacyModConversionTest.cs @@ -0,0 +1,29 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps.Legacy; +using osu.Game.Rulesets.Catch.Mods; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Catch.Tests +{ + [TestFixture] + public class CatchLegacyModConversionTest : LegacyModConversionTest + { + [TestCase(LegacyMods.Easy, new[] { typeof(CatchModEasy) })] + [TestCase(LegacyMods.HardRock | LegacyMods.DoubleTime, new[] { typeof(CatchModHardRock), typeof(CatchModDoubleTime) })] + [TestCase(LegacyMods.DoubleTime, new[] { typeof(CatchModDoubleTime) })] + [TestCase(LegacyMods.Nightcore, new[] { typeof(CatchModNightcore) })] + [TestCase(LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(CatchModNightcore) })] + [TestCase(LegacyMods.Flashlight | LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(CatchModFlashlight), typeof(CatchModNightcore) })] + [TestCase(LegacyMods.Perfect, new[] { typeof(CatchModPerfect) })] + [TestCase(LegacyMods.SuddenDeath, new[] { typeof(CatchModSuddenDeath) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath, new[] { typeof(CatchModPerfect) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath | LegacyMods.DoubleTime, new[] { typeof(CatchModDoubleTime), typeof(CatchModPerfect) })] + public new void Test(LegacyMods legacyMods, Type[] expectedMods) => base.Test(legacyMods, expectedMods); + + protected override Ruleset CreateRuleset() => new CatchRuleset(); + } +} diff --git a/osu.Game.Rulesets.Catch/CatchRuleset.cs b/osu.Game.Rulesets.Catch/CatchRuleset.cs index 5428b4eeb8..71d68ace94 100644 --- a/osu.Game.Rulesets.Catch/CatchRuleset.cs +++ b/osu.Game.Rulesets.Catch/CatchRuleset.cs @@ -46,6 +46,11 @@ namespace osu.Game.Rulesets.Catch else if (mods.HasFlag(LegacyMods.DoubleTime)) yield return new CatchModDoubleTime(); + if (mods.HasFlag(LegacyMods.Perfect)) + yield return new CatchModPerfect(); + else if (mods.HasFlag(LegacyMods.SuddenDeath)) + yield return new CatchModSuddenDeath(); + if (mods.HasFlag(LegacyMods.Autoplay)) yield return new CatchModAutoplay(); @@ -67,14 +72,8 @@ namespace osu.Game.Rulesets.Catch if (mods.HasFlag(LegacyMods.NoFail)) yield return new CatchModNoFail(); - if (mods.HasFlag(LegacyMods.Perfect)) - yield return new CatchModPerfect(); - if (mods.HasFlag(LegacyMods.Relax)) yield return new CatchModRelax(); - - if (mods.HasFlag(LegacyMods.SuddenDeath)) - yield return new CatchModSuddenDeath(); } public override IEnumerable GetModsFor(ModType type) diff --git a/osu.Game.Rulesets.Mania.Tests/ManiaLegacyModConversionTest.cs b/osu.Game.Rulesets.Mania.Tests/ManiaLegacyModConversionTest.cs new file mode 100644 index 0000000000..957743c5f1 --- /dev/null +++ b/osu.Game.Rulesets.Mania.Tests/ManiaLegacyModConversionTest.cs @@ -0,0 +1,30 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps.Legacy; +using osu.Game.Rulesets.Mania.Mods; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Mania.Tests +{ + [TestFixture] + public class ManiaLegacyModConversionTest : LegacyModConversionTest + { + [TestCase(LegacyMods.Easy, new[] { typeof(ManiaModEasy) })] + [TestCase(LegacyMods.HardRock | LegacyMods.DoubleTime, new[] { typeof(ManiaModHardRock), typeof(ManiaModDoubleTime) })] + [TestCase(LegacyMods.DoubleTime, new[] { typeof(ManiaModDoubleTime) })] + [TestCase(LegacyMods.Nightcore, new[] { typeof(ManiaModNightcore) })] + [TestCase(LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(ManiaModNightcore) })] + [TestCase(LegacyMods.Flashlight | LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(ManiaModFlashlight), typeof(ManiaModNightcore) })] + [TestCase(LegacyMods.Perfect, new[] { typeof(ManiaModPerfect) })] + [TestCase(LegacyMods.SuddenDeath, new[] { typeof(ManiaModSuddenDeath) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath, new[] { typeof(ManiaModPerfect) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath | LegacyMods.DoubleTime, new[] { typeof(ManiaModDoubleTime), typeof(ManiaModPerfect) })] + [TestCase(LegacyMods.Random | LegacyMods.SuddenDeath, new[] { typeof(ManiaModRandom), typeof(ManiaModSuddenDeath) })] + public new void Test(LegacyMods legacyMods, Type[] expectedMods) => base.Test(legacyMods, expectedMods); + + protected override Ruleset CreateRuleset() => new ManiaRuleset(); + } +} diff --git a/osu.Game.Rulesets.Mania/ManiaRuleset.cs b/osu.Game.Rulesets.Mania/ManiaRuleset.cs index 0c4e7d4858..c74a292331 100644 --- a/osu.Game.Rulesets.Mania/ManiaRuleset.cs +++ b/osu.Game.Rulesets.Mania/ManiaRuleset.cs @@ -46,6 +46,11 @@ namespace osu.Game.Rulesets.Mania else if (mods.HasFlag(LegacyMods.DoubleTime)) yield return new ManiaModDoubleTime(); + if (mods.HasFlag(LegacyMods.Perfect)) + yield return new ManiaModPerfect(); + else if (mods.HasFlag(LegacyMods.SuddenDeath)) + yield return new ManiaModSuddenDeath(); + if (mods.HasFlag(LegacyMods.Autoplay)) yield return new ManiaModAutoplay(); @@ -97,14 +102,8 @@ namespace osu.Game.Rulesets.Mania if (mods.HasFlag(LegacyMods.NoFail)) yield return new ManiaModNoFail(); - if (mods.HasFlag(LegacyMods.Perfect)) - yield return new ManiaModPerfect(); - if (mods.HasFlag(LegacyMods.Random)) yield return new ManiaModRandom(); - - if (mods.HasFlag(LegacyMods.SuddenDeath)) - yield return new ManiaModSuddenDeath(); } public override IEnumerable GetModsFor(ModType type) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuLegacyModConversionTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuLegacyModConversionTest.cs new file mode 100644 index 0000000000..495f2738b5 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/OsuLegacyModConversionTest.cs @@ -0,0 +1,30 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps.Legacy; +using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Osu.Tests +{ + [TestFixture] + public class OsuLegacyModConversionTest : LegacyModConversionTest + { + [TestCase(LegacyMods.Easy, new[] { typeof(OsuModEasy) })] + [TestCase(LegacyMods.HardRock | LegacyMods.DoubleTime, new[] { typeof(OsuModHardRock), typeof(OsuModDoubleTime) })] + [TestCase(LegacyMods.DoubleTime, new[] { typeof(OsuModDoubleTime) })] + [TestCase(LegacyMods.Nightcore, new[] { typeof(OsuModNightcore) })] + [TestCase(LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(OsuModNightcore) })] + [TestCase(LegacyMods.Flashlight | LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(OsuModFlashlight), typeof(OsuModFlashlight) })] + [TestCase(LegacyMods.Perfect, new[] { typeof(OsuModPerfect) })] + [TestCase(LegacyMods.SuddenDeath, new[] { typeof(OsuModSuddenDeath) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath, new[] { typeof(OsuModPerfect) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath | LegacyMods.DoubleTime, new[] { typeof(OsuModDoubleTime), typeof(OsuModPerfect) })] + [TestCase(LegacyMods.SpunOut | LegacyMods.Easy, new[] { typeof(OsuModSpunOut), typeof(OsuModEasy) })] + public new void Test(LegacyMods legacyMods, Type[] expectedMods) => base.Test(legacyMods, expectedMods); + + protected override Ruleset CreateRuleset() => new OsuRuleset(); + } +} diff --git a/osu.Game.Rulesets.Osu/OsuRuleset.cs b/osu.Game.Rulesets.Osu/OsuRuleset.cs index ceb9ed9343..df2ae81a5a 100644 --- a/osu.Game.Rulesets.Osu/OsuRuleset.cs +++ b/osu.Game.Rulesets.Osu/OsuRuleset.cs @@ -52,6 +52,11 @@ namespace osu.Game.Rulesets.Osu else if (mods.HasFlag(LegacyMods.DoubleTime)) yield return new OsuModDoubleTime(); + if (mods.HasFlag(LegacyMods.Perfect)) + yield return new OsuModPerfect(); + else if (mods.HasFlag(LegacyMods.SuddenDeath)) + yield return new OsuModSuddenDeath(); + if (mods.HasFlag(LegacyMods.Autopilot)) yield return new OsuModAutopilot(); @@ -76,18 +81,12 @@ namespace osu.Game.Rulesets.Osu if (mods.HasFlag(LegacyMods.NoFail)) yield return new OsuModNoFail(); - if (mods.HasFlag(LegacyMods.Perfect)) - yield return new OsuModPerfect(); - if (mods.HasFlag(LegacyMods.Relax)) yield return new OsuModRelax(); if (mods.HasFlag(LegacyMods.SpunOut)) yield return new OsuModSpunOut(); - if (mods.HasFlag(LegacyMods.SuddenDeath)) - yield return new OsuModSuddenDeath(); - if (mods.HasFlag(LegacyMods.Target)) yield return new OsuModTarget(); diff --git a/osu.Game.Rulesets.Taiko.Tests/TaikoLegacyModConversionTest.cs b/osu.Game.Rulesets.Taiko.Tests/TaikoLegacyModConversionTest.cs new file mode 100644 index 0000000000..a59544386b --- /dev/null +++ b/osu.Game.Rulesets.Taiko.Tests/TaikoLegacyModConversionTest.cs @@ -0,0 +1,29 @@ +// 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 NUnit.Framework; +using osu.Game.Beatmaps.Legacy; +using osu.Game.Rulesets.Taiko.Mods; +using osu.Game.Tests.Beatmaps; + +namespace osu.Game.Rulesets.Taiko.Tests +{ + [TestFixture] + public class TaikoLegacyModConversionTest : LegacyModConversionTest + { + [TestCase(LegacyMods.Easy, new[] { typeof(TaikoModEasy) })] + [TestCase(LegacyMods.HardRock | LegacyMods.DoubleTime, new[] { typeof(TaikoModHardRock), typeof(TaikoModDoubleTime) })] + [TestCase(LegacyMods.DoubleTime, new[] { typeof(TaikoModDoubleTime) })] + [TestCase(LegacyMods.Nightcore, new[] { typeof(TaikoModNightcore) })] + [TestCase(LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(TaikoModNightcore) })] + [TestCase(LegacyMods.Flashlight | LegacyMods.Nightcore | LegacyMods.DoubleTime, new[] { typeof(TaikoModFlashlight), typeof(TaikoModNightcore) })] + [TestCase(LegacyMods.Perfect, new[] { typeof(TaikoModPerfect) })] + [TestCase(LegacyMods.SuddenDeath, new[] { typeof(TaikoModSuddenDeath) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath, new[] { typeof(TaikoModPerfect) })] + [TestCase(LegacyMods.Perfect | LegacyMods.SuddenDeath | LegacyMods.DoubleTime, new[] { typeof(TaikoModDoubleTime), typeof(TaikoModPerfect) })] + public new void Test(LegacyMods legacyMods, Type[] expectedMods) => base.Test(legacyMods, expectedMods); + + protected override Ruleset CreateRuleset() => new TaikoRuleset(); + } +} diff --git a/osu.Game.Rulesets.Taiko/TaikoRuleset.cs b/osu.Game.Rulesets.Taiko/TaikoRuleset.cs index 7fdb823388..b2655f592c 100644 --- a/osu.Game.Rulesets.Taiko/TaikoRuleset.cs +++ b/osu.Game.Rulesets.Taiko/TaikoRuleset.cs @@ -45,6 +45,11 @@ namespace osu.Game.Rulesets.Taiko else if (mods.HasFlag(LegacyMods.DoubleTime)) yield return new TaikoModDoubleTime(); + if (mods.HasFlag(LegacyMods.Perfect)) + yield return new TaikoModPerfect(); + else if (mods.HasFlag(LegacyMods.SuddenDeath)) + yield return new TaikoModSuddenDeath(); + if (mods.HasFlag(LegacyMods.Autoplay)) yield return new TaikoModAutoplay(); @@ -66,14 +71,8 @@ namespace osu.Game.Rulesets.Taiko if (mods.HasFlag(LegacyMods.NoFail)) yield return new TaikoModNoFail(); - if (mods.HasFlag(LegacyMods.Perfect)) - yield return new TaikoModPerfect(); - if (mods.HasFlag(LegacyMods.Relax)) yield return new TaikoModRelax(); - - if (mods.HasFlag(LegacyMods.SuddenDeath)) - yield return new TaikoModSuddenDeath(); } public override IEnumerable GetModsFor(ModType type) diff --git a/osu.Game/Tests/Beatmaps/LegacyModConversionTest.cs b/osu.Game/Tests/Beatmaps/LegacyModConversionTest.cs new file mode 100644 index 0000000000..e9251f8011 --- /dev/null +++ b/osu.Game/Tests/Beatmaps/LegacyModConversionTest.cs @@ -0,0 +1,35 @@ +// 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.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps.Legacy; +using osu.Game.Rulesets; + +namespace osu.Game.Tests.Beatmaps +{ + /// + /// Base class for tests of converting enumeration flags to ruleset mod instances. + /// + public abstract class LegacyModConversionTest + { + /// + /// Creates the whose legacy mod conversion is to be tested. + /// + /// + protected abstract Ruleset CreateRuleset(); + + protected void Test(LegacyMods legacyMods, Type[] expectedMods) + { + var ruleset = CreateRuleset(); + var mods = ruleset.ConvertLegacyMods(legacyMods).ToList(); + Assert.AreEqual(expectedMods.Length, mods.Count); + + foreach (var modType in expectedMods) + { + Assert.IsNotNull(mods.SingleOrDefault(mod => mod.GetType() == modType)); + } + } + } +}