From 666d9b153cd583d6ae8915ae4e52832ddc7107aa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 May 2025 16:00:20 +0900 Subject: [PATCH 1/4] Add failing test showing double filter on entering song select --- .../Visual/SongSelectV2/BeatmapCarouselTestScene.cs | 2 +- .../Visual/SongSelectV2/TestSceneSongSelect.cs | 9 +++++++++ osu.Game/Graphics/Carousel/Carousel.cs | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index f9f7f3e89c..da339bbc3e 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -356,7 +356,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 """); createHeader("carousel"); stats.AddParagraph($""" - sorting: {Carousel.IsFiltering} + filtering: {Carousel.IsFiltering} (total {Carousel.FilterCount} times) tracked: {Carousel.ItemsTracked} displayable: {Carousel.DisplayableItems} displayed: {Carousel.VisibleItems} diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs index b81484d3da..dcd745395b 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs @@ -78,6 +78,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddUntilStep("wait for results screen", () => Stack.CurrentScreen is ResultsScreen); } + [Test] + public void TestSingleFilterWhenEntering() + { + ImportBeatmapForRuleset(0); + LoadSongSelect(); + + AddAssert("single filter", () => Carousel.FilterCount, () => Is.EqualTo(1)); + } + [Test] public void TestCookieDoesNothingIfNothingSelected() { diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index 9da4d0e187..ce5d015775 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -71,6 +71,11 @@ namespace osu.Game.Graphics.Carousel /// public bool IsFiltering => !filterTask.IsCompleted; + /// + /// The number of times filter operations have been triggered. + /// + internal int FilterCount { get; private set; } + /// /// The number of displayable items currently being tracked (before filtering). /// @@ -187,6 +192,8 @@ namespace osu.Game.Graphics.Carousel /// Whether all existing drawable panels should be reset post filter. protected virtual Task> FilterAsync(bool clearExistingPanels = false) { + FilterCount++; + if (clearExistingPanels) filterReusesPanels.Invalidate(); From bb938be0b6e2d43ced4637d961fa8251bcf4916d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 May 2025 03:02:33 +0900 Subject: [PATCH 2/4] Fix carousel filtering twice on entering song select --- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 17 ++++++++++++++++- osu.Game/Screens/SelectV2/SongSelect.cs | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index e007ae54ce..32e14ad43a 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -7,6 +7,7 @@ using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using System.Threading; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; @@ -49,6 +50,8 @@ namespace osu.Game.Screens.SelectV2 private readonly BeatmapCarouselFilterMatching matching; private readonly BeatmapCarouselFilterGrouping grouping; + private bool waitingForInitialCriteria; + /// /// Total number of beatmap difficulties displayed with the filter. /// @@ -67,8 +70,10 @@ namespace osu.Game.Screens.SelectV2 return -SPACING; } - public BeatmapCarousel() + public BeatmapCarousel(bool waitForInitialCriteria = false) { + waitingForInitialCriteria = waitForInitialCriteria; + DebounceDelay = 100; DistanceOffscreenToPreload = 100; @@ -473,6 +478,8 @@ namespace osu.Game.Screens.SelectV2 public void Filter(FilterCriteria criteria) { + waitingForInitialCriteria = false; + bool resetDisplay = grouping.BeatmapSetsGroupedTogether != BeatmapCarouselFilterGrouping.ShouldGroupBeatmapsTogether(criteria); Criteria = criteria; @@ -493,6 +500,14 @@ namespace osu.Game.Screens.SelectV2 })); } + protected override Task> FilterAsync(bool clearExistingPanels = false) + { + if (waitingForInitialCriteria) + return Task.FromResult(Enumerable.Empty()); + + return base.FilterAsync(clearExistingPanels); + } + #endregion #region Drawable pooling diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index 1056c2ff71..59dae1f28a 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -208,7 +208,7 @@ namespace osu.Game.Screens.SelectV2 }, Children = new Drawable[] { - carousel = new BeatmapCarousel + carousel = new BeatmapCarousel(waitForInitialCriteria: true) { BleedTop = FilterControl.HEIGHT_FROM_SCREEN_TOP + 5, BleedBottom = ScreenFooter.HEIGHT + 5, From 5222c3013976837654653bf24481410ee5c7a1b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 May 2025 18:07:37 +0900 Subject: [PATCH 3/4] Fix unintentional initial filter delay on entering song select --- osu.Game/Screens/SelectV2/SongSelect.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index 59dae1f28a..ebc6adeb93 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -548,8 +548,11 @@ namespace osu.Game.Screens.SelectV2 private void criteriaChanged(FilterCriteria criteria) { + // The first filter needs to be applied immediately as this triggers the initial carousel load. + double filterDelay = filterDebounce == null ? 0 : filter_delay; + filterDebounce?.Cancel(); - filterDebounce = Scheduler.AddDelayed(() => { carousel.Filter(criteria); }, filter_delay); + filterDebounce = Scheduler.AddDelayed(() => { carousel.Filter(criteria); }, filterDelay); } private void newItemsPresented(IEnumerable carouselItems) From 5bf235f2dcb908ccd039577104e1f6a96c056564 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 May 2025 18:15:27 +0900 Subject: [PATCH 4/4] Use nullable `Criteria` instead of `ctor` flag --- .../SongSelectV2/BeatmapCarouselTestScene.cs | 4 +++- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 18 ++++++------------ osu.Game/Screens/SelectV2/SongSelect.cs | 5 ++++- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs index da339bbc3e..d8f173aed2 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs @@ -149,6 +149,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 TextAnchor = Anchor.CentreLeft, }, }; + + Carousel.Filter(new FilterCriteria()); }); // Prefer title sorting so that order of carousel panels match order of BeatmapSets bindable. @@ -171,7 +173,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { AddStep(description, () => { - var criteria = Carousel.Criteria; + var criteria = Carousel.Criteria ?? new FilterCriteria(); apply?.Invoke(criteria); Carousel.Filter(criteria); }); diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 32e14ad43a..d3c5fbadf7 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -50,8 +50,6 @@ namespace osu.Game.Screens.SelectV2 private readonly BeatmapCarouselFilterMatching matching; private readonly BeatmapCarouselFilterGrouping grouping; - private bool waitingForInitialCriteria; - /// /// Total number of beatmap difficulties displayed with the filter. /// @@ -70,18 +68,16 @@ namespace osu.Game.Screens.SelectV2 return -SPACING; } - public BeatmapCarousel(bool waitForInitialCriteria = false) + public BeatmapCarousel() { - waitingForInitialCriteria = waitForInitialCriteria; - DebounceDelay = 100; DistanceOffscreenToPreload = 100; Filters = new ICarouselFilter[] { - matching = new BeatmapCarouselFilterMatching(() => Criteria), - new BeatmapCarouselFilterSorting(() => Criteria), - grouping = new BeatmapCarouselFilterGrouping(() => Criteria), + matching = new BeatmapCarouselFilterMatching(() => Criteria!), + new BeatmapCarouselFilterSorting(() => Criteria!), + grouping = new BeatmapCarouselFilterGrouping(() => Criteria!), }; AddInternal(loading = new LoadingLayer()); @@ -472,14 +468,12 @@ namespace osu.Game.Screens.SelectV2 #region Filtering - public FilterCriteria Criteria { get; private set; } = new FilterCriteria(); + public FilterCriteria? Criteria { get; private set; } private ScheduledDelegate? loadingDebounce; public void Filter(FilterCriteria criteria) { - waitingForInitialCriteria = false; - bool resetDisplay = grouping.BeatmapSetsGroupedTogether != BeatmapCarouselFilterGrouping.ShouldGroupBeatmapsTogether(criteria); Criteria = criteria; @@ -502,7 +496,7 @@ namespace osu.Game.Screens.SelectV2 protected override Task> FilterAsync(bool clearExistingPanels = false) { - if (waitingForInitialCriteria) + if (Criteria == null) return Task.FromResult(Enumerable.Empty()); return base.FilterAsync(clearExistingPanels); diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs index ebc6adeb93..279cf9e3b1 100644 --- a/osu.Game/Screens/SelectV2/SongSelect.cs +++ b/osu.Game/Screens/SelectV2/SongSelect.cs @@ -208,7 +208,7 @@ namespace osu.Game.Screens.SelectV2 }, Children = new Drawable[] { - carousel = new BeatmapCarousel(waitForInitialCriteria: true) + carousel = new BeatmapCarousel { BleedTop = FilterControl.HEIGHT_FROM_SCREEN_TOP + 5, BleedBottom = ScreenFooter.HEIGHT + 5, @@ -557,6 +557,9 @@ namespace osu.Game.Screens.SelectV2 private void newItemsPresented(IEnumerable carouselItems) { + if (carousel.Criteria == null) + return; + int count = carousel.MatchedBeatmapsCount; if (count == 0)