From 67bed57cbdf3f0921a6a721c74cc28317afc74c7 Mon Sep 17 00:00:00 2001 From: iiSaLMaN Date: Fri, 27 Sep 2019 08:46:49 +0300 Subject: [PATCH 1/4] Bind value changed event of cursor trail appearence outside BDL https://github.com/ppy/osu/pull/6270#discussion_r328899728 --- osu.Game.Rulesets.Osu/UI/Cursor/OsuCursorContainer.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursorContainer.cs b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursorContainer.cs index a944ff88c6..6dbdf0114d 100644 --- a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursorContainer.cs +++ b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursorContainer.cs @@ -40,6 +40,11 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor private void load(OsuRulesetConfigManager config) { config?.BindWith(OsuRulesetSetting.ShowCursorTrail, showTrail); + } + + protected override void LoadComplete() + { + base.LoadComplete(); showTrail.BindValueChanged(v => cursorTrail.FadeTo(v.NewValue ? 1 : 0, 200), true); } From 62c4c1266ef8c1dc71324021bd4808abbb7b6205 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2019 14:45:26 +0900 Subject: [PATCH 2/4] Move private functions to bottom --- .../SongSelect/TestSceneBeatmapCarousel.cs | 202 +++++++++--------- 1 file changed, 101 insertions(+), 101 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 6bdd94db21..527367fdb8 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -58,107 +58,6 @@ namespace osu.Game.Tests.Visual.SongSelect }); } - private void loadBeatmaps(List beatmapSets = null) - { - if (beatmapSets == null) - { - beatmapSets = new List(); - - for (int i = 1; i <= set_count; i++) - beatmapSets.Add(createTestBeatmapSet(i)); - } - - bool changed = false; - AddStep($"Load {beatmapSets.Count} Beatmaps", () => - { - carousel.Filter(new FilterCriteria()); - carousel.BeatmapSetsChanged = () => changed = true; - carousel.BeatmapSets = beatmapSets; - }); - - AddUntilStep("Wait for load", () => changed); - } - - private void ensureRandomFetchSuccess() => - AddAssert("ensure prev random fetch worked", () => selectedSets.Peek() == carousel.SelectedBeatmapSet); - - private void waitForSelection(int set, int? diff = null) => - AddUntilStep($"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}", () => - carousel.SelectBeatmap(carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First())); - - private void advanceSelection(bool diff, int direction = 1, int count = 1) - { - if (count == 1) - AddStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => - carousel.SelectNext(direction, !diff)); - else - { - AddRepeatStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => - carousel.SelectNext(direction, !diff), count); - } - } - - private void checkVisibleItemCount(bool diff, int count) => - 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 checkNoSelection() => AddAssert("Selection is null", () => currentSelection == null); - - private 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); - }); - - private void ensureRandomDidntRepeat() => - AddAssert("ensure no repeats", () => selectedSets.Distinct().Count() == selectedSets.Count); - - private void prevRandom() => AddStep("select random last", () => - { - carousel.SelectPreviousRandom(); - selectedSets.Pop(); - }); - - private bool selectedBeatmapVisible() - { - var currentlySelected = carousel.Items.Find(s => s.Item is CarouselBeatmap && s.Item.State.Value == CarouselItemState.Selected); - if (currentlySelected == null) - return true; - - return currentlySelected.Item.Visible; - } - - private void checkInvisibleDifficultiesUnselectable() - { - nextRandom(); - AddAssert("Selection is visible", selectedBeatmapVisible); - } - - private void checkNonmatchingFilter() - { - AddStep("Toggle non-matching filter", () => - { - carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false); - carousel.Filter(new FilterCriteria(), false); - eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID); - }); - } - /// /// Test keyboard traversal /// @@ -482,6 +381,107 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("Selection was random", () => eagerSelectedIDs.Count > 1); } + private void loadBeatmaps(List beatmapSets = null) + { + if (beatmapSets == null) + { + beatmapSets = new List(); + + for (int i = 1; i <= set_count; i++) + beatmapSets.Add(createTestBeatmapSet(i)); + } + + bool changed = false; + AddStep($"Load {beatmapSets.Count} Beatmaps", () => + { + carousel.Filter(new FilterCriteria()); + carousel.BeatmapSetsChanged = () => changed = true; + carousel.BeatmapSets = beatmapSets; + }); + + AddUntilStep("Wait for load", () => changed); + } + + private void ensureRandomFetchSuccess() => + AddAssert("ensure prev random fetch worked", () => selectedSets.Peek() == carousel.SelectedBeatmapSet); + + private void waitForSelection(int set, int? diff = null) => + AddUntilStep($"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}", () => + carousel.SelectBeatmap(carousel.BeatmapSets.Skip(set - 1).First().Beatmaps.Skip(diff - 1).First())); + + private void advanceSelection(bool diff, int direction = 1, int count = 1) + { + if (count == 1) + AddStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => + carousel.SelectNext(direction, !diff)); + else + { + AddRepeatStep($"select {(direction > 0 ? "next" : "prev")} {(diff ? "diff" : "set")}", () => + carousel.SelectNext(direction, !diff), count); + } + } + + private void checkVisibleItemCount(bool diff, int count) => + 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 checkNoSelection() => AddAssert("Selection is null", () => currentSelection == null); + + private 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); + }); + + private void ensureRandomDidntRepeat() => + AddAssert("ensure no repeats", () => selectedSets.Distinct().Count() == selectedSets.Count); + + private void prevRandom() => AddStep("select random last", () => + { + carousel.SelectPreviousRandom(); + selectedSets.Pop(); + }); + + private bool selectedBeatmapVisible() + { + var currentlySelected = carousel.Items.Find(s => s.Item is CarouselBeatmap && s.Item.State.Value == CarouselItemState.Selected); + if (currentlySelected == null) + return true; + + return currentlySelected.Item.Visible; + } + + private void checkInvisibleDifficultiesUnselectable() + { + nextRandom(); + AddAssert("Selection is visible", selectedBeatmapVisible); + } + + private void checkNonmatchingFilter() + { + AddStep("Toggle non-matching filter", () => + { + carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false); + carousel.Filter(new FilterCriteria(), false); + eagerSelectedIDs.Add(carousel.SelectedBeatmapSet.ID); + }); + } + private BeatmapSetInfo createTestBeatmapSet(int id) { return new BeatmapSetInfo From 46d6c5ec3b6832d353f033d9284d761f423943db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2019 15:13:58 +0900 Subject: [PATCH 3/4] Add failing test --- .../SongSelect/TestSceneBeatmapCarousel.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 527367fdb8..f87d6ebebb 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -245,6 +245,30 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert($"Check #{set_count} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith($"#{set_count}!")); } + [Test] + public void TestSortingWithFiltered() + { + List sets = new List(); + + for (int i = 0; i < 3; i++) + { + var set = createTestBeatmapSet(i); + set.Beatmaps[0].StarDifficulty = 3 - i; + set.Beatmaps[2].StarDifficulty = 6 + i; + sets.Add(set); + } + + loadBeatmaps(sets); + + AddStep("Filter to normal", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Normal" }, false)); + AddAssert("Check first set at end", () => carousel.BeatmapSets.First() == sets.Last()); + AddAssert("Check last set at start", () => carousel.BeatmapSets.Last() == sets.First()); + + AddStep("Filter to insane", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Insane" }, false)); + AddAssert("Check first set at start", () => carousel.BeatmapSets.First() == sets.First()); + AddAssert("Check last set at end", () => carousel.BeatmapSets.Last() == sets.Last()); + } + [Test] public void TestRemoveAll() { From f15953d65ce00c38a2544234c426c33f55429cb0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 7 Oct 2019 15:16:26 +0900 Subject: [PATCH 4/4] Fix carousel including filtered difficulties in sort comparisons --- .../Select/Carousel/CarouselBeatmapSet.cs | 23 ++++++++++++++++--- .../Screens/Select/Carousel/CarouselGroup.cs | 2 +- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index 5a3996bb49..35816fe620 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -49,16 +49,33 @@ namespace osu.Game.Screens.Select.Carousel return otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded); case SortMode.BPM: - return BeatmapSet.MaxBPM.CompareTo(otherSet.BeatmapSet.MaxBPM); + return compareUsingAggregateMax(otherSet, b => b.BPM); case SortMode.Length: - return BeatmapSet.MaxLength.CompareTo(otherSet.BeatmapSet.MaxLength); + return compareUsingAggregateMax(otherSet, b => b.Length); case SortMode.Difficulty: - return BeatmapSet.MaxStarDifficulty.CompareTo(otherSet.BeatmapSet.MaxStarDifficulty); + return compareUsingAggregateMax(otherSet, b => b.StarDifficulty); } } + /// + /// All beatmaps which are not filtered and valid for display. + /// + protected IEnumerable ValidBeatmaps => Beatmaps.Where(b => !b.Filtered.Value).Select(b => b.Beatmap); + + private int compareUsingAggregateMax(CarouselBeatmapSet other, Func func) + { + var ourBeatmaps = ValidBeatmaps.Any(); + var otherBeatmaps = other.ValidBeatmaps.Any(); + + if (!ourBeatmaps && !otherBeatmaps) return 0; + if (!ourBeatmaps) return -1; + if (!otherBeatmaps) return 1; + + return ValidBeatmaps.Max(func).CompareTo(other.ValidBeatmaps.Max(func)); + } + public override void Filter(FilterCriteria criteria) { base.Filter(criteria); diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 6ebd2d41cc..09b728abeb 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -83,8 +83,8 @@ namespace osu.Game.Screens.Select.Carousel var children = new List(InternalChildren); - children.Sort((x, y) => x.CompareTo(criteria, y)); children.ForEach(c => c.Filter(criteria)); + children.Sort((x, y) => x.CompareTo(criteria, y)); InternalChildren = children; }