From bd9056c709f72bd47c10b485a06fd529298df9d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 17:32:21 +0900 Subject: [PATCH] Better choose new selection when multiple items are removed including current --- .../Visual/TestCaseBeatmapCarousel.cs | 8 ++++- .../Carousel/CarouselGroupEagerSelect.cs | 32 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 973ea71cbf..34d2ceacb1 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -257,9 +257,15 @@ namespace osu.Game.Tests.Visual private void testRemoveAll() { setSelected(2, 1); - AddAssert("Selection is non-null", () => currentSelection != null); + AddStep("Remove selected", () => carousel.RemoveBeatmapSet(carousel.SelectedBeatmapSet)); + checkSelected(2); + + AddStep("Remove first", () => carousel.RemoveBeatmapSet(carousel.BeatmapSets.First())); + AddStep("Remove first", () => carousel.RemoveBeatmapSet(carousel.BeatmapSets.First())); + checkSelected(1); + AddUntilStep(() => { carousel.RemoveBeatmapSet(carousel.BeatmapSets.Last()); diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 4fca0c69b6..2d79742cc8 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -1,6 +1,7 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE +using System; using System.Linq; namespace osu.Game.Screens.Select.Carousel @@ -19,14 +20,33 @@ namespace osu.Game.Screens.Select.Carousel }; } + /// + /// We need to keep track of the index for cases where the selection is removed but we want to select a new item based on its old location. + /// private int lastSelectedIndex; + private CarouselItem lastSelected; + public override void Filter(FilterCriteria criteria) { base.Filter(criteria); attemptSelection(); } + public override void RemoveChild(CarouselItem i) + { + base.RemoveChild(i); + + if (i != lastSelected) + updateSelectedIndex(); + } + + public override void AddChild(CarouselItem i) + { + base.AddChild(i); + updateSelectedIndex(); + } + protected override void ChildItemStateChanged(CarouselItem item, CarouselItemState value) { base.ChildItemStateChanged(item, value); @@ -34,7 +54,7 @@ namespace osu.Game.Screens.Select.Carousel switch (value) { case CarouselItemState.Selected: - lastSelectedIndex = InternalChildren.IndexOf(item); + updateSelected(item); break; case CarouselItemState.NotSelected: attemptSelection(); @@ -56,6 +76,16 @@ namespace osu.Game.Screens.Select.Carousel if (nextToSelect != null) nextToSelect.State.Value = CarouselItemState.Selected; + else + updateSelected(null); } + + private void updateSelected(CarouselItem newSelection) + { + lastSelected = newSelection; + updateSelectedIndex(); + } + + private void updateSelectedIndex() => lastSelectedIndex = Math.Max(0, InternalChildren.IndexOf(lastSelected)); } }