1
0
mirror of https://github.com/ppy/osu.git synced 2025-01-14 22:42:57 +08:00

Ensure correct selection after deletion of currently selected

Also fixes a lot of bad interactions and simplifies further.
This commit is contained in:
Dean Herbert 2017-12-16 16:14:37 +09:00
parent 49ce42d90c
commit 3c406662ed
7 changed files with 79 additions and 68 deletions

View File

@ -74,9 +74,14 @@ namespace osu.Game.Tests.Visual
private void ensureRandomFetchSuccess() =>
AddAssert("ensure prev random fetch worked", () => selectedSets.Peek() == carousel.SelectedBeatmapSet);
private void checkSelected(int set, int diff) =>
AddAssert($"selected is set{set} diff{diff}", () =>
carousel.SelectedBeatmap == carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First());
private void checkSelected(int set, int? diff = null) =>
AddAssert($"selected is set{set}{(diff.HasValue ? $" diff{diff.Value}" : "")}", () =>
{
if (diff != null)
return carousel.SelectedBeatmap == carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff.Value - 1).First();
return carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Contains(carousel.SelectedBeatmap);
});
private void setSelected(int set, int diff) =>
AddStep($"select set{set} diff{diff}", () =>
@ -95,7 +100,7 @@ namespace osu.Game.Tests.Visual
}
private void checkVisibleItemCount(bool diff, int count) =>
AddAssert($"{count} {(diff ? "diff" : "set")} visible", () =>
AddAssert($"{count} {(diff ? "diffs" : "sets")} visible", () =>
carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count);
private void nextRandom() =>
@ -138,8 +143,9 @@ namespace osu.Game.Tests.Visual
advanceSelection(diff: false);
advanceSelection(diff: false);
checkSelected(1, 1);
checkSelected(1, 2);
advanceSelection(direction: -1, diff: true);
advanceSelection(direction: -1, diff: true);
checkSelected(set_count, 3);
}
@ -234,7 +240,7 @@ namespace osu.Game.Tests.Visual
checkVisibleItemCount(false, set_count);
checkSelected(set_count, 1);
checkSelected(set_count, 2);
}
/// <summary>

View File

@ -74,6 +74,7 @@ namespace osu.Game.Screens.Select
{
root = newRoot;
updateItems();
BeatmapSetsChanged?.Invoke();
});
});
}
@ -85,11 +86,11 @@ namespace osu.Game.Screens.Select
private void updateItems()
{
scrollableContent.Clear(false);
Items = root.Drawables.ToList();
yPositionsCache.Invalidate();
BeatmapSetsChanged?.Invoke();
if (root.Children == null || root.Children.All(c => c.Filtered))
SelectionChanged?.Invoke(null);
}
private readonly List<float> yPositions = new List<float>();
@ -126,9 +127,6 @@ namespace osu.Game.Screens.Select
root.RemoveChild(existingSet);
updateItems();
if (existingSet.State == CarouselItemState.Selected)
SelectNext();
}
public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet)
@ -188,13 +186,6 @@ namespace osu.Game.Screens.Select
/// <param name="skipDifficulties">Whether to skip individual difficulties and only increment over full groups.</param>
public void SelectNext(int direction = 1, bool skipDifficulties = true)
{
// todo: we may want to refactor and remove this as an optimisation in the future.
if (beatmapSets.All(g => g.Filtered))
{
SelectionChanged?.Invoke(null);
return;
}
int originalIndex = Items.IndexOf(selectedBeatmap?.Drawables.First());
int currentIndex = originalIndex;
@ -214,13 +205,14 @@ namespace osu.Game.Screens.Select
select(beatmap);
return;
case CarouselBeatmapSet set:
if (!skipDifficulties) continue;
select(set);
return;
}
}
}
private IEnumerable<CarouselBeatmapSet> getVisibleSets() => beatmapSets.Where(select => select.Visible);
private IEnumerable<CarouselBeatmapSet> getVisibleSets() => beatmapSets.Where(select => !select.Filtered);
public void SelectNextRandom()
{
@ -298,28 +290,9 @@ namespace osu.Game.Screens.Select
{
FilterTask = null;
var lastSet = selectedBeatmapSet;
var lastBeatmap = selectedBeatmap;
root.Filter(criteria);
updateItems();
if (selectedBeatmap != null && !selectedBeatmap.Filtered)
select(selectedBeatmap);
else if (lastBeatmap != null && !lastSet.Filtered)
{
var searchable = lastSet.Beatmaps.AsEnumerable();
// search forwards first then backwards if nothing found.
var nextAvailable =
searchable.SkipWhile(b => b != lastBeatmap).FirstOrDefault(b => !b.Filtered) ??
searchable.Reverse().SkipWhile(b => b != lastBeatmap).FirstOrDefault(b => !b.Filtered);
select(nextAvailable);
}
else
SelectNext();
ScrollToSelected(false);
};

View File

@ -49,5 +49,7 @@ namespace osu.Game.Screens.Select.Carousel
return Beatmap.StarDifficulty.CompareTo(otherBeatmap.Beatmap.StarDifficulty);
}
}
public override string ToString() => Beatmap.ToString();
}
}

View File

@ -52,5 +52,7 @@ namespace osu.Game.Screens.Select.Carousel
base.Filter(criteria);
Filtered.Value = InternalChildren.All(i => i.Filtered);
}
public override string ToString() => BeatmapSet.ToString();
}
}

View File

@ -2,7 +2,6 @@
// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
using System.Collections.Generic;
using osu.Framework.Configuration;
namespace osu.Game.Screens.Select.Carousel
{
@ -13,13 +12,11 @@ namespace osu.Game.Screens.Select.Carousel
{
private readonly List<CarouselItem> items;
public readonly Bindable<CarouselItem> Selected = new Bindable<CarouselItem>();
protected override DrawableCarouselItem CreateDrawableRepresentation() => null;
public override void AddChild(CarouselItem i)
{
i.State.ValueChanged += v => ItemStateChanged(i, v);
i.State.ValueChanged += v => ChildItemStateChanged(i, v);
base.AddChild(i);
}
@ -28,7 +25,7 @@ namespace osu.Game.Screens.Select.Carousel
if (items != null) InternalChildren = items;
}
protected virtual void ItemStateChanged(CarouselItem item, CarouselItemState value)
protected virtual void ChildItemStateChanged(CarouselItem item, CarouselItemState value)
{
// todo: check state of selected item.
@ -42,7 +39,6 @@ namespace osu.Game.Screens.Select.Carousel
}
State.Value = CarouselItemState.Selected;
Selected.Value = item;
}
}
}

View File

@ -15,31 +15,47 @@ namespace osu.Game.Screens.Select.Carousel
State.ValueChanged += v =>
{
if (v == CarouselItemState.Selected)
{
foreach (var c in InternalChildren.Where(c => c.State.Value == CarouselItemState.Hidden))
c.State.Value = CarouselItemState.NotSelected;
if (InternalChildren.Any(c => c.Visible) && InternalChildren.All(c => c.State != CarouselItemState.Selected))
InternalChildren.First(c => c.Visible).State.Value = CarouselItemState.Selected;
}
attemptSelection();
};
}
protected override void ItemStateChanged(CarouselItem item, CarouselItemState value)
{
base.ItemStateChanged(item, value);
private int lastSelectedIndex;
if (value == CarouselItemState.NotSelected)
public override void Filter(FilterCriteria criteria)
{
base.Filter(criteria);
attemptSelection();
}
protected override void ChildItemStateChanged(CarouselItem item, CarouselItemState value)
{
base.ChildItemStateChanged(item, value);
switch (value)
{
if (Children.All(i => i.State != CarouselItemState.Selected))
{
var first = Children.FirstOrDefault(i => !i.Filtered);
if (first != null)
{
first.State.Value = CarouselItemState.Selected;
}
}
case CarouselItemState.Selected:
lastSelectedIndex = InternalChildren.IndexOf(item);
break;
case CarouselItemState.NotSelected:
attemptSelection();
break;
}
}
private void attemptSelection()
{
// we only perform eager selection if we are a currently selected group.
if (State != CarouselItemState.Selected) return;
// we only perform eager selection if none of our children are in a selected state already.
if (Children.Any(i => i.State == CarouselItemState.Selected)) return;
CarouselItem nextToSelect =
Children.Skip(lastSelectedIndex).FirstOrDefault(i => !i.Filtered) ??
Children.Reverse().Skip(InternalChildren.Count - lastSelectedIndex).FirstOrDefault(i => !i.Filtered);
if (nextToSelect != null)
nextToSelect.State.Value = CarouselItemState.Selected;
}
}
}

View File

@ -18,7 +18,10 @@ namespace osu.Game.Screens.Select.Carousel
protected List<CarouselItem> InternalChildren { get; set; }
public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered.Value;
/// <summary>
/// This item is not in a hidden state.
/// </summary>
public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered;
public IEnumerable<DrawableCarouselItem> Drawables
{
@ -39,7 +42,14 @@ namespace osu.Game.Screens.Select.Carousel
public virtual void AddChild(CarouselItem i) => (InternalChildren ?? (InternalChildren = new List<CarouselItem>())).Add(i);
public virtual void RemoveChild(CarouselItem i) => InternalChildren?.Remove(i);
public virtual void RemoveChild(CarouselItem i)
{
InternalChildren?.Remove(i);
// it's important we do the deselection after removing, so any further actions based on
// State.ValueChanged make decisions post-removal.
if (i.State.Value == CarouselItemState.Selected) i.State.Value = CarouselItemState.NotSelected;
}
protected CarouselItem()
{
@ -47,9 +57,9 @@ namespace osu.Game.Screens.Select.Carousel
State.ValueChanged += v =>
{
if (InternalChildren == null) return;
Logger.Log($"State of {this} changed to {v}");
Logger.Log($"State changed to {v}");
if (InternalChildren == null) return;
switch (v)
{
@ -57,6 +67,12 @@ namespace osu.Game.Screens.Select.Carousel
case CarouselItemState.NotSelected:
InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Hidden);
break;
case CarouselItemState.Selected:
InternalChildren.ForEach(c =>
{
if (c.State == CarouselItemState.Hidden) c.State.Value = CarouselItemState.NotSelected;
});
break;
}
};