From 0a45aa80cb011e658dc1d7c3504d2fe4ef12f079 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:16:49 +0900 Subject: [PATCH 1/3] Remove unnecessary double-schedule in `UpdateBeatmapSet` --- osu.Game/Screens/Select/BeatmapCarousel.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index dff2c598c3..94e520ba1c 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -311,13 +311,10 @@ namespace osu.Game.Screens.Select itemsCache.Invalidate(); - Schedule(() => - { - if (!Scroll.UserScrolling) - ScrollToSelected(true); + if (!Scroll.UserScrolling) + ScrollToSelected(true); - BeatmapSetsChanged?.Invoke(); - }); + BeatmapSetsChanged?.Invoke(); }); /// From 449e9bcf5c123e1e1c92534f8f3a710412a5028d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:17:38 +0900 Subject: [PATCH 2/3] Ensure beatmap carousel scroll position is maintained during deletion operations --- osu.Game/Screens/Select/BeatmapCarousel.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 94e520ba1c..c27915c383 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -286,6 +286,9 @@ namespace osu.Game.Screens.Select root.RemoveChild(existingSet); itemsCache.Invalidate(); + + if (!Scroll.UserScrolling) + ScrollToSelected(true); }); public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => From f2cecad83b2e1df2b2e15c5cd49e04534cf55dca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:38:02 +0900 Subject: [PATCH 3/3] Add failing test coverage showing carousel deletions don't keep scroll position --- .../SongSelect/TestSceneBeatmapCarousel.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 0298c3bea9..2f2001c4f2 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -9,6 +9,7 @@ using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Rulesets; @@ -40,6 +41,36 @@ namespace osu.Game.Tests.Visual.SongSelect this.rulesets = rulesets; } + [Test] + public void TestScrollPositionMaintainedOnAdd() + { + loadBeatmaps(count: 1, randomDifficulties: false); + + for (int i = 0; i < 10; i++) + { + AddRepeatStep("Add some sets", () => carousel.UpdateBeatmapSet(TestResources.CreateTestBeatmapSetInfo()), 4); + + checkSelectionIsCentered(); + } + } + + [Test] + public void TestScrollPositionMaintainedOnDelete() + { + loadBeatmaps(count: 50, randomDifficulties: false); + + for (int i = 0; i < 10; i++) + { + AddRepeatStep("Remove some sets", () => + carousel.RemoveBeatmapSet(carousel.Items.Select(item => item.Item) + .OfType() + .OrderBy(item => item.GetHashCode()) + .First(item => item.State.Value != CarouselItemState.Selected && item.Visible).BeatmapSet), 4); + + checkSelectionIsCentered(); + } + } + [Test] public void TestManyPanels() { @@ -813,6 +844,18 @@ namespace osu.Game.Tests.Visual.SongSelect carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count); } + private void checkSelectionIsCentered() + { + AddAssert("Selected panel is centered", () => + { + return Precision.AlmostEquals( + carousel.ScreenSpaceDrawQuad.Centre, + carousel.Items + .First(i => i.Item.State.Value == CarouselItemState.Selected) + .ScreenSpaceDrawQuad.Centre, 100); + }); + } + private void checkNoSelection() => AddAssert("Selection is null", () => currentSelection == null); private void nextRandom() =>