From 63edcddaf13453d66e86d4368e63ed90d2636962 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 12 Mar 2020 15:01:43 +0900 Subject: [PATCH 1/5] Apply ruleset filter in all cases (even when bypassing filter for selection purposes) --- .../Screens/Select/Carousel/CarouselBeatmap.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 8c264ce974..e0d59e3b18 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -25,18 +25,18 @@ namespace osu.Game.Screens.Select.Carousel { base.Filter(criteria); - if (Beatmap.BeatmapSet?.Equals(criteria.SelectedBeatmapSet) == true) - { - // bypass filtering for selected beatmap - Filtered.Value = false; - return; - } - bool match = criteria.Ruleset == null || Beatmap.RulesetID == criteria.Ruleset.ID || (Beatmap.RulesetID == 0 && criteria.Ruleset.ID > 0 && criteria.AllowConvertedBeatmaps); + if (Beatmap.BeatmapSet?.Equals(criteria.SelectedBeatmapSet) == true) + { + // bypass filtering for selected beatmap + Filtered.Value = !match; + return; + } + match &= !criteria.StarDifficulty.HasFilter || criteria.StarDifficulty.IsInRange(Beatmap.StarDifficulty); match &= !criteria.ApproachRate.HasFilter || criteria.ApproachRate.IsInRange(Beatmap.BaseDifficulty.ApproachRate); match &= !criteria.DrainRate.HasFilter || criteria.DrainRate.IsInRange(Beatmap.BaseDifficulty.DrainRate); From 933a8ffc8a8152e6e5680d39557bd3e2958f407b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 12 Mar 2020 17:10:51 +0900 Subject: [PATCH 2/5] Add test coverage --- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 16 +++++++++++++++- .../Carousel/DrawableCarouselBeatmapSet.cs | 8 +++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 105d96cdfe..fb287a4f70 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -436,6 +436,9 @@ namespace osu.Game.Tests.Visual.SongSelect changeRuleset(0); + // used for filter check below + AddStep("allow convert display", () => config.Set(OsuSetting.ShowConvertedBeatmaps, true)); + AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmap != null); AddStep("set filter text", () => songSelect.FilterControl.ChildrenOfType().First().Text = "nonono"); @@ -446,9 +449,11 @@ namespace osu.Game.Tests.Visual.SongSelect BeatmapInfo target = null; + int targetRuleset = differentRuleset ? 1 : 0; + AddStep("select beatmap externally", () => { - target = manager.GetAllUsableBeatmapSets().Where(b => b.Beatmaps.Any(bi => bi.RulesetID == (differentRuleset ? 1 : 0))) + target = manager.GetAllUsableBeatmapSets().Where(b => b.Beatmaps.Any(bi => bi.RulesetID == targetRuleset)) .ElementAt(5).Beatmaps.First(); Beatmap.Value = manager.GetWorkingBeatmap(target); @@ -456,6 +461,15 @@ namespace osu.Game.Tests.Visual.SongSelect AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmap != null); + AddAssert("selected only shows expected ruleset (plus converts)", () => + { + var selectedPanel = songSelect.Carousel.ChildrenOfType().First(s => s.Item.State.Value == CarouselItemState.Selected); + + // special case for converts checked here. + return selectedPanel.ChildrenOfType().All(i => + i.IsFiltered || i.Item.Beatmap.Ruleset.ID == targetRuleset || i.Item.Beatmap.Ruleset.ID == 0); + }); + AddUntilStep("carousel has correct", () => songSelect.Carousel.SelectedBeatmap?.OnlineBeatmapID == target.OnlineBeatmapID); AddUntilStep("game has correct", () => Beatmap.Value.BeatmapInfo.OnlineBeatmapID == target.OnlineBeatmapID); diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index 6cd145cfef..1454784f03 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -205,7 +205,9 @@ namespace osu.Game.Screens.Select.Carousel { private readonly BindableBool filtered = new BindableBool(); - private readonly CarouselBeatmap item; + public bool IsFiltered => filtered.Value; + + public readonly CarouselBeatmap Item; public FilterableDifficultyIcon(CarouselBeatmap item) : base(item.Beatmap) @@ -214,13 +216,13 @@ namespace osu.Game.Screens.Select.Carousel filtered.ValueChanged += isFiltered => Schedule(() => this.FadeTo(isFiltered.NewValue ? 0.1f : 1, 100)); filtered.TriggerChange(); - this.item = item; + this.Item = item; } protected override bool OnClick(ClickEvent e) { if (!filtered.Value) - item.State.Value = CarouselItemState.Selected; + Item.State.Value = CarouselItemState.Selected; return true; } From fc058f8896eb8f023b6e7702d21355167f1a78b4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 12 Mar 2020 18:03:18 +0900 Subject: [PATCH 3/5] Remove unnecessary this. prefix --- osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index 1454784f03..547aeaddc6 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -216,7 +216,7 @@ namespace osu.Game.Screens.Select.Carousel filtered.ValueChanged += isFiltered => Schedule(() => this.FadeTo(isFiltered.NewValue ? 0.1f : 1, 100)); filtered.TriggerChange(); - this.Item = item; + Item = item; } protected override bool OnClick(ClickEvent e) From 5537b279de1b3707496f4bf8aac49aa359f13cbc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 12 Mar 2020 18:39:43 +0900 Subject: [PATCH 4/5] Fix failing test occasionally getting wrong ruleset beatmap --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index fb287a4f70..55c1d8451f 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -453,8 +453,9 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("select beatmap externally", () => { - target = manager.GetAllUsableBeatmapSets().Where(b => b.Beatmaps.Any(bi => bi.RulesetID == targetRuleset)) - .ElementAt(5).Beatmaps.First(); + target = manager.GetAllUsableBeatmapSets() + .Where(b => b.Beatmaps.Any(bi => bi.RulesetID == targetRuleset)) + .ElementAt(5).Beatmaps.First(bi => bi.RulesetID == targetRuleset); Beatmap.Value = manager.GetWorkingBeatmap(target); }); From 3f8b454ff4783b896516506b4ef192cda78134f2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 13 Mar 2020 10:01:28 +0900 Subject: [PATCH 5/5] Reword comment to match new filtering behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bartłomiej Dach --- osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index e0d59e3b18..6d760df065 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -32,7 +32,7 @@ namespace osu.Game.Screens.Select.Carousel if (Beatmap.BeatmapSet?.Equals(criteria.SelectedBeatmapSet) == true) { - // bypass filtering for selected beatmap + // only check ruleset equality or convertability for selected beatmap Filtered.Value = !match; return; }