Merge pull request #8481 from LittleEndu/rewrite-select-next

Rewrite beatmap carousel's select next logic to not use drawables
This commit is contained in:
Dean Herbert 2020-03-30 01:21:31 +09:00 committed by GitHub
commit 12d44b6335
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 99 additions and 32 deletions

View File

@ -83,6 +83,82 @@ namespace osu.Game.Tests.Visual.SongSelect
waitForSelection(set_count, 3);
}
[TestCase(true)]
[TestCase(false)]
public void TestTraversalBeyondVisible(bool forwards)
{
var sets = new List<BeatmapSetInfo>();
const int total_set_count = 200;
for (int i = 0; i < total_set_count; i++)
sets.Add(createTestBeatmapSet(i + 1));
loadBeatmaps(sets);
for (int i = 1; i < total_set_count; i += i)
selectNextAndAssert(i);
void selectNextAndAssert(int amount)
{
setSelected(forwards ? 1 : total_set_count, 1);
AddStep($"{(forwards ? "Next" : "Previous")} beatmap {amount} times", () =>
{
for (int i = 0; i < amount; i++)
{
carousel.SelectNext(forwards ? 1 : -1);
}
});
waitForSelection(forwards ? amount + 1 : total_set_count - amount);
}
}
[Test]
public void TestTraversalBeyondVisibleDifficulties()
{
var sets = new List<BeatmapSetInfo>();
const int total_set_count = 20;
for (int i = 0; i < total_set_count; i++)
sets.Add(createTestBeatmapSet(i + 1));
loadBeatmaps(sets);
// Selects next set once, difficulty index doesn't change
selectNextAndAssert(3, true, 2, 1);
// Selects next set 16 times (50 \ 3 == 16), difficulty index changes twice (50 % 3 == 2)
selectNextAndAssert(50, true, 17, 3);
// Travels around the carousel thrice (200 \ 60 == 3)
// continues to select 20 times (200 \ 60 == 20)
// selects next set 6 times (20 \ 3 == 6)
// difficulty index changes twice (20 % 3 == 2)
selectNextAndAssert(200, true, 7, 3);
// All same but in reverse
selectNextAndAssert(3, false, 19, 3);
selectNextAndAssert(50, false, 4, 1);
selectNextAndAssert(200, false, 14, 1);
void selectNextAndAssert(int amount, bool forwards, int expectedSet, int expectedDiff)
{
// Select very first or very last difficulty
setSelected(forwards ? 1 : 20, forwards ? 1 : 3);
AddStep($"{(forwards ? "Next" : "Previous")} difficulty {amount} times", () =>
{
for (int i = 0; i < amount; i++)
carousel.SelectNext(forwards ? 1 : -1, false);
});
waitForSelection(expectedSet, expectedDiff);
}
}
/// <summary>
/// Test filtering
/// </summary>

View File

@ -253,46 +253,37 @@ 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)
{
var visibleItems = Items.Where(s => !s.Item.Filtered.Value).ToList();
if (!visibleItems.Any())
if (beatmapSets.All(s => s.Filtered.Value))
return;
DrawableCarouselItem drawable = null;
if (skipDifficulties)
selectNextSet(direction, true);
else
selectNextDifficulty(direction);
}
if (selectedBeatmap != null && (drawable = selectedBeatmap.Drawables.FirstOrDefault()) == null)
// if the selected beatmap isn't present yet, we can't correctly change selection.
// we can fix this by changing this method to not reference drawables / Items in the first place.
return;
private void selectNextSet(int direction, bool skipDifficulties)
{
var unfilteredSets = beatmapSets.Where(s => !s.Filtered.Value).ToList();
int originalIndex = visibleItems.IndexOf(drawable);
int currentIndex = originalIndex;
var nextSet = unfilteredSets[(unfilteredSets.IndexOf(selectedBeatmapSet) + direction + unfilteredSets.Count) % unfilteredSets.Count];
// local function to increment the index in the required direction, wrapping over extremities.
int incrementIndex() => currentIndex = (currentIndex + direction + visibleItems.Count) % visibleItems.Count;
if (skipDifficulties)
select(nextSet);
else
select(direction > 0 ? nextSet.Beatmaps.First(b => !b.Filtered.Value) : nextSet.Beatmaps.Last(b => !b.Filtered.Value));
}
while (incrementIndex() != originalIndex)
{
var item = visibleItems[currentIndex].Item;
private void selectNextDifficulty(int direction)
{
var unfilteredDifficulties = selectedBeatmapSet.Children.Where(s => !s.Filtered.Value).ToList();
if (item.Filtered.Value || item.State.Value == CarouselItemState.Selected) continue;
int index = unfilteredDifficulties.IndexOf(selectedBeatmap);
switch (item)
{
case CarouselBeatmap beatmap:
if (skipDifficulties) continue;
select(beatmap);
return;
case CarouselBeatmapSet set:
if (skipDifficulties)
select(set);
else
select(direction > 0 ? set.Beatmaps.First(b => !b.Filtered.Value) : set.Beatmaps.Last(b => !b.Filtered.Value));
return;
}
}
if (index + direction < 0 || index + direction >= unfilteredDifficulties.Count)
selectNextSet(direction, false);
else
select(unfilteredDifficulties[index + direction]);
}
/// <summary>