From bca1be0bfa69c0c6c6e5bb1a756eb573f1e0d0e0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Nov 2019 18:54:33 +0900 Subject: [PATCH 1/7] Add failing test --- .../SongSelect/TestScenePlaySongSelect.cs | 53 +++++++++++++------ osu.Game/Screens/Select/SongSelect.cs | 4 +- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 794d135b06..2d1a786bb9 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -57,23 +57,6 @@ namespace osu.Game.Tests.Visual.SongSelect typeof(DrawableCarouselBeatmapSet), }; - private class TestSongSelect : PlaySongSelect - { - public Action StartRequested; - - public new Bindable Ruleset => base.Ruleset; - - public WorkingBeatmap CurrentBeatmap => Beatmap.Value; - public WorkingBeatmap CurrentBeatmapDetailsBeatmap => BeatmapDetails.Beatmap; - public new BeatmapCarousel Carousel => base.Carousel; - - protected override bool OnStart() - { - StartRequested?.Invoke(); - return base.OnStart(); - } - } - private TestSongSelect songSelect; [BackgroundDependencyLoader] @@ -101,6 +84,17 @@ namespace osu.Game.Tests.Visual.SongSelect manager?.Delete(manager.GetAllUsableBeatmapSets()); }); + [Test] + public void TestSingleFilterOnEnter() + { + addRulesetImportStep(0); + addRulesetImportStep(0); + + createSongSelect(); + + AddAssert("filter count is 1", () => songSelect.FilterCount == 1); + } + [Test] public void TestAudioResuming() { @@ -357,5 +351,30 @@ namespace osu.Game.Tests.Visual.SongSelect base.Dispose(isDisposing); rulesets?.Dispose(); } + + private class TestSongSelect : PlaySongSelect + { + public Action StartRequested; + + public new Bindable Ruleset => base.Ruleset; + + public WorkingBeatmap CurrentBeatmap => Beatmap.Value; + public WorkingBeatmap CurrentBeatmapDetailsBeatmap => BeatmapDetails.Beatmap; + public new BeatmapCarousel Carousel => base.Carousel; + + protected override bool OnStart() + { + StartRequested?.Invoke(); + return base.OnStart(); + } + + public int FilterCount; + + protected override void ApplyFilterToCarousel(FilterCriteria criteria) + { + FilterCount++; + base.ApplyFilterToCarousel(criteria); + } + } } } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 375b994169..6810f77b93 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -165,7 +165,7 @@ namespace osu.Game.Screens.Select { RelativeSizeAxes = Axes.X, Height = FilterControl.HEIGHT, - FilterChanged = c => Carousel.Filter(c), + FilterChanged = ApplyFilterToCarousel, Background = { Width = 2 }, }, } @@ -217,6 +217,8 @@ namespace osu.Game.Screens.Select BeatmapDetails.Leaderboard.ScoreSelected += score => this.Push(new SoloResults(score)); } + protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); + [BackgroundDependencyLoader(true)] private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) { From 280c1a0eb42b16954c4ffd98257bfb7e92ec0440 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Nov 2019 18:00:45 +0900 Subject: [PATCH 2/7] Fix carousel filtering twice on startup due to unpopulated ruleset --- osu.Game/Screens/Select/SongSelect.cs | 37 +++++++++++++++------------ 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 6810f77b93..269fb1b683 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -46,40 +46,40 @@ namespace osu.Game.Screens.Select protected const float BACKGROUND_BLUR = 20; private const float left_area_padding = 20; - public readonly FilterControl FilterControl; + public FilterControl FilterControl; protected virtual bool ShowFooter => true; /// /// Can be null if is false. /// - protected readonly BeatmapOptionsOverlay BeatmapOptions; + protected BeatmapOptionsOverlay BeatmapOptions; /// /// Can be null if is false. /// - protected readonly Footer Footer; + protected Footer Footer; /// /// Contains any panel which is triggered by a footer button. /// Helps keep them located beneath the footer itself. /// - protected readonly Container FooterPanels; + protected Container FooterPanels; protected override BackgroundScreen CreateBackground() => new BackgroundScreenBeatmap(); - protected readonly BeatmapCarousel Carousel; - private readonly BeatmapInfoWedge beatmapInfoWedge; + protected BeatmapCarousel Carousel; + private BeatmapInfoWedge beatmapInfoWedge; private DialogOverlay dialogOverlay; private BeatmapManager beatmaps; - protected readonly ModSelectOverlay ModSelect; + protected ModSelectOverlay ModSelect; protected SampleChannel SampleConfirm; private SampleChannel sampleChangeDifficulty; private SampleChannel sampleChangeBeatmap; - protected readonly BeatmapDetailArea BeatmapDetails; + protected BeatmapDetailArea BeatmapDetails; private readonly Bindable decoupledRuleset = new Bindable(); @@ -90,8 +90,14 @@ namespace osu.Game.Screens.Select [Cached(Type = typeof(IBindable>))] private readonly Bindable> mods = new Bindable>(Array.Empty()); // Bound to the game's mods, but is not reset on exiting - protected SongSelect() + protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); + + [BackgroundDependencyLoader(true)] + private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) { + // transfer initial value so filter is in a good state (it uses our re-cached bindables). + transferRulesetValue(); + AddRangeInternal(new Drawable[] { new ParallaxContainer @@ -215,13 +221,7 @@ namespace osu.Game.Screens.Select } BeatmapDetails.Leaderboard.ScoreSelected += score => this.Push(new SoloResults(score)); - } - protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); - - [BackgroundDependencyLoader(true)] - private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) - { if (Footer != null) { Footer.AddButton(new FooterButtonMods { Current = mods }, ModSelect); @@ -640,7 +640,7 @@ namespace osu.Game.Screens.Select return; // manual binding to parent ruleset to allow for delayed load in the incoming direction. - rulesetNoDebounce = decoupledRuleset.Value = Ruleset.Value; + transferRulesetValue(); Ruleset.ValueChanged += r => updateSelectedRuleset(r.NewValue); decoupledRuleset.ValueChanged += r => Ruleset.Value = r.NewValue; @@ -652,6 +652,11 @@ namespace osu.Game.Screens.Select boundLocalBindables = true; } + private void transferRulesetValue() + { + rulesetNoDebounce = decoupledRuleset.Value = Ruleset.Value; + } + private void delete(BeatmapSetInfo beatmap) { if (beatmap == null || beatmap.ID <= 0) return; From a580b9079a7995718fb7a1083882a9288a339ea7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2019 08:28:41 +0900 Subject: [PATCH 3/7] Reword comment --- osu.Game/Screens/Select/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 269fb1b683..1f16b134c4 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -95,7 +95,7 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(true)] private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) { - // transfer initial value so filter is in a good state (it uses our re-cached bindables). + // initial value transfer is requried for FilterControl (it uses our re-cached bindables in its asynd loac for the initial filter). transferRulesetValue(); AddRangeInternal(new Drawable[] From d8a5750e5d1f3462bf7edc4d7d48ba5f35341fd1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2019 08:38:01 +0900 Subject: [PATCH 4/7] Fix typo 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/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 1f16b134c4..b595003f0a 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -95,7 +95,7 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(true)] private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) { - // initial value transfer is requried for FilterControl (it uses our re-cached bindables in its asynd loac for the initial filter). + // initial value transfer is required for FilterControl (it uses our re-cached bindables in its async load for the initial filter). transferRulesetValue(); AddRangeInternal(new Drawable[] From e9e37fc821b1ce075d3cc2fba3ff92f5a260f8cb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Nov 2019 18:23:35 +0900 Subject: [PATCH 5/7] Add private setter for FilterControl --- osu.Game/Screens/Select/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 974120e658..089510513b 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.Select protected const float BACKGROUND_BLUR = 20; private const float left_area_padding = 20; - public FilterControl FilterControl; + public FilterControl FilterControl { get; private set; } protected virtual bool ShowFooter => true; From e0f59d8e24ea0c0f2f64dcda6fc8a6ddf5e72cf9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Nov 2019 15:43:00 +0900 Subject: [PATCH 6/7] Move method --- osu.Game/Screens/Select/SongSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 089510513b..b5e62740f2 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -88,8 +88,6 @@ namespace osu.Game.Screens.Select [Resolved(canBeNull: true)] private MusicController music { get; set; } - protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); - [BackgroundDependencyLoader(true)] private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuColour colours, SkinManager skins, ScoreManager scores) { @@ -262,6 +260,8 @@ namespace osu.Game.Screens.Select } } + protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); + private DependencyContainer dependencies; protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) From 5b416eb7ba35c261810368b2115079e375d17dd0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Nov 2019 17:24:43 +0900 Subject: [PATCH 7/7] Move initial filter to run on entering --- osu.Game/Screens/Select/SongSelect.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index b5e62740f2..a52edb70db 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -260,7 +260,11 @@ namespace osu.Game.Screens.Select } } - protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) => Carousel.Filter(criteria); + protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) + { + if (this.IsCurrentScreen()) + Carousel.Filter(criteria); + } private DependencyContainer dependencies; @@ -433,6 +437,8 @@ namespace osu.Game.Screens.Select { base.OnEntering(last); + Carousel.Filter(FilterControl.CreateCriteria(), false); + this.FadeInFromZero(250); FilterControl.Activate(); }