From 8f9e97b4ccbec6e9de72dba2fca7bfe29e1b7b88 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Mar 2020 18:07:39 +0900 Subject: [PATCH 1/3] Fix carousel not remembering last selection correctly --- osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 045c682dc3..6ce12f7b89 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -104,7 +104,8 @@ namespace osu.Game.Screens.Select.Carousel private void updateSelected(CarouselItem newSelection) { - LastSelected = newSelection; + if (newSelection != null) + LastSelected = newSelection; updateSelectedIndex(); } From 3a50c4bb51bf0377ad51e830656ef2a24c32417f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Mar 2020 18:58:22 +0900 Subject: [PATCH 2/3] Update tests --- .../SongSelect/TestSceneBeatmapCarousel.cs | 86 +++++++++++++++---- 1 file changed, 69 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 8df75c78f5..c2534e2cc7 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -419,7 +419,7 @@ namespace osu.Game.Tests.Visual.SongSelect } [Test] - public void TestCarouselRootIsRandom() + public void TestCarouselRemembersSelection() { List manySets = new List(); @@ -429,12 +429,74 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(manySets); advanceSelection(direction: 1, diff: false); - checkNonmatchingFilter(); - checkNonmatchingFilter(); - checkNonmatchingFilter(); - checkNonmatchingFilter(); - checkNonmatchingFilter(); - AddAssert("Selection was random", () => eagerSelectedIDs.Count > 1); + + for (int i = 0; i < 5; i++) + { + AddStep("Toggle non-matching filter", () => + { + carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false); + }); + + AddStep("Restore no filter", () => + { + carousel.Filter(new FilterCriteria(), false); + eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID); + }); + } + + // always returns to same selection as long as it's available. + AddAssert("Selection was remembered", () => eagerSelectedIDs.Count == 1); + } + + [Test] + public void TestRandomFallbackOnNonMatchingPrevious() + { + List manySets = new List(); + + AddStep("populate maps", () => + { + for (int i = 0; i < 10; i++) + { + var set = createTestBeatmapSet(i); + + foreach (var b in set.Beatmaps) + { + // all taiko except for first + int ruleset = i > 0 ? 1 : 0; + + b.Ruleset = rulesets.GetRuleset(ruleset); + b.RulesetID = ruleset; + } + + manySets.Add(set); + } + }); + + loadBeatmaps(manySets); + + for (int i = 0; i < 10; i++) + { + AddStep("Reset filter", () => carousel.Filter(new FilterCriteria(), false)); + + AddStep("select first beatmap", () => carousel.SelectBeatmap(manySets.First().Beatmaps.First())); + + AddStep("Toggle non-matching filter", () => + { + carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false); + }); + + AddAssert("selection lost", () => carousel.SelectedBeatmap == null); + + AddStep("Restore different ruleset filter", () => + { + carousel.Filter(new FilterCriteria { Ruleset = rulesets.GetRuleset(1) }, false); + eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID); + }); + + AddAssert("selection changed", () => carousel.SelectedBeatmap != manySets.First().Beatmaps.First()); + } + + AddAssert("Selection was random", () => eagerSelectedIDs.Count > 2); } [Test] @@ -593,16 +655,6 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("Selection is visible", selectedBeatmapVisible); } - private void checkNonmatchingFilter() - { - AddStep("Toggle non-matching filter", () => - { - carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false); - carousel.Filter(new FilterCriteria(), false); - eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID); - }); - } - private BeatmapSetInfo createTestBeatmapSet(int id) { return new BeatmapSetInfo From be4a97c2894282c794c763f269dcdb991aacf512 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Mar 2020 13:01:24 +0900 Subject: [PATCH 3/3] Correctly bypass last selected when it is filtered --- osu.Game/Screens/Select/BeatmapCarousel.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 9f8b201eff..389ae918b9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -333,8 +333,7 @@ namespace osu.Game.Screens.Select else set = visibleSets.ElementAt(RNG.Next(visibleSets.Count)); - var visibleBeatmaps = set.Beatmaps.Where(s => !s.Filtered.Value).ToList(); - select(visibleBeatmaps[RNG.Next(visibleBeatmaps.Count)]); + select(set); return true; } @@ -756,7 +755,7 @@ namespace osu.Game.Screens.Select protected override void PerformSelection() { - if (LastSelected == null) + if (LastSelected == null || LastSelected.Filtered.Value) carousel.SelectNextRandom(); else base.PerformSelection();