mirror of
https://github.com/ppy/osu.git
synced 2026-05-21 07:49:52 +08:00
Fix new (beatmap) carousel not correctly accounting for user scroll overrides
The new carousel implementation was lacking some scroll related behaviours. This makes sure that post-filter, the selection is re-centered *unless* the user has scrolled away manually. This matches the old carousel's behaviour. See https://github.com/ppy/osu/pull/16647 for original implementation. Closes https://github.com/ppy/osu/issues/33052.
This commit is contained in:
@@ -5,7 +5,6 @@ using System.Linq;
|
||||
using NUnit.Framework;
|
||||
using osu.Framework.Graphics.Primitives;
|
||||
using osu.Framework.Testing;
|
||||
using osu.Game.Screens.Select.Filter;
|
||||
using osu.Game.Screens.SelectV2;
|
||||
|
||||
namespace osu.Game.Tests.Visual.SongSelectV2
|
||||
@@ -19,19 +18,16 @@ namespace osu.Game.Tests.Visual.SongSelectV2
|
||||
RemoveAllBeatmaps();
|
||||
CreateCarousel();
|
||||
|
||||
SortBy(SortMode.Artist);
|
||||
|
||||
AddBeatmaps(10);
|
||||
WaitForDrawablePanels();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestScrollPositionMaintainedOnAddSecondSelected()
|
||||
public void TestScrollPositionMaintainedOnAdd_SecondSelected()
|
||||
{
|
||||
Quad positionBefore = default;
|
||||
|
||||
AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First());
|
||||
AddStep("scroll to selected item", () => Scroll.ScrollTo(Scroll.ChildrenOfType<PanelBeatmap>().Single(p => p.Selected.Value)));
|
||||
|
||||
WaitForScrolling();
|
||||
|
||||
@@ -45,11 +41,35 @@ namespace osu.Game.Tests.Visual.SongSelectV2
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestScrollPositionMaintainedOnAddLastSelected()
|
||||
public void TestScrollPositionMaintainedOnAdd_SecondSelected_WithUserScroll()
|
||||
{
|
||||
Quad positionBefore = default;
|
||||
|
||||
AddStep("scroll to last item", () => Scroll.ScrollToEnd(false));
|
||||
AddStep("select middle beatmap", () => Carousel.CurrentSelection = BeatmapSets.ElementAt(BeatmapSets.Count - 2).Beatmaps.First());
|
||||
WaitForScrolling();
|
||||
|
||||
AddStep("override scroll with user scroll", () =>
|
||||
{
|
||||
InputManager.MoveMouseTo(Scroll.ScreenSpaceDrawQuad.Centre);
|
||||
InputManager.ScrollVerticalBy(-1);
|
||||
});
|
||||
WaitForScrolling();
|
||||
|
||||
AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<PanelBeatmap>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
|
||||
|
||||
RemoveFirstBeatmap();
|
||||
WaitForFiltering();
|
||||
|
||||
AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<PanelBeatmap>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
|
||||
() => Is.EqualTo(positionBefore));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestScrollPositionMaintainedOnAdd_LastSelected()
|
||||
{
|
||||
Quad positionBefore = default;
|
||||
|
||||
AddStep("scroll to end", () => Scroll.ScrollToEnd(false));
|
||||
|
||||
AddStep("select last beatmap", () => Carousel.CurrentSelection = BeatmapSets.Last().Beatmaps.Last());
|
||||
|
||||
@@ -62,5 +82,50 @@ namespace osu.Game.Tests.Visual.SongSelectV2
|
||||
AddAssert("select screen position unchanged", () => Carousel.ChildrenOfType<PanelBeatmap>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
|
||||
() => Is.EqualTo(positionBefore));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestScrollToSelectionAfterFilter()
|
||||
{
|
||||
Quad positionBefore = default;
|
||||
|
||||
AddStep("select first beatmap", () => Carousel.CurrentSelection = BeatmapSets.First().Beatmaps.First());
|
||||
|
||||
WaitForScrolling();
|
||||
|
||||
AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<PanelBeatmap>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
|
||||
|
||||
AddStep("scroll to end", () => Scroll.ScrollToEnd());
|
||||
WaitForScrolling();
|
||||
|
||||
ApplyToFilter("search", f => f.SearchText = "Some");
|
||||
WaitForFiltering();
|
||||
|
||||
AddUntilStep("select screen position returned to selection", () => Carousel.ChildrenOfType<PanelBeatmap>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
|
||||
() => Is.EqualTo(positionBefore));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestScrollToSelectionAfterFilter_WithUserScroll()
|
||||
{
|
||||
Quad positionBefore = default;
|
||||
|
||||
AddStep("select first beatmap", () => Carousel.CurrentSelection = BeatmapSets.First().Beatmaps.First());
|
||||
WaitForScrolling();
|
||||
|
||||
AddStep("override scroll with user scroll", () =>
|
||||
{
|
||||
InputManager.MoveMouseTo(Scroll.ScreenSpaceDrawQuad.Centre);
|
||||
InputManager.ScrollVerticalBy(-1);
|
||||
});
|
||||
WaitForScrolling();
|
||||
|
||||
AddStep("save selected screen position", () => positionBefore = Carousel.ChildrenOfType<PanelBeatmap>().FirstOrDefault(p => p.Selected.Value)!.ScreenSpaceDrawQuad);
|
||||
|
||||
ApplyToFilter("search", f => f.SearchText = "Some");
|
||||
WaitForFiltering();
|
||||
|
||||
AddUntilStep("select screen position returned to selection", () => Carousel.ChildrenOfType<PanelBeatmap>().Single(p => p.Selected.Value).ScreenSpaceDrawQuad,
|
||||
() => Is.EqualTo(positionBefore));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -315,6 +315,8 @@ namespace osu.Game.Graphics.Carousel
|
||||
HandleItemSelected(currentSelection.Model);
|
||||
|
||||
refreshAfterSelection();
|
||||
if (!Scroll.UserScrolling)
|
||||
scrollToSelection();
|
||||
|
||||
NewItemsPresented?.Invoke();
|
||||
});
|
||||
@@ -469,6 +471,9 @@ namespace osu.Game.Graphics.Carousel
|
||||
|
||||
#region Selection handling
|
||||
|
||||
/// <summary>
|
||||
/// Becomes invalid when the current selection has changed and needs to be updated visually.
|
||||
/// </summary>
|
||||
private readonly Cached selectionValid = new Cached();
|
||||
|
||||
private Selection currentKeyboardSelection = new Selection();
|
||||
@@ -569,7 +574,10 @@ namespace osu.Game.Graphics.Carousel
|
||||
if (!selectionValid.IsValid)
|
||||
{
|
||||
refreshAfterSelection();
|
||||
|
||||
// Always scroll to selection in this case (regardless of `UserScrolling` state), centering the selection.
|
||||
scrollToSelection();
|
||||
|
||||
selectionValid.Validate();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user