From 5cbb9b9b1886f750e7a7a16d33ef7f062db3aac4 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 14 Dec 2017 12:59:35 +0900
Subject: [PATCH] Fix random and add tests
Also exposes SelectedBeatmapSet.
---
.../Visual/TestCaseBeatmapCarousel.cs | 58 +++++++++++++-
osu.Game/Screens/Select/BeatmapCarousel.cs | 79 ++++++++++---------
osu.Game/Screens/Select/SongSelect.cs | 4 +-
3 files changed, 97 insertions(+), 44 deletions(-)
diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs
index f034dc7baf..971760239f 100644
--- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs
+++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs
@@ -10,6 +10,7 @@ using osu.Framework.Allocation;
using osu.Framework.Extensions;
using osu.Framework.Graphics;
using osu.Game.Beatmaps;
+using osu.Game.Configuration;
using osu.Game.Screens.Select;
using osu.Game.Screens.Select.Carousel;
@@ -44,7 +45,7 @@ namespace osu.Game.Tests.Visual
AddStep("Load Beatmaps", () =>
{
- carousel.Beatmaps = new[]
+ carousel.BeatmapSets = new[]
{
createTestBeatmapSet(1),
createTestBeatmapSet(2),
@@ -55,11 +56,11 @@ namespace osu.Game.Tests.Visual
void checkSelected(int set, int diff) =>
AddAssert($"selected is set{set} diff{diff}", () =>
- carousel.SelectedBeatmap == carousel.Beatmaps.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First());
+ carousel.SelectedBeatmap == carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First());
void setSelected(int set, int diff) =>
AddStep($"select set{set} diff{diff}", () =>
- carousel.SelectBeatmap(carousel.Beatmaps.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First()));
+ carousel.SelectBeatmap(carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First()));
void advanceSelection(bool diff, int direction = 1, int count = 1)
{
@@ -77,7 +78,7 @@ namespace osu.Game.Tests.Visual
AddAssert($"{count} {(diff ? "diff" : "set")} visible", () =>
carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count);
- AddUntilStep(() => carousel.Beatmaps.Any(), "Wait for load");
+ AddUntilStep(() => carousel.BeatmapSets.Any(), "Wait for load");
// test traversal
@@ -122,6 +123,55 @@ namespace osu.Game.Tests.Visual
checkSelected(1, 1);
AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false));
checkSelected(1, 1);
+
+ // test random non-repeating algorithm
+
+ Stack<BeatmapSetInfo> selectedSets = new Stack<BeatmapSetInfo>();
+
+ void nextRandom() =>
+ AddStep("select random next", () =>
+ {
+ carousel.RandomAlgorithm.Value = RandomSelectAlgorithm.RandomPermutation;
+
+ if (!selectedSets.Any() && carousel.SelectedBeatmap != null)
+ selectedSets.Push(carousel.SelectedBeatmapSet);
+
+ carousel.SelectNextRandom();
+ selectedSets.Push(carousel.SelectedBeatmapSet);
+ });
+
+ void ensureRandomDidntRepeat() =>
+ AddAssert("ensure no repeats", () => selectedSets.Distinct().Count() == selectedSets.Count());
+
+ void prevRandom() => AddStep("select random last", () =>
+ {
+ carousel.SelectPreviousRandom();
+ selectedSets.Pop();
+ });
+
+ void ensureRandomFetchSuccess() =>
+ AddAssert("ensure prev random fetch worked", () => selectedSets.Peek() == carousel.SelectedBeatmapSet);
+
+ setSelected(1, 1);
+ nextRandom();
+ ensureRandomDidntRepeat();
+ nextRandom();
+ ensureRandomDidntRepeat();
+ nextRandom();
+ ensureRandomDidntRepeat();
+
+ prevRandom();
+ ensureRandomFetchSuccess();
+ prevRandom();
+ ensureRandomFetchSuccess();
+
+ nextRandom();
+ ensureRandomDidntRepeat();
+ nextRandom();
+ ensureRandomDidntRepeat();
+
+ nextRandom();
+ AddAssert("ensure repeat", () => selectedSets.Contains(carousel.SelectedBeatmapSet));
}
private BeatmapSetInfo createTestBeatmapSet(int i)
diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs
index 5e3efb55c9..810cc8c408 100644
--- a/osu.Game/Screens/Select/BeatmapCarousel.cs
+++ b/osu.Game/Screens/Select/BeatmapCarousel.cs
@@ -27,15 +27,24 @@ namespace osu.Game.Screens.Select
public class BeatmapCarousel : OsuScrollContainer
{
/// <summary>
- /// Triggered when the <see cref="Beatmaps"/> loaded change and are completely loaded.
+ /// Triggered when the <see cref="BeatmapSets"/> loaded change and are completely loaded.
/// </summary>
- public Action BeatmapsChanged;
+ public Action BeatmapSetsChanged;
/// <summary>
/// The currently selected beatmap.
/// </summary>
public BeatmapInfo SelectedBeatmap => selectedBeatmap?.Beatmap;
+ private CarouselBeatmap selectedBeatmap => selectedBeatmapSet?.Beatmaps.FirstOrDefault(s => s.State == CarouselItemState.Selected);
+
+ /// <summary>
+ /// The currently selected beatmap set.
+ /// </summary>
+ public BeatmapSetInfo SelectedBeatmapSet => selectedBeatmapSet?.BeatmapSet;
+
+ private CarouselBeatmapSet selectedBeatmapSet => carouselSets.FirstOrDefault(s => s.State == CarouselItemState.Selected);
+
/// <summary>
/// Raised when the <see cref="SelectedBeatmap"/> is changed.
/// </summary>
@@ -43,7 +52,7 @@ namespace osu.Game.Screens.Select
public override bool HandleInput => AllowSelection;
- public IEnumerable<BeatmapSetInfo> Beatmaps
+ public IEnumerable<BeatmapSetInfo> BeatmapSets
{
get { return carouselSets.Select(g => g.BeatmapSet); }
set
@@ -72,7 +81,7 @@ namespace osu.Game.Screens.Select
Items = root.Drawables.Value.ToList();
yPositionsCache.Invalidate();
- BeatmapsChanged?.Invoke();
+ BeatmapSetsChanged?.Invoke();
});
});
}
@@ -85,17 +94,14 @@ namespace osu.Game.Screens.Select
private readonly List<CarouselBeatmapSet> carouselSets = new List<CarouselBeatmapSet>();
- private Bindable<RandomSelectAlgorithm> randomSelectAlgorithm;
- private readonly List<CarouselBeatmapSet> seenSets = new List<CarouselBeatmapSet>();
+ public Bindable<RandomSelectAlgorithm> RandomAlgorithm = new Bindable<RandomSelectAlgorithm>();
+ private readonly List<CarouselBeatmapSet> previouslyVisitedRandomSets = new List<CarouselBeatmapSet>();
protected List<DrawableCarouselItem> Items = new List<DrawableCarouselItem>();
private CarouselGroup root = new CarouselGroup();
private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
- private CarouselBeatmap selectedBeatmap;
- private CarouselBeatmapSet selectedBeatmapSet => carouselSets.FirstOrDefault(s => s.State == CarouselItemState.Selected);
-
public BeatmapCarousel()
{
Add(new OsuContextMenuContainer
@@ -173,12 +179,6 @@ namespace osu.Game.Screens.Select
}
}
- private void selectNullBeatmap()
- {
- selectedBeatmap = null;
- SelectionChanged?.Invoke(null);
- }
-
/// <summary>
/// Increment selection in the carousel in a chosen direction.
/// </summary>
@@ -187,9 +187,9 @@ namespace osu.Game.Screens.Select
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 (carouselSets.All(g => g.State == CarouselItemState.Hidden))
+ if (carouselSets.All(g => !g.Visible))
{
- selectNullBeatmap();
+ SelectionChanged?.Invoke(null);
return;
}
@@ -218,53 +218,57 @@ namespace osu.Game.Screens.Select
}
}
- private IEnumerable<CarouselBeatmapSet> getVisibleGroups() => carouselSets.Where(select => select.State != CarouselItemState.NotSelected);
+ private IEnumerable<CarouselBeatmapSet> getVisibleSets() => carouselSets.Where(select => select.Visible);
public void SelectNextRandom()
{
if (carouselSets.Count == 0)
return;
- var visibleGroups = getVisibleGroups();
- if (!visibleGroups.Any())
+ var visible = getVisibleSets().ToList();
+ if (!visible.Any())
return;
if (selectedBeatmap != null)
+ {
randomSelectedBeatmaps.Push(selectedBeatmap);
- CarouselBeatmapSet group;
+ // when performing a random, we want to add the current set to the previously visited list
+ // else the user may be "randomised" to the existing selection.
+ if (previouslyVisitedRandomSets.LastOrDefault() != selectedBeatmapSet)
+ previouslyVisitedRandomSets.Add(selectedBeatmapSet);
+ }
- if (randomSelectAlgorithm == RandomSelectAlgorithm.RandomPermutation)
+ CarouselBeatmapSet set;
+
+ if (RandomAlgorithm == RandomSelectAlgorithm.RandomPermutation)
{
- var notSeenGroups = visibleGroups.Except(seenSets);
- if (!notSeenGroups.Any())
+ var notYetVisitedSets = visible.Except(previouslyVisitedRandomSets).ToList();
+ if (!notYetVisitedSets.Any())
{
- seenSets.Clear();
- notSeenGroups = visibleGroups;
+ previouslyVisitedRandomSets.Clear();
+ notYetVisitedSets = visible;
}
- group = notSeenGroups.ElementAt(RNG.Next(notSeenGroups.Count()));
- seenSets.Add(group);
+ set = notYetVisitedSets.ElementAt(RNG.Next(notYetVisitedSets.Count()));
+ previouslyVisitedRandomSets.Add(set);
}
else
- group = visibleGroups.ElementAt(RNG.Next(visibleGroups.Count()));
+ set = visible.ElementAt(RNG.Next(visible.Count()));
- CarouselBeatmap item = group.Beatmaps[RNG.Next(group.Beatmaps.Count)];
-
- select(item);
+ select(set.Beatmaps[RNG.Next(set.Beatmaps.Count)]);
}
public void SelectPreviousRandom()
{
- if (!randomSelectedBeatmaps.Any())
- return;
-
while (randomSelectedBeatmaps.Any())
{
var beatmap = randomSelectedBeatmaps.Pop();
- if (beatmap.Visible)
+ if (!beatmap.Filtered)
{
+ if (RandomAlgorithm == RandomSelectAlgorithm.RandomPermutation)
+ previouslyVisitedRandomSets.Remove(selectedBeatmapSet);
select(beatmap);
break;
}
@@ -351,7 +355,6 @@ namespace osu.Game.Screens.Select
{
if (v == CarouselItemState.Selected)
{
- selectedBeatmap = c;
SelectionChanged?.Invoke(c.Beatmap);
yPositionsCache.Invalidate();
Schedule(() => ScrollToSelected());
@@ -365,7 +368,7 @@ namespace osu.Game.Screens.Select
[BackgroundDependencyLoader(permitNulls: true)]
private void load(OsuConfigManager config)
{
- randomSelectAlgorithm = config.GetBindable<RandomSelectAlgorithm>(OsuSetting.RandomSelectAlgorithm);
+ config.BindWith(OsuSetting.RandomSelectAlgorithm, RandomAlgorithm);
}
private void removeBeatmapSet(CarouselBeatmapSet set)
diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs
index eec4679bfd..0c3baf2c07 100644
--- a/osu.Game/Screens/Select/SongSelect.cs
+++ b/osu.Game/Screens/Select/SongSelect.cs
@@ -113,7 +113,7 @@ namespace osu.Game.Screens.Select
Anchor = Anchor.CentreRight,
Origin = Anchor.CentreRight,
SelectionChanged = carouselSelectionChanged,
- BeatmapsChanged = carouselBeatmapsLoaded,
+ BeatmapSetsChanged = carouselBeatmapsLoaded,
},
FilterControl = new FilterControl
{
@@ -193,7 +193,7 @@ namespace osu.Game.Screens.Select
initialAddSetsTask = new CancellationTokenSource();
- carousel.Beatmaps = this.beatmaps.GetAllUsableBeatmapSets();
+ carousel.BeatmapSets = this.beatmaps.GetAllUsableBeatmapSets();
Beatmap.ValueChanged += beatmap_ValueChanged;