From 3e5e717fce3a47983793f60ef8147aa624178b84 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 14 Oct 2022 01:59:55 +0300 Subject: [PATCH 1/5] Add failing test cases --- .../TestSceneModSelectOverlay.cs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs index 4c43a2fdcd..0292ce5905 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs @@ -17,6 +17,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Overlays.Mods; using osu.Game.Overlays.Settings; using osu.Game.Rulesets; +using osu.Game.Rulesets.Catch.Mods; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; @@ -338,26 +339,36 @@ namespace osu.Game.Tests.Visual.UserInterface } [Test] - public void TestRulesetChanges() + public void TestCommonModsMaintainedOnRulesetChange() { createScreen(); changeRuleset(0); - var noFailMod = new OsuRuleset().GetModsFor(ModType.DifficultyReduction).FirstOrDefault(m => m is OsuModNoFail); - - AddStep("set mods externally", () => { SelectedMods.Value = new[] { noFailMod }; }); + AddStep("select relax mod", () => SelectedMods.Value = new[] { Ruleset.Value.CreateInstance().CreateMod() }); changeRuleset(0); + AddAssert("ensure mod still selected", () => SelectedMods.Value.SingleOrDefault() is OsuModRelax); - AddAssert("ensure mods still selected", () => SelectedMods.Value.SingleOrDefault(m => m is OsuModNoFail) != null); + changeRuleset(2); + AddAssert("catch variant selected", () => SelectedMods.Value.SingleOrDefault() is CatchModRelax); changeRuleset(3); + AddAssert("no mod selected", () => SelectedMods.Value.Count == 0); + } - AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0); - + [Test] + public void TestUncommonModsDiscardedOnRulesetChange() + { + createScreen(); changeRuleset(0); - AddAssert("ensure mods not selected", () => SelectedMods.Value.Count == 0); + AddStep("select single tap mod", () => SelectedMods.Value = new[] { new OsuModSingleTap() }); + + changeRuleset(0); + AddAssert("ensure mod still selected", () => SelectedMods.Value.SingleOrDefault() is OsuModSingleTap); + + changeRuleset(3); + AddAssert("no mod selected", () => SelectedMods.Value.Count == 0); } [Test] From 739b21ab3bb3245910ce1115e6f6003fa589e2c1 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 14 Oct 2022 02:02:24 +0300 Subject: [PATCH 2/5] Maintain mod selection on ruleset change for common mods --- osu.Game/OsuGameBase.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 478f154d58..662d165580 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -586,11 +586,16 @@ namespace osu.Game return; } + var previouslySelectedMods = SelectedMods.Value.ToArray(); + if (!SelectedMods.Disabled) SelectedMods.Value = Array.Empty(); AvailableMods.Value = dict; + if (!SelectedMods.Disabled) + SelectedMods.Value = previouslySelectedMods.Select(m => instance.CreateModFromAcronym(m.Acronym)).Where(m => m != null).ToArray(); + void revertRulesetChange() => Ruleset.Value = r.OldValue?.Available == true ? r.OldValue : RulesetStore.AvailableRulesets.First(); } From 01c65d3cc1281da68e501506785724ac5e2f9f58 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 14 Oct 2022 02:16:23 +0300 Subject: [PATCH 3/5] Remove seemingly unnecessary/leftover code --- osu.Game/Screens/Select/SongSelect.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index ece94b5365..f5a058e945 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -502,8 +502,6 @@ namespace osu.Game.Screens.Select if (transferRulesetValue()) { - Mods.Value = Array.Empty(); - // transferRulesetValue() may trigger a re-filter. If the current selection does not match the new ruleset, we want to switch away from it. // The default logic on WorkingBeatmap change is to switch to a matching ruleset (see workingBeatmapChanged()), but we don't want that here. // We perform an early selection attempt and clear out the beatmap selection to avoid a second ruleset change (revert). From 7d53d35bf619aa209391edd773d5c594d7bce15d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 Oct 2022 16:23:54 +0300 Subject: [PATCH 4/5] Remove duplicate & outdated test case --- .../SongSelect/TestScenePlaySongSelect.cs | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index cc8746959b..248bf9f5ed 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -538,36 +538,6 @@ namespace osu.Game.Tests.Visual.SongSelect AddUntilStep("selection shown on wedge", () => songSelect!.CurrentBeatmapDetailsBeatmap.BeatmapInfo.MatchesOnlineID(target)); } - [Test] - public void TestRulesetChangeResetsMods() - { - createSongSelect(); - changeRuleset(0); - - changeMods(new OsuModHardRock()); - - int actionIndex = 0; - int modChangeIndex = 0; - int rulesetChangeIndex = 0; - - AddStep("change ruleset", () => - { - SelectedMods.ValueChanged += onModChange; - songSelect!.Ruleset.ValueChanged += onRulesetChange; - - Ruleset.Value = new TaikoRuleset().RulesetInfo; - - SelectedMods.ValueChanged -= onModChange; - songSelect!.Ruleset.ValueChanged -= onRulesetChange; - }); - - AddAssert("mods changed before ruleset", () => modChangeIndex < rulesetChangeIndex); - AddAssert("empty mods", () => !SelectedMods.Value.Any()); - - void onModChange(ValueChangedEvent> e) => modChangeIndex = actionIndex++; - void onRulesetChange(ValueChangedEvent e) => rulesetChangeIndex = actionIndex++; - } - [Test] public void TestModsRetainedBetweenSongSelect() { From 841e20c3363ebe2dd5e9fd0c408d7342ca78273d Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 15 Oct 2022 17:16:08 +0300 Subject: [PATCH 5/5] Remove unused usings --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 248bf9f5ed..63532fdba8 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Threading.Tasks; @@ -32,7 +31,6 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; -using osu.Game.Rulesets.Taiko; using osu.Game.Scoring; using osu.Game.Screens.Play; using osu.Game.Screens.Select;