From fe7df808b6065dcbb405f7775fdaaa0d5a441f52 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 15 Apr 2024 21:07:08 +0900 Subject: [PATCH 1/2] Add tests --- .../SongSelect/TestScenePlaySongSelect.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index ce241f3676..e03ffd48f1 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -29,6 +29,7 @@ using osu.Game.Overlays.Dialog; using osu.Game.Overlays.Mods; using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; +using osu.Game.Rulesets.Mania.Mods; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; @@ -1147,6 +1148,62 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("filter text cleared", () => songSelect!.FilterControl.ChildrenOfType().First().Text, () => Is.Empty); } + [Test] + public void TestNonFilterableModChange() + { + addRulesetImportStep(0); + + createSongSelect(); + + // Mod that is guaranteed to never re-filter. + AddStep("add non-filterable mod", () => SelectedMods.Value = new Mod[] { new OsuModCinema() }); + AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); + + // Removing the mod should still not re-filter. + AddStep("remove non-filterable mod", () => SelectedMods.Value = Array.Empty()); + AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); + } + + [Test] + public void TestFilterableModChange() + { + addRulesetImportStep(3); + + createSongSelect(); + + // Change to mania ruleset. + AddStep("filter to mania ruleset", () => Ruleset.Value = rulesets.AvailableRulesets.First(r => r.OnlineID == 3)); + AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(2)); + + // Apply a mod, but this should NOT re-filter because there's no search text. + AddStep("add filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey3() }); + AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(2)); + + // Set search text. Should re-filter. + AddStep("set search text to match mods", () => songSelect!.FilterControl.CurrentTextSearch.Value = "keys=3"); + AddAssert("filter count is 3", () => songSelect!.FilterCount, () => Is.EqualTo(3)); + + // Change filterable mod. Should re-filter. + AddStep("change new filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey5() }); + AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); + + // Add non-filterable mod. Should NOT re-filter. + AddStep("apply non-filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModNoFail(), new ManiaModKey5() }); + AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); + + // Remove filterable mod. Should re-filter. + AddStep("remove filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModNoFail() }); + AddAssert("filter count is 5", () => songSelect!.FilterCount, () => Is.EqualTo(5)); + + // Remove non-filterable mod. Should NOT re-filter. + AddStep("remove filterable mod", () => SelectedMods.Value = Array.Empty()); + AddAssert("filter count is 5", () => songSelect!.FilterCount, () => Is.EqualTo(5)); + + // Add filterable mod. Should re-filter. + AddStep("add filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey3() }); + AddAssert("filter count is 6", () => songSelect!.FilterCount, () => Is.EqualTo(6)); + } + private void waitForInitialSelection() { AddUntilStep("wait for initial selection", () => !Beatmap.IsDefault); From 343b3ba0e614dcfd68e9f3b6046b7d119776c95e Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 15 Apr 2024 21:07:36 +0900 Subject: [PATCH 2/2] Don't re-filter unless mods may change the filter --- .../ManiaFilterCriteria.cs | 20 +++++++++++++++++++ .../NonVisual/Filtering/FilterMatchingTest.cs | 5 +++++ .../Filtering/FilterQueryParserTest.cs | 5 +++++ .../Rulesets/Filter/IRulesetFilterCriteria.cs | 10 ++++++++++ osu.Game/Screens/Select/FilterControl.cs | 13 ++++++------ 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/osu.Game.Rulesets.Mania/ManiaFilterCriteria.cs b/osu.Game.Rulesets.Mania/ManiaFilterCriteria.cs index ea7eb5b8f0..8c6efbc72d 100644 --- a/osu.Game.Rulesets.Mania/ManiaFilterCriteria.cs +++ b/osu.Game.Rulesets.Mania/ManiaFilterCriteria.cs @@ -1,9 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; +using System.Linq; +using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Rulesets.Filter; using osu.Game.Rulesets.Mania.Beatmaps; +using osu.Game.Rulesets.Mania.Mods; +using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Scoring.Legacy; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; @@ -30,5 +35,20 @@ namespace osu.Game.Rulesets.Mania return false; } + + public bool FilterMayChangeFromMods(ValueChangedEvent> mods) + { + if (keys.HasFilter) + { + // Interpreting as the Mod type is required for equality comparison. + HashSet oldSet = mods.OldValue.OfType().AsEnumerable().ToHashSet(); + HashSet newSet = mods.NewValue.OfType().AsEnumerable().ToHashSet(); + + if (!oldSet.SetEquals(newSet)) + return true; + } + + return false; + } } } diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs index 78d8eabba7..10e0e46f4c 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterMatchingTest.cs @@ -1,10 +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.Collections.Generic; using NUnit.Framework; +using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Rulesets; using osu.Game.Rulesets.Filter; +using osu.Game.Rulesets.Mods; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; using osu.Game.Screens.Select.Filter; @@ -311,6 +314,8 @@ namespace osu.Game.Tests.NonVisual.Filtering public bool Matches(BeatmapInfo beatmapInfo, FilterCriteria criteria) => match; public bool TryParseCustomKeywordCriteria(string key, Operator op, string value) => false; + + public bool FilterMayChangeFromMods(ValueChangedEvent> mods) => false; } } } diff --git a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs index b0ceed45b9..7897b3d8c0 100644 --- a/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs +++ b/osu.Game.Tests/NonVisual/Filtering/FilterQueryParserTest.cs @@ -2,10 +2,13 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using NUnit.Framework; +using osu.Framework.Bindables; using osu.Game.Beatmaps; using osu.Game.Rulesets.Filter; +using osu.Game.Rulesets.Mods; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; using osu.Game.Screens.Select.Filter; @@ -514,6 +517,8 @@ namespace osu.Game.Tests.NonVisual.Filtering return false; } + + public bool FilterMayChangeFromMods(ValueChangedEvent> mods) => false; } private static readonly object[] correct_date_query_examples = diff --git a/osu.Game/Rulesets/Filter/IRulesetFilterCriteria.cs b/osu.Game/Rulesets/Filter/IRulesetFilterCriteria.cs index f926b04db4..c374fe315d 100644 --- a/osu.Game/Rulesets/Filter/IRulesetFilterCriteria.cs +++ b/osu.Game/Rulesets/Filter/IRulesetFilterCriteria.cs @@ -1,7 +1,10 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Collections.Generic; +using osu.Framework.Bindables; using osu.Game.Beatmaps; +using osu.Game.Rulesets.Mods; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Filter; @@ -52,5 +55,12 @@ namespace osu.Game.Rulesets.Filter /// while ignored criteria are included in . /// bool TryParseCustomKeywordCriteria(string key, Operator op, string value); + + /// + /// Whether to reapply the filter as a result of the given change in applied mods. + /// + /// The change in mods. + /// Whether the filter should be re-applied. + bool FilterMayChangeFromMods(ValueChangedEvent> mods); } } diff --git a/osu.Game/Screens/Select/FilterControl.cs b/osu.Game/Screens/Select/FilterControl.cs index 0bfd927234..73c122dda6 100644 --- a/osu.Game/Screens/Select/FilterControl.cs +++ b/osu.Game/Screens/Select/FilterControl.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -49,15 +50,14 @@ namespace osu.Game.Screens.Select } private OsuTabControl sortTabs; - private Bindable sortMode; - private Bindable groupMode; - private FilterControlTextBox searchTextBox; - private CollectionDropdown collectionDropdown; + [CanBeNull] + private FilterCriteria currentCriteria; + public FilterCriteria CreateCriteria() { string query = searchTextBox.Text; @@ -228,7 +228,8 @@ namespace osu.Game.Screens.Select if (m.NewValue.SequenceEqual(m.OldValue)) return; - updateCriteria(); + if (currentCriteria?.RulesetCriteria?.FilterMayChangeFromMods(m) == true) + updateCriteria(); }); groupMode.BindValueChanged(_ => updateCriteria()); @@ -263,7 +264,7 @@ namespace osu.Game.Screens.Select private readonly Bindable minimumStars = new BindableDouble(); private readonly Bindable maximumStars = new BindableDouble(); - private void updateCriteria() => FilterChanged?.Invoke(CreateCriteria()); + private void updateCriteria() => FilterChanged?.Invoke(currentCriteria = CreateCriteria()); protected override bool OnClick(ClickEvent e) => true;