From 78dd975a351440a675cafda188a39b5392ba875b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 12 Dec 2017 17:48:38 +0900 Subject: [PATCH 01/53] Initial carousel infrastructue changes --- .../Visual/TestCasePlaySongSelect.cs | 13 +- osu.Game/Beatmaps/Drawables/BeatmapGroup.cs | 147 ------ osu.Game/Screens/Select/BeatmapCarousel.cs | 419 ++++++++---------- .../Select/Carousel/CarouselBeatmap.cs | 42 ++ .../Select/Carousel/CarouselBeatmapSet.cs | 62 +++ .../Screens/Select/Carousel/CarouselGroup.cs | 54 +++ .../Carousel/CarouselGroupEagerSelect.cs | 28 ++ .../Screens/Select/Carousel/CarouselItem.cs | 57 +++ .../Carousel/DrawableCarouselBeatmap.cs} | 115 ++--- .../Carousel/DrawableCarouselBeatmapSet.cs} | 75 ++-- .../Select/Carousel/DrawableCarouselItem.cs} | 80 ++-- osu.Game/Screens/Select/EditSongSelect.cs | 4 +- osu.Game/Screens/Select/FilterCriteria.cs | 54 --- osu.Game/Screens/Select/MatchSongSelect.cs | 4 +- osu.Game/Screens/Select/PlaySongSelect.cs | 25 +- osu.Game/Screens/Select/SongSelect.cs | 36 +- osu.Game/osu.Game.csproj | 14 +- 17 files changed, 594 insertions(+), 635 deletions(-) delete mode 100644 osu.Game/Beatmaps/Drawables/BeatmapGroup.cs create mode 100644 osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs create mode 100644 osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs create mode 100644 osu.Game/Screens/Select/Carousel/CarouselGroup.cs create mode 100644 osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs create mode 100644 osu.Game/Screens/Select/Carousel/CarouselItem.cs rename osu.Game/{Beatmaps/Drawables/BeatmapPanel.cs => Screens/Select/Carousel/DrawableCarouselBeatmap.cs} (81%) rename osu.Game/{Beatmaps/Drawables/BeatmapSetHeader.cs => Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs} (75%) rename osu.Game/{Beatmaps/Drawables/Panel.cs => Screens/Select/Carousel/DrawableCarouselItem.cs} (71%) diff --git a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs index 7c070fd3df..16c757702c 100644 --- a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs @@ -26,10 +26,16 @@ namespace osu.Game.Tests.Visual private DependencyContainer dependencies; + public override IReadOnlyList RequiredTypes => new[] + { + typeof(BeatmapCarousel), + typeof(SongSelect), + }; + protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); [BackgroundDependencyLoader] - private void load() + private void load(BeatmapManager baseMaanger) { PlaySongSelect songSelect; @@ -43,7 +49,10 @@ namespace osu.Game.Tests.Visual Func contextFactory = () => context; dependencies.Cache(rulesets = new RulesetStore(contextFactory)); - dependencies.Cache(manager = new BeatmapManager(storage, contextFactory, rulesets, null)); + dependencies.Cache(manager = new BeatmapManager(storage, contextFactory, rulesets, null) + { + DefaultBeatmap = baseMaanger.GetWorkingBeatmap(null) + }); for (int i = 0; i < 100; i += 10) manager.Import(createTestBeatmapSet(i)); diff --git a/osu.Game/Beatmaps/Drawables/BeatmapGroup.cs b/osu.Game/Beatmaps/Drawables/BeatmapGroup.cs deleted file mode 100644 index 9eb84421d4..0000000000 --- a/osu.Game/Beatmaps/Drawables/BeatmapGroup.cs +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright (c) 2007-2017 ppy Pty Ltd . -// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE - -using System; -using System.Collections.Generic; -using System.Linq; -using osu.Framework; -using osu.Framework.Graphics; - -namespace osu.Game.Beatmaps.Drawables -{ - public class BeatmapGroup : IStateful - { - public event Action StateChanged; - - public BeatmapPanel SelectedPanel; - - /// - /// Fires when one of our difficulties was selected. Will fire on first expand. - /// - public Action SelectionChanged; - - /// - /// Fires when one of our difficulties is clicked when already selected. Should start playing the map. - /// - public Action StartRequested; - - public Action DeleteRequested; - - public Action RestoreHiddenRequested; - - public Action HideDifficultyRequested; - - public Action EditRequested; - - public BeatmapSetHeader Header; - - public List BeatmapPanels; - - public BeatmapSetInfo BeatmapSet; - - private BeatmapGroupState state; - - public BeatmapGroupState State - { - get { return state; } - set - { - state = value; - UpdateState(); - StateChanged?.Invoke(state); - } - } - - public void UpdateState() - { - switch (state) - { - case BeatmapGroupState.Expanded: - Header.State = PanelSelectedState.Selected; - foreach (BeatmapPanel panel in BeatmapPanels) - if (panel == SelectedPanel) - panel.State = PanelSelectedState.Selected; - else if (panel.Filtered) - panel.State = PanelSelectedState.Hidden; - else - panel.State = PanelSelectedState.NotSelected; - break; - case BeatmapGroupState.Collapsed: - Header.State = PanelSelectedState.NotSelected; - foreach (BeatmapPanel panel in BeatmapPanels) - panel.State = PanelSelectedState.Hidden; - break; - case BeatmapGroupState.Hidden: - Header.State = PanelSelectedState.Hidden; - foreach (BeatmapPanel panel in BeatmapPanels) - panel.State = PanelSelectedState.Hidden; - break; - } - } - - public BeatmapGroup(BeatmapSetInfo beatmapSet, BeatmapManager manager) - { - if (beatmapSet == null) - throw new ArgumentNullException(nameof(beatmapSet)); - if (manager == null) - throw new ArgumentNullException(nameof(manager)); - - BeatmapSet = beatmapSet; - WorkingBeatmap beatmap = manager.GetWorkingBeatmap(BeatmapSet.Beatmaps.FirstOrDefault()); - - Header = new BeatmapSetHeader(beatmap) - { - GainedSelection = headerGainedSelection, - DeleteRequested = b => DeleteRequested(b), - RestoreHiddenRequested = b => RestoreHiddenRequested(b), - RelativeSizeAxes = Axes.X, - }; - - BeatmapPanels = BeatmapSet.Beatmaps.Where(b => !b.Hidden).OrderBy(b => b.RulesetID).ThenBy(b => b.StarDifficulty).Select(b => new BeatmapPanel(b) - { - Alpha = 0, - GainedSelection = panelGainedSelection, - HideRequested = p => HideDifficultyRequested?.Invoke(p), - StartRequested = p => StartRequested?.Invoke(p.Beatmap), - EditRequested = p => EditRequested?.Invoke(p.Beatmap), - RelativeSizeAxes = Axes.X, - }).ToList(); - - Header.AddDifficultyIcons(BeatmapPanels); - } - - - private void headerGainedSelection(BeatmapSetHeader panel) - { - State = BeatmapGroupState.Expanded; - - //we want to make sure one of our children is selected in the case none have been selected yet. - if (SelectedPanel == null) - BeatmapPanels.First(p => !p.Filtered).State = PanelSelectedState.Selected; - } - - private void panelGainedSelection(BeatmapPanel panel) - { - try - { - if (SelectedPanel == panel) return; - - if (SelectedPanel != null) - SelectedPanel.State = PanelSelectedState.NotSelected; - SelectedPanel = panel; - } - finally - { - State = BeatmapGroupState.Expanded; - SelectionChanged?.Invoke(this, SelectedPanel); - } - } - } - - public enum BeatmapGroupState - { - Collapsed, - Expanded, - Hidden, - } -} diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index f6b832d0f7..cede452402 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -7,7 +7,6 @@ using osu.Framework.Graphics.Containers; using System; using System.Collections.Generic; using System.Linq; -using osu.Game.Beatmaps.Drawables; using osu.Game.Configuration; using osu.Framework.Input; using OpenTK.Input; @@ -15,17 +14,19 @@ using osu.Framework.MathUtils; using System.Diagnostics; using System.Threading.Tasks; using osu.Framework.Allocation; +using osu.Framework.Caching; using osu.Framework.Threading; using osu.Framework.Configuration; using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Cursor; +using osu.Game.Screens.Select.Carousel; namespace osu.Game.Screens.Select { public class BeatmapCarousel : OsuScrollContainer { - public BeatmapInfo SelectedBeatmap => selectedPanel?.Beatmap; + public BeatmapInfo SelectedBeatmap => selectedBeatmap?.Beatmap; public override bool HandleInput => AllowSelection; @@ -33,27 +34,30 @@ namespace osu.Game.Screens.Select public IEnumerable Beatmaps { - get { return groups.Select(g => g.BeatmapSet); } + get { return carouselSets.Select(g => g.BeatmapSet); } set { scrollableContent.Clear(false); - panels.Clear(); - groups.Clear(); + items.Clear(); + carouselSets.Clear(); - List newGroups = null; + List newSets = null; Task.Run(() => { - newGroups = value.Select(createGroup).Where(g => g != null).ToList(); - criteria.Filter(newGroups); + newSets = value.Select(createGroup).Where(g => g != null).ToList(); + newSets.ForEach(g => g.Filter(criteria)); }).ContinueWith(t => { Schedule(() => { - foreach (var g in newGroups) + foreach (var g in newSets) addGroup(g); - computeYPositions(); + root = new CarouselGroup(newSets.OfType().ToList()); + items = root.Drawables.Value.ToList(); + + yPositionsCache.Invalidate(); BeatmapsChanged?.Invoke(); }); }); @@ -62,24 +66,19 @@ namespace osu.Game.Screens.Select private readonly List yPositions = new List(); - /// - /// Required for now unfortunately. - /// - private BeatmapManager manager; + private readonly Container scrollableContent; - private readonly Container scrollableContent; - - private readonly List groups = new List(); + private readonly List carouselSets = new List(); private Bindable randomType; - private readonly List seenGroups = new List(); + private readonly List seenSets = new List(); - private readonly List panels = new List(); + private List items = new List(); + private CarouselGroup root = new CarouselGroup(); - private readonly Stack> randomSelectedBeatmaps = new Stack>(); + private readonly Stack randomSelectedBeatmaps = new Stack(); - private BeatmapGroup selectedGroup; - private BeatmapPanel selectedPanel; + private CarouselBeatmap selectedBeatmap; public BeatmapCarousel() { @@ -87,7 +86,7 @@ namespace osu.Game.Screens.Select { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, - Child = scrollableContent = new Container + Child = scrollableContent = new Container { RelativeSizeAxes = Axes.X, } @@ -96,45 +95,44 @@ namespace osu.Game.Screens.Select public void RemoveBeatmap(BeatmapSetInfo beatmapSet) { - Schedule(() => removeGroup(groups.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); + Schedule(() => removeGroup(carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) { - // todo: this method should be smarter as to not recreate panels that haven't changed, etc. - var oldGroup = groups.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + // todo: this method should be smarter as to not recreate items that haven't changed, etc. + var oldGroup = carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); - var newGroup = createGroup(beatmapSet); + bool hadSelection = oldGroup?.State == CarouselItemState.Selected; - int index = groups.IndexOf(oldGroup); + var newSet = createGroup(beatmapSet); + + int index = carouselSets.IndexOf(oldGroup); if (index >= 0) - groups.RemoveAt(index); + carouselSets.RemoveAt(index); - if (newGroup != null) + if (newSet != null) { if (index >= 0) - groups.Insert(index, newGroup); + carouselSets.Insert(index, newSet); else - addGroup(newGroup); + addGroup(newSet); } - bool hadSelection = selectedGroup == oldGroup; - - if (hadSelection && newGroup == null) - selectedGroup = null; + if (hadSelection && newSet == null) + SelectNext(); Filter(null, false); //check if we can/need to maintain our current selection. - if (hadSelection && newGroup != null) + if (hadSelection && newSet != null) { - var newSelection = - newGroup.BeatmapPanels.Find(p => p.Beatmap.ID == selectedPanel?.Beatmap.ID); + var newSelection = newSet.Beatmaps.Find(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID); - if (newSelection == null && oldGroup != null && selectedPanel != null) - newSelection = newGroup.BeatmapPanels[Math.Min(newGroup.BeatmapPanels.Count - 1, oldGroup.BeatmapPanels.IndexOf(selectedPanel))]; + if (newSelection == null && selectedBeatmap != null) + newSelection = newSet.Beatmaps[Math.Min(newSet.Beatmaps.Count - 1, oldGroup.Beatmaps.IndexOf(selectedBeatmap))]; - selectGroup(newGroup, newSelection); + select(newSelection); } } @@ -148,12 +146,12 @@ namespace osu.Game.Screens.Select if (beatmap == SelectedBeatmap) return; - foreach (BeatmapGroup group in groups) + foreach (CarouselBeatmapSet group in carouselSets) { - var panel = group.BeatmapPanels.FirstOrDefault(p => p.Beatmap.Equals(beatmap)); - if (panel != null) + var item = group.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap)); + if (item != null) { - selectGroup(group, panel, animated); + select(item, animated); return; } } @@ -161,20 +159,9 @@ namespace osu.Game.Screens.Select public Action SelectionChanged; - public Action StartRequested; - - public Action DeleteRequested; - - public Action RestoreRequested; - - public Action EditRequested; - - public Action HideDifficultyRequested; - private void selectNullBeatmap() { - selectedGroup = null; - selectedPanel = null; + selectedBeatmap = null; SelectionChanged?.Invoke(null); } @@ -186,90 +173,71 @@ 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 (groups.All(g => g.State == BeatmapGroupState.Hidden)) + if (carouselSets.All(g => g.State == CarouselItemState.Hidden)) { selectNullBeatmap(); return; } - int originalIndex = Math.Max(0, groups.IndexOf(selectedGroup)); + int originalIndex = Math.Max(0, items.IndexOf(selectedBeatmap?.Drawables.Value.First())); int currentIndex = originalIndex; // local function to increment the index in the required direction, wrapping over extremities. - int incrementIndex() => currentIndex = (currentIndex + direction + groups.Count) % groups.Count; + int incrementIndex() => currentIndex = (currentIndex + direction + items.Count) % items.Count; - // in the case we are skipping difficulties, we want to increment the index once before starting to find out new target - // (we don't care about the currently selected group). - if (skipDifficulties) - incrementIndex(); - - do + while (incrementIndex() != originalIndex) { - var group = groups[currentIndex]; + var item = items[currentIndex].Item; - if (group.State == BeatmapGroupState.Hidden) continue; + if (item.Filtered || item.State == CarouselItemState.Selected) continue; - // we are only interested in non-filtered panels. - IEnumerable validPanels = group.BeatmapPanels.Where(p => !p.Filtered); - - // if we are considering difficulties, we need to do a few extrea steps. - if (!skipDifficulties) + switch (item) { - // we want to reverse the panel order if we are searching backwards. - if (direction < 0) - validPanels = validPanels.Reverse(); - - // if we are currently on the selected panel, let's try to find a valid difficulty before leaving to the next group. - // the first valid difficulty is found by skipping to the selected panel and then one further. - if (currentIndex == originalIndex) - validPanels = validPanels.SkipWhile(p => p != selectedPanel).Skip(1); + case CarouselBeatmap beatmap: + if (skipDifficulties) continue; + select(beatmap); + return; + case CarouselBeatmapSet set: + select(set); + return; } - - var next = validPanels.FirstOrDefault(); - - // at this point, we can perform the selection change if we have a valid new target, else continue to increment in the specified direction. - if (next != null) - { - selectGroup(group, next); - return; - } - } while (incrementIndex() != originalIndex); + } } - private IEnumerable getVisibleGroups() => groups.Where(selectGroup => selectGroup.State != BeatmapGroupState.Hidden); + private IEnumerable getVisibleGroups() => carouselSets.Where(select => select.State != CarouselItemState.NotSelected); public void SelectNextRandom() { - if (groups.Count == 0) + if (carouselSets.Count == 0) return; var visibleGroups = getVisibleGroups(); if (!visibleGroups.Any()) return; - if (selectedGroup != null) - randomSelectedBeatmaps.Push(new KeyValuePair(selectedGroup, selectedGroup.SelectedPanel)); + if (selectedBeatmap != null) + randomSelectedBeatmaps.Push(selectedBeatmap); - BeatmapGroup group; + CarouselBeatmapSet group; if (randomType == SelectionRandomType.RandomPermutation) { - var notSeenGroups = visibleGroups.Except(seenGroups); + var notSeenGroups = visibleGroups.Except(seenSets); if (!notSeenGroups.Any()) { - seenGroups.Clear(); + seenSets.Clear(); notSeenGroups = visibleGroups; } group = notSeenGroups.ElementAt(RNG.Next(notSeenGroups.Count())); - seenGroups.Add(group); + seenSets.Add(group); } else group = visibleGroups.ElementAt(RNG.Next(visibleGroups.Count())); - BeatmapPanel panel = group.BeatmapPanels[RNG.Next(group.BeatmapPanels.Count)]; + CarouselBeatmap item = group.Beatmaps[RNG.Next(group.Beatmaps.Count)]; - selectGroup(group, panel); + select(item); } public void SelectPreviousRandom() @@ -277,17 +245,13 @@ namespace osu.Game.Screens.Select if (!randomSelectedBeatmaps.Any()) return; - var visibleGroups = getVisibleGroups(); - if (!visibleGroups.Any()) - return; - while (randomSelectedBeatmaps.Any()) { - var beatmapCoordinates = randomSelectedBeatmaps.Pop(); - var group = beatmapCoordinates.Key; - if (visibleGroups.Contains(group)) + var beatmap = randomSelectedBeatmaps.Pop(); + + if (beatmap.Visible) { - selectGroup(group, beatmapCoordinates.Value); + select(beatmap); break; } } @@ -314,25 +278,14 @@ namespace osu.Game.Screens.Select { filterTask = null; - criteria.Filter(groups); + carouselSets.ForEach(s => s.Filter(criteria)); - var filtered = new List(groups); + yPositionsCache.Invalidate(); - scrollableContent.Clear(false); - panels.Clear(); - groups.Clear(); - - foreach (var g in filtered) - addGroup(g); - - computeYPositions(); - - selectedGroup?.UpdateState(); - - if (selectedGroup == null || selectedGroup.State == BeatmapGroupState.Hidden) + if (selectedBeatmap?.Visible != true) SelectNext(); else - selectGroup(selectedGroup, selectedPanel); + select(selectedBeatmap); }; filterTask?.Cancel(); @@ -350,54 +303,60 @@ namespace osu.Game.Screens.Select ScrollTo(selectedY, animated); } - private BeatmapGroup createGroup(BeatmapSetInfo beatmapSet) + private CarouselBeatmapSet createGroup(BeatmapSetInfo beatmapSet) { if (beatmapSet.Beatmaps.All(b => b.Hidden)) return null; + // todo: remove the need for this. foreach (var b in beatmapSet.Beatmaps) { if (b.Metadata == null) b.Metadata = beatmapSet.Metadata; } - return new BeatmapGroup(beatmapSet, manager) + var set = new CarouselBeatmapSet(beatmapSet); + + foreach (var c in set.Beatmaps) { - SelectionChanged = (g, p) => selectGroup(g, p), - StartRequested = b => StartRequested?.Invoke(), - DeleteRequested = b => DeleteRequested?.Invoke(b), - RestoreHiddenRequested = s => RestoreRequested?.Invoke(s), - EditRequested = b => EditRequested?.Invoke(b), - HideDifficultyRequested = b => HideDifficultyRequested?.Invoke(b), - State = BeatmapGroupState.Collapsed - }; + c.State.ValueChanged += v => + { + if (v == CarouselItemState.Selected) + { + selectedBeatmap = c; + SelectionChanged?.Invoke(c.Beatmap); + yPositionsCache.Invalidate(); + Schedule(() => ScrollToSelected()); + } + }; + } + + return set; } [BackgroundDependencyLoader(permitNulls: true)] - private void load(BeatmapManager manager, OsuConfigManager config) + private void load(OsuConfigManager config) { - this.manager = manager; - randomType = config.GetBindable(OsuSetting.SelectionRandomType); } - private void addGroup(BeatmapGroup group) + private void addGroup(CarouselBeatmapSet set) { // prevent duplicates by concurrent independent actions trying to add a group - if (groups.Any(g => g.BeatmapSet.ID == group.BeatmapSet.ID)) + //todo: check this + if (carouselSets.Any(g => g.BeatmapSet.ID == set.BeatmapSet.ID)) return; - groups.Add(group); - panels.Add(group.Header); - panels.AddRange(group.BeatmapPanels); + //todo: add to root + carouselSets.Add(set); } - private void removeGroup(BeatmapGroup group) + private void removeGroup(CarouselBeatmapSet set) { - if (group == null) + if (set == null) return; - if (selectedGroup == group) + if (set.State == CarouselItemState.Selected) { if (getVisibleGroups().Count() == 1) selectNullBeatmap(); @@ -405,21 +364,23 @@ namespace osu.Game.Screens.Select SelectNext(); } - groups.Remove(group); - panels.Remove(group.Header); - foreach (var p in group.BeatmapPanels) - panels.Remove(p); + carouselSets.Remove(set); - scrollableContent.Remove(group.Header); - scrollableContent.RemoveRange(group.BeatmapPanels); + foreach (var d in set.Drawables.Value) + { + items.Remove(d); + scrollableContent.Remove(d); + } - computeYPositions(); + yPositionsCache.Invalidate(); } + private Cached yPositionsCache = new Cached(); + /// - /// Computes the target Y positions for every panel in the carousel. + /// Computes the target Y positions for every item in the carousel. /// - /// The Y position of the currently selected panel. + /// The Y position of the currently selected item. private float computeYPositions(bool animated = true) { yPositions.Clear(); @@ -427,88 +388,61 @@ namespace osu.Game.Screens.Select float currentY = DrawHeight / 2; float selectedY = currentY; - foreach (BeatmapGroup group in groups) + float lastSetY = 0; + + foreach (DrawableCarouselItem d in items) { - movePanel(group.Header, group.State != BeatmapGroupState.Hidden, animated, ref currentY); - - if (group.State == BeatmapGroupState.Expanded) + switch (d) { - group.Header.MoveToX(-100, 500, Easing.OutExpo); - var headerY = group.Header.Position.Y; + case DrawableCarouselBeatmapSet set: + set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); - foreach (BeatmapPanel panel in group.BeatmapPanels) - { - if (panel == selectedPanel) - selectedY = currentY + panel.DrawHeight / 2 - DrawHeight / 2; + lastSetY = set.Position.Y; - panel.MoveToX(-50, 500, Easing.OutExpo); + movePanel(set, set.Item.Visible, animated, ref currentY); + break; + case DrawableCarouselBeatmap beatmap: + beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); - bool isHidden = panel.State == PanelSelectedState.Hidden; + if (beatmap.Item == selectedBeatmap) + selectedY = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; - //on first display we want to begin hidden under our group's header. - if (isHidden || panel.Alpha == 0) - panel.MoveToY(headerY); + // on first display we want to begin hidden under our group's header. + if (animated && !beatmap.IsPresent) + beatmap.MoveToY(lastSetY); - movePanel(panel, !isHidden, animated, ref currentY); - } - } - else - { - group.Header.MoveToX(0, 500, Easing.OutExpo); - - foreach (BeatmapPanel panel in group.BeatmapPanels) - { - panel.MoveToX(0, 500, Easing.OutExpo); - movePanel(panel, false, animated, ref currentY); - } + movePanel(beatmap, beatmap.Item.Visible, animated, ref currentY); + break; } } currentY += DrawHeight / 2; scrollableContent.Height = currentY; + yPositionsCache.Validate(); + return selectedY; } - private void movePanel(Panel panel, bool advance, bool animated, ref float currentY) + private void movePanel(DrawableCarouselItem item, bool advance, bool animated, ref float currentY) { yPositions.Add(currentY); - panel.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); + item.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); if (advance) - currentY += panel.DrawHeight + 5; + currentY += item.DrawHeight + 5; } - private void selectGroup(BeatmapGroup group, BeatmapPanel panel = null, bool animated = true) + private void select(CarouselBeatmapSet beatmapSet = null) { - try - { - if (panel == null || panel.Filtered == true) - panel = group.BeatmapPanels.First(p => !p.Filtered); + if (beatmapSet == null) return; + beatmapSet.State.Value = CarouselItemState.Selected; + } - if (selectedPanel == panel) return; - - Trace.Assert(group.BeatmapPanels.Contains(panel), @"Selected panel must be in provided group"); - - if (selectedGroup != null && selectedGroup != group && selectedGroup.State != BeatmapGroupState.Hidden) - selectedGroup.State = BeatmapGroupState.Collapsed; - - group.State = BeatmapGroupState.Expanded; - group.SelectedPanel = panel; - - panel.State = PanelSelectedState.Selected; - - if (selectedPanel == panel) return; - - selectedPanel = panel; - selectedGroup = group; - - SelectionChanged?.Invoke(panel.Beatmap); - } - finally - { - ScrollToSelected(animated); - } + private void select(CarouselBeatmap beatmap = null, bool animated = true) + { + if (beatmap == null) return; + beatmap.State.Value = CarouselItemState.Selected; } protected override bool OnKeyDown(InputState state, KeyDownEventArgs args) @@ -547,66 +481,67 @@ namespace osu.Game.Screens.Select float drawHeight = DrawHeight; - // Remove all panels that should no longer be on-screen - scrollableContent.RemoveAll(delegate(Panel p) + if (!yPositionsCache.IsValid) + computeYPositions(); + + // Remove all items that should no longer be on-screen + scrollableContent.RemoveAll(delegate (DrawableCarouselItem p) { - float panelPosY = p.Position.Y; - bool remove = panelPosY < Current - p.DrawHeight || panelPosY > Current + drawHeight || !p.IsPresent; + float itemPosY = p.Position.Y; + bool remove = itemPosY < Current - p.DrawHeight || itemPosY > Current + drawHeight || !p.IsPresent; return remove; }); - // Find index range of all panels that should be on-screen - Trace.Assert(panels.Count == yPositions.Count); + // Find index range of all items that should be on-screen + Trace.Assert(items.Count == yPositions.Count); - int firstIndex = yPositions.BinarySearch(Current - Panel.MAX_HEIGHT); + int firstIndex = yPositions.BinarySearch(Current - DrawableCarouselItem.MAX_HEIGHT); if (firstIndex < 0) firstIndex = ~firstIndex; int lastIndex = yPositions.BinarySearch(Current + drawHeight); if (lastIndex < 0) { lastIndex = ~lastIndex; - // Add the first panel of the last visible beatmap group to preload its data. - if (lastIndex != 0 && panels[lastIndex - 1] is BeatmapSetHeader) + // Add the first item of the last visible beatmap group to preload its data. + if (lastIndex != 0 && items[lastIndex - 1] is DrawableCarouselBeatmapSet) lastIndex++; } - // Add those panels within the previously found index range that should be displayed. + // Add those items within the previously found index range that should be displayed. for (int i = firstIndex; i < lastIndex; ++i) { - Panel panel = panels[i]; - if (panel.State == PanelSelectedState.Hidden) - continue; + DrawableCarouselItem item = items[i]; // Only add if we're not already part of the content. - if (!scrollableContent.Contains(panel)) + if (!scrollableContent.Contains(item)) { - // Makes sure headers are always _below_ panels, + // Makes sure headers are always _below_ items, // and depth flows downward. - panel.Depth = i + (panel is BeatmapSetHeader ? panels.Count : 0); + item.Depth = i + (item is DrawableCarouselBeatmapSet ? items.Count : 0); - switch (panel.LoadState) + switch (item.LoadState) { case LoadState.NotLoaded: - LoadComponentAsync(panel); + LoadComponentAsync(item); break; case LoadState.Loading: break; default: - scrollableContent.Add(panel); + scrollableContent.Add(item); break; } } } - // Update externally controlled state of currently visible panels + // Update externally controlled state of currently visible items // (e.g. x-offset and opacity). float halfHeight = drawHeight / 2; - foreach (Panel p in scrollableContent.Children) - updatePanel(p, halfHeight); + foreach (DrawableCarouselItem p in scrollableContent.Children) + updateItem(p, halfHeight); } /// - /// Computes the x-offset of currently visible panels. Makes the carousel appear round. + /// Computes the x-offset of currently visible items. Makes the carousel appear round. /// /// /// Vertical distance from the center of the carousel container @@ -624,20 +559,20 @@ namespace osu.Game.Screens.Select } /// - /// Update a panel's x position and multiplicative alpha based on its y position and + /// Update a item's x position and multiplicative alpha based on its y position and /// the current scroll position. /// - /// The panel to be updated. + /// The item to be updated. /// Half the draw height of the carousel container. - private void updatePanel(Panel p, float halfHeight) + private void updateItem(DrawableCarouselItem p, float halfHeight) { var height = p.IsPresent ? p.DrawHeight : 0; - float panelDrawY = p.Position.Y - Current + height / 2; - float dist = Math.Abs(1f - panelDrawY / halfHeight); + float itemDrawY = p.Position.Y - Current + height / 2; + float dist = Math.Abs(1f - itemDrawY / halfHeight); // Setting the origin position serves as an additive position on top of potential - // local transformation we may want to apply (e.g. when a panel gets selected, we + // local transformation we may want to apply (e.g. when a item gets selected, we // may want to smoothly transform it leftwards.) p.OriginPosition = new Vector2(-offsetX(dist, halfHeight), 0); diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs new file mode 100644 index 0000000000..f1bc24db50 --- /dev/null +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -0,0 +1,42 @@ +// 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; +using osu.Game.Beatmaps; + +namespace osu.Game.Screens.Select.Carousel +{ + public class CarouselBeatmap : CarouselItem + { + public readonly BeatmapInfo Beatmap; + + public CarouselBeatmap(BeatmapInfo beatmap) + { + Beatmap = beatmap; + State.Value = CarouselItemState.Hidden; + } + + protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmap(this) + { + /*GainedSelection = panelGainedSelection, + HideRequested = p => HideDifficultyRequested?.Invoke(p), + StartRequested = p => StartRequested?.Invoke(p.beatmap), + EditRequested = p => EditRequested?.Invoke(p.beatmap),*/ + }; + + public override void Filter(FilterCriteria criteria) + { + base.Filter(criteria); + + bool match = criteria.Ruleset == null || (Beatmap.RulesetID == criteria.Ruleset.ID || Beatmap.RulesetID == 0 && criteria.Ruleset.ID > 0 && criteria.AllowConvertedBeatmaps); + + if (!string.IsNullOrEmpty(criteria.SearchText)) + match &= + Beatmap.Metadata.SearchableTerms.Any(term => term.IndexOf(criteria.SearchText, StringComparison.InvariantCultureIgnoreCase) >= 0) || + Beatmap.Version.IndexOf(criteria.SearchText, StringComparison.InvariantCultureIgnoreCase) >= 0; + + Filtered.Value = !match; + } + } +} diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs new file mode 100644 index 0000000000..525bb6c600 --- /dev/null +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -0,0 +1,62 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System; +using System.Collections.Generic; +using System.Linq; +using osu.Game.Beatmaps; + +namespace osu.Game.Screens.Select.Carousel +{ + public class CarouselBeatmapSet : CarouselGroupEagerSelect + { + public readonly List Beatmaps; + + public BeatmapSetInfo BeatmapSet; + + public CarouselBeatmapSet(BeatmapSetInfo beatmapSet) + { + if (beatmapSet == null) throw new ArgumentNullException(nameof(beatmapSet)); + + BeatmapSet = beatmapSet; + + Children = Beatmaps = beatmapSet.Beatmaps + .Where(b => !b.Hidden) + .OrderBy(b => b.RulesetID).ThenBy(b => b.StarDifficulty) + .Select(b => new CarouselBeatmap(b)) + .ToList(); + } + + protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmapSet(this); + + public override void Filter(FilterCriteria criteria) + { + base.Filter(criteria); + Filtered.Value = Children.All(i => i.Filtered); + + /*switch (criteria.Sort) + { + default: + case SortMode.Artist: + groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Artist, y.BeatmapSet.Metadata.Artist, StringComparison.InvariantCultureIgnoreCase)); + break; + case SortMode.Title: + groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Title, y.BeatmapSet.Metadata.Title, StringComparison.InvariantCultureIgnoreCase)); + break; + case SortMode.Author: + groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Author.Username, y.BeatmapSet.Metadata.Author.Username, StringComparison.InvariantCultureIgnoreCase)); + break; + case SortMode.Difficulty: + groups.Sort((x, y) => x.BeatmapSet.MaxStarDifficulty.CompareTo(y.BeatmapSet.MaxStarDifficulty)); + break; + }*/ + } + } + + public enum CarouselItemState + { + Hidden, + NotSelected, + Selected, + } +} diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs new file mode 100644 index 0000000000..597fed7a34 --- /dev/null +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -0,0 +1,54 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System.Collections.Generic; +using osu.Framework.Configuration; +using osu.Framework.Extensions.IEnumerableExtensions; + +namespace osu.Game.Screens.Select.Carousel +{ + /// + /// A group which ensures only one child is selected. + /// + public class CarouselGroup : CarouselItem + { + private readonly List items; + + public readonly Bindable Selected = new Bindable(); + + protected override DrawableCarouselItem CreateDrawableRepresentation() => null; + + protected override IEnumerable Children + { + get { return base.Children; } + set + { + base.Children = value; + value.ForEach(i => i.State.ValueChanged += v => itemStateChanged(i, v)); + } + } + + public CarouselGroup(List items = null) + { + if (items != null) Children = items; + } + + private void itemStateChanged(CarouselItem item, CarouselItemState value) + { + // todo: check state of selected item. + + // ensure we are the only item selected + if (value == CarouselItemState.Selected) + { + foreach (var b in Children) + { + if (item == b) continue; + b.State.Value = CarouselItemState.NotSelected; + } + + State.Value = CarouselItemState.Selected; + Selected.Value = item; + } + } + } +} diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs new file mode 100644 index 0000000000..351f0ed55b --- /dev/null +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -0,0 +1,28 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System.Linq; + +namespace osu.Game.Screens.Select.Carousel +{ + /// + /// A group which ensures at least one child is selected (if the group itself is selected). + /// + public class CarouselGroupEagerSelect : CarouselGroup + { + public CarouselGroupEagerSelect() + { + State.ValueChanged += v => + { + if (v == CarouselItemState.Selected) + { + foreach (var c in Children.Where(c => c.State.Value == CarouselItemState.Hidden)) + c.State.Value = CarouselItemState.NotSelected; + + if (Children.Any(c => c.Visible) && Children.All(c => c.State != CarouselItemState.Selected)) + Children.First(c => c.Visible).State.Value = CarouselItemState.Selected; + } + }; + } + } +} diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs new file mode 100644 index 0000000000..d6d0615b6e --- /dev/null +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -0,0 +1,57 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System; +using System.Collections.Generic; +using osu.Framework.Configuration; +using osu.Framework.Extensions.IEnumerableExtensions; + +namespace osu.Game.Screens.Select.Carousel +{ + public abstract class CarouselItem + { + public readonly BindableBool Filtered = new BindableBool(); + + public readonly Bindable State = new Bindable(CarouselItemState.NotSelected); + + protected virtual IEnumerable Children { get; set; } + + public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered.Value; + + public readonly Lazy> Drawables; + + protected CarouselItem() + { + Drawables = new Lazy>(() => + { + List items = new List(); + + var self = CreateDrawableRepresentation(); + if (self != null) items.Add(self); + + if (Children != null) + foreach (var c in Children) + items.AddRange(c.Drawables.Value); + + return items; + }); + + State.ValueChanged += v => + { + if (Children == null) return; + + switch (v) + { + case CarouselItemState.Hidden: + case CarouselItemState.NotSelected: + Children.ForEach(c => c.State.Value = CarouselItemState.Hidden); + break; + } + }; + } + + protected abstract DrawableCarouselItem CreateDrawableRepresentation(); + + public virtual void Filter(FilterCriteria criteria) => Children?.ForEach(c => c.Filter(criteria)); + } +} diff --git a/osu.Game/Beatmaps/Drawables/BeatmapPanel.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs similarity index 81% rename from osu.Game/Beatmaps/Drawables/BeatmapPanel.cs rename to osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index d8ba1e9195..5170e0b98b 100644 --- a/osu.Game/Beatmaps/Drawables/BeatmapPanel.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -2,84 +2,50 @@ // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE using System; -using osu.Framework.Configuration; +using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.UserInterface; +using osu.Framework.Input; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Drawables; using osu.Game.Graphics; using osu.Game.Graphics.Backgrounds; +using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using OpenTK; using OpenTK.Graphics; -using osu.Framework.Input; -using osu.Game.Graphics.Sprites; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Graphics.UserInterface; -namespace osu.Game.Beatmaps.Drawables +namespace osu.Game.Screens.Select.Carousel { - public class BeatmapPanel : Panel, IHasContextMenu + public class DrawableCarouselBeatmap : DrawableCarouselItem, IHasContextMenu { - public BeatmapInfo Beatmap; + private readonly BeatmapInfo beatmap; + private readonly Sprite background; - public Action GainedSelection; - public Action StartRequested; - public Action EditRequested; + public Action StartRequested; + public Action EditRequested; public Action HideRequested; private readonly Triangles triangles; private readonly StarCounter starCounter; - protected override void Selected() + [BackgroundDependencyLoader] + private void load(SongSelect songSelect) { - base.Selected(); - - GainedSelection?.Invoke(this); - - background.Colour = ColourInfo.GradientVertical( - new Color4(20, 43, 51, 255), - new Color4(40, 86, 102, 255)); - - triangles.Colour = Color4.White; + StartRequested = songSelect.Start; + EditRequested = songSelect.Edit; } - protected override void Deselected() + public DrawableCarouselBeatmap(CarouselBeatmap panel) + : base(panel) { - base.Deselected(); - - background.Colour = new Color4(20, 43, 51, 255); - triangles.Colour = OsuColour.Gray(0.5f); - } - - protected override bool OnClick(InputState state) - { - if (State == PanelSelectedState.Selected) - StartRequested?.Invoke(this); - - return base.OnClick(state); - } - - public BindableBool Filtered = new BindableBool(); - - protected override void ApplyState(PanelSelectedState last = PanelSelectedState.Hidden) - { - if (!IsLoaded) return; - - base.ApplyState(last); - - if (last == PanelSelectedState.Hidden && State != last) - starCounter.ReplayAnimation(); - } - - public BeatmapPanel(BeatmapInfo beatmap) - { - if (beatmap == null) - throw new ArgumentNullException(nameof(beatmap)); - - Beatmap = beatmap; + beatmap = panel.Beatmap; Height *= 0.60f; Children = new Drawable[] @@ -160,11 +126,46 @@ namespace osu.Game.Beatmaps.Drawables }; } + protected override void Selected() + { + base.Selected(); + + background.Colour = ColourInfo.GradientVertical( + new Color4(20, 43, 51, 255), + new Color4(40, 86, 102, 255)); + + triangles.Colour = Color4.White; + } + + protected override void Deselected() + { + base.Deselected(); + + background.Colour = new Color4(20, 43, 51, 255); + triangles.Colour = OsuColour.Gray(0.5f); + } + + protected override bool OnClick(InputState state) + { + if (Item.State == CarouselItemState.Selected) + StartRequested?.Invoke(beatmap); + + return base.OnClick(state); + } + + protected override void ApplyState() + { + if (Item.State.Value != CarouselItemState.Hidden && Alpha == 0) + starCounter.ReplayAnimation(); + + base.ApplyState(); + } + public MenuItem[] ContextMenuItems => new MenuItem[] { - new OsuMenuItem("Play", MenuItemType.Highlighted, () => StartRequested?.Invoke(this)), - new OsuMenuItem("Edit", MenuItemType.Standard, () => EditRequested?.Invoke(this)), - new OsuMenuItem("Hide", MenuItemType.Destructive, () => HideRequested?.Invoke(Beatmap)), + new OsuMenuItem("Play", MenuItemType.Highlighted, () => StartRequested?.Invoke(beatmap)), + new OsuMenuItem("Edit", MenuItemType.Standard, () => EditRequested?.Invoke(beatmap)), + new OsuMenuItem("Hide", MenuItemType.Destructive, () => HideRequested?.Invoke(beatmap)), }; } } diff --git a/osu.Game/Beatmaps/Drawables/BeatmapSetHeader.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs similarity index 75% rename from osu.Game/Beatmaps/Drawables/BeatmapSetHeader.cs rename to osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index 9bb7b5c737..f4e43d14aa 100644 --- a/osu.Game/Beatmaps/Drawables/BeatmapSetHeader.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -4,46 +4,40 @@ using System; using System.Collections.Generic; using System.Linq; -using OpenTK; -using OpenTK.Graphics; using osu.Framework.Allocation; using osu.Framework.Configuration; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; -using osu.Framework.Localisation; -using osu.Game.Graphics.Sprites; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.UserInterface; +using osu.Framework.Localisation; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.Drawables; +using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; +using OpenTK; +using OpenTK.Graphics; -namespace osu.Game.Beatmaps.Drawables +namespace osu.Game.Screens.Select.Carousel { - public class BeatmapSetHeader : Panel, IHasContextMenu + public class DrawableCarouselBeatmapSet : DrawableCarouselItem, IHasContextMenu { - public Action GainedSelection; + public Action GainedSelection; public Action DeleteRequested; public Action RestoreHiddenRequested; - private readonly WorkingBeatmap beatmap; + private readonly BeatmapSetInfo beatmapSet; private readonly FillFlowContainer difficultyIcons; - public BeatmapSetHeader(WorkingBeatmap beatmap) + public DrawableCarouselBeatmapSet(CarouselBeatmapSet set) + : base(set) { - if (beatmap == null) - throw new ArgumentNullException(nameof(beatmap)); - - this.beatmap = beatmap; - - difficultyIcons = new FillFlowContainer - { - Margin = new MarginPadding { Top = 5 }, - AutoSizeAxes = Axes.Both, - }; + beatmapSet = set.BeatmapSet; } protected override void Selected() @@ -53,15 +47,17 @@ namespace osu.Game.Beatmaps.Drawables } [BackgroundDependencyLoader] - private void load(LocalisationEngine localisation) + private void load(LocalisationEngine localisation, BeatmapManager manager) { if (localisation == null) throw new ArgumentNullException(nameof(localisation)); + var working = manager.GetWorkingBeatmap(beatmapSet.Beatmaps.FirstOrDefault()); + Children = new Drawable[] { new DelayedLoadWrapper( - new PanelBackground(beatmap) + new PanelBackground(working) { RelativeSizeAxes = Axes.Both, OnLoadComplete = d => d.FadeInFromZero(400, Easing.Out), @@ -76,18 +72,23 @@ namespace osu.Game.Beatmaps.Drawables new OsuSpriteText { Font = @"Exo2.0-BoldItalic", - Current = localisation.GetUnicodePreference(beatmap.Metadata.TitleUnicode, beatmap.Metadata.Title), + Current = localisation.GetUnicodePreference(beatmapSet.Metadata.TitleUnicode, beatmapSet.Metadata.Title), TextSize = 22, Shadow = true, }, new OsuSpriteText { Font = @"Exo2.0-SemiBoldItalic", - Current = localisation.GetUnicodePreference(beatmap.Metadata.ArtistUnicode, beatmap.Metadata.Artist), + Current = localisation.GetUnicodePreference(beatmapSet.Metadata.ArtistUnicode, beatmapSet.Metadata.Artist), TextSize = 17, Shadow = true, }, - difficultyIcons + new FillFlowContainer + { + Margin = new MarginPadding { Top = 5 }, + AutoSizeAxes = Axes.Both, + Children = ((CarouselBeatmapSet)Item).Beatmaps.Select(b => new FilterableDifficultyIcon(b)).ToList() + } } } }; @@ -153,27 +154,19 @@ namespace osu.Game.Beatmaps.Drawables } } - public void AddDifficultyIcons(IEnumerable panels) - { - if (panels == null) - throw new ArgumentNullException(nameof(panels)); - - difficultyIcons.AddRange(panels.Select(p => new FilterableDifficultyIcon(p))); - } - public MenuItem[] ContextMenuItems { get { List items = new List(); - if (State == PanelSelectedState.NotSelected) - items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => State = PanelSelectedState.Selected)); + if (Item.State == CarouselItemState.NotSelected) + items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => Item.State.Value = CarouselItemState.Selected)); - if (beatmap.BeatmapSetInfo.Beatmaps.Any(b => b.Hidden)) - items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => RestoreHiddenRequested?.Invoke(beatmap.BeatmapSetInfo))); + if (beatmapSet.Beatmaps.Any(b => b.Hidden)) + items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => RestoreHiddenRequested?.Invoke(beatmapSet))); - items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => DeleteRequested?.Invoke(beatmap.BeatmapSetInfo))); + items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => DeleteRequested?.Invoke(beatmapSet))); return items.ToArray(); } @@ -183,11 +176,11 @@ namespace osu.Game.Beatmaps.Drawables { private readonly BindableBool filtered = new BindableBool(); - public FilterableDifficultyIcon(BeatmapPanel panel) - : base(panel.Beatmap) + public FilterableDifficultyIcon(CarouselBeatmap item) + : base(item.Beatmap) { - filtered.BindTo(panel.Filtered); - filtered.ValueChanged += v => this.FadeTo(v ? 0.1f : 1, 100); + filtered.BindTo(item.Filtered); + filtered.ValueChanged += v => Schedule(() => this.FadeTo(v ? 0.1f : 1, 100)); filtered.TriggerChange(); } } diff --git a/osu.Game/Beatmaps/Drawables/Panel.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs similarity index 71% rename from osu.Game/Beatmaps/Drawables/Panel.cs rename to osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs index c990a0ea46..040ff22f4e 100644 --- a/osu.Game/Beatmaps/Drawables/Panel.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs @@ -1,41 +1,44 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE -using System; -using osu.Framework; +using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; -using osu.Framework.Allocation; +using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; using osu.Framework.Input; +using osu.Framework.MathUtils; +using osu.Game.Graphics; using OpenTK; using OpenTK.Graphics; -using osu.Framework.Extensions.Color4Extensions; -using osu.Framework.MathUtils; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics; -namespace osu.Game.Beatmaps.Drawables +namespace osu.Game.Screens.Select.Carousel { - public class Panel : Container, IStateful + public abstract class DrawableCarouselItem : Container { public const float MAX_HEIGHT = 80; - public event Action StateChanged; - public override bool RemoveWhenNotAlive => false; - private readonly Container nestedContainer; + public override bool IsPresent => base.IsPresent || Item.Visible; + public readonly CarouselItem Item; + + private readonly Container nestedContainer; private readonly Container borderContainer; private readonly Box hoverLayer; protected override Container Content => nestedContainer; - protected Panel() + protected DrawableCarouselItem(CarouselItem item) { + Item = item; + Item.Filtered.ValueChanged += v => Schedule(ApplyState); + Item.State.ValueChanged += v => Schedule(ApplyState); + Height = MAX_HEIGHT; RelativeSizeAxes = Axes.X; @@ -59,8 +62,6 @@ namespace osu.Game.Beatmaps.Drawables }, } }); - - Alpha = 0; } private SampleChannel sampleHover; @@ -86,10 +87,7 @@ namespace osu.Game.Beatmaps.Drawables base.OnHoverLost(state); } - public void SetMultiplicativeAlpha(float alpha) - { - borderContainer.Alpha = alpha; - } + public void SetMultiplicativeAlpha(float alpha) => borderContainer.Alpha = alpha; protected override void LoadComplete() { @@ -97,49 +95,30 @@ namespace osu.Game.Beatmaps.Drawables ApplyState(); } - protected virtual void ApplyState(PanelSelectedState last = PanelSelectedState.Hidden) + protected virtual void ApplyState() { if (!IsLoaded) return; - switch (state) + switch (Item.State.Value) { - case PanelSelectedState.Hidden: - case PanelSelectedState.NotSelected: + case CarouselItemState.NotSelected: Deselected(); break; - case PanelSelectedState.Selected: + case CarouselItemState.Selected: Selected(); break; } - if (state == PanelSelectedState.Hidden) + if (!Item.Visible) this.FadeOut(300, Easing.OutQuint); else this.FadeIn(250); } - private PanelSelectedState state = PanelSelectedState.NotSelected; - - public PanelSelectedState State - { - get { return state; } - - set - { - if (state == value) - return; - - var last = state; - state = value; - - ApplyState(last); - - StateChanged?.Invoke(State); - } - } - protected virtual void Selected() { + Item.State.Value = CarouselItemState.Selected; + borderContainer.BorderThickness = 2.5f; borderContainer.EdgeEffect = new EdgeEffectParameters { @@ -152,6 +131,8 @@ namespace osu.Game.Beatmaps.Drawables protected virtual void Deselected() { + Item.State.Value = CarouselItemState.NotSelected; + borderContainer.BorderThickness = 0; borderContainer.EdgeEffect = new EdgeEffectParameters { @@ -164,15 +145,8 @@ namespace osu.Game.Beatmaps.Drawables protected override bool OnClick(InputState state) { - State = PanelSelectedState.Selected; + Item.State.Value = CarouselItemState.Selected; return true; } } - - public enum PanelSelectedState - { - Hidden, - NotSelected, - Selected - } } diff --git a/osu.Game/Screens/Select/EditSongSelect.cs b/osu.Game/Screens/Select/EditSongSelect.cs index 907c080729..f02d25501e 100644 --- a/osu.Game/Screens/Select/EditSongSelect.cs +++ b/osu.Game/Screens/Select/EditSongSelect.cs @@ -1,14 +1,12 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE -using osu.Framework.Input; - namespace osu.Game.Screens.Select { public class EditSongSelect : SongSelect { protected override bool ShowFooter => false; - protected override void OnSelected(InputState state) => Exit(); + protected override void Start() => Exit(); } } diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index f410c69212..8e99e29c1f 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -1,11 +1,6 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE -using System; -using System.Collections.Generic; -using System.Linq; -using osu.Game.Beatmaps; -using osu.Game.Beatmaps.Drawables; using osu.Game.Rulesets; using osu.Game.Screens.Select.Filter; @@ -18,54 +13,5 @@ namespace osu.Game.Screens.Select public string SearchText; public RulesetInfo Ruleset; public bool AllowConvertedBeatmaps; - - private bool canConvert(BeatmapInfo beatmapInfo) => beatmapInfo.RulesetID == Ruleset.ID || beatmapInfo.RulesetID == 0 && Ruleset.ID > 0 && AllowConvertedBeatmaps; - - public void Filter(List groups) - { - foreach (var g in groups) - { - var set = g.BeatmapSet; - - // we only support converts from osu! mode to other modes for now. - // in the future this will have to change, at which point this condition will become a touch more complicated. - bool hasCurrentMode = set.Beatmaps.Any(canConvert); - - bool match = hasCurrentMode; - - if (!string.IsNullOrEmpty(SearchText)) - match &= set.Metadata.SearchableTerms.Any(term => term.IndexOf(SearchText, StringComparison.InvariantCultureIgnoreCase) >= 0); - - foreach (var panel in g.BeatmapPanels) - panel.Filtered.Value = !canConvert(panel.Beatmap); - - switch (g.State) - { - case BeatmapGroupState.Hidden: - if (match) g.State = BeatmapGroupState.Collapsed; - break; - default: - if (!match) g.State = BeatmapGroupState.Hidden; - break; - } - } - - switch (Sort) - { - default: - case SortMode.Artist: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Artist, y.BeatmapSet.Metadata.Artist, StringComparison.InvariantCultureIgnoreCase)); - break; - case SortMode.Title: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Title, y.BeatmapSet.Metadata.Title, StringComparison.InvariantCultureIgnoreCase)); - break; - case SortMode.Author: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Author.Username, y.BeatmapSet.Metadata.Author.Username, StringComparison.InvariantCultureIgnoreCase)); - break; - case SortMode.Difficulty: - groups.Sort((x, y) => x.BeatmapSet.MaxStarDifficulty.CompareTo(y.BeatmapSet.MaxStarDifficulty)); - break; - } - } } } diff --git a/osu.Game/Screens/Select/MatchSongSelect.cs b/osu.Game/Screens/Select/MatchSongSelect.cs index 2d3b198478..898c195432 100644 --- a/osu.Game/Screens/Select/MatchSongSelect.cs +++ b/osu.Game/Screens/Select/MatchSongSelect.cs @@ -1,12 +1,10 @@ // Copyright (c) 2007-2017 ppy Pty Ltd . // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE -using osu.Framework.Input; - namespace osu.Game.Screens.Select { public class MatchSongSelect : SongSelect { - protected override void OnSelected(InputState state) => Exit(); + protected override void Start() => Exit(); } } diff --git a/osu.Game/Screens/Select/PlaySongSelect.cs b/osu.Game/Screens/Select/PlaySongSelect.cs index bba6ddf577..565a7a0a01 100644 --- a/osu.Game/Screens/Select/PlaySongSelect.cs +++ b/osu.Game/Screens/Select/PlaySongSelect.cs @@ -8,7 +8,6 @@ using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Input; using osu.Framework.Screens; using osu.Game.Beatmaps; using osu.Game.Graphics; @@ -114,22 +113,22 @@ namespace osu.Game.Screens.Select return false; } - protected override void OnSelected(InputState state) + protected override void Start() { if (player != null) return; - if (state?.Keyboard.ControlPressed == true) - { - var auto = Ruleset.Value.CreateInstance().GetAutoplayMod(); - var autoType = auto.GetType(); + //if (state?.Keyboard.ControlPressed == true) + //{ + // var auto = Ruleset.Value.CreateInstance().GetAutoplayMod(); + // var autoType = auto.GetType(); - var mods = modSelect.SelectedMods.Value; - if (mods.All(m => m.GetType() != autoType)) - { - modSelect.SelectedMods.Value = mods.Concat(new[] { auto }); - removeAutoModOnResume = true; - } - } + // var mods = modSelect.SelectedMods.Value; + // if (mods.All(m => m.GetType() != autoType)) + // { + // modSelect.SelectedMods.Value = mods.Concat(new[] { auto }); + // removeAutoModOnResume = true; + // } + //} Beatmap.Value.Track.Looping = false; Beatmap.Disabled = true; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 7fb6a82981..b2e2c8bac6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -67,6 +67,10 @@ namespace osu.Game.Screens.Select public readonly FilterControl FilterControl; + private DependencyContainer dependencies; + + protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); + protected SongSelect() { const float carousel_width = 640; @@ -106,13 +110,11 @@ namespace osu.Game.Screens.Select Size = new Vector2(carousel_width, 1), Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, + + //todo: clicking play on another map doesn't work bindable disabled SelectionChanged = carouselSelectionChanged, BeatmapsChanged = carouselBeatmapsLoaded, - DeleteRequested = promptDelete, - RestoreRequested = s => { foreach (var b in s.Beatmaps) beatmaps.Restore(b); }, - EditRequested = editRequested, - HideDifficultyRequested = b => beatmaps.Hide(b), - StartRequested = () => carouselRaisedStart(), + //RestoreRequested = s => { foreach (var b in s.Beatmaps) beatmaps.Restore(b); }, }); Add(FilterControl = new FilterControl { @@ -163,12 +165,14 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(permitNulls: true)] private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuGame osu, OsuColour colours) { + dependencies.Cache(this); + if (Footer != null) { Footer.AddButton(@"random", colours.Green, triggerRandom, Key.F2); Footer.AddButton(@"options", colours.Blue, BeatmapOptions, Key.F3); - BeatmapOptions.AddButton(@"Delete", @"Beatmap", FontAwesome.fa_trash, colours.Pink, () => promptDelete(Beatmap.Value.BeatmapSetInfo), Key.Number4, float.MaxValue); + BeatmapOptions.AddButton(@"Delete", @"Beatmap", FontAwesome.fa_trash, colours.Pink, () => Delete(Beatmap.Value.BeatmapSetInfo), Key.Number4, float.MaxValue); } if (this.beatmaps == null) @@ -197,7 +201,7 @@ namespace osu.Game.Screens.Select carousel.AllowSelection = !Beatmap.Disabled; } - private void editRequested(BeatmapInfo beatmap) + public void Edit(BeatmapInfo beatmap) { Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap, Beatmap); Push(new Editor()); @@ -229,7 +233,7 @@ namespace osu.Game.Screens.Select carousel.SelectNextRandom(); } - private void carouselRaisedStart(InputState state = null) + public void Start(BeatmapInfo beatmap) { // if we have a pending filter operation, we want to run it now. // it could change selection (ie. if the ruleset has been changed). @@ -242,7 +246,9 @@ namespace osu.Game.Screens.Select selectionChangedDebounce = null; } - OnSelected(state); + carousel.SelectBeatmap(beatmap); + + Start(); } private ScheduledDelegate selectionChangedDebounce; @@ -261,7 +267,7 @@ namespace osu.Game.Screens.Select // In these cases, the other component has already loaded the beatmap, so we don't need to do so again. if (beatmap?.Equals(Beatmap.Value.BeatmapInfo) != true) { - bool preview = beatmap?.BeatmapSetInfoID != Beatmap.Value.BeatmapInfo.BeatmapSetInfoID; + bool preview = beatmap?.BeatmapSetInfoID != Beatmap.Value?.BeatmapInfo.BeatmapSetInfoID; Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap, Beatmap); ensurePlayingSelected(preview); @@ -301,7 +307,7 @@ namespace osu.Game.Screens.Select carousel.SelectNextRandom(); } - protected abstract void OnSelected(InputState state); + protected abstract void Start(); private void filterChanged(FilterCriteria criteria, bool debounce = true) { @@ -346,7 +352,7 @@ namespace osu.Game.Screens.Select logo.Action = () => { - carouselRaisedStart(); + Start(); return false; }; } @@ -458,7 +464,7 @@ namespace osu.Game.Screens.Select Beatmap.SetDefault(); } - private void promptDelete(BeatmapSetInfo beatmap) + public void Delete(BeatmapSetInfo beatmap) { if (beatmap == null) return; @@ -474,13 +480,13 @@ namespace osu.Game.Screens.Select { case Key.KeypadEnter: case Key.Enter: - carouselRaisedStart(state); + Start(); return true; case Key.Delete: if (state.Keyboard.ShiftPressed) { if (!Beatmap.IsDefault) - promptDelete(Beatmap.Value.BeatmapSetInfo); + Delete(Beatmap.Value.BeatmapSetInfo); return true; } break; diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 53ad323134..0bc7fa3f67 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -262,10 +262,7 @@ - - - @@ -316,7 +313,6 @@ - @@ -756,6 +752,14 @@ + + + + + + + + @@ -855,4 +859,4 @@ - + \ No newline at end of file From 99b00143ebf59b5a0846a6c923f0100f62293739 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 12:46:02 +0900 Subject: [PATCH 02/53] More clean-ups and event bindings --- osu.Game/Configuration/OsuConfigManager.cs | 2 +- osu.Game/Configuration/SelectionRandomType.cs | 4 +- .../Sections/Gameplay/SongSelectSettings.cs | 4 +- osu.Game/Screens/Select/BeatmapCarousel.cs | 63 ++++----- .../Select/Carousel/CarouselBeatmap.cs | 10 +- .../Carousel/DrawableCarouselBeatmap.cs | 3 +- .../Carousel/DrawableCarouselBeatmapSet.cs | 60 ++++----- osu.Game/Screens/Select/SongSelect.cs | 120 +++++++++--------- osu.Game/osu.Game.csproj | 2 +- 9 files changed, 116 insertions(+), 152 deletions(-) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index 1a7d29e907..1f7808dceb 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -20,7 +20,7 @@ namespace osu.Game.Configuration Set(OsuSetting.DisplayStarsMinimum, 0.0, 0, 10, 0.1); Set(OsuSetting.DisplayStarsMaximum, 10.0, 0, 10, 0.1); - Set(OsuSetting.SelectionRandomType, SelectionRandomType.RandomPermutation); + Set(OsuSetting.SelectionRandomType, SongSelectRandomMode.RandomPermutation); Set(OsuSetting.ChatDisplayHeight, ChatOverlay.DEFAULT_HEIGHT, 0.2, 1); diff --git a/osu.Game/Configuration/SelectionRandomType.cs b/osu.Game/Configuration/SelectionRandomType.cs index 298ee71e36..4ff52fab37 100644 --- a/osu.Game/Configuration/SelectionRandomType.cs +++ b/osu.Game/Configuration/SelectionRandomType.cs @@ -5,11 +5,11 @@ using System.ComponentModel; namespace osu.Game.Configuration { - public enum SelectionRandomType + public enum SongSelectRandomMode { [Description("Never repeat")] RandomPermutation, [Description("Random")] Random } -} \ No newline at end of file +} diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs index 9875ee8004..890c28aa07 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs @@ -34,10 +34,10 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay Bindable = config.GetBindable(OsuSetting.DisplayStarsMaximum), KeyboardStep = 1f }, - new SettingsEnumDropdown + new SettingsEnumDropdown { LabelText = "Random beatmap selection", - Bindable = config.GetBindable(OsuSetting.SelectionRandomType), + Bindable = config.GetBindable(OsuSetting.SelectionRandomType), } }; } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index cede452402..c332f26c09 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -52,7 +52,7 @@ namespace osu.Game.Screens.Select Schedule(() => { foreach (var g in newSets) - addGroup(g); + addBeatmapSet(g); root = new CarouselGroup(newSets.OfType().ToList()); items = root.Drawables.Value.ToList(); @@ -65,12 +65,13 @@ namespace osu.Game.Screens.Select } private readonly List yPositions = new List(); + private Cached yPositionsCache = new Cached(); private readonly Container scrollableContent; private readonly List carouselSets = new List(); - private Bindable randomType; + private Bindable randomType; private readonly List seenSets = new List(); private List items = new List(); @@ -95,7 +96,7 @@ namespace osu.Game.Screens.Select public void RemoveBeatmap(BeatmapSetInfo beatmapSet) { - Schedule(() => removeGroup(carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); + Schedule(() => removeBeatmapSet(carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) @@ -116,7 +117,7 @@ namespace osu.Game.Screens.Select if (index >= 0) carouselSets.Insert(index, newSet); else - addGroup(newSet); + addBeatmapSet(newSet); } if (hadSelection && newSet == null) @@ -151,7 +152,7 @@ namespace osu.Game.Screens.Select var item = group.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap)); if (item != null) { - select(item, animated); + select(item); return; } } @@ -220,7 +221,7 @@ namespace osu.Game.Screens.Select CarouselBeatmapSet group; - if (randomType == SelectionRandomType.RandomPermutation) + if (randomType == SongSelectRandomMode.RandomPermutation) { var notSeenGroups = visibleGroups.Except(seenSets); if (!notSeenGroups.Any()) @@ -337,10 +338,10 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config) { - randomType = config.GetBindable(OsuSetting.SelectionRandomType); + randomType = config.GetBindable(OsuSetting.SelectionRandomType); } - private void addGroup(CarouselBeatmapSet set) + private void addBeatmapSet(CarouselBeatmapSet set) { // prevent duplicates by concurrent independent actions trying to add a group //todo: check this @@ -351,19 +352,11 @@ namespace osu.Game.Screens.Select carouselSets.Add(set); } - private void removeGroup(CarouselBeatmapSet set) + private void removeBeatmapSet(CarouselBeatmapSet set) { if (set == null) return; - if (set.State == CarouselItemState.Selected) - { - if (getVisibleGroups().Count() == 1) - selectNullBeatmap(); - else - SelectNext(); - } - carouselSets.Remove(set); foreach (var d in set.Drawables.Value) @@ -372,11 +365,12 @@ namespace osu.Game.Screens.Select scrollableContent.Remove(d); } + if (set.State == CarouselItemState.Selected) + SelectNext(); + yPositionsCache.Invalidate(); } - private Cached yPositionsCache = new Cached(); - /// /// Computes the target Y positions for every item in the carousel. /// @@ -396,10 +390,7 @@ namespace osu.Game.Screens.Select { case DrawableCarouselBeatmapSet set: set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); - lastSetY = set.Position.Y; - - movePanel(set, set.Item.Visible, animated, ref currentY); break; case DrawableCarouselBeatmap beatmap: beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); @@ -410,10 +401,14 @@ namespace osu.Game.Screens.Select // on first display we want to begin hidden under our group's header. if (animated && !beatmap.IsPresent) beatmap.MoveToY(lastSetY); - - movePanel(beatmap, beatmap.Item.Visible, animated, ref currentY); break; } + + yPositions.Add(currentY); + d.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); + + if (d.Item.Visible) + currentY += d.DrawHeight + 5; } currentY += DrawHeight / 2; @@ -424,25 +419,11 @@ namespace osu.Game.Screens.Select return selectedY; } - private void movePanel(DrawableCarouselItem item, bool advance, bool animated, ref float currentY) + private void select(CarouselItem item) { - yPositions.Add(currentY); - item.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); + if (item == null) return; - if (advance) - currentY += item.DrawHeight + 5; - } - - private void select(CarouselBeatmapSet beatmapSet = null) - { - if (beatmapSet == null) return; - beatmapSet.State.Value = CarouselItemState.Selected; - } - - private void select(CarouselBeatmap beatmap = null, bool animated = true) - { - if (beatmap == null) return; - beatmap.State.Value = CarouselItemState.Selected; + item.State.Value = CarouselItemState.Selected; } protected override bool OnKeyDown(InputState state, KeyDownEventArgs args) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index f1bc24db50..655579269f 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -17,19 +17,13 @@ namespace osu.Game.Screens.Select.Carousel State.Value = CarouselItemState.Hidden; } - protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmap(this) - { - /*GainedSelection = panelGainedSelection, - HideRequested = p => HideDifficultyRequested?.Invoke(p), - StartRequested = p => StartRequested?.Invoke(p.beatmap), - EditRequested = p => EditRequested?.Invoke(p.beatmap),*/ - }; + protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmap(this); public override void Filter(FilterCriteria criteria) { base.Filter(criteria); - bool match = criteria.Ruleset == null || (Beatmap.RulesetID == criteria.Ruleset.ID || Beatmap.RulesetID == 0 && criteria.Ruleset.ID > 0 && criteria.AllowConvertedBeatmaps); + bool match = criteria.Ruleset == null || Beatmap.RulesetID == criteria.Ruleset.ID || Beatmap.RulesetID == 0 && criteria.Ruleset.ID > 0 && criteria.AllowConvertedBeatmaps; if (!string.IsNullOrEmpty(criteria.SearchText)) match &= diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 5170e0b98b..61a916d0a3 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -36,10 +36,11 @@ namespace osu.Game.Screens.Select.Carousel private readonly StarCounter starCounter; [BackgroundDependencyLoader] - private void load(SongSelect songSelect) + private void load(SongSelect songSelect, BeatmapManager manager) { StartRequested = songSelect.Start; EditRequested = songSelect.Edit; + HideRequested = manager.Hide; } public DrawableCarouselBeatmap(CarouselBeatmap panel) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index f4e43d14aa..9a1ee663a1 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -24,10 +24,7 @@ namespace osu.Game.Screens.Select.Carousel { public class DrawableCarouselBeatmapSet : DrawableCarouselItem, IHasContextMenu { - public Action GainedSelection; - public Action DeleteRequested; - public Action RestoreHiddenRequested; private readonly BeatmapSetInfo beatmapSet; @@ -40,18 +37,15 @@ namespace osu.Game.Screens.Select.Carousel beatmapSet = set.BeatmapSet; } - protected override void Selected() - { - base.Selected(); - GainedSelection?.Invoke(this); - } - [BackgroundDependencyLoader] private void load(LocalisationEngine localisation, BeatmapManager manager) { if (localisation == null) throw new ArgumentNullException(nameof(localisation)); + RestoreHiddenRequested = s => s.Beatmaps.ForEach(manager.Restore); + DeleteRequested = manager.Delete; + var working = manager.GetWorkingBeatmap(beatmapSet.Beatmaps.FirstOrDefault()); Children = new Drawable[] @@ -61,7 +55,8 @@ namespace osu.Game.Screens.Select.Carousel { RelativeSizeAxes = Axes.Both, OnLoadComplete = d => d.FadeInFromZero(400, Easing.Out), - }, 300), + }, 300 + ), new FillFlowContainer { Direction = FillDirection.Vertical, @@ -94,6 +89,24 @@ namespace osu.Game.Screens.Select.Carousel }; } + public MenuItem[] ContextMenuItems + { + get + { + List items = new List(); + + if (Item.State == CarouselItemState.NotSelected) + items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => Item.State.Value = CarouselItemState.Selected)); + + if (beatmapSet.Beatmaps.Any(b => b.Hidden)) + items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => RestoreHiddenRequested?.Invoke(beatmapSet))); + + items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => DeleteRequested?.Invoke(beatmapSet))); + + return items.ToArray(); + } + } + private class PanelBackground : BufferedContainer { public PanelBackground(WorkingBeatmap working) @@ -130,22 +143,19 @@ namespace osu.Game.Screens.Select.Carousel new Box { RelativeSizeAxes = Axes.Both, - Colour = ColourInfo.GradientHorizontal( - Color4.Black, new Color4(0f, 0f, 0f, 0.9f)), + Colour = ColourInfo.GradientHorizontal(Color4.Black, new Color4(0f, 0f, 0f, 0.9f)), Width = 0.05f, }, new Box { RelativeSizeAxes = Axes.Both, - Colour = ColourInfo.GradientHorizontal( - new Color4(0f, 0f, 0f, 0.9f), new Color4(0f, 0f, 0f, 0.1f)), + Colour = ColourInfo.GradientHorizontal(new Color4(0f, 0f, 0f, 0.9f), new Color4(0f, 0f, 0f, 0.1f)), Width = 0.2f, }, new Box { RelativeSizeAxes = Axes.Both, - Colour = ColourInfo.GradientHorizontal( - new Color4(0f, 0f, 0f, 0.1f), new Color4(0, 0, 0, 0)), + Colour = ColourInfo.GradientHorizontal(new Color4(0f, 0f, 0f, 0.1f), new Color4(0, 0, 0, 0)), Width = 0.05f, }, } @@ -154,24 +164,6 @@ namespace osu.Game.Screens.Select.Carousel } } - public MenuItem[] ContextMenuItems - { - get - { - List items = new List(); - - if (Item.State == CarouselItemState.NotSelected) - items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => Item.State.Value = CarouselItemState.Selected)); - - if (beatmapSet.Beatmaps.Any(b => b.Hidden)) - items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => RestoreHiddenRequested?.Invoke(beatmapSet))); - - items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => DeleteRequested?.Invoke(beatmapSet))); - - return items.ToArray(); - } - } - public class FilterableDifficultyIcon : DifficultyIcon { private readonly BindableBool filtered = new BindableBool(); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index b2e2c8bac6..759a9efa1c 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -76,68 +76,68 @@ namespace osu.Game.Screens.Select const float carousel_width = 640; const float filter_height = 100; - Add(new ParallaxContainer + AddRange(new Drawable[] { - Padding = new MarginPadding { Top = filter_height }, - ParallaxAmount = 0.005f, - RelativeSizeAxes = Axes.Both, - Children = new[] + new ParallaxContainer { - new WedgeBackground + Padding = new MarginPadding { Top = filter_height }, + ParallaxAmount = 0.005f, + RelativeSizeAxes = Axes.Both, + Children = new[] { - RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Right = carousel_width * 0.76f }, + new WedgeBackground + { + RelativeSizeAxes = Axes.Both, + Padding = new MarginPadding { Right = carousel_width * 0.76f }, + } } - } - }); - Add(LeftContent = new Container - { - Origin = Anchor.BottomLeft, - Anchor = Anchor.BottomLeft, - RelativeSizeAxes = Axes.Both, - Size = new Vector2(wedged_container_size.X, 1), - Padding = new MarginPadding - { - Bottom = 50, - Top = wedged_container_size.Y + left_area_padding, - Left = left_area_padding, - Right = left_area_padding * 2, - } - }); - Add(carousel = new BeatmapCarousel - { - RelativeSizeAxes = Axes.Y, - Size = new Vector2(carousel_width, 1), - Anchor = Anchor.CentreRight, - Origin = Anchor.CentreRight, - - //todo: clicking play on another map doesn't work bindable disabled - SelectionChanged = carouselSelectionChanged, - BeatmapsChanged = carouselBeatmapsLoaded, - //RestoreRequested = s => { foreach (var b in s.Beatmaps) beatmaps.Restore(b); }, - }); - Add(FilterControl = new FilterControl - { - RelativeSizeAxes = Axes.X, - Height = filter_height, - FilterChanged = criteria => filterChanged(criteria), - Exit = Exit, - }); - Add(beatmapInfoWedge = new BeatmapInfoWedge - { - Alpha = 0, - Size = wedged_container_size, - RelativeSizeAxes = Axes.X, - Margin = new MarginPadding - { - Top = left_area_padding, - Right = left_area_padding, }, - }); - Add(new ResetScrollContainer(() => carousel.ScrollToSelected()) - { - RelativeSizeAxes = Axes.Y, - Width = 250, + LeftContent = new Container + { + Origin = Anchor.BottomLeft, + Anchor = Anchor.BottomLeft, + RelativeSizeAxes = Axes.Both, + Size = new Vector2(wedged_container_size.X, 1), + Padding = new MarginPadding + { + Bottom = 50, + Top = wedged_container_size.Y + left_area_padding, + Left = left_area_padding, + Right = left_area_padding * 2, + } + }, + carousel = new BeatmapCarousel + { + RelativeSizeAxes = Axes.Y, + Size = new Vector2(carousel_width, 1), + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + SelectionChanged = carouselSelectionChanged, + BeatmapsChanged = carouselBeatmapsLoaded, + }, + FilterControl = new FilterControl + { + RelativeSizeAxes = Axes.X, + Height = filter_height, + FilterChanged = c => carousel.Filter(c), + Exit = Exit, + }, + beatmapInfoWedge = new BeatmapInfoWedge + { + Alpha = 0, + Size = wedged_container_size, + RelativeSizeAxes = Axes.X, + Margin = new MarginPadding + { + Top = left_area_padding, + Right = left_area_padding, + }, + }, + new ResetScrollContainer(() => carousel.ScrollToSelected()) + { + RelativeSizeAxes = Axes.Y, + Width = 250, + } }); if (ShowFooter) @@ -309,11 +309,6 @@ namespace osu.Game.Screens.Select protected abstract void Start(); - private void filterChanged(FilterCriteria criteria, bool debounce = true) - { - carousel.Filter(criteria, debounce); - } - private void onBeatmapSetAdded(BeatmapSetInfo s) => Schedule(() => carousel.UpdateBeatmapSet(s)); private void onBeatmapSetRemoved(BeatmapSetInfo s) => Schedule(() => removeBeatmapSet(s)); @@ -489,6 +484,7 @@ namespace osu.Game.Screens.Select Delete(Beatmap.Value.BeatmapSetInfo); return true; } + break; } diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 0bc7fa3f67..54c41ead80 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -343,7 +343,7 @@ - + From 1146ba02d713fd3781d7450e4e4f91eb93d2f2fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 18:09:14 +0900 Subject: [PATCH 03/53] Make GetWorkingBeatmap return a sane default rather than exception on lookup failure --- osu.Game/Beatmaps/BeatmapManager.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index b0fbef235b..c4b2c93d7e 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -374,12 +374,9 @@ namespace osu.Game.Beatmaps /// A instance correlating to the provided . public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo beatmapInfo, WorkingBeatmap previous = null) { - if (beatmapInfo == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) + if (beatmapInfo?.BeatmapSet == null || beatmapInfo == DefaultBeatmap?.BeatmapInfo) return DefaultBeatmap; - if (beatmapInfo.BeatmapSet == null) - throw new InvalidOperationException($@"Beatmap set {beatmapInfo.BeatmapSetInfoID} is not in the local database."); - if (beatmapInfo.Metadata == null) beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; From b9298325a3168f191289a278af4061814ecf6a67 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 18:10:32 +0900 Subject: [PATCH 04/53] Rename weird config setting --- osu.Game/Configuration/OsuConfigManager.cs | 4 +-- ...RandomType.cs => RandomSelectAlgorithm.cs} | 30 +++++++++---------- .../Sections/Gameplay/SongSelectSettings.cs | 6 ++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 ++-- osu.Game/osu.Game.csproj | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) rename osu.Game/Configuration/{SelectionRandomType.cs => RandomSelectAlgorithm.cs} (87%) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index 1f7808dceb..f4c7bdb586 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -20,7 +20,7 @@ namespace osu.Game.Configuration Set(OsuSetting.DisplayStarsMinimum, 0.0, 0, 10, 0.1); Set(OsuSetting.DisplayStarsMaximum, 10.0, 0, 10, 0.1); - Set(OsuSetting.SelectionRandomType, SongSelectRandomMode.RandomPermutation); + Set(OsuSetting.RandomSelectAlgorithm, RandomSelectAlgorithm.RandomPermutation); Set(OsuSetting.ChatDisplayHeight, ChatOverlay.DEFAULT_HEIGHT, 0.2, 1); @@ -108,7 +108,7 @@ namespace osu.Game.Configuration SaveUsername, DisplayStarsMinimum, DisplayStarsMaximum, - SelectionRandomType, + RandomSelectAlgorithm, SnakingInSliders, SnakingOutSliders, ShowFpsDisplay, diff --git a/osu.Game/Configuration/SelectionRandomType.cs b/osu.Game/Configuration/RandomSelectAlgorithm.cs similarity index 87% rename from osu.Game/Configuration/SelectionRandomType.cs rename to osu.Game/Configuration/RandomSelectAlgorithm.cs index 4ff52fab37..9ed6b9357b 100644 --- a/osu.Game/Configuration/SelectionRandomType.cs +++ b/osu.Game/Configuration/RandomSelectAlgorithm.cs @@ -1,15 +1,15 @@ -// Copyright (c) 2007-2017 ppy Pty Ltd . -// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE - -using System.ComponentModel; - -namespace osu.Game.Configuration -{ - public enum SongSelectRandomMode - { - [Description("Never repeat")] - RandomPermutation, - [Description("Random")] - Random - } -} +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System.ComponentModel; + +namespace osu.Game.Configuration +{ + public enum RandomSelectAlgorithm + { + [Description("Never repeat")] + RandomPermutation, + [Description("Random")] + Random + } +} diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs index 890c28aa07..03cd2118b9 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/SongSelectSettings.cs @@ -34,10 +34,10 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay Bindable = config.GetBindable(OsuSetting.DisplayStarsMaximum), KeyboardStep = 1f }, - new SettingsEnumDropdown + new SettingsEnumDropdown { - LabelText = "Random beatmap selection", - Bindable = config.GetBindable(OsuSetting.SelectionRandomType), + LabelText = "Random selection algorithm", + Bindable = config.GetBindable(OsuSetting.RandomSelectAlgorithm), } }; } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index c332f26c09..f61c8f60ef 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -71,7 +71,7 @@ namespace osu.Game.Screens.Select private readonly List carouselSets = new List(); - private Bindable randomType; + private Bindable randomSelectAlgorithm; private readonly List seenSets = new List(); private List items = new List(); @@ -221,7 +221,7 @@ namespace osu.Game.Screens.Select CarouselBeatmapSet group; - if (randomType == SongSelectRandomMode.RandomPermutation) + if (randomSelectAlgorithm == RandomSelectAlgorithm.RandomPermutation) { var notSeenGroups = visibleGroups.Except(seenSets); if (!notSeenGroups.Any()) @@ -338,7 +338,6 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config) { - randomType = config.GetBindable(OsuSetting.SelectionRandomType); } private void addBeatmapSet(CarouselBeatmapSet set) @@ -350,6 +349,7 @@ namespace osu.Game.Screens.Select //todo: add to root carouselSets.Add(set); + randomSelectAlgorithm = config.GetBindable(OsuSetting.RandomSelectAlgorithm); } private void removeBeatmapSet(CarouselBeatmapSet set) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 54c41ead80..c190d988fa 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -343,7 +343,7 @@ - + From 1b859524418c876c51056cca7e648ff27bb83c78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 19:56:16 +0900 Subject: [PATCH 05/53] Cleanup and basic TestCase --- .../Visual/TestCaseBeatmapCarousel.cs | 132 ++++++++++++++++++ .../Visual/TestCasePlaySongSelect.cs | 15 +- osu.Game.Tests/osu.Game.Tests.csproj | 1 + osu.Game/Screens/Select/BeatmapCarousel.cs | 52 +++---- .../Select/Carousel/CarouselBeatmapSet.cs | 7 - .../Screens/Select/Carousel/CarouselItem.cs | 7 + .../Carousel/DrawableCarouselBeatmap.cs | 13 +- .../Carousel/DrawableCarouselBeatmapSet.cs | 4 +- .../Select/Carousel/DrawableCarouselItem.cs | 2 + osu.Game/Screens/Select/SongSelect.cs | 2 +- 10 files changed, 193 insertions(+), 42 deletions(-) create mode 100644 osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs new file mode 100644 index 0000000000..f0a0e3405a --- /dev/null +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -0,0 +1,132 @@ +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using osu.Framework.Allocation; +using osu.Framework.Extensions; +using osu.Framework.Graphics; +using osu.Framework.MathUtils; +using osu.Game.Beatmaps; +using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Carousel; + +namespace osu.Game.Tests.Visual +{ + internal class TestCaseBeatmapCarousel : OsuTestCase + { + private BeatmapCarousel carousel; + + public override IReadOnlyList RequiredTypes => new[] + { + typeof(CarouselItem), + typeof(CarouselGroup), + typeof(CarouselGroupEagerSelect), + typeof(CarouselBeatmap), + typeof(CarouselBeatmapSet), + + typeof(DrawableCarouselItem), + typeof(CarouselItemState), + + typeof(DrawableCarouselBeatmap), + typeof(DrawableCarouselBeatmapSet), + }; + + [BackgroundDependencyLoader] + private void load() + { + Add(carousel = new BeatmapCarousel + { + RelativeSizeAxes = Axes.Both, + }); + + AddStep("Load Beatmaps", () => + { + carousel.Beatmaps = new[] + { + createTestBeatmapSet(0), + createTestBeatmapSet(1), + createTestBeatmapSet(2), + createTestBeatmapSet(3), + }; + }); + + AddUntilStep(() => carousel.Beatmaps.Any(), "Wait for load"); + + AddStep("SelectNext set", () => carousel.SelectNext()); + AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + + AddStep("SelectNext diff", () => carousel.SelectNext(1, false)); + AddAssert("set1 diff2", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.Skip(1).First()); + + AddStep("SelectNext backwards", () => carousel.SelectNext(-1)); + AddAssert("set4 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.First()); + + AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); + AddAssert("set3 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Reverse().Skip(1).First().Beatmaps.Last()); + + AddStep("SelectNext", () => carousel.SelectNext()); + AddStep("SelectNext", () => carousel.SelectNext()); + AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + + AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); + AddAssert("set4 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.Last()); + + // AddStep("Clear beatmaps", () => carousel.Beatmaps = new BeatmapSetInfo[] { }); + // AddStep("SelectNext (noop)", () => carousel.SelectNext()); + } + + private BeatmapSetInfo createTestBeatmapSet(int i) + { + return new BeatmapSetInfo + { + OnlineBeatmapSetID = 1234 + i, + Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(), + Metadata = new BeatmapMetadata + { + OnlineBeatmapSetID = 1234 + i, + // Create random metadata, then we can check if sorting works based on these + Artist = "MONACA " + RNG.Next(0, 9), + Title = "Black Song " + RNG.Next(0, 9), + AuthorString = "Some Guy " + RNG.Next(0, 9), + }, + Beatmaps = new List(new[] + { + new BeatmapInfo + { + OnlineBeatmapID = 1234 + i, + Path = "normal.osu", + Version = "Normal", + BaseDifficulty = new BeatmapDifficulty + { + OverallDifficulty = 3.5f, + } + }, + new BeatmapInfo + { + OnlineBeatmapID = 1235 + i, + Path = "hard.osu", + Version = "Hard", + BaseDifficulty = new BeatmapDifficulty + { + OverallDifficulty = 5, + } + }, + new BeatmapInfo + { + OnlineBeatmapID = 1236 + i, + Path = "insane.osu", + Version = "Insane", + BaseDifficulty = new BeatmapDifficulty + { + OverallDifficulty = 7, + } + }, + }), + }; + } + } +} diff --git a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs index 16c757702c..d4cef5b160 100644 --- a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs @@ -13,6 +13,7 @@ using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Rulesets; using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Carousel; using osu.Game.Screens.Select.Filter; using osu.Game.Tests.Platform; @@ -28,8 +29,20 @@ namespace osu.Game.Tests.Visual public override IReadOnlyList RequiredTypes => new[] { - typeof(BeatmapCarousel), typeof(SongSelect), + typeof(BeatmapCarousel), + + typeof(CarouselItem), + typeof(CarouselGroup), + typeof(CarouselGroupEagerSelect), + typeof(CarouselBeatmap), + typeof(CarouselBeatmapSet), + + typeof(DrawableCarouselItem), + typeof(CarouselItemState), + + typeof(DrawableCarouselBeatmap), + typeof(DrawableCarouselBeatmapSet), }; protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 1542269f00..184561faa7 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -88,6 +88,7 @@ + diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index f61c8f60ef..970aba0a6d 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -26,20 +26,35 @@ namespace osu.Game.Screens.Select { public class BeatmapCarousel : OsuScrollContainer { + /// + /// Triggered when the loaded change and are completely loaded. + /// + public Action BeatmapsChanged; + + /// + /// The currently selected beatmap. + /// public BeatmapInfo SelectedBeatmap => selectedBeatmap?.Beatmap; - public override bool HandleInput => AllowSelection; + /// + /// Raised when the is changed. + /// + public Action SelectionChanged; - public Action BeatmapsChanged; + public override bool HandleInput => AllowSelection; public IEnumerable Beatmaps { get { return carouselSets.Select(g => g.BeatmapSet); } set { - scrollableContent.Clear(false); - items.Clear(); - carouselSets.Clear(); + Schedule(() => + { + scrollableContent.Clear(false); + items.Clear(); + carouselSets.Clear(); + yPositionsCache.Invalidate(); + }); List newSets = null; @@ -51,8 +66,7 @@ namespace osu.Game.Screens.Select { Schedule(() => { - foreach (var g in newSets) - addBeatmapSet(g); + carouselSets.AddRange(newSets); root = new CarouselGroup(newSets.OfType().ToList()); items = root.Drawables.Value.ToList(); @@ -94,7 +108,7 @@ namespace osu.Game.Screens.Select }); } - public void RemoveBeatmap(BeatmapSetInfo beatmapSet) + public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { Schedule(() => removeBeatmapSet(carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); } @@ -116,8 +130,8 @@ namespace osu.Game.Screens.Select { if (index >= 0) carouselSets.Insert(index, newSet); - else - addBeatmapSet(newSet); + //else + // addBeatmapSet(newSet); } if (hadSelection && newSet == null) @@ -158,8 +172,6 @@ namespace osu.Game.Screens.Select } } - public Action SelectionChanged; - private void selectNullBeatmap() { selectedBeatmap = null; @@ -180,7 +192,7 @@ namespace osu.Game.Screens.Select return; } - int originalIndex = Math.Max(0, items.IndexOf(selectedBeatmap?.Drawables.Value.First())); + int originalIndex = items.IndexOf(selectedBeatmap?.Drawables.Value.First()); int currentIndex = originalIndex; // local function to increment the index in the required direction, wrapping over extremities. @@ -338,17 +350,6 @@ namespace osu.Game.Screens.Select [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config) { - } - - private void addBeatmapSet(CarouselBeatmapSet set) - { - // prevent duplicates by concurrent independent actions trying to add a group - //todo: check this - if (carouselSets.Any(g => g.BeatmapSet.ID == set.BeatmapSet.ID)) - return; - - //todo: add to root - carouselSets.Add(set); randomSelectAlgorithm = config.GetBindable(OsuSetting.RandomSelectAlgorithm); } @@ -422,7 +423,6 @@ namespace osu.Game.Screens.Select private void select(CarouselItem item) { if (item == null) return; - item.State.Value = CarouselItemState.Selected; } @@ -466,7 +466,7 @@ namespace osu.Game.Screens.Select computeYPositions(); // Remove all items that should no longer be on-screen - scrollableContent.RemoveAll(delegate (DrawableCarouselItem p) + scrollableContent.RemoveAll(delegate(DrawableCarouselItem p) { float itemPosY = p.Position.Y; bool remove = itemPosY < Current - p.DrawHeight || itemPosY > Current + drawHeight || !p.IsPresent; diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index 525bb6c600..d0ccda0e28 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -52,11 +52,4 @@ namespace osu.Game.Screens.Select.Carousel }*/ } } - - public enum CarouselItemState - { - Hidden, - NotSelected, - Selected, - } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index d6d0615b6e..68148814e0 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -54,4 +54,11 @@ namespace osu.Game.Screens.Select.Carousel public virtual void Filter(FilterCriteria criteria) => Children?.ForEach(c => c.Filter(criteria)); } + + public enum CarouselItemState + { + Hidden, + NotSelected, + Selected, + } } diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 61a916d0a3..1fca23a2ec 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -35,12 +35,17 @@ namespace osu.Game.Screens.Select.Carousel private readonly Triangles triangles; private readonly StarCounter starCounter; - [BackgroundDependencyLoader] + [BackgroundDependencyLoader(true)] private void load(SongSelect songSelect, BeatmapManager manager) { - StartRequested = songSelect.Start; - EditRequested = songSelect.Edit; - HideRequested = manager.Hide; + if (songSelect != null) + { + StartRequested = songSelect.Start; + EditRequested = songSelect.Edit; + } + + if (manager != null) + HideRequested = manager.Hide; } public DrawableCarouselBeatmap(CarouselBeatmap panel) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index 9a1ee663a1..9987cc4a38 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -46,12 +46,10 @@ namespace osu.Game.Screens.Select.Carousel RestoreHiddenRequested = s => s.Beatmaps.ForEach(manager.Restore); DeleteRequested = manager.Delete; - var working = manager.GetWorkingBeatmap(beatmapSet.Beatmaps.FirstOrDefault()); - Children = new Drawable[] { new DelayedLoadWrapper( - new PanelBackground(working) + new PanelBackground(manager.GetWorkingBeatmap(beatmapSet.Beatmaps.FirstOrDefault())) { RelativeSizeAxes = Axes.Both, OnLoadComplete = d => d.FadeInFromZero(400, Easing.Out), diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs index 040ff22f4e..ad91706154 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs @@ -42,6 +42,8 @@ namespace osu.Game.Screens.Select.Carousel Height = MAX_HEIGHT; RelativeSizeAxes = Axes.X; + Alpha = 0; + AddInternal(borderContainer = new Container { RelativeSizeAxes = Axes.Both, diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 759a9efa1c..eec4679bfd 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -454,7 +454,7 @@ namespace osu.Game.Screens.Select private void removeBeatmapSet(BeatmapSetInfo beatmapSet) { - carousel.RemoveBeatmap(beatmapSet); + carousel.RemoveBeatmapSet(beatmapSet); if (carousel.SelectedBeatmap == null) Beatmap.SetDefault(); } From ec4f99c92ef9d530074477edeea7e2b5c1e1ca52 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 21:08:12 +0900 Subject: [PATCH 06/53] Clean up tests some more --- .../Visual/TestCaseBeatmapCarousel.cs | 80 ++++++++++++++----- osu.Game/Screens/Select/BeatmapCarousel.cs | 36 ++++----- 2 files changed, 76 insertions(+), 40 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index f0a0e3405a..da5865c96d 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -9,7 +9,6 @@ using System.Text; using osu.Framework.Allocation; using osu.Framework.Extensions; using osu.Framework.Graphics; -using osu.Framework.MathUtils; using osu.Game.Beatmaps; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; @@ -18,7 +17,7 @@ namespace osu.Game.Tests.Visual { internal class TestCaseBeatmapCarousel : OsuTestCase { - private BeatmapCarousel carousel; + private TestBeatmapCarousel carousel; public override IReadOnlyList RequiredTypes => new[] { @@ -38,7 +37,7 @@ namespace osu.Game.Tests.Visual [BackgroundDependencyLoader] private void load() { - Add(carousel = new BeatmapCarousel + Add(carousel = new TestBeatmapCarousel { RelativeSizeAxes = Axes.Both, }); @@ -47,36 +46,66 @@ namespace osu.Game.Tests.Visual { carousel.Beatmaps = new[] { - createTestBeatmapSet(0), createTestBeatmapSet(1), createTestBeatmapSet(2), createTestBeatmapSet(3), + createTestBeatmapSet(4), }; }); + 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()); + + 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); + } + } + + void checkVisibleItemCount(bool diff, int count) => + 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"); - AddStep("SelectNext set", () => carousel.SelectNext()); - AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + advanceSelection(direction: 1, diff: false); + checkSelected(1, 1); - AddStep("SelectNext diff", () => carousel.SelectNext(1, false)); - AddAssert("set1 diff2", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.Skip(1).First()); + advanceSelection(direction: 1, diff: true); + checkSelected(1, 2); - AddStep("SelectNext backwards", () => carousel.SelectNext(-1)); - AddAssert("set4 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.First()); + advanceSelection(direction: -1, diff: false); + checkSelected(4, 1); - AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); - AddAssert("set3 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Reverse().Skip(1).First().Beatmaps.Last()); + advanceSelection(direction: -1, diff: true); + checkSelected(3, 3); - AddStep("SelectNext", () => carousel.SelectNext()); - AddStep("SelectNext", () => carousel.SelectNext()); - AddAssert("set1 diff1", () => carousel.SelectedBeatmap == carousel.Beatmaps.First().Beatmaps.First()); + advanceSelection(diff: false); + advanceSelection(diff: false); + checkSelected(1, 1); - AddStep("SelectNext diff backwards", () => carousel.SelectNext(-1, false)); - AddAssert("set4 diff3", () => carousel.SelectedBeatmap == carousel.Beatmaps.Last().Beatmaps.Last()); + advanceSelection(direction: -1, diff: true); + checkSelected(4, 3); - // AddStep("Clear beatmaps", () => carousel.Beatmaps = new BeatmapSetInfo[] { }); - // AddStep("SelectNext (noop)", () => carousel.SelectNext()); + AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3" }, false)); + checkVisibleItemCount(diff: false, count: 1); + checkVisibleItemCount(diff: true, count: 3); + checkSelected(3, 1); + + advanceSelection(diff: true, count: 4); + checkSelected(3, 2); + + AddStep("Un-filter (debounce)", () => carousel.Filter(new FilterCriteria())); + AddUntilStep(() => !carousel.PendingFilterTask, "Wait for debounce"); + checkVisibleItemCount(diff: false, count: 4); + checkVisibleItemCount(diff: true, count: 3); } private BeatmapSetInfo createTestBeatmapSet(int i) @@ -89,9 +118,9 @@ namespace osu.Game.Tests.Visual { OnlineBeatmapSetID = 1234 + i, // Create random metadata, then we can check if sorting works based on these - Artist = "MONACA " + RNG.Next(0, 9), - Title = "Black Song " + RNG.Next(0, 9), - AuthorString = "Some Guy " + RNG.Next(0, 9), + Artist = "peppy", + Title = "test set #" + i, + AuthorString = "peppy" }, Beatmaps = new List(new[] { @@ -128,5 +157,12 @@ namespace osu.Game.Tests.Visual }), }; } + + private class TestBeatmapCarousel : BeatmapCarousel + { + public new List Items => base.Items; + + public bool PendingFilterTask => FilterTask != null; + } } } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 970aba0a6d..9f210e023b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -51,7 +51,7 @@ namespace osu.Game.Screens.Select Schedule(() => { scrollableContent.Clear(false); - items.Clear(); + Items.Clear(); carouselSets.Clear(); yPositionsCache.Invalidate(); }); @@ -69,7 +69,7 @@ namespace osu.Game.Screens.Select carouselSets.AddRange(newSets); root = new CarouselGroup(newSets.OfType().ToList()); - items = root.Drawables.Value.ToList(); + Items = root.Drawables.Value.ToList(); yPositionsCache.Invalidate(); BeatmapsChanged?.Invoke(); @@ -88,7 +88,7 @@ namespace osu.Game.Screens.Select private Bindable randomSelectAlgorithm; private readonly List seenSets = new List(); - private List items = new List(); + protected List Items = new List(); private CarouselGroup root = new CarouselGroup(); private readonly Stack randomSelectedBeatmaps = new Stack(); @@ -192,15 +192,15 @@ namespace osu.Game.Screens.Select return; } - int originalIndex = items.IndexOf(selectedBeatmap?.Drawables.Value.First()); + int originalIndex = Items.IndexOf(selectedBeatmap?.Drawables.Value.First()); int currentIndex = originalIndex; // local function to increment the index in the required direction, wrapping over extremities. - int incrementIndex() => currentIndex = (currentIndex + direction + items.Count) % items.Count; + int incrementIndex() => currentIndex = (currentIndex + direction + Items.Count) % Items.Count; while (incrementIndex() != originalIndex) { - var item = items[currentIndex].Item; + var item = Items[currentIndex].Item; if (item.Filtered || item.State == CarouselItemState.Selected) continue; @@ -272,13 +272,13 @@ namespace osu.Game.Screens.Select private FilterCriteria criteria = new FilterCriteria(); - private ScheduledDelegate filterTask; + protected ScheduledDelegate FilterTask; public bool AllowSelection = true; public void FlushPendingFilters() { - if (filterTask?.Completed == false) + if (FilterTask?.Completed == false) Filter(null, false); } @@ -289,7 +289,7 @@ namespace osu.Game.Screens.Select Action perform = delegate { - filterTask = null; + FilterTask = null; carouselSets.ForEach(s => s.Filter(criteria)); @@ -301,11 +301,11 @@ namespace osu.Game.Screens.Select select(selectedBeatmap); }; - filterTask?.Cancel(); - filterTask = null; + FilterTask?.Cancel(); + FilterTask = null; if (debounce) - filterTask = Scheduler.AddDelayed(perform, 250); + FilterTask = Scheduler.AddDelayed(perform, 250); else perform(); } @@ -362,7 +362,7 @@ namespace osu.Game.Screens.Select foreach (var d in set.Drawables.Value) { - items.Remove(d); + Items.Remove(d); scrollableContent.Remove(d); } @@ -385,7 +385,7 @@ namespace osu.Game.Screens.Select float lastSetY = 0; - foreach (DrawableCarouselItem d in items) + foreach (DrawableCarouselItem d in Items) { switch (d) { @@ -474,7 +474,7 @@ namespace osu.Game.Screens.Select }); // Find index range of all items that should be on-screen - Trace.Assert(items.Count == yPositions.Count); + Trace.Assert(Items.Count == yPositions.Count); int firstIndex = yPositions.BinarySearch(Current - DrawableCarouselItem.MAX_HEIGHT); if (firstIndex < 0) firstIndex = ~firstIndex; @@ -484,21 +484,21 @@ namespace osu.Game.Screens.Select lastIndex = ~lastIndex; // Add the first item of the last visible beatmap group to preload its data. - if (lastIndex != 0 && items[lastIndex - 1] is DrawableCarouselBeatmapSet) + if (lastIndex != 0 && Items[lastIndex - 1] is DrawableCarouselBeatmapSet) lastIndex++; } // Add those items within the previously found index range that should be displayed. for (int i = firstIndex; i < lastIndex; ++i) { - DrawableCarouselItem item = items[i]; + DrawableCarouselItem item = Items[i]; // Only add if we're not already part of the content. if (!scrollableContent.Contains(item)) { // Makes sure headers are always _below_ items, // and depth flows downward. - item.Depth = i + (item is DrawableCarouselBeatmapSet ? items.Count : 0); + item.Depth = i + (item is DrawableCarouselBeatmapSet ? Items.Count : 0); switch (item.LoadState) { From 48f30d2bb5b10302b4b159da05476a1e7454738a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 21:24:33 +0900 Subject: [PATCH 07/53] Get ready for more tests --- osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index da5865c96d..e77337d488 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -57,6 +57,10 @@ namespace osu.Game.Tests.Visual AddAssert($"selected is set{set} diff{diff}", () => carousel.SelectedBeatmap == carousel.Beatmaps.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())); + void advanceSelection(bool diff, int direction = 1, int count = 1) { if (count == 1) @@ -106,6 +110,8 @@ namespace osu.Game.Tests.Visual AddUntilStep(() => !carousel.PendingFilterTask, "Wait for debounce"); checkVisibleItemCount(diff: false, count: 4); checkVisibleItemCount(diff: true, count: 3); + + setSelected(1, diff: 2); } private BeatmapSetInfo createTestBeatmapSet(int i) From 8646d5d1e081d850482766f60dd471734663c214 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 13 Dec 2017 21:47:42 +0900 Subject: [PATCH 08/53] Add testing and fix filtering only some difficulties --- .../Visual/TestCaseBeatmapCarousel.cs | 12 ++++++++++- osu.Game/Screens/Select/BeatmapCarousel.cs | 21 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index e77337d488..f034dc7baf 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -79,6 +79,8 @@ namespace osu.Game.Tests.Visual AddUntilStep(() => carousel.Beatmaps.Any(), "Wait for load"); + // test traversal + advanceSelection(direction: 1, diff: false); checkSelected(1, 1); @@ -98,6 +100,8 @@ namespace osu.Game.Tests.Visual advanceSelection(direction: -1, diff: true); checkSelected(4, 3); + // test basic filtering + AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3" }, false)); checkVisibleItemCount(diff: false, count: 1); checkVisibleItemCount(diff: true, count: 3); @@ -111,7 +115,13 @@ namespace osu.Game.Tests.Visual checkVisibleItemCount(diff: false, count: 4); checkVisibleItemCount(diff: true, count: 3); - setSelected(1, diff: 2); + // test filtering some difficulties (and keeping current beatmap set selected). + + setSelected(1, 2); + AddStep("Filter some difficulties", () => carousel.Filter(new FilterCriteria { SearchText = "Normal" }, false)); + checkSelected(1, 1); + AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); + checkSelected(1, 1); } private BeatmapSetInfo createTestBeatmapSet(int i) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 9f210e023b..5e3efb55c9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -94,6 +94,7 @@ namespace osu.Game.Screens.Select private readonly Stack randomSelectedBeatmaps = new Stack(); private CarouselBeatmap selectedBeatmap; + private CarouselBeatmapSet selectedBeatmapSet => carouselSets.FirstOrDefault(s => s.State == CarouselItemState.Selected); public BeatmapCarousel() { @@ -291,14 +292,28 @@ namespace osu.Game.Screens.Select { FilterTask = null; + var lastSet = selectedBeatmapSet; + var lastBeatmap = selectedBeatmap; + carouselSets.ForEach(s => s.Filter(criteria)); yPositionsCache.Invalidate(); - if (selectedBeatmap?.Visible != true) - SelectNext(); - else + if (selectedBeatmap?.Filtered == false) 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(); }; FilterTask?.Cancel(); From 5cbb9b9b1886f750e7a7a16d33ef7f062db3aac4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 12:59:35 +0900 Subject: [PATCH 09/53] 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 selectedSets = new Stack(); + + 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 { /// - /// Triggered when the loaded change and are completely loaded. + /// Triggered when the loaded change and are completely loaded. /// - public Action BeatmapsChanged; + public Action BeatmapSetsChanged; /// /// The currently selected beatmap. /// public BeatmapInfo SelectedBeatmap => selectedBeatmap?.Beatmap; + private CarouselBeatmap selectedBeatmap => selectedBeatmapSet?.Beatmaps.FirstOrDefault(s => s.State == CarouselItemState.Selected); + + /// + /// The currently selected beatmap set. + /// + public BeatmapSetInfo SelectedBeatmapSet => selectedBeatmapSet?.BeatmapSet; + + private CarouselBeatmapSet selectedBeatmapSet => carouselSets.FirstOrDefault(s => s.State == CarouselItemState.Selected); + /// /// Raised when the is changed. /// @@ -43,7 +52,7 @@ namespace osu.Game.Screens.Select public override bool HandleInput => AllowSelection; - public IEnumerable Beatmaps + public IEnumerable 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 carouselSets = new List(); - private Bindable randomSelectAlgorithm; - private readonly List seenSets = new List(); + public Bindable RandomAlgorithm = new Bindable(); + private readonly List previouslyVisitedRandomSets = new List(); protected List Items = new List(); private CarouselGroup root = new CarouselGroup(); private readonly Stack randomSelectedBeatmaps = new Stack(); - 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); - } - /// /// Increment selection in the carousel in a chosen direction. /// @@ -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 getVisibleGroups() => carouselSets.Where(select => select.State != CarouselItemState.NotSelected); + private IEnumerable 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(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; From 7814b2df1470e90d5409fef7a7452f6d96832481 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 13:49:43 +0900 Subject: [PATCH 10/53] More renaming --- osu.Game/Screens/Select/BeatmapCarousel.cs | 38 ++++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 810cc8c408..94fc9ebdb7 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -43,7 +43,7 @@ namespace osu.Game.Screens.Select /// public BeatmapSetInfo SelectedBeatmapSet => selectedBeatmapSet?.BeatmapSet; - private CarouselBeatmapSet selectedBeatmapSet => carouselSets.FirstOrDefault(s => s.State == CarouselItemState.Selected); + private CarouselBeatmapSet selectedBeatmapSet => beatmapSets.FirstOrDefault(s => s.State == CarouselItemState.Selected); /// /// Raised when the is changed. @@ -52,16 +52,18 @@ namespace osu.Game.Screens.Select public override bool HandleInput => AllowSelection; + private readonly List beatmapSets = new List(); + public IEnumerable BeatmapSets { - get { return carouselSets.Select(g => g.BeatmapSet); } + get { return beatmapSets.Select(g => g.BeatmapSet); } set { Schedule(() => { scrollableContent.Clear(false); Items.Clear(); - carouselSets.Clear(); + beatmapSets.Clear(); yPositionsCache.Invalidate(); }); @@ -75,7 +77,7 @@ namespace osu.Game.Screens.Select { Schedule(() => { - carouselSets.AddRange(newSets); + beatmapSets.AddRange(newSets); root = new CarouselGroup(newSets.OfType().ToList()); Items = root.Drawables.Value.ToList(); @@ -92,7 +94,7 @@ namespace osu.Game.Screens.Select private readonly Container scrollableContent; - private readonly List carouselSets = new List(); + public Bindable RandomAlgorithm = new Bindable(); private readonly List previouslyVisitedRandomSets = new List(); @@ -104,7 +106,7 @@ namespace osu.Game.Screens.Select public BeatmapCarousel() { - Add(new OsuContextMenuContainer + Child = new OsuContextMenuContainer { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, @@ -112,31 +114,31 @@ namespace osu.Game.Screens.Select { RelativeSizeAxes = Axes.X, } - }); + }; } public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { - Schedule(() => removeBeatmapSet(carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); + Schedule(() => removeBeatmapSet(beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) { // todo: this method should be smarter as to not recreate items that haven't changed, etc. - var oldGroup = carouselSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + var oldGroup = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); bool hadSelection = oldGroup?.State == CarouselItemState.Selected; var newSet = createGroup(beatmapSet); - int index = carouselSets.IndexOf(oldGroup); + int index = beatmapSets.IndexOf(oldGroup); if (index >= 0) - carouselSets.RemoveAt(index); + beatmapSets.RemoveAt(index); if (newSet != null) { if (index >= 0) - carouselSets.Insert(index, newSet); + beatmapSets.Insert(index, newSet); //else // addBeatmapSet(newSet); } @@ -168,7 +170,7 @@ namespace osu.Game.Screens.Select if (beatmap == SelectedBeatmap) return; - foreach (CarouselBeatmapSet group in carouselSets) + foreach (CarouselBeatmapSet group in beatmapSets) { var item = group.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap)); if (item != null) @@ -187,7 +189,7 @@ 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.Visible)) + if (beatmapSets.All(g => !g.Visible)) { SelectionChanged?.Invoke(null); return; @@ -218,11 +220,11 @@ namespace osu.Game.Screens.Select } } - private IEnumerable getVisibleSets() => carouselSets.Where(select => select.Visible); + private IEnumerable getVisibleSets() => beatmapSets.Where(select => select.Visible); public void SelectNextRandom() { - if (carouselSets.Count == 0) + if (beatmapSets.Count == 0) return; var visible = getVisibleSets().ToList(); @@ -299,7 +301,7 @@ namespace osu.Game.Screens.Select var lastSet = selectedBeatmapSet; var lastBeatmap = selectedBeatmap; - carouselSets.ForEach(s => s.Filter(criteria)); + beatmapSets.ForEach(s => s.Filter(criteria)); yPositionsCache.Invalidate(); @@ -376,7 +378,7 @@ namespace osu.Game.Screens.Select if (set == null) return; - carouselSets.Remove(set); + beatmapSets.Remove(set); foreach (var d in set.Drawables.Value) { From b4b2f12116ba9a7c5038661f50a7c9e23d6ac6b5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 16:59:08 +0900 Subject: [PATCH 11/53] Add support for adding/removing items Tests accompany of course --- .../Visual/TestCaseBeatmapCarousel.cs | 32 ++++++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 82 +++++++++---------- .../Carousel/DrawableCarouselBeatmapSet.cs | 2 +- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 971760239f..f7f2108f33 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -141,7 +141,7 @@ namespace osu.Game.Tests.Visual }); void ensureRandomDidntRepeat() => - AddAssert("ensure no repeats", () => selectedSets.Distinct().Count() == selectedSets.Count()); + AddAssert("ensure no repeats", () => selectedSets.Distinct().Count() == selectedSets.Count); void prevRandom() => AddStep("select random last", () => { @@ -153,6 +153,7 @@ namespace osu.Game.Tests.Visual AddAssert("ensure prev random fetch worked", () => selectedSets.Peek() == carousel.SelectedBeatmapSet); setSelected(1, 1); + nextRandom(); ensureRandomDidntRepeat(); nextRandom(); @@ -172,29 +173,44 @@ namespace osu.Game.Tests.Visual nextRandom(); AddAssert("ensure repeat", () => selectedSets.Contains(carousel.SelectedBeatmapSet)); + + // test adding and removing + + AddStep("Add new set #5", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(5))); + AddStep("Add new set #6", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(6))); + + checkVisibleItemCount(false, 6); + + AddStep("Remove set #4", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(4))); + + checkVisibleItemCount(false, 5); + + } private BeatmapSetInfo createTestBeatmapSet(int i) { return new BeatmapSetInfo { - OnlineBeatmapSetID = 1234 + i, + ID = i, + OnlineBeatmapSetID = i, Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(), Metadata = new BeatmapMetadata { - OnlineBeatmapSetID = 1234 + i, + OnlineBeatmapSetID = i, // Create random metadata, then we can check if sorting works based on these Artist = "peppy", Title = "test set #" + i, - AuthorString = "peppy" + AuthorString = "peppy", }, Beatmaps = new List(new[] { new BeatmapInfo { - OnlineBeatmapID = 1234 + i, + OnlineBeatmapID = i * 10, Path = "normal.osu", Version = "Normal", + StarDifficulty = 2, BaseDifficulty = new BeatmapDifficulty { OverallDifficulty = 3.5f, @@ -202,9 +218,10 @@ namespace osu.Game.Tests.Visual }, new BeatmapInfo { - OnlineBeatmapID = 1235 + i, + OnlineBeatmapID = i * 10 + 1, Path = "hard.osu", Version = "Hard", + StarDifficulty = 5, BaseDifficulty = new BeatmapDifficulty { OverallDifficulty = 5, @@ -212,9 +229,10 @@ namespace osu.Game.Tests.Visual }, new BeatmapInfo { - OnlineBeatmapID = 1236 + i, + OnlineBeatmapID = i * 10 + 2, Path = "insane.osu", Version = "Insane", + StarDifficulty = 6, BaseDifficulty = new BeatmapDifficulty { OverallDifficulty = 7, diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 94fc9ebdb7..bdfb772704 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -71,39 +71,45 @@ namespace osu.Game.Screens.Select Task.Run(() => { - newSets = value.Select(createGroup).Where(g => g != null).ToList(); + newSets = value.Select(createCarouselSet).Where(g => g != null).ToList(); newSets.ForEach(g => g.Filter(criteria)); }).ContinueWith(t => { Schedule(() => { beatmapSets.AddRange(newSets); - - root = new CarouselGroup(newSets.OfType().ToList()); - Items = root.Drawables.Value.ToList(); - - yPositionsCache.Invalidate(); - BeatmapSetsChanged?.Invoke(); + updateItems(); }); }); } } + /// + /// Call after altering in any way. + /// + private void updateItems() + { + scrollableContent.Clear(false); + + root = new CarouselGroup(beatmapSets.OfType().ToList()); + Items = root.Drawables.Value.ToList(); + + yPositionsCache.Invalidate(); + BeatmapSetsChanged?.Invoke(); + } + private readonly List yPositions = new List(); private Cached yPositionsCache = new Cached(); private readonly Container scrollableContent; - - public Bindable RandomAlgorithm = new Bindable(); private readonly List previouslyVisitedRandomSets = new List(); + private readonly Stack randomSelectedBeatmaps = new Stack(); protected List Items = new List(); private CarouselGroup root = new CarouselGroup(); - private readonly Stack randomSelectedBeatmaps = new Stack(); - public BeatmapCarousel() { Child = new OsuContextMenuContainer @@ -119,19 +125,28 @@ namespace osu.Game.Screens.Select public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { - Schedule(() => removeBeatmapSet(beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID))); + var existingSet = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + + if (existingSet == null) + return; + + beatmapSets.Remove(existingSet); + + updateItems(); + + if (existingSet.State == CarouselItemState.Selected) + SelectNext(); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) { - // todo: this method should be smarter as to not recreate items that haven't changed, etc. - var oldGroup = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + CarouselBeatmapSet existingSet = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); - bool hadSelection = oldGroup?.State == CarouselItemState.Selected; + bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; - var newSet = createGroup(beatmapSet); + var newSet = createCarouselSet(beatmapSet); - int index = beatmapSets.IndexOf(oldGroup); + int index = beatmapSets.IndexOf(existingSet); if (index >= 0) beatmapSets.RemoveAt(index); @@ -139,8 +154,8 @@ namespace osu.Game.Screens.Select { if (index >= 0) beatmapSets.Insert(index, newSet); - //else - // addBeatmapSet(newSet); + else + beatmapSets.Add(newSet); } if (hadSelection && newSet == null) @@ -154,10 +169,12 @@ namespace osu.Game.Screens.Select var newSelection = newSet.Beatmaps.Find(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID); if (newSelection == null && selectedBeatmap != null) - newSelection = newSet.Beatmaps[Math.Min(newSet.Beatmaps.Count - 1, oldGroup.Beatmaps.IndexOf(selectedBeatmap))]; + newSelection = newSet.Beatmaps[Math.Min(newSet.Beatmaps.Count - 1, existingSet.Beatmaps.IndexOf(selectedBeatmap))]; select(newSelection); } + + updateItems(); } public void SelectBeatmap(BeatmapInfo beatmap, bool animated = true) @@ -252,11 +269,11 @@ namespace osu.Game.Screens.Select notYetVisitedSets = visible; } - set = notYetVisitedSets.ElementAt(RNG.Next(notYetVisitedSets.Count())); + set = notYetVisitedSets.ElementAt(RNG.Next(notYetVisitedSets.Count)); previouslyVisitedRandomSets.Add(set); } else - set = visible.ElementAt(RNG.Next(visible.Count())); + set = visible.ElementAt(RNG.Next(visible.Count)); select(set.Beatmaps[RNG.Next(set.Beatmaps.Count)]); } @@ -337,7 +354,7 @@ namespace osu.Game.Screens.Select ScrollTo(selectedY, animated); } - private CarouselBeatmapSet createGroup(BeatmapSetInfo beatmapSet) + private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) { if (beatmapSet.Beatmaps.All(b => b.Hidden)) return null; @@ -373,25 +390,6 @@ namespace osu.Game.Screens.Select config.BindWith(OsuSetting.RandomSelectAlgorithm, RandomAlgorithm); } - private void removeBeatmapSet(CarouselBeatmapSet set) - { - if (set == null) - return; - - beatmapSets.Remove(set); - - foreach (var d in set.Drawables.Value) - { - Items.Remove(d); - scrollableContent.Remove(d); - } - - if (set.State == CarouselItemState.Selected) - SelectNext(); - - yPositionsCache.Invalidate(); - } - /// /// Computes the target Y positions for every item in the carousel. /// diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index 9987cc4a38..b5636ae9ed 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -52,7 +52,7 @@ namespace osu.Game.Screens.Select.Carousel new PanelBackground(manager.GetWorkingBeatmap(beatmapSet.Beatmaps.FirstOrDefault())) { RelativeSizeAxes = Axes.Both, - OnLoadComplete = d => d.FadeInFromZero(400, Easing.Out), + OnLoadComplete = d => d.FadeInFromZero(1000, Easing.OutQuint), }, 300 ), new FillFlowContainer From 67f05977ea017e14804a1c546ef350c5da3d42ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 20:40:58 +0900 Subject: [PATCH 12/53] Add sorting support --- .../Visual/TestCaseBeatmapCarousel.cs | 156 +++++++++++------- osu.Game/Screens/Select/BeatmapCarousel.cs | 74 ++++----- .../Select/Carousel/CarouselBeatmap.cs | 17 ++ .../Select/Carousel/CarouselBeatmapSet.cs | 45 ++--- .../Screens/Select/Carousel/CarouselGroup.cs | 15 +- .../Carousel/CarouselGroupEagerSelect.cs | 6 +- .../Screens/Select/Carousel/CarouselItem.cs | 44 +++-- 7 files changed, 204 insertions(+), 153 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index f7f2108f33..6163daaf39 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -13,6 +13,7 @@ using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; +using osu.Game.Screens.Select.Filter; namespace osu.Game.Tests.Visual { @@ -35,6 +36,9 @@ namespace osu.Game.Tests.Visual typeof(DrawableCarouselBeatmapSet), }; + + private readonly Stack selectedSets = new Stack(); + [BackgroundDependencyLoader] private void load() { @@ -54,34 +58,68 @@ namespace osu.Game.Tests.Visual }; }); - 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()); - - 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())); - - 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); - } - } - - void checkVisibleItemCount(bool diff, int count) => - 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.BeatmapSets.Any(), "Wait for load"); - // test traversal + testTraversal(); + testFiltering(); + testRandom(); + testAddRemove(); + testSorting(); + } + 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 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 ? "diff" : "set")} visible", () => + carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count); + + 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(); + }); + + /// + /// Test keyboard traversal + /// + private void testTraversal() + { advanceSelection(direction: 1, diff: false); checkSelected(1, 1); @@ -100,8 +138,14 @@ namespace osu.Game.Tests.Visual advanceSelection(direction: -1, diff: true); checkSelected(4, 3); + } - // test basic filtering + /// + /// Test filtering + /// + private void testFiltering() + { + // basic filtering AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3" }, false)); checkVisibleItemCount(diff: false, count: 1); @@ -123,35 +167,13 @@ 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 selectedSets = new Stack(); - - 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); - + /// + /// Test random non-repeating algorithm + /// + private void testRandom() + { setSelected(1, 1); nextRandom(); @@ -173,21 +195,37 @@ namespace osu.Game.Tests.Visual nextRandom(); AddAssert("ensure repeat", () => selectedSets.Contains(carousel.SelectedBeatmapSet)); + } - // test adding and removing - + /// + /// Test adding and removing beatmap sets + /// + private void testAddRemove() + { AddStep("Add new set #5", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(5))); AddStep("Add new set #6", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(6))); checkVisibleItemCount(false, 6); - AddStep("Remove set #4", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(4))); + AddStep("Remove set #5", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(5))); checkVisibleItemCount(false, 5); - + AddStep("Remove set #6", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(6))); } + /// + /// Test sorting + /// + private void testSorting() + { + AddStep("Sort by author", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Author }, false)); + AddAssert("Check yyyyy is at bottom", () => carousel.BeatmapSets.Last().Metadata.AuthorString == "yyyyy"); + AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); + AddAssert("Check #4 is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith("#4")); + } + + private BeatmapSetInfo createTestBeatmapSet(int i) { return new BeatmapSetInfo @@ -201,7 +239,7 @@ namespace osu.Game.Tests.Visual // Create random metadata, then we can check if sorting works based on these Artist = "peppy", Title = "test set #" + i, - AuthorString = "peppy", + AuthorString = string.Concat(Enumerable.Repeat((char)('z' - i), 5)) }, Beatmaps = new List(new[] { diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index bdfb772704..e51ce85546 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -17,6 +17,7 @@ using osu.Framework.Allocation; using osu.Framework.Caching; using osu.Framework.Threading; using osu.Framework.Configuration; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Cursor; @@ -52,32 +53,26 @@ namespace osu.Game.Screens.Select public override bool HandleInput => AllowSelection; - private readonly List beatmapSets = new List(); + private IEnumerable beatmapSets => root.Children?.OfType() ?? new CarouselBeatmapSet[] { }; public IEnumerable BeatmapSets { get { return beatmapSets.Select(g => g.BeatmapSet); } set { - Schedule(() => - { - scrollableContent.Clear(false); - Items.Clear(); - beatmapSets.Clear(); - yPositionsCache.Invalidate(); - }); - List newSets = null; + CarouselGroup newRoot = new CarouselGroup(); + Task.Run(() => { - newSets = value.Select(createCarouselSet).Where(g => g != null).ToList(); - newSets.ForEach(g => g.Filter(criteria)); + value.Select(createCarouselSet).Where(g => g != null).ForEach(newRoot.AddChild); + newRoot.Filter(criteria); }).ContinueWith(t => { Schedule(() => { - beatmapSets.AddRange(newSets); + root = newRoot; updateItems(); }); }); @@ -91,8 +86,7 @@ namespace osu.Game.Screens.Select { scrollableContent.Clear(false); - root = new CarouselGroup(beatmapSets.OfType().ToList()); - Items = root.Drawables.Value.ToList(); + Items = root.Drawables.ToList(); yPositionsCache.Invalidate(); BeatmapSetsChanged?.Invoke(); @@ -125,13 +119,12 @@ namespace osu.Game.Screens.Select public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { - var existingSet = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + var existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); if (existingSet == null) return; - beatmapSets.Remove(existingSet); - + root.RemoveChild(existingSet); updateItems(); if (existingSet.State == CarouselItemState.Selected) @@ -140,39 +133,29 @@ namespace osu.Game.Screens.Select public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) { - CarouselBeatmapSet existingSet = beatmapSets.Find(b => b.BeatmapSet.ID == beatmapSet.ID); + CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; var newSet = createCarouselSet(beatmapSet); - int index = beatmapSets.IndexOf(existingSet); - if (index >= 0) - beatmapSets.RemoveAt(index); + if (existingSet != null) + root.RemoveChild(existingSet); - if (newSet != null) + if (newSet == null) { - if (index >= 0) - beatmapSets.Insert(index, newSet); - else - beatmapSets.Add(newSet); + updateItems(); + SelectNext(); + return; } - if (hadSelection && newSet == null) - SelectNext(); + root.AddChild(newSet); - Filter(null, false); + Filter(debounce: false); //check if we can/need to maintain our current selection. - if (hadSelection && newSet != null) - { - var newSelection = newSet.Beatmaps.Find(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID); - - if (newSelection == null && selectedBeatmap != null) - newSelection = newSet.Beatmaps[Math.Min(newSet.Beatmaps.Count - 1, existingSet.Beatmaps.IndexOf(selectedBeatmap))]; - - select(newSelection); - } + if (hadSelection) + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); updateItems(); } @@ -212,7 +195,7 @@ namespace osu.Game.Screens.Select return; } - int originalIndex = Items.IndexOf(selectedBeatmap?.Drawables.Value.First()); + int originalIndex = Items.IndexOf(selectedBeatmap?.Drawables.First()); int currentIndex = originalIndex; // local function to increment the index in the required direction, wrapping over extremities. @@ -241,7 +224,7 @@ namespace osu.Game.Screens.Select public void SelectNextRandom() { - if (beatmapSets.Count == 0) + if (!beatmapSets.Any()) return; var visible = getVisibleSets().ToList(); @@ -275,7 +258,7 @@ namespace osu.Game.Screens.Select else set = visible.ElementAt(RNG.Next(visible.Count)); - select(set.Beatmaps[RNG.Next(set.Beatmaps.Count)]); + select(set.Beatmaps.Skip(RNG.Next(set.Beatmaps.Count())).FirstOrDefault()); } public void SelectPreviousRandom() @@ -303,7 +286,7 @@ namespace osu.Game.Screens.Select public void FlushPendingFilters() { if (FilterTask?.Completed == false) - Filter(null, false); + Filter(debounce: false); } public void Filter(FilterCriteria newCriteria = null, bool debounce = true) @@ -318,9 +301,8 @@ namespace osu.Game.Screens.Select var lastSet = selectedBeatmapSet; var lastBeatmap = selectedBeatmap; - beatmapSets.ForEach(s => s.Filter(criteria)); - - yPositionsCache.Invalidate(); + root.Filter(criteria); + updateItems(); if (selectedBeatmap?.Filtered == false) select(selectedBeatmap); @@ -337,6 +319,8 @@ namespace osu.Game.Screens.Select } else SelectNext(); + + ScrollToSelected(false); }; FilterTask?.Cancel(); diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index 655579269f..a0d91cf23b 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -4,6 +4,7 @@ using System; using System.Linq; using osu.Game.Beatmaps; +using osu.Game.Screens.Select.Filter; namespace osu.Game.Screens.Select.Carousel { @@ -32,5 +33,21 @@ namespace osu.Game.Screens.Select.Carousel Filtered.Value = !match; } + + public override int CompareTo(FilterCriteria criteria, CarouselItem other) + { + if (!(other is CarouselBeatmap otherBeatmap)) + return base.CompareTo(criteria, other); + + switch (criteria.Sort) + { + default: + case SortMode.Difficulty: + var ruleset = Beatmap.RulesetID.CompareTo(otherBeatmap.Beatmap.RulesetID); + if (ruleset != 0) return ruleset; + + return Beatmap.StarDifficulty.CompareTo(otherBeatmap.Beatmap.StarDifficulty); + } + } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index d0ccda0e28..655ed2057d 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -4,52 +4,53 @@ using System; using System.Collections.Generic; using System.Linq; +using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; +using osu.Game.Screens.Select.Filter; namespace osu.Game.Screens.Select.Carousel { public class CarouselBeatmapSet : CarouselGroupEagerSelect { - public readonly List Beatmaps; + public IEnumerable Beatmaps => InternalChildren.OfType(); public BeatmapSetInfo BeatmapSet; public CarouselBeatmapSet(BeatmapSetInfo beatmapSet) { - if (beatmapSet == null) throw new ArgumentNullException(nameof(beatmapSet)); + BeatmapSet = beatmapSet ?? throw new ArgumentNullException(nameof(beatmapSet)); - BeatmapSet = beatmapSet; - - Children = Beatmaps = beatmapSet.Beatmaps - .Where(b => !b.Hidden) - .OrderBy(b => b.RulesetID).ThenBy(b => b.StarDifficulty) - .Select(b => new CarouselBeatmap(b)) - .ToList(); + beatmapSet.Beatmaps + .Where(b => !b.Hidden) + .Select(b => new CarouselBeatmap(b)) + .ForEach(AddChild); } protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmapSet(this); - public override void Filter(FilterCriteria criteria) + public override int CompareTo(FilterCriteria criteria, CarouselItem other) { - base.Filter(criteria); - Filtered.Value = Children.All(i => i.Filtered); + if (!(other is CarouselBeatmapSet otherSet)) + return base.CompareTo(criteria, other); - /*switch (criteria.Sort) + switch (criteria.Sort) { default: case SortMode.Artist: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Artist, y.BeatmapSet.Metadata.Artist, StringComparison.InvariantCultureIgnoreCase)); - break; + return string.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist, StringComparison.InvariantCultureIgnoreCase); case SortMode.Title: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Title, y.BeatmapSet.Metadata.Title, StringComparison.InvariantCultureIgnoreCase)); - break; + return string.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title, StringComparison.InvariantCultureIgnoreCase); case SortMode.Author: - groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Author.Username, y.BeatmapSet.Metadata.Author.Username, StringComparison.InvariantCultureIgnoreCase)); - break; + return string.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username, StringComparison.InvariantCultureIgnoreCase); case SortMode.Difficulty: - groups.Sort((x, y) => x.BeatmapSet.MaxStarDifficulty.CompareTo(y.BeatmapSet.MaxStarDifficulty)); - break; - }*/ + return BeatmapSet.MaxStarDifficulty.CompareTo(otherSet.BeatmapSet.MaxStarDifficulty); + } + } + + public override void Filter(FilterCriteria criteria) + { + base.Filter(criteria); + Filtered.Value = InternalChildren.All(i => i.Filtered); } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 597fed7a34..695a8e1bc4 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using osu.Framework.Configuration; -using osu.Framework.Extensions.IEnumerableExtensions; namespace osu.Game.Screens.Select.Carousel { @@ -18,19 +17,15 @@ namespace osu.Game.Screens.Select.Carousel protected override DrawableCarouselItem CreateDrawableRepresentation() => null; - protected override IEnumerable Children + public override void AddChild(CarouselItem i) { - get { return base.Children; } - set - { - base.Children = value; - value.ForEach(i => i.State.ValueChanged += v => itemStateChanged(i, v)); - } + i.State.ValueChanged += v => itemStateChanged(i, v); + base.AddChild(i); } public CarouselGroup(List items = null) { - if (items != null) Children = items; + if (items != null) InternalChildren = items; } private void itemStateChanged(CarouselItem item, CarouselItemState value) @@ -40,7 +35,7 @@ namespace osu.Game.Screens.Select.Carousel // ensure we are the only item selected if (value == CarouselItemState.Selected) { - foreach (var b in Children) + foreach (var b in InternalChildren) { if (item == b) continue; b.State.Value = CarouselItemState.NotSelected; diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 351f0ed55b..b9312882a1 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -16,11 +16,11 @@ namespace osu.Game.Screens.Select.Carousel { if (v == CarouselItemState.Selected) { - foreach (var c in Children.Where(c => c.State.Value == CarouselItemState.Hidden)) + foreach (var c in InternalChildren.Where(c => c.State.Value == CarouselItemState.Hidden)) c.State.Value = CarouselItemState.NotSelected; - if (Children.Any(c => c.Visible) && Children.All(c => c.State != CarouselItemState.Selected)) - Children.First(c => c.Visible).State.Value = CarouselItemState.Selected; + if (InternalChildren.Any(c => c.Visible) && InternalChildren.All(c => c.State != CarouselItemState.Selected)) + InternalChildren.First(c => c.Visible).State.Value = CarouselItemState.Selected; } }; } diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index 68148814e0..7b0202cd87 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using osu.Framework.Configuration; -using osu.Framework.Extensions.IEnumerableExtensions; namespace osu.Game.Screens.Select.Carousel { @@ -14,45 +13,62 @@ namespace osu.Game.Screens.Select.Carousel public readonly Bindable State = new Bindable(CarouselItemState.NotSelected); - protected virtual IEnumerable Children { get; set; } + public IReadOnlyList Children => InternalChildren; + + protected List InternalChildren { get; set; } public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered.Value; - public readonly Lazy> Drawables; - - protected CarouselItem() + public IEnumerable Drawables { - Drawables = new Lazy>(() => + get { List items = new List(); - var self = CreateDrawableRepresentation(); + var self = drawableRepresentation.Value; if (self != null) items.Add(self); - if (Children != null) - foreach (var c in Children) - items.AddRange(c.Drawables.Value); + if (InternalChildren != null) + foreach (var c in InternalChildren) + items.AddRange(c.Drawables); return items; - }); + } + } + + public virtual void AddChild(CarouselItem i) => (InternalChildren ?? (InternalChildren = new List())).Add(i); + + public virtual void RemoveChild(CarouselItem i) => InternalChildren?.Remove(i); + + protected CarouselItem() + { + drawableRepresentation = new Lazy(CreateDrawableRepresentation); State.ValueChanged += v => { - if (Children == null) return; + if (InternalChildren == null) return; switch (v) { case CarouselItemState.Hidden: case CarouselItemState.NotSelected: - Children.ForEach(c => c.State.Value = CarouselItemState.Hidden); + InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Hidden); break; } }; } + private readonly Lazy drawableRepresentation; + protected abstract DrawableCarouselItem CreateDrawableRepresentation(); - public virtual void Filter(FilterCriteria criteria) => Children?.ForEach(c => c.Filter(criteria)); + public virtual void Filter(FilterCriteria criteria) + { + InternalChildren?.Sort((x, y) => x.CompareTo(criteria, y)); + InternalChildren?.ForEach(c => c.Filter(criteria)); + } + + public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => GetHashCode().CompareTo(other.GetHashCode()); } public enum CarouselItemState From 2817ed0d4678b6c5aec1a9bb22f20f29d3eeefb3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 20:45:59 +0900 Subject: [PATCH 13/53] Fix typo --- osu.Game.Tests/Visual/TestCasePlaySongSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs index d4cef5b160..7421546c43 100644 --- a/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/TestCasePlaySongSelect.cs @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Visual protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); [BackgroundDependencyLoader] - private void load(BeatmapManager baseMaanger) + private void load(BeatmapManager baseManager) { PlaySongSelect songSelect; @@ -64,7 +64,7 @@ namespace osu.Game.Tests.Visual dependencies.Cache(rulesets = new RulesetStore(contextFactory)); dependencies.Cache(manager = new BeatmapManager(storage, contextFactory, rulesets, null) { - DefaultBeatmap = baseMaanger.GetWorkingBeatmap(null) + DefaultBeatmap = baseManager.GetWorkingBeatmap(null) }); for (int i = 0; i < 100; i += 10) From 48e53a76b0e5e02d9a2b18a8a34ae3ee8e99c728 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Dec 2017 21:51:09 +0900 Subject: [PATCH 14/53] Fix incorrect line endings --- .../Configuration/RandomSelectAlgorithm.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/osu.Game/Configuration/RandomSelectAlgorithm.cs b/osu.Game/Configuration/RandomSelectAlgorithm.cs index 9ed6b9357b..cde657dba8 100644 --- a/osu.Game/Configuration/RandomSelectAlgorithm.cs +++ b/osu.Game/Configuration/RandomSelectAlgorithm.cs @@ -1,15 +1,15 @@ -// Copyright (c) 2007-2017 ppy Pty Ltd . -// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE - -using System.ComponentModel; - -namespace osu.Game.Configuration -{ - public enum RandomSelectAlgorithm - { - [Description("Never repeat")] - RandomPermutation, - [Description("Random")] - Random - } -} +// Copyright (c) 2007-2017 ppy Pty Ltd . +// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE + +using System.ComponentModel; + +namespace osu.Game.Configuration +{ + public enum RandomSelectAlgorithm + { + [Description("Never repeat")] + RandomPermutation, + [Description("Random")] + Random + } +} From 2e3332e3fe2b3c69e07012a9f63176c74eff3596 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 00:11:47 +0900 Subject: [PATCH 15/53] Shortcut non-visible panels to avoid adding as drawables --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e51ce85546..1f2bee52a5 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -498,6 +498,8 @@ namespace osu.Game.Screens.Select // Only add if we're not already part of the content. if (!scrollableContent.Contains(item)) { + if (!item.Item.Visible) continue; + // Makes sure headers are always _below_ items, // and depth flows downward. item.Depth = i + (item is DrawableCarouselBeatmapSet ? Items.Count : 0); From 192ceb5465b1df5b1769091c5fcc7250ad11b7ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 00:23:39 +0900 Subject: [PATCH 16/53] Avoid multiple access to selectedBeatmap during y position computation --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 1f2bee52a5..b0b5c932ba 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -387,6 +387,8 @@ namespace osu.Game.Screens.Select float lastSetY = 0; + var selected = selectedBeatmap; + foreach (DrawableCarouselItem d in Items) { switch (d) @@ -398,7 +400,7 @@ namespace osu.Game.Screens.Select case DrawableCarouselBeatmap beatmap: beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); - if (beatmap.Item == selectedBeatmap) + if (beatmap.Item == selected) selectedY = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; // on first display we want to begin hidden under our group's header. From ed5b6cc16f884db3cf6d809fb922118b2ef6d512 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 12:08:11 +0900 Subject: [PATCH 17/53] Add back ctrl-enter autoplay shortcut --- osu.Game/Screens/Select/PlaySongSelect.cs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Select/PlaySongSelect.cs b/osu.Game/Screens/Select/PlaySongSelect.cs index 565a7a0a01..4a0ee31fbb 100644 --- a/osu.Game/Screens/Select/PlaySongSelect.cs +++ b/osu.Game/Screens/Select/PlaySongSelect.cs @@ -117,18 +117,19 @@ namespace osu.Game.Screens.Select { if (player != null) return; - //if (state?.Keyboard.ControlPressed == true) - //{ - // var auto = Ruleset.Value.CreateInstance().GetAutoplayMod(); - // var autoType = auto.GetType(); + // Ctrl+Enter should start map with autoplay enabled. + if (GetContainingInputManager().CurrentState?.Keyboard.ControlPressed == true) + { + var auto = Ruleset.Value.CreateInstance().GetAutoplayMod(); + var autoType = auto.GetType(); - // var mods = modSelect.SelectedMods.Value; - // if (mods.All(m => m.GetType() != autoType)) - // { - // modSelect.SelectedMods.Value = mods.Concat(new[] { auto }); - // removeAutoModOnResume = true; - // } - //} + var mods = modSelect.SelectedMods.Value; + if (mods.All(m => m.GetType() != autoType)) + { + modSelect.SelectedMods.Value = mods.Concat(new[] { auto }); + removeAutoModOnResume = true; + } + } Beatmap.Value.Track.Looping = false; Beatmap.Disabled = true; From 59d512762e1f5a780f1f86dcdba77e82da75cd4f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 12:48:15 +0900 Subject: [PATCH 18/53] SongSelect tidying --- osu.Game/Screens/Select/SongSelect.cs | 131 ++++++++++++-------------- 1 file changed, 61 insertions(+), 70 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 0c3baf2c07..d71108a686 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -27,25 +27,11 @@ namespace osu.Game.Screens.Select { public abstract class SongSelect : OsuScreen { - private BeatmapManager beatmaps; - protected override BackgroundScreen CreateBackground() => new BackgroundScreenBeatmap(); - - private readonly BeatmapCarousel carousel; - private DialogOverlay dialogOverlay; - private static readonly Vector2 wedged_container_size = new Vector2(0.5f, 245); - + private static readonly Vector2 background_blur = new Vector2(20); private const float left_area_padding = 20; - private readonly BeatmapInfoWedge beatmapInfoWedge; - - protected Container LeftContent; - - private static readonly Vector2 background_blur = new Vector2(20); - private CancellationTokenSource initialAddSetsTask; - - private SampleChannel sampleChangeDifficulty; - private SampleChannel sampleChangeBeatmap; + public readonly FilterControl FilterControl; protected virtual bool ShowFooter => true; @@ -65,10 +51,21 @@ namespace osu.Game.Screens.Select /// protected readonly Container FooterPanels; - public readonly FilterControl FilterControl; + protected override BackgroundScreen CreateBackground() => new BackgroundScreenBeatmap(); + + protected Container LeftContent; + + private readonly BeatmapCarousel carousel; + private readonly BeatmapInfoWedge beatmapInfoWedge; + private DialogOverlay dialogOverlay; + private BeatmapManager beatmaps; + + private SampleChannel sampleChangeDifficulty; + private SampleChannel sampleChangeBeatmap; + + private CancellationTokenSource initialAddSetsTask; private DependencyContainer dependencies; - protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => dependencies = new DependencyContainer(parent); protected SongSelect() @@ -172,7 +169,7 @@ namespace osu.Game.Screens.Select Footer.AddButton(@"random", colours.Green, triggerRandom, Key.F2); Footer.AddButton(@"options", colours.Blue, BeatmapOptions, Key.F3); - BeatmapOptions.AddButton(@"Delete", @"Beatmap", FontAwesome.fa_trash, colours.Pink, () => Delete(Beatmap.Value.BeatmapSetInfo), Key.Number4, float.MaxValue); + BeatmapOptions.AddButton(@"Delete", @"Beatmap", FontAwesome.fa_trash, colours.Pink, () => delete(Beatmap.Value.BeatmapSetInfo), Key.Number4, float.MaxValue); } if (this.beatmaps == null) @@ -195,10 +192,14 @@ namespace osu.Game.Screens.Select carousel.BeatmapSets = this.beatmaps.GetAllUsableBeatmapSets(); - Beatmap.ValueChanged += beatmap_ValueChanged; - Beatmap.DisabledChanged += disabled => carousel.AllowSelection = !disabled; - carousel.AllowSelection = !Beatmap.Disabled; + Beatmap.TriggerChange(); + + Beatmap.ValueChanged += b => + { + if (!IsCurrentScreen) return; + carousel.SelectBeatmap(b?.BeatmapInfo); + }; } public void Edit(BeatmapInfo beatmap) @@ -207,32 +208,6 @@ namespace osu.Game.Screens.Select Push(new Editor()); } - private void onBeatmapRestored(BeatmapInfo beatmap) - { - Schedule(() => - { - var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); - carousel.UpdateBeatmapSet(beatmapSet); - }); - } - - private void onBeatmapHidden(BeatmapInfo beatmap) - { - Schedule(() => - { - var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); - carousel.UpdateBeatmapSet(beatmapSet); - }); - } - - private void carouselBeatmapsLoaded() - { - if (Beatmap.Value.BeatmapSetInfo?.DeletePending == false) - carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false); - else - carousel.SelectNextRandom(); - } - public void Start(BeatmapInfo beatmap) { // if we have a pending filter operation, we want to run it now. @@ -251,6 +226,11 @@ namespace osu.Game.Screens.Select Start(); } + /// + /// Called when a selection is made. + /// + protected abstract void Start(); + private ScheduledDelegate selectionChangedDebounce; // We need to keep track of the last selected beatmap ignoring debounce to play the correct selection sounds. @@ -307,18 +287,11 @@ namespace osu.Game.Screens.Select carousel.SelectNextRandom(); } - protected abstract void Start(); - - private void onBeatmapSetAdded(BeatmapSetInfo s) => Schedule(() => carousel.UpdateBeatmapSet(s)); - - private void onBeatmapSetRemoved(BeatmapSetInfo s) => Schedule(() => removeBeatmapSet(s)); - protected override void OnEntering(Screen last) { base.OnEntering(last); Content.FadeInFromZero(250); - FilterControl.Activate(); } @@ -359,13 +332,6 @@ namespace osu.Game.Screens.Select logo.FadeOut(logo_transition / 2, Easing.Out); } - private void beatmap_ValueChanged(WorkingBeatmap beatmap) - { - if (!IsCurrentScreen) return; - - carousel.SelectBeatmap(beatmap?.BeatmapInfo); - } - protected override void OnResuming(Screen last) { if (Beatmap != null && !Beatmap.Value.BeatmapSetInfo.DeletePending) @@ -426,8 +392,7 @@ namespace osu.Game.Screens.Select /// The working beatmap. protected virtual void UpdateBeatmap(WorkingBeatmap beatmap) { - var backgroundModeBeatmap = Background as BackgroundScreenBeatmap; - if (backgroundModeBeatmap != null) + if (Background is BackgroundScreenBeatmap backgroundModeBeatmap) { backgroundModeBeatmap.Beatmap = beatmap; backgroundModeBeatmap.BlurTo(background_blur, 750, Easing.OutQuint); @@ -452,18 +417,44 @@ namespace osu.Game.Screens.Select } } - private void removeBeatmapSet(BeatmapSetInfo beatmapSet) + private void onBeatmapSetAdded(BeatmapSetInfo s) => Schedule(() => carousel.UpdateBeatmapSet(s)); + + private void onBeatmapSetRemoved(BeatmapSetInfo s) => Schedule(() => { - carousel.RemoveBeatmapSet(beatmapSet); + carousel.RemoveBeatmapSet(s); if (carousel.SelectedBeatmap == null) Beatmap.SetDefault(); + }); + + private void onBeatmapRestored(BeatmapInfo beatmap) + { + Schedule(() => + { + var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); + carousel.UpdateBeatmapSet(beatmapSet); + }); } - public void Delete(BeatmapSetInfo beatmap) + private void onBeatmapHidden(BeatmapInfo beatmap) { - if (beatmap == null) - return; + Schedule(() => + { + var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); + carousel.UpdateBeatmapSet(beatmapSet); + }); + } + private void carouselBeatmapsLoaded() + { + if (Beatmap.Value.BeatmapSetInfo?.DeletePending == false) + carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false); + else + carousel.SelectNextRandom(); + } + + private void delete(BeatmapSetInfo beatmap) + { + if (beatmap == null) return; dialogOverlay?.Push(new BeatmapDeleteDialog(beatmap)); } @@ -481,7 +472,7 @@ namespace osu.Game.Screens.Select if (state.Keyboard.ShiftPressed) { if (!Beatmap.IsDefault) - Delete(Beatmap.Value.BeatmapSetInfo); + delete(Beatmap.Value.BeatmapSetInfo); return true; } From e6cac4a6753d505322b22ea4d92899067d333055 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 14:36:11 +0900 Subject: [PATCH 19/53] Allow tests to work with a variable number of beatmap sets loaded --- .../Visual/TestCaseBeatmapCarousel.cs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 6163daaf39..12e6e292f6 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -39,6 +39,8 @@ namespace osu.Game.Tests.Visual private readonly Stack selectedSets = new Stack(); + private const int set_count = 1000; + [BackgroundDependencyLoader] private void load() { @@ -47,16 +49,12 @@ namespace osu.Game.Tests.Visual RelativeSizeAxes = Axes.Both, }); - AddStep("Load Beatmaps", () => - { - carousel.BeatmapSets = new[] - { - createTestBeatmapSet(1), - createTestBeatmapSet(2), - createTestBeatmapSet(3), - createTestBeatmapSet(4), - }; - }); + List beatmapSets = new List(); + + for (int i = 1; i <= set_count; i++) + beatmapSets.Add(createTestBeatmapSet(i)); + + AddStep("Load Beatmaps", () => { carousel.BeatmapSets = beatmapSets; }); AddUntilStep(() => carousel.BeatmapSets.Any(), "Wait for load"); @@ -127,17 +125,17 @@ namespace osu.Game.Tests.Visual checkSelected(1, 2); advanceSelection(direction: -1, diff: false); - checkSelected(4, 1); + checkSelected(set_count, 1); advanceSelection(direction: -1, diff: true); - checkSelected(3, 3); + checkSelected(set_count - 1, 3); advanceSelection(diff: false); advanceSelection(diff: false); checkSelected(1, 1); advanceSelection(direction: -1, diff: true); - checkSelected(4, 3); + checkSelected(set_count, 3); } /// @@ -147,7 +145,7 @@ namespace osu.Game.Tests.Visual { // basic filtering - AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3" }, false)); + AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3!" }, false)); checkVisibleItemCount(diff: false, count: 1); checkVisibleItemCount(diff: true, count: 3); checkSelected(3, 1); @@ -157,7 +155,7 @@ namespace osu.Game.Tests.Visual AddStep("Un-filter (debounce)", () => carousel.Filter(new FilterCriteria())); AddUntilStep(() => !carousel.PendingFilterTask, "Wait for debounce"); - checkVisibleItemCount(diff: false, count: 4); + checkVisibleItemCount(diff: false, count: set_count); checkVisibleItemCount(diff: true, count: 3); // test filtering some difficulties (and keeping current beatmap set selected). @@ -202,16 +200,18 @@ namespace osu.Game.Tests.Visual /// private void testAddRemove() { - AddStep("Add new set #5", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(5))); - AddStep("Add new set #6", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(6))); + AddStep("Add new set", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(set_count + 1))); + AddStep("Add new set", () => carousel.UpdateBeatmapSet(createTestBeatmapSet(set_count + 2))); - checkVisibleItemCount(false, 6); + checkVisibleItemCount(false, set_count + 2); - AddStep("Remove set #5", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(5))); + AddStep("Remove set", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(set_count + 2))); - checkVisibleItemCount(false, 5); + checkVisibleItemCount(false, set_count + 1); - AddStep("Remove set #6", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(6))); + AddStep("Remove set", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(set_count + 1))); + + checkVisibleItemCount(false, set_count); } /// @@ -220,9 +220,9 @@ namespace osu.Game.Tests.Visual private void testSorting() { AddStep("Sort by author", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Author }, false)); - AddAssert("Check yyyyy is at bottom", () => carousel.BeatmapSets.Last().Metadata.AuthorString == "yyyyy"); - AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); - AddAssert("Check #4 is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith("#4")); + AddAssert("Check zzzzz is at bottom", () => carousel.BeatmapSets.Last().Metadata.AuthorString == "zzzzz"); + AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddAssert($"Check #{set_count} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith($"#{set_count}!")); } @@ -237,9 +237,9 @@ namespace osu.Game.Tests.Visual { OnlineBeatmapSetID = i, // Create random metadata, then we can check if sorting works based on these - Artist = "peppy", - Title = "test set #" + i, - AuthorString = string.Concat(Enumerable.Repeat((char)('z' - i), 5)) + Artist = $"peppy{i.ToString().PadLeft(6,'0')}", + Title = $"test set #{i}!", + AuthorString = string.Concat(Enumerable.Repeat((char)('z' - Math.Min(25,i - 1)), 5)) }, Beatmaps = new List(new[] { From acfdd3278331069d508a1a01697bdb7c8fbac265 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 18:38:09 +0900 Subject: [PATCH 20/53] Move DrawableCarouselBeatmap initialisation to BDL oops --- .../Carousel/DrawableCarouselBeatmap.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 1fca23a2ec..cd6cc55037 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -26,14 +26,20 @@ namespace osu.Game.Screens.Select.Carousel { private readonly BeatmapInfo beatmap; - private readonly Sprite background; + private Sprite background; public Action StartRequested; public Action EditRequested; public Action HideRequested; - private readonly Triangles triangles; - private readonly StarCounter starCounter; + private Triangles triangles; + private StarCounter starCounter; + + public DrawableCarouselBeatmap(CarouselBeatmap panel) : base(panel) + { + beatmap = panel.Beatmap; + Height *= 0.60f; + } [BackgroundDependencyLoader(true)] private void load(SongSelect songSelect, BeatmapManager manager) @@ -46,13 +52,6 @@ namespace osu.Game.Screens.Select.Carousel if (manager != null) HideRequested = manager.Hide; - } - - public DrawableCarouselBeatmap(CarouselBeatmap panel) - : base(panel) - { - beatmap = panel.Beatmap; - Height *= 0.60f; Children = new Drawable[] { From fd9d900ae00bfe9ac5c1a59332162671fd6b8e9c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Dec 2017 18:40:03 +0900 Subject: [PATCH 21/53] Simplify StarCounter and SpriteIcon --- osu.Game/Graphics/SpriteIcon.cs | 29 ++++++++++--------- .../Graphics/UserInterface/StarCounter.cs | 23 +++++---------- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/osu.Game/Graphics/SpriteIcon.cs b/osu.Game/Graphics/SpriteIcon.cs index e752b1d91a..326aa2fb79 100644 --- a/osu.Game/Graphics/SpriteIcon.cs +++ b/osu.Game/Graphics/SpriteIcon.cs @@ -15,14 +15,19 @@ namespace osu.Game.Graphics { public class SpriteIcon : CompositeDrawable { - private readonly Sprite spriteShadow; - private readonly Sprite spriteMain; + private Sprite spriteShadow; + private Sprite spriteMain; private Cached layout = new Cached(); - private readonly Container shadowVisibility; + private Container shadowVisibility; - public SpriteIcon() + private FontStore store; + + [BackgroundDependencyLoader] + private void load(FontStore store) { + this.store = store; + InternalChildren = new Drawable[] { shadowVisibility = new Container @@ -39,7 +44,7 @@ namespace osu.Game.Graphics Y = 2, Colour = new Color4(0f, 0f, 0f, 0.2f), }, - Alpha = 0, + Alpha = shadow ? 1 : 0, }, spriteMain = new Sprite { @@ -49,14 +54,7 @@ namespace osu.Game.Graphics FillMode = FillMode.Fit }, }; - } - private FontStore store; - - [BackgroundDependencyLoader] - private void load(FontStore store) - { - this.store = store; updateTexture(); } @@ -105,12 +103,15 @@ namespace osu.Game.Graphics } } + private bool shadow; public bool Shadow { - get { return spriteShadow.IsPresent; } + get { return shadow; } set { - shadowVisibility.Alpha = value ? 1 : 0; + shadow = value; + if (shadowVisibility != null) + shadowVisibility.Alpha = value ? 1 : 0; } } diff --git a/osu.Game/Graphics/UserInterface/StarCounter.cs b/osu.Game/Graphics/UserInterface/StarCounter.cs index e581d19d54..b4ca0d2e02 100644 --- a/osu.Game/Graphics/UserInterface/StarCounter.cs +++ b/osu.Game/Graphics/UserInterface/StarCounter.cs @@ -6,6 +6,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.MathUtils; using System; +using System.Linq; namespace osu.Game.Graphics.UserInterface { @@ -72,16 +73,9 @@ namespace osu.Game.Graphics.UserInterface AutoSizeAxes = Axes.Both, Direction = FillDirection.Horizontal, Spacing = new Vector2(star_spacing), + ChildrenEnumerable = Enumerable.Range(0, StarCount).Select(i => new Star { Alpha = minStarAlpha }) } }; - - for (int i = 0; i < StarCount; i++) - { - stars.Add(new Star - { - Alpha = minStarAlpha, - }); - } } protected override void LoadComplete() @@ -147,15 +141,12 @@ namespace osu.Game.Graphics.UserInterface { Size = new Vector2(star_size); - Children = new[] + Child = Icon = new SpriteIcon { - Icon = new SpriteIcon - { - Size = new Vector2(star_size), - Icon = FontAwesome.fa_star, - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - } + Size = new Vector2(star_size), + Icon = FontAwesome.fa_star, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, }; } } From a8a2c233a004adf866e155fd0b95c701109475c9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 00:33:09 +0900 Subject: [PATCH 22/53] Add tests for (and fix) removal of last item in carousel --- .../Visual/TestCaseBeatmapCarousel.cs | 40 +++++++++++++++++-- osu.Game/Screens/Select/BeatmapCarousel.cs | 8 ++-- .../Screens/Select/Carousel/CarouselGroup.cs | 4 +- .../Carousel/CarouselGroupEagerSelect.cs | 17 ++++++++ .../Screens/Select/Carousel/CarouselItem.cs | 9 +++++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 12e6e292f6..8c8ec8a670 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -39,7 +39,9 @@ namespace osu.Game.Tests.Visual private readonly Stack selectedSets = new Stack(); - private const int set_count = 1000; + private BeatmapInfo currentSelection; + + private const int set_count = 5; [BackgroundDependencyLoader] private void load() @@ -54,6 +56,8 @@ namespace osu.Game.Tests.Visual for (int i = 1; i <= set_count; i++) beatmapSets.Add(createTestBeatmapSet(i)); + carousel.SelectionChanged = s => currentSelection = s; + AddStep("Load Beatmaps", () => { carousel.BeatmapSets = beatmapSets; }); AddUntilStep(() => carousel.BeatmapSets.Any(), "Wait for load"); @@ -63,6 +67,8 @@ namespace osu.Game.Tests.Visual testRandom(); testAddRemove(); testSorting(); + + testRemoveAll(); } private void ensureRandomFetchSuccess() => @@ -145,6 +151,8 @@ namespace osu.Game.Tests.Visual { // basic filtering + setSelected(1, 1); + AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = "set #3!" }, false)); checkVisibleItemCount(diff: false, count: 1); checkVisibleItemCount(diff: true, count: 3); @@ -163,8 +171,19 @@ namespace osu.Game.Tests.Visual setSelected(1, 2); AddStep("Filter some difficulties", () => carousel.Filter(new FilterCriteria { SearchText = "Normal" }, false)); checkSelected(1, 1); + AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); checkSelected(1, 1); + + AddStep("Filter all", () => carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false)); + + checkVisibleItemCount(false, 0); + checkVisibleItemCount(true, 0); + AddAssert("Selection is null", () => currentSelection == null); + + AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); + + AddAssert("Selection is non-null", () => currentSelection != null); } /// @@ -225,6 +244,21 @@ namespace osu.Game.Tests.Visual AddAssert($"Check #{set_count} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Title.EndsWith($"#{set_count}!")); } + private void testRemoveAll() + { + setSelected(2, 1); + + AddAssert("Selection is non-null", () => currentSelection != null); + + AddUntilStep(() => + { + carousel.RemoveBeatmapSet(carousel.BeatmapSets.Last()); + return !carousel.BeatmapSets.Any(); + }, "Remove all"); + + AddAssert("Selection is null", () => currentSelection == null); + } + private BeatmapSetInfo createTestBeatmapSet(int i) { @@ -237,9 +271,9 @@ namespace osu.Game.Tests.Visual { OnlineBeatmapSetID = i, // Create random metadata, then we can check if sorting works based on these - Artist = $"peppy{i.ToString().PadLeft(6,'0')}", + Artist = $"peppy{i.ToString().PadLeft(6, '0')}", Title = $"test set #{i}!", - AuthorString = string.Concat(Enumerable.Repeat((char)('z' - Math.Min(25,i - 1)), 5)) + AuthorString = string.Concat(Enumerable.Repeat((char)('z' - Math.Min(25, i - 1)), 5)) }, Beatmaps = new List(new[] { diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index b0b5c932ba..fab5ce473a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -62,7 +62,7 @@ namespace osu.Game.Screens.Select { List newSets = null; - CarouselGroup newRoot = new CarouselGroup(); + CarouselGroup newRoot = new CarouselGroupEagerSelect(); Task.Run(() => { @@ -102,7 +102,7 @@ namespace osu.Game.Screens.Select private readonly Stack randomSelectedBeatmaps = new Stack(); protected List Items = new List(); - private CarouselGroup root = new CarouselGroup(); + private CarouselGroup root = new CarouselGroupEagerSelect(); public BeatmapCarousel() { @@ -189,7 +189,7 @@ 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 (beatmapSets.All(g => !g.Visible)) + if (beatmapSets.All(g => g.Filtered)) { SelectionChanged?.Invoke(null); return; @@ -304,7 +304,7 @@ namespace osu.Game.Screens.Select root.Filter(criteria); updateItems(); - if (selectedBeatmap?.Filtered == false) + if (selectedBeatmap != null && !selectedBeatmap.Filtered) select(selectedBeatmap); else if (lastBeatmap != null && !lastSet.Filtered) { diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 695a8e1bc4..151e73f45f 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -19,7 +19,7 @@ namespace osu.Game.Screens.Select.Carousel public override void AddChild(CarouselItem i) { - i.State.ValueChanged += v => itemStateChanged(i, v); + i.State.ValueChanged += v => ItemStateChanged(i, v); base.AddChild(i); } @@ -28,7 +28,7 @@ namespace osu.Game.Screens.Select.Carousel if (items != null) InternalChildren = items; } - private void itemStateChanged(CarouselItem item, CarouselItemState value) + protected virtual void ItemStateChanged(CarouselItem item, CarouselItemState value) { // todo: check state of selected item. diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index b9312882a1..0a3c7b40ba 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -24,5 +24,22 @@ namespace osu.Game.Screens.Select.Carousel } }; } + + protected override void ItemStateChanged(CarouselItem item, CarouselItemState value) + { + base.ItemStateChanged(item, value); + + if (value == CarouselItemState.NotSelected) + { + if (Children.All(i => i.State != CarouselItemState.Selected)) + { + var first = Children.FirstOrDefault(i => !i.Filtered); + if (first != null) + { + first.State.Value = CarouselItemState.Selected; + } + } + } + } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index 7b0202cd87..fba7e4321c 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using osu.Framework.Configuration; +using osu.Framework.Logging; namespace osu.Game.Screens.Select.Carousel { @@ -48,6 +49,8 @@ namespace osu.Game.Screens.Select.Carousel { if (InternalChildren == null) return; + Logger.Log($"State changed to {v}"); + switch (v) { case CarouselItemState.Hidden: @@ -56,6 +59,12 @@ namespace osu.Game.Screens.Select.Carousel break; } }; + + Filtered.ValueChanged += v => + { + if (v && State == CarouselItemState.Selected) + State.Value = CarouselItemState.NotSelected; + }; } private readonly Lazy drawableRepresentation; From 49ce42d90cbe3f0be99573bc773a58dda02b3155 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 16:14:01 +0900 Subject: [PATCH 23/53] Add ToString() overrides on many classes to make debugging easier --- osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs | 4 ++++ osu.Game/Beatmaps/BeatmapInfo.cs | 2 ++ osu.Game/Beatmaps/BeatmapMetadata.cs | 2 ++ osu.Game/Beatmaps/BeatmapSetInfo.cs | 2 ++ osu.Game/Users/User.cs | 2 ++ 5 files changed, 12 insertions(+) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 8c8ec8a670..ccf9ecb7f9 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -228,9 +228,13 @@ namespace osu.Game.Tests.Visual checkVisibleItemCount(false, set_count + 1); + setSelected(set_count + 1, 1); + AddStep("Remove set", () => carousel.RemoveBeatmapSet(createTestBeatmapSet(set_count + 1))); checkVisibleItemCount(false, set_count); + + checkSelected(set_count, 1); } /// diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index f3a9694982..5856eab303 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -118,6 +118,8 @@ namespace osu.Game.Beatmaps [JsonProperty("difficulty_rating")] public double StarDifficulty { get; set; } + public override string ToString() => $"{Metadata} [{Version}]"; + public bool Equals(BeatmapInfo other) { if (ID == 0 || other?.ID == 0) diff --git a/osu.Game/Beatmaps/BeatmapMetadata.cs b/osu.Game/Beatmaps/BeatmapMetadata.cs index a78ef25166..e0e0bb4eb2 100644 --- a/osu.Game/Beatmaps/BeatmapMetadata.cs +++ b/osu.Game/Beatmaps/BeatmapMetadata.cs @@ -57,6 +57,8 @@ namespace osu.Game.Beatmaps public string AudioFile { get; set; } public string BackgroundFile { get; set; } + public override string ToString() => $"{Artist} - {Title} ({Author})"; + public string[] SearchableTerms => new[] { Author?.Username, diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index c870c31a8b..a41beaab81 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -33,6 +33,8 @@ namespace osu.Game.Beatmaps public List Files { get; set; } + public override string ToString() => Metadata.ToString(); + public bool Protected { get; set; } } } diff --git a/osu.Game/Users/User.cs b/osu.Game/Users/User.cs index a6fa8637fd..5c0e5f1f95 100644 --- a/osu.Game/Users/User.cs +++ b/osu.Game/Users/User.cs @@ -136,5 +136,7 @@ namespace osu.Game.Users [JsonProperty(@"rankHistory")] public RankHistoryData RankHistory; + + public override string ToString() => Username; } } From 3c406662ed2330c26305da18616376816008234d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 16:14:37 +0900 Subject: [PATCH 24/53] Ensure correct selection after deletion of currently selected Also fixes a lot of bad interactions and simplifies further. --- .../Visual/TestCaseBeatmapCarousel.cs | 18 ++++--- osu.Game/Screens/Select/BeatmapCarousel.cs | 39 +++----------- .../Select/Carousel/CarouselBeatmap.cs | 2 + .../Select/Carousel/CarouselBeatmapSet.cs | 2 + .../Screens/Select/Carousel/CarouselGroup.cs | 8 +-- .../Carousel/CarouselGroupEagerSelect.cs | 54 ++++++++++++------- .../Screens/Select/Carousel/CarouselItem.cs | 24 +++++++-- 7 files changed, 79 insertions(+), 68 deletions(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index ccf9ecb7f9..973ea71cbf 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -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); } /// diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index fab5ce473a..e37040b3de 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -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 yPositions = new List(); @@ -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 /// Whether to skip individual difficulties and only increment over full groups. 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 getVisibleSets() => beatmapSets.Where(select => select.Visible); + private IEnumerable 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); }; diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index a0d91cf23b..cd3fefe8e6 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -49,5 +49,7 @@ namespace osu.Game.Screens.Select.Carousel return Beatmap.StarDifficulty.CompareTo(otherBeatmap.Beatmap.StarDifficulty); } } + + public override string ToString() => Beatmap.ToString(); } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index 655ed2057d..885595fc51 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -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(); } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 151e73f45f..98aa7cd5c5 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -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 items; - public readonly Bindable Selected = new Bindable(); - 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; } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 0a3c7b40ba..4fca0c69b6 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -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; + } } } diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index fba7e4321c..91fe600dc7 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -18,7 +18,10 @@ namespace osu.Game.Screens.Select.Carousel protected List InternalChildren { get; set; } - public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered.Value; + /// + /// This item is not in a hidden state. + /// + public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered; public IEnumerable Drawables { @@ -39,7 +42,14 @@ namespace osu.Game.Screens.Select.Carousel public virtual void AddChild(CarouselItem i) => (InternalChildren ?? (InternalChildren = new List())).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; } }; From df7e795aa3af6b959d78cb862f5d9ed23d69241a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 16:27:39 +0900 Subject: [PATCH 25/53] Simplify and rename filter methods --- osu.Game/Screens/Select/BeatmapCarousel.cs | 27 ++++++++++++---------- osu.Game/Screens/Select/SongSelect.cs | 4 ++-- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e37040b3de..40c3ae0fdc 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -67,7 +67,7 @@ namespace osu.Game.Screens.Select Task.Run(() => { value.Select(createCarouselSet).Where(g => g != null).ForEach(newRoot.AddChild); - newRoot.Filter(criteria); + newRoot.Filter(activeCriteria); }).ContinueWith(t => { Schedule(() => @@ -149,7 +149,7 @@ namespace osu.Game.Screens.Select root.AddChild(newSet); - Filter(debounce: false); + applyActiveCriteria(false); //check if we can/need to maintain our current selection. if (hadSelection) @@ -158,7 +158,7 @@ namespace osu.Game.Screens.Select updateItems(); } - public void SelectBeatmap(BeatmapInfo beatmap, bool animated = true) + public void SelectBeatmap(BeatmapInfo beatmap) { if (beatmap == null || beatmap.Hidden) { @@ -212,14 +212,12 @@ namespace osu.Game.Screens.Select } } - private IEnumerable getVisibleSets() => beatmapSets.Where(select => !select.Filtered); - public void SelectNextRandom() { if (!beatmapSets.Any()) return; - var visible = getVisibleSets().ToList(); + var visible = beatmapSets.Where(select => !select.Filtered).ToList(); if (!visible.Any()) return; @@ -269,28 +267,33 @@ namespace osu.Game.Screens.Select } } - private FilterCriteria criteria = new FilterCriteria(); + private FilterCriteria activeCriteria = new FilterCriteria(); protected ScheduledDelegate FilterTask; public bool AllowSelection = true; - public void FlushPendingFilters() + public void FlushPendingFilterOperations() { if (FilterTask?.Completed == false) - Filter(debounce: false); + applyActiveCriteria(false); } - public void Filter(FilterCriteria newCriteria = null, bool debounce = true) + public void Filter(FilterCriteria newCriteria, bool debounce = true) { if (newCriteria != null) - criteria = newCriteria; + activeCriteria = newCriteria; + applyActiveCriteria(debounce); + } + + private void applyActiveCriteria(bool debounce) + { Action perform = delegate { FilterTask = null; - root.Filter(criteria); + root.Filter(activeCriteria); updateItems(); ScrollToSelected(false); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index d71108a686..d9a3f2faea 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -212,7 +212,7 @@ namespace osu.Game.Screens.Select { // if we have a pending filter operation, we want to run it now. // it could change selection (ie. if the ruleset has been changed). - carousel.FlushPendingFilters(); + carousel.FlushPendingFilterOperations(); if (selectionChangedDebounce?.Completed == false) { @@ -447,7 +447,7 @@ namespace osu.Game.Screens.Select private void carouselBeatmapsLoaded() { if (Beatmap.Value.BeatmapSetInfo?.DeletePending == false) - carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false); + carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo); else carousel.SelectNextRandom(); } From 59dbca26129c49b17f3e166610a261c9980d07c8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 16:44:42 +0900 Subject: [PATCH 26/53] Fix ScrollToSelected being called in too many cases --- osu.Game/Screens/Select/BeatmapCarousel.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 40c3ae0fdc..6e43f6fb01 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -149,7 +149,7 @@ namespace osu.Game.Screens.Select root.AddChild(newSet); - applyActiveCriteria(false); + applyActiveCriteria(false, false); //check if we can/need to maintain our current selection. if (hadSelection) @@ -276,7 +276,7 @@ namespace osu.Game.Screens.Select public void FlushPendingFilterOperations() { if (FilterTask?.Completed == false) - applyActiveCriteria(false); + applyActiveCriteria(false, false); } public void Filter(FilterCriteria newCriteria, bool debounce = true) @@ -284,10 +284,10 @@ namespace osu.Game.Screens.Select if (newCriteria != null) activeCriteria = newCriteria; - applyActiveCriteria(debounce); + applyActiveCriteria(debounce, true); } - private void applyActiveCriteria(bool debounce) + private void applyActiveCriteria(bool debounce, bool scroll) { Action perform = delegate { @@ -296,7 +296,7 @@ namespace osu.Game.Screens.Select root.Filter(activeCriteria); updateItems(); - ScrollToSelected(false); + if (scroll) ScrollToSelected(false); }; FilterTask?.Cancel(); @@ -446,7 +446,7 @@ namespace osu.Game.Screens.Select computeYPositions(); // Remove all items that should no longer be on-screen - scrollableContent.RemoveAll(delegate(DrawableCarouselItem p) + scrollableContent.RemoveAll(delegate (DrawableCarouselItem p) { float itemPosY = p.Position.Y; bool remove = itemPosY < Current - p.DrawHeight || itemPosY > Current + drawHeight || !p.IsPresent; From bd9056c709f72bd47c10b485a06fd529298df9d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 17:32:21 +0900 Subject: [PATCH 27/53] 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)); } } From 5aee8f80bb4f56c9e71ad9dd1c83b601210f79f8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 17:33:50 +0900 Subject: [PATCH 28/53] Fix incorrect test assumption (affected by random select above) --- osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 34d2ceacb1..52db5b4883 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -240,7 +240,7 @@ namespace osu.Game.Tests.Visual checkVisibleItemCount(false, set_count); - checkSelected(set_count, 2); + checkSelected(set_count); } /// From 33f8c8419ab279e1b4331ad76a5d299ff462d130 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 21:25:54 +0900 Subject: [PATCH 29/53] Fix initial beatmap selection potentially being incorrect --- osu.Game/Screens/Select/BeatmapCarousel.cs | 7 +------ osu.Game/Screens/Select/SongSelect.cs | 4 ++-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 6e43f6fb01..2dba274831 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -160,13 +160,8 @@ namespace osu.Game.Screens.Select public void SelectBeatmap(BeatmapInfo beatmap) { - if (beatmap == null || beatmap.Hidden) - { - SelectNext(); + if (beatmap?.Hidden != false) return; - } - - if (beatmap == SelectedBeatmap) return; foreach (CarouselBeatmapSet group in beatmapSets) { diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index d9a3f2faea..1fea4d0ccb 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -197,8 +197,8 @@ namespace osu.Game.Screens.Select Beatmap.ValueChanged += b => { - if (!IsCurrentScreen) return; - carousel.SelectBeatmap(b?.BeatmapInfo); + if (IsCurrentScreen) + carousel.SelectBeatmap(b?.BeatmapInfo); }; } From da0940ae0b0a7c4743da685a9388af15dbe4dbad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 23:55:55 +0900 Subject: [PATCH 30/53] Only apply criteria if there are items populated in the carousel --- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 2dba274831..98902b3f6b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -284,7 +284,9 @@ namespace osu.Game.Screens.Select private void applyActiveCriteria(bool debounce, bool scroll) { - Action perform = delegate + if (root.Children?.Any() != true) return; + + void perform() { FilterTask = null; @@ -292,7 +294,7 @@ namespace osu.Game.Screens.Select updateItems(); if (scroll) ScrollToSelected(false); - }; + } FilterTask?.Cancel(); FilterTask = null; From 29a8ade59f59838796af698c9cb8243d31e771ff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 16 Dec 2017 23:56:14 +0900 Subject: [PATCH 31/53] Rename "Hidden" to "Collapsed" --- osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs | 2 +- osu.Game/Screens/Select/Carousel/CarouselItem.cs | 10 +++++----- .../Screens/Select/Carousel/DrawableCarouselBeatmap.cs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs index cd3fefe8e6..d7e7b1e265 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmap.cs @@ -15,7 +15,7 @@ namespace osu.Game.Screens.Select.Carousel public CarouselBeatmap(BeatmapInfo beatmap) { Beatmap = beatmap; - State.Value = CarouselItemState.Hidden; + State.Value = CarouselItemState.Collapsed; } protected override DrawableCarouselItem CreateDrawableRepresentation() => new DrawableCarouselBeatmap(this); diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index 91fe600dc7..12125fe5e0 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -21,7 +21,7 @@ namespace osu.Game.Screens.Select.Carousel /// /// This item is not in a hidden state. /// - public bool Visible => State.Value != CarouselItemState.Hidden && !Filtered; + public bool Visible => State.Value != CarouselItemState.Collapsed && !Filtered; public IEnumerable Drawables { @@ -63,14 +63,14 @@ namespace osu.Game.Screens.Select.Carousel switch (v) { - case CarouselItemState.Hidden: + case CarouselItemState.Collapsed: case CarouselItemState.NotSelected: - InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Hidden); + InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Collapsed); break; case CarouselItemState.Selected: InternalChildren.ForEach(c => { - if (c.State == CarouselItemState.Hidden) c.State.Value = CarouselItemState.NotSelected; + if (c.State == CarouselItemState.Collapsed) c.State.Value = CarouselItemState.NotSelected; }); break; } @@ -98,7 +98,7 @@ namespace osu.Game.Screens.Select.Carousel public enum CarouselItemState { - Hidden, + Collapsed, NotSelected, Selected, } diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index cd6cc55037..92768cd88f 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -160,7 +160,7 @@ namespace osu.Game.Screens.Select.Carousel protected override void ApplyState() { - if (Item.State.Value != CarouselItemState.Hidden && Alpha == 0) + if (Item.State.Value != CarouselItemState.Collapsed && Alpha == 0) starCounter.ReplayAnimation(); base.ApplyState(); From e2710a309cc8a5ccc531fe0d327dbc8fe9be903c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 17 Dec 2017 02:33:01 +0900 Subject: [PATCH 32/53] Fix panel animation and depth --- osu.Game/Screens/Select/BeatmapCarousel.cs | 67 +++++++++++++--------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 98902b3f6b..dac01f788c 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -305,11 +305,9 @@ namespace osu.Game.Screens.Select perform(); } - public void ScrollToSelected(bool animated = true) - { - float selectedY = computeYPositions(animated); - ScrollTo(selectedY, animated); - } + private float? scrollTarget; + + public void ScrollToSelected(bool animated = true) => Schedule(() => { if (scrollTarget != null) ScrollTo(scrollTarget.Value, animated); }); private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) { @@ -351,39 +349,56 @@ namespace osu.Game.Screens.Select /// Computes the target Y positions for every item in the carousel. /// /// The Y position of the currently selected item. - private float computeYPositions(bool animated = true) + private void computeYPositions(bool animated = true) { yPositions.Clear(); float currentY = DrawHeight / 2; - float selectedY = currentY; + DrawableCarouselBeatmapSet lastSet = null; - float lastSetY = 0; - - var selected = selectedBeatmap; + scrollTarget = null; foreach (DrawableCarouselItem d in Items) { - switch (d) + if (d.IsPresent) { - case DrawableCarouselBeatmapSet set: - set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); - lastSetY = set.Position.Y; - break; - case DrawableCarouselBeatmap beatmap: - beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); + switch (d) + { + case DrawableCarouselBeatmapSet set: + lastSet = set; - if (beatmap.Item == selected) - selectedY = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; + set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); + set.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); + break; + case DrawableCarouselBeatmap beatmap: + if (beatmap.Item.State.Value == CarouselItemState.Selected) + scrollTarget = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; - // on first display we want to begin hidden under our group's header. - if (animated && !beatmap.IsPresent) - beatmap.MoveToY(lastSetY); - break; + void performMove(float y, float? startY = null) + { + if (startY != null) beatmap.MoveTo(new Vector2(0, startY.Value)); + beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); + beatmap.MoveToY(y, animated ? 750 : 0, Easing.OutExpo); + } + + Debug.Assert(lastSet != null); + + float? setY = null; + if (!d.IsLoaded || beatmap.Alpha == 0) // can't use IsPresent due to DrawableCarouselItem override. + setY = lastSet.Y + lastSet.DrawHeight + 5; + + if (d.IsLoaded) + performMove(currentY, setY); + else + { + float y = currentY; + d.OnLoadComplete = _ => performMove(y, setY); + } + break; + } } yPositions.Add(currentY); - d.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); if (d.Item.Visible) currentY += d.DrawHeight + 5; @@ -393,8 +408,6 @@ namespace osu.Game.Screens.Select scrollableContent.Height = currentY; yPositionsCache.Validate(); - - return selectedY; } private void select(CarouselItem item) @@ -477,7 +490,7 @@ namespace osu.Game.Screens.Select // Makes sure headers are always _below_ items, // and depth flows downward. - item.Depth = i + (item is DrawableCarouselBeatmapSet ? Items.Count : 0); + item.Depth = i + (item is DrawableCarouselBeatmapSet ? -Items.Count : 0); switch (item.LoadState) { From d27047f94dc89cf7c33de049b6ca332ab85684a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 17 Dec 2017 02:56:29 +0900 Subject: [PATCH 33/53] Remove logging --- osu.Game/Screens/Select/Carousel/CarouselItem.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index 12125fe5e0..b7c5227414 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using osu.Framework.Configuration; -using osu.Framework.Logging; namespace osu.Game.Screens.Select.Carousel { @@ -57,8 +56,6 @@ namespace osu.Game.Screens.Select.Carousel State.ValueChanged += v => { - Logger.Log($"State of {this} changed to {v}"); - if (InternalChildren == null) return; switch (v) From c02ce16f47db9c7e60ad6d89e76818cdb4e6b5c4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 17 Dec 2017 03:04:57 +0900 Subject: [PATCH 34/53] Remove unnecessary capture --- osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs index ad91706154..ee4c958c13 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs @@ -36,8 +36,8 @@ namespace osu.Game.Screens.Select.Carousel protected DrawableCarouselItem(CarouselItem item) { Item = item; - Item.Filtered.ValueChanged += v => Schedule(ApplyState); - Item.State.ValueChanged += v => Schedule(ApplyState); + Item.Filtered.ValueChanged += _ => Schedule(ApplyState); + Item.State.ValueChanged += _ => Schedule(ApplyState); Height = MAX_HEIGHT; RelativeSizeAxes = Axes.X; From 5d7413f19c7a9f888d68fec6acc9086b43d967e8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 17 Dec 2017 04:30:56 +0900 Subject: [PATCH 35/53] Improve performance with large numbers of panels visible --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 ++-- .../Select/Carousel/CarouselGroupEagerSelect.cs | 12 ++++++++++++ osu.Game/Screens/Select/Carousel/CarouselItem.cs | 3 ++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index dac01f788c..459d364f73 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -483,11 +483,11 @@ namespace osu.Game.Screens.Select { DrawableCarouselItem item = Items[i]; + if (!item.Item.Visible) continue; + // Only add if we're not already part of the content. if (!scrollableContent.Contains(item)) { - if (!item.Item.Visible) continue; - // Makes sure headers are always _below_ items, // and depth flows downward. item.Depth = i + (item is DrawableCarouselBeatmapSet ? -Items.Count : 0); diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 2d79742cc8..65282542e6 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -27,9 +27,19 @@ namespace osu.Game.Screens.Select.Carousel private CarouselItem lastSelected; + /// + /// To avoid overhead during filter operations, we don't attempt any selections until after all + /// children have been filtered. This bool will be true during the base + /// operation. + /// + private bool filteringChildren; + public override void Filter(FilterCriteria criteria) { + filteringChildren = true; base.Filter(criteria); + filteringChildren = false; + attemptSelection(); } @@ -64,6 +74,8 @@ namespace osu.Game.Screens.Select.Carousel private void attemptSelection() { + if (filteringChildren) return; + // we only perform eager selection if we are a currently selected group. if (State != CarouselItemState.Selected) return; diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index b7c5227414..c289481d16 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -33,7 +33,8 @@ namespace osu.Game.Screens.Select.Carousel if (InternalChildren != null) foreach (var c in InternalChildren) - items.AddRange(c.Drawables); + // if (!c.Filtered) <- potential optimisation at the cost of no fade out animations. + items.AddRange(c.Drawables); return items; } From 54cc6fadf97be79ff4cd4b3a8bed752853f2068a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 17 Dec 2017 05:53:13 +0900 Subject: [PATCH 36/53] Greatly improve performance when many hidden panels are on-screen --- osu.Game/Screens/Select/BeatmapCarousel.cs | 75 ++++++++++--------- .../Carousel/CarouselGroupEagerSelect.cs | 1 + .../Screens/Select/Carousel/CarouselItem.cs | 7 +- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 459d364f73..63c8aee3f9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -44,7 +44,7 @@ namespace osu.Game.Screens.Select /// public BeatmapSetInfo SelectedBeatmapSet => selectedBeatmapSet?.BeatmapSet; - private CarouselBeatmapSet selectedBeatmapSet => beatmapSets.FirstOrDefault(s => s.State == CarouselItemState.Selected); + private CarouselBeatmapSet selectedBeatmapSet; /// /// Raised when the is changed. @@ -73,26 +73,14 @@ namespace osu.Game.Screens.Select Schedule(() => { root = newRoot; - updateItems(); + scrollableContent.Clear(false); + yPositionsCache.Invalidate(); BeatmapSetsChanged?.Invoke(); }); }); } } - /// - /// Call after altering in any way. - /// - private void updateItems() - { - scrollableContent.Clear(false); - Items = root.Drawables.ToList(); - yPositionsCache.Invalidate(); - - if (root.Children == null || root.Children.All(c => c.Filtered)) - SelectionChanged?.Invoke(null); - } - private readonly List yPositions = new List(); private Cached yPositionsCache = new Cached(); @@ -126,7 +114,7 @@ namespace osu.Game.Screens.Select return; root.RemoveChild(existingSet); - updateItems(); + yPositionsCache.Invalidate(); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) @@ -142,7 +130,7 @@ namespace osu.Game.Screens.Select if (newSet == null) { - updateItems(); + yPositionsCache.Invalidate(); SelectNext(); return; } @@ -155,7 +143,7 @@ namespace osu.Game.Screens.Select if (hadSelection) select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); - updateItems(); + yPositionsCache.Invalidate(); } public void SelectBeatmap(BeatmapInfo beatmap) @@ -200,8 +188,10 @@ namespace osu.Game.Screens.Select select(beatmap); return; case CarouselBeatmapSet set: - if (!skipDifficulties) continue; - select(set); + if (skipDifficulties) + select(set); + else + select(direction > 0 ? set.Beatmaps.First() : set.Beatmaps.Last()); return; } } @@ -209,10 +199,7 @@ namespace osu.Game.Screens.Select public void SelectNextRandom() { - if (!beatmapSets.Any()) - return; - - var visible = beatmapSets.Where(select => !select.Filtered).ToList(); + var visible = beatmapSets.Where(s => !s.Filtered).ToList(); if (!visible.Any()) return; @@ -291,7 +278,7 @@ namespace osu.Game.Screens.Select FilterTask = null; root.Filter(activeCriteria); - updateItems(); + yPositionsCache.Invalidate(); if (scroll) ScrollToSelected(false); } @@ -329,6 +316,7 @@ namespace osu.Game.Screens.Select { if (v == CarouselItemState.Selected) { + selectedBeatmapSet = set; SelectionChanged?.Invoke(c.Beatmap); yPositionsCache.Invalidate(); Schedule(() => ScrollToSelected()); @@ -410,6 +398,12 @@ namespace osu.Game.Screens.Select yPositionsCache.Validate(); } + private void noSelection() + { + if (root.Children == null || root.Children.All(c => c.Filtered)) + SelectionChanged?.Invoke(null); + } + private void select(CarouselItem item) { if (item == null) return; @@ -450,11 +444,18 @@ namespace osu.Game.Screens.Select { base.Update(); - float drawHeight = DrawHeight; - + // todo: scheduled scrolls... if (!yPositionsCache.IsValid) + { + Items = root.Drawables.ToList(); computeYPositions(); + if (selectedBeatmapSet != null && beatmapSets.All(s => s.Filtered)) + SelectionChanged?.Invoke(null); + } + + float drawHeight = DrawHeight; + // Remove all items that should no longer be on-screen scrollableContent.RemoveAll(delegate (DrawableCarouselItem p) { @@ -469,21 +470,21 @@ namespace osu.Game.Screens.Select int firstIndex = yPositions.BinarySearch(Current - DrawableCarouselItem.MAX_HEIGHT); if (firstIndex < 0) firstIndex = ~firstIndex; int lastIndex = yPositions.BinarySearch(Current + drawHeight); - if (lastIndex < 0) - { - lastIndex = ~lastIndex; + if (lastIndex < 0) lastIndex = ~lastIndex; - // Add the first item of the last visible beatmap group to preload its data. - if (lastIndex != 0 && Items[lastIndex - 1] is DrawableCarouselBeatmapSet) - lastIndex++; - } + int notVisibleCount = 0; // Add those items within the previously found index range that should be displayed. for (int i = firstIndex; i < lastIndex; ++i) { DrawableCarouselItem item = Items[i]; - if (!item.Item.Visible) continue; + if (!item.Item.Visible) + { + if (!item.IsPresent) + notVisibleCount++; + continue; + } // Only add if we're not already part of the content. if (!scrollableContent.Contains(item)) @@ -506,6 +507,10 @@ namespace osu.Game.Screens.Select } } + // this is not actually useful right now, but once we have groups may well be. + if (notVisibleCount > 50) + yPositionsCache.Invalidate(); + // Update externally controlled state of currently visible items // (e.g. x-offset and opacity). float halfHeight = drawHeight / 2; diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 65282542e6..1e6c809f2c 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -67,6 +67,7 @@ namespace osu.Game.Screens.Select.Carousel updateSelected(item); break; case CarouselItemState.NotSelected: + case CarouselItemState.Collapsed: attemptSelection(); break; } diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index c289481d16..586e959a74 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -29,12 +29,11 @@ namespace osu.Game.Screens.Select.Carousel List items = new List(); var self = drawableRepresentation.Value; - if (self != null) items.Add(self); + if (self?.IsPresent == true) items.Add(self); if (InternalChildren != null) foreach (var c in InternalChildren) - // if (!c.Filtered) <- potential optimisation at the cost of no fade out animations. - items.AddRange(c.Drawables); + items.AddRange(c.Drawables); return items; } @@ -48,7 +47,7 @@ namespace osu.Game.Screens.Select.Carousel // 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; + i.State.Value = CarouselItemState.Collapsed; } protected CarouselItem() From 19643ba5e675fe98e06e5328e409bb2a41525e4f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 02:23:03 +0900 Subject: [PATCH 37/53] Resolve scroll animation/position issues --- osu.Game/Screens/Select/BeatmapCarousel.cs | 62 +++++++++++++--------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63c8aee3f9..1991fb608c 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -74,7 +74,8 @@ namespace osu.Game.Screens.Select { root = newRoot; scrollableContent.Clear(false); - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); + scrollPositionCache.Invalidate(); BeatmapSetsChanged?.Invoke(); }); }); @@ -82,7 +83,8 @@ namespace osu.Game.Screens.Select } private readonly List yPositions = new List(); - private Cached yPositionsCache = new Cached(); + private Cached itemsCache = new Cached(); + private Cached scrollPositionCache = new Cached(); private readonly Container scrollableContent; @@ -114,7 +116,7 @@ namespace osu.Game.Screens.Select return; root.RemoveChild(existingSet); - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) @@ -130,7 +132,7 @@ namespace osu.Game.Screens.Select if (newSet == null) { - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); SelectNext(); return; } @@ -143,7 +145,7 @@ namespace osu.Game.Screens.Select if (hadSelection) select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); } public void SelectBeatmap(BeatmapInfo beatmap) @@ -278,9 +280,16 @@ namespace osu.Game.Screens.Select FilterTask = null; root.Filter(activeCriteria); - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); - if (scroll) ScrollToSelected(false); + if (scroll) + { + Schedule(() => + { + updateItems(false); + updateScrollPosition(false); + }); + } } FilterTask?.Cancel(); @@ -294,7 +303,7 @@ namespace osu.Game.Screens.Select private float? scrollTarget; - public void ScrollToSelected(bool animated = true) => Schedule(() => { if (scrollTarget != null) ScrollTo(scrollTarget.Value, animated); }); + public void ScrollToSelected() => scrollPositionCache.Invalidate(); private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) { @@ -318,8 +327,9 @@ namespace osu.Game.Screens.Select { selectedBeatmapSet = set; SelectionChanged?.Invoke(c.Beatmap); - yPositionsCache.Invalidate(); - Schedule(() => ScrollToSelected()); + + itemsCache.Invalidate(); + scrollPositionCache.Invalidate(); } }; } @@ -337,8 +347,10 @@ namespace osu.Game.Screens.Select /// Computes the target Y positions for every item in the carousel. /// /// The Y position of the currently selected item. - private void computeYPositions(bool animated = true) + private void updateItems(bool animated = true) { + Items = root.Drawables.ToList(); + yPositions.Clear(); float currentY = DrawHeight / 2; @@ -395,13 +407,19 @@ namespace osu.Game.Screens.Select currentY += DrawHeight / 2; scrollableContent.Height = currentY; - yPositionsCache.Validate(); + if (selectedBeatmapSet?.Filtered.Value == true) + { + selectedBeatmapSet = null; + SelectionChanged?.Invoke(null); + } + + itemsCache.Validate(); } - private void noSelection() + private void updateScrollPosition(bool animated = true) { - if (root.Children == null || root.Children.All(c => c.Filtered)) - SelectionChanged?.Invoke(null); + if (scrollTarget != null) ScrollTo(scrollTarget.Value, animated); + scrollPositionCache.Validate(); } private void select(CarouselItem item) @@ -444,15 +462,11 @@ namespace osu.Game.Screens.Select { base.Update(); - // todo: scheduled scrolls... - if (!yPositionsCache.IsValid) - { - Items = root.Drawables.ToList(); - computeYPositions(); + if (!itemsCache.IsValid) + updateItems(); - if (selectedBeatmapSet != null && beatmapSets.All(s => s.Filtered)) - SelectionChanged?.Invoke(null); - } + if (!scrollPositionCache.IsValid) + updateScrollPosition(); float drawHeight = DrawHeight; @@ -509,7 +523,7 @@ namespace osu.Game.Screens.Select // this is not actually useful right now, but once we have groups may well be. if (notVisibleCount > 50) - yPositionsCache.Invalidate(); + itemsCache.Invalidate(); // Update externally controlled state of currently visible items // (e.g. x-offset and opacity). From 942054a30ff7fc9120a9192c2c70f2b34592108b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 02:26:40 +0900 Subject: [PATCH 38/53] Re-fix null selection --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 1991fb608c..4ec62877bf 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -407,7 +407,7 @@ namespace osu.Game.Screens.Select currentY += DrawHeight / 2; scrollableContent.Height = currentY; - if (selectedBeatmapSet?.Filtered.Value == true) + if (selectedBeatmapSet != null && selectedBeatmapSet.State.Value != CarouselItemState.Selected) { selectedBeatmapSet = null; SelectionChanged?.Invoke(null); From 30a15729ec351e199b37654a5743f90af6cfd36f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 02:46:53 +0900 Subject: [PATCH 39/53] Fix event handling from outside carousel being scheduled at the wrong level Was causing BeatmapSet's Set to run *after* newer events were received. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 62 ++++++++++++---------- osu.Game/Screens/Select/SongSelect.cs | 30 ++--------- 2 files changed, 38 insertions(+), 54 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 4ec62877bf..c18f917645 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -110,42 +110,48 @@ namespace osu.Game.Screens.Select public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { - var existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); + Schedule(() => + { + var existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - if (existingSet == null) - return; + if (existingSet == null) + return; - root.RemoveChild(existingSet); - itemsCache.Invalidate(); + root.RemoveChild(existingSet); + itemsCache.Invalidate(); + }); } public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) { - CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - - bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; - - var newSet = createCarouselSet(beatmapSet); - - if (existingSet != null) - root.RemoveChild(existingSet); - - if (newSet == null) + Schedule(() => { + CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); + + bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; + + var newSet = createCarouselSet(beatmapSet); + + if (existingSet != null) + root.RemoveChild(existingSet); + + if (newSet == null) + { + itemsCache.Invalidate(); + SelectNext(); + return; + } + + root.AddChild(newSet); + + applyActiveCriteria(false, false); + + //check if we can/need to maintain our current selection. + if (hadSelection) + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); + itemsCache.Invalidate(); - SelectNext(); - return; - } - - root.AddChild(newSet); - - applyActiveCriteria(false, false); - - //check if we can/need to maintain our current selection. - if (hadSelection) - select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); - - itemsCache.Invalidate(); + }); } public void SelectBeatmap(BeatmapInfo beatmap) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 1fea4d0ccb..b5fba7d88d 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -417,32 +417,10 @@ namespace osu.Game.Screens.Select } } - private void onBeatmapSetAdded(BeatmapSetInfo s) => Schedule(() => carousel.UpdateBeatmapSet(s)); - - private void onBeatmapSetRemoved(BeatmapSetInfo s) => Schedule(() => - { - carousel.RemoveBeatmapSet(s); - if (carousel.SelectedBeatmap == null) - Beatmap.SetDefault(); - }); - - private void onBeatmapRestored(BeatmapInfo beatmap) - { - Schedule(() => - { - var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); - carousel.UpdateBeatmapSet(beatmapSet); - }); - } - - private void onBeatmapHidden(BeatmapInfo beatmap) - { - Schedule(() => - { - var beatmapSet = beatmaps.QueryBeatmapSet(s => s.ID == beatmap.BeatmapSetInfoID); - carousel.UpdateBeatmapSet(beatmapSet); - }); - } + private void onBeatmapSetAdded(BeatmapSetInfo s) => carousel.UpdateBeatmapSet(s); + private void onBeatmapSetRemoved(BeatmapSetInfo s) => carousel.RemoveBeatmapSet(s); + private void onBeatmapRestored(BeatmapInfo b) => carousel.UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); + private void onBeatmapHidden(BeatmapInfo b) => carousel.UpdateBeatmapSet(beatmaps.QueryBeatmapSet(s => s.ID == b.BeatmapSetInfoID)); private void carouselBeatmapsLoaded() { From 3759c39f00444da97d2848536d0a781b2822274d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 02:51:09 +0900 Subject: [PATCH 40/53] Update test case to handle scheduled removal --- osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs index 52db5b4883..5758e893a6 100644 --- a/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs @@ -268,8 +268,10 @@ namespace osu.Game.Tests.Visual AddUntilStep(() => { + if (!carousel.BeatmapSets.Any()) return true; + carousel.RemoveBeatmapSet(carousel.BeatmapSets.Last()); - return !carousel.BeatmapSets.Any(); + return false; }, "Remove all"); AddAssert("Selection is null", () => currentSelection == null); From 482941b33321f3fdf360c692e7a98e60790d4697 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 06:59:32 +0900 Subject: [PATCH 41/53] Preload drawables to force asynchronous construction --- osu.Game/Screens/Select/BeatmapCarousel.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index c18f917645..7d5a0c859a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -68,6 +68,9 @@ namespace osu.Game.Screens.Select { value.Select(createCarouselSet).Where(g => g != null).ForEach(newRoot.AddChild); newRoot.Filter(activeCriteria); + + // preload drawables as the ctor overhead is quite high currently. + var drawables = newRoot.Drawables; }).ContinueWith(t => { Schedule(() => From c10288541c796667d5da5b1a3e8481a0ec91441d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 07:58:34 +0900 Subject: [PATCH 42/53] Avoid redundant IndexOf calls --- osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 1e6c809f2c..5701760221 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -54,7 +54,7 @@ namespace osu.Game.Screens.Select.Carousel public override void AddChild(CarouselItem i) { base.AddChild(i); - updateSelectedIndex(); + attemptSelection(); } protected override void ChildItemStateChanged(CarouselItem item, CarouselItemState value) @@ -99,6 +99,6 @@ namespace osu.Game.Screens.Select.Carousel updateSelectedIndex(); } - private void updateSelectedIndex() => lastSelectedIndex = Math.Max(0, InternalChildren.IndexOf(lastSelected)); + private void updateSelectedIndex() => lastSelectedIndex = lastSelected == null ? 0 : Math.Max(0, InternalChildren.IndexOf(lastSelected)); } } From dfd7787b1586a65b6b668db6bf9ed69c3cbf9142 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 07:58:48 +0900 Subject: [PATCH 43/53] Move more overhead from ctor to BDL --- .../Select/Carousel/DrawableCarouselItem.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs index ee4c958c13..cb354b3602 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselItem.cs @@ -26,25 +26,28 @@ namespace osu.Game.Screens.Select.Carousel public readonly CarouselItem Item; - private readonly Container nestedContainer; - private readonly Container borderContainer; + private Container nestedContainer; + private Container borderContainer; - private readonly Box hoverLayer; + private Box hoverLayer; protected override Container Content => nestedContainer; protected DrawableCarouselItem(CarouselItem item) { Item = item; - Item.Filtered.ValueChanged += _ => Schedule(ApplyState); - Item.State.ValueChanged += _ => Schedule(ApplyState); Height = MAX_HEIGHT; RelativeSizeAxes = Axes.X; - Alpha = 0; + } - AddInternal(borderContainer = new Container + private SampleChannel sampleHover; + + [BackgroundDependencyLoader] + private void load(AudioManager audio, OsuColour colours) + { + InternalChild = borderContainer = new Container { RelativeSizeAxes = Axes.Both, Masking = true, @@ -63,14 +66,8 @@ namespace osu.Game.Screens.Select.Carousel Blending = BlendingMode.Additive, }, } - }); - } + }; - private SampleChannel sampleHover; - - [BackgroundDependencyLoader] - private void load(AudioManager audio, OsuColour colours) - { sampleHover = audio.Sample.Get($@"SongSelect/song-ping-variation-{RNG.Next(1, 5)}"); hoverLayer.Colour = colours.Blue.Opacity(0.1f); } @@ -94,7 +91,10 @@ namespace osu.Game.Screens.Select.Carousel protected override void LoadComplete() { base.LoadComplete(); + ApplyState(); + Item.Filtered.ValueChanged += _ => Schedule(ApplyState); + Item.State.ValueChanged += _ => Schedule(ApplyState); } protected virtual void ApplyState() From 954bc77a71049ba8062e3f7cdc7e332898afdd3a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 08:05:57 +0900 Subject: [PATCH 44/53] Indicate unused variable --- osu.Game/Screens/Select/BeatmapCarousel.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 7d5a0c859a..21e12c48b9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -70,18 +70,15 @@ namespace osu.Game.Screens.Select newRoot.Filter(activeCriteria); // preload drawables as the ctor overhead is quite high currently. - var drawables = newRoot.Drawables; - }).ContinueWith(t => + var _ = newRoot.Drawables; + }).ContinueWith(_ => Schedule(() => { - Schedule(() => - { - root = newRoot; - scrollableContent.Clear(false); - itemsCache.Invalidate(); - scrollPositionCache.Invalidate(); - BeatmapSetsChanged?.Invoke(); - }); - }); + root = newRoot; + scrollableContent.Clear(false); + itemsCache.Invalidate(); + scrollPositionCache.Invalidate(); + BeatmapSetsChanged?.Invoke(); + })); } } From d6ba53acb3697d8f1d63949c9d5085373d61c62f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 09:47:53 +0900 Subject: [PATCH 45/53] Update framework --- osu-framework | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu-framework b/osu-framework index fc6de01ad6..5da6990a8e 160000 --- a/osu-framework +++ b/osu-framework @@ -1 +1 @@ -Subproject commit fc6de01ad6045544991bf278316c9eed8ea01ef1 +Subproject commit 5da6990a8e68dea852495950996e1362a293dbd5 From b21c22085d899a7a2574639e5954b77b51c3147b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:13:16 +0900 Subject: [PATCH 46/53] Make more things private --- .../Carousel/DrawableCarouselBeatmap.cs | 20 +++++++++---------- .../Carousel/DrawableCarouselBeatmapSet.cs | 12 +++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs index 92768cd88f..6c0cc341fd 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmap.cs @@ -28,9 +28,9 @@ namespace osu.Game.Screens.Select.Carousel private Sprite background; - public Action StartRequested; - public Action EditRequested; - public Action HideRequested; + private Action startRequested; + private Action editRequested; + private Action hideRequested; private Triangles triangles; private StarCounter starCounter; @@ -46,12 +46,12 @@ namespace osu.Game.Screens.Select.Carousel { if (songSelect != null) { - StartRequested = songSelect.Start; - EditRequested = songSelect.Edit; + startRequested = songSelect.Start; + editRequested = songSelect.Edit; } if (manager != null) - HideRequested = manager.Hide; + hideRequested = manager.Hide; Children = new Drawable[] { @@ -153,7 +153,7 @@ namespace osu.Game.Screens.Select.Carousel protected override bool OnClick(InputState state) { if (Item.State == CarouselItemState.Selected) - StartRequested?.Invoke(beatmap); + startRequested?.Invoke(beatmap); return base.OnClick(state); } @@ -168,9 +168,9 @@ namespace osu.Game.Screens.Select.Carousel public MenuItem[] ContextMenuItems => new MenuItem[] { - new OsuMenuItem("Play", MenuItemType.Highlighted, () => StartRequested?.Invoke(beatmap)), - new OsuMenuItem("Edit", MenuItemType.Standard, () => EditRequested?.Invoke(beatmap)), - new OsuMenuItem("Hide", MenuItemType.Destructive, () => HideRequested?.Invoke(beatmap)), + new OsuMenuItem("Play", MenuItemType.Highlighted, () => startRequested?.Invoke(beatmap)), + new OsuMenuItem("Edit", MenuItemType.Standard, () => editRequested?.Invoke(beatmap)), + new OsuMenuItem("Hide", MenuItemType.Destructive, () => hideRequested?.Invoke(beatmap)), }; } } diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs index b5636ae9ed..8abb93950f 100644 --- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs @@ -24,8 +24,8 @@ namespace osu.Game.Screens.Select.Carousel { public class DrawableCarouselBeatmapSet : DrawableCarouselItem, IHasContextMenu { - public Action DeleteRequested; - public Action RestoreHiddenRequested; + private Action deleteRequested; + private Action restoreHiddenRequested; private readonly BeatmapSetInfo beatmapSet; @@ -43,8 +43,8 @@ namespace osu.Game.Screens.Select.Carousel if (localisation == null) throw new ArgumentNullException(nameof(localisation)); - RestoreHiddenRequested = s => s.Beatmaps.ForEach(manager.Restore); - DeleteRequested = manager.Delete; + restoreHiddenRequested = s => s.Beatmaps.ForEach(manager.Restore); + deleteRequested = manager.Delete; Children = new Drawable[] { @@ -97,9 +97,9 @@ namespace osu.Game.Screens.Select.Carousel items.Add(new OsuMenuItem("Expand", MenuItemType.Highlighted, () => Item.State.Value = CarouselItemState.Selected)); if (beatmapSet.Beatmaps.Any(b => b.Hidden)) - items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => RestoreHiddenRequested?.Invoke(beatmapSet))); + items.Add(new OsuMenuItem("Restore all hidden", MenuItemType.Standard, () => restoreHiddenRequested?.Invoke(beatmapSet))); - items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => DeleteRequested?.Invoke(beatmapSet))); + items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => deleteRequested?.Invoke(beatmapSet))); return items.ToArray(); } From 5bfb6d1f58ef072f39bc65b0c4586095fe74e857 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:13:51 +0900 Subject: [PATCH 47/53] Remove unused variable --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 21e12c48b9..48bfe01d51 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -60,8 +60,6 @@ namespace osu.Game.Screens.Select get { return beatmapSets.Select(g => g.BeatmapSet); } set { - List newSets = null; - CarouselGroup newRoot = new CarouselGroupEagerSelect(); Task.Run(() => From 4e46565f6e24082c7739f71aba1033584d25e4b6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:25:02 +0900 Subject: [PATCH 48/53] Remove todo --- osu.Game/Screens/Select/Carousel/CarouselGroup.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 98aa7cd5c5..b8d7ddaf3e 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -27,8 +27,6 @@ namespace osu.Game.Screens.Select.Carousel protected virtual void ChildItemStateChanged(CarouselItem item, CarouselItemState value) { - // todo: check state of selected item. - // ensure we are the only item selected if (value == CarouselItemState.Selected) { From 4c1f00567bd25e3506bb5dba1c4031a24e5c0845 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:31:38 +0900 Subject: [PATCH 49/53] Fix incorrect flush logic when starting play from non-selected difficulty using context menu --- osu.Game/Screens/Select/SongSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index b5fba7d88d..4d5101447a 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -214,6 +214,8 @@ namespace osu.Game.Screens.Select // it could change selection (ie. if the ruleset has been changed). carousel.FlushPendingFilterOperations(); + carousel.SelectBeatmap(beatmap); + if (selectionChangedDebounce?.Completed == false) { selectionChangedDebounce.RunTask(); @@ -221,8 +223,6 @@ namespace osu.Game.Screens.Select selectionChangedDebounce = null; } - carousel.SelectBeatmap(beatmap); - Start(); } From 7173829896539cba3843cefabed90ffabfc2f82a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:43:10 +0900 Subject: [PATCH 50/53] Add filter checks to difficulty selection --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 48bfe01d51..e5f3d93a7d 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -197,7 +197,7 @@ namespace osu.Game.Screens.Select if (skipDifficulties) select(set); else - select(direction > 0 ? set.Beatmaps.First() : set.Beatmaps.Last()); + select(direction > 0 ? set.Beatmaps.First(b => !b.Filtered) : set.Beatmaps.Last(b => !b.Filtered)); return; } } From b2cd32eb950d208183924c0f5ace1b4e07a80d78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 11:57:13 +0900 Subject: [PATCH 51/53] Move children to CarouselGroup --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 +- .../Screens/Select/Carousel/CarouselGroup.cs | 52 ++++++++++++++++++- .../Screens/Select/Carousel/CarouselItem.cs | 44 +--------------- 3 files changed, 54 insertions(+), 46 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e5f3d93a7d..c090ea6f0a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -53,7 +53,7 @@ namespace osu.Game.Screens.Select public override bool HandleInput => AllowSelection; - private IEnumerable beatmapSets => root.Children?.OfType() ?? new CarouselBeatmapSet[] { }; + private IEnumerable beatmapSets => root.Children.OfType(); public IEnumerable BeatmapSets { @@ -277,7 +277,7 @@ namespace osu.Game.Screens.Select private void applyActiveCriteria(bool debounce, bool scroll) { - if (root.Children?.Any() != true) return; + if (root.Children.Any() != true) return; void perform() { diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index b8d7ddaf3e..a54eeb562e 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -14,15 +14,63 @@ namespace osu.Game.Screens.Select.Carousel protected override DrawableCarouselItem CreateDrawableRepresentation() => null; - public override void AddChild(CarouselItem i) + public IReadOnlyList Children => InternalChildren; + + protected List InternalChildren = new List(); + + public override List Drawables + { + get + { + var drawables = base.Drawables; + foreach (var c in InternalChildren) + drawables.AddRange(c.Drawables); + return drawables; + } + } + + 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. + i.State.Value = CarouselItemState.Collapsed; + } + + public virtual void AddChild(CarouselItem i) { i.State.ValueChanged += v => ChildItemStateChanged(i, v); - base.AddChild(i); + InternalChildren.Add(i); } public CarouselGroup(List items = null) { if (items != null) InternalChildren = items; + + State.ValueChanged += v => + { + switch (v) + { + case CarouselItemState.Collapsed: + case CarouselItemState.NotSelected: + InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Collapsed); + break; + case CarouselItemState.Selected: + InternalChildren.ForEach(c => + { + if (c.State == CarouselItemState.Collapsed) c.State.Value = CarouselItemState.NotSelected; + }); + break; + } + }; + } + + public override void Filter(FilterCriteria criteria) + { + base.Filter(criteria); + InternalChildren.Sort((x, y) => x.CompareTo(criteria, y)); + InternalChildren.ForEach(c => c.Filter(criteria)); } protected virtual void ChildItemStateChanged(CarouselItem item, CarouselItemState value) diff --git a/osu.Game/Screens/Select/Carousel/CarouselItem.cs b/osu.Game/Screens/Select/Carousel/CarouselItem.cs index 586e959a74..7d76aee253 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselItem.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselItem.cs @@ -13,66 +13,28 @@ namespace osu.Game.Screens.Select.Carousel public readonly Bindable State = new Bindable(CarouselItemState.NotSelected); - public IReadOnlyList Children => InternalChildren; - - protected List InternalChildren { get; set; } - /// /// This item is not in a hidden state. /// public bool Visible => State.Value != CarouselItemState.Collapsed && !Filtered; - public IEnumerable Drawables + public virtual List Drawables { get { - List items = new List(); + var items = new List(); var self = drawableRepresentation.Value; if (self?.IsPresent == true) items.Add(self); - if (InternalChildren != null) - foreach (var c in InternalChildren) - items.AddRange(c.Drawables); - return items; } } - public virtual void AddChild(CarouselItem i) => (InternalChildren ?? (InternalChildren = new List())).Add(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. - i.State.Value = CarouselItemState.Collapsed; - } - protected CarouselItem() { drawableRepresentation = new Lazy(CreateDrawableRepresentation); - State.ValueChanged += v => - { - if (InternalChildren == null) return; - - switch (v) - { - case CarouselItemState.Collapsed: - case CarouselItemState.NotSelected: - InternalChildren.ForEach(c => c.State.Value = CarouselItemState.Collapsed); - break; - case CarouselItemState.Selected: - InternalChildren.ForEach(c => - { - if (c.State == CarouselItemState.Collapsed) c.State.Value = CarouselItemState.NotSelected; - }); - break; - } - }; - Filtered.ValueChanged += v => { if (v && State == CarouselItemState.Selected) @@ -86,8 +48,6 @@ namespace osu.Game.Screens.Select.Carousel public virtual void Filter(FilterCriteria criteria) { - InternalChildren?.Sort((x, y) => x.CompareTo(criteria, y)); - InternalChildren?.ForEach(c => c.Filter(criteria)); } public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => GetHashCode().CompareTo(other.GetHashCode()); From 23e014b52dd7837e397b4625d066604fdfb8e168 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 12:05:49 +0900 Subject: [PATCH 52/53] Simplify drawable removal logic --- osu.Game/Screens/Select/BeatmapCarousel.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index c090ea6f0a..6dc7dd8922 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -475,12 +475,7 @@ namespace osu.Game.Screens.Select float drawHeight = DrawHeight; // Remove all items that should no longer be on-screen - scrollableContent.RemoveAll(delegate (DrawableCarouselItem p) - { - float itemPosY = p.Position.Y; - bool remove = itemPosY < Current - p.DrawHeight || itemPosY > Current + drawHeight || !p.IsPresent; - return remove; - }); + scrollableContent.RemoveAll(p => p.Y < Current - p.DrawHeight || p.Y > Current + drawHeight || !p.IsPresent); // Find index range of all items that should be on-screen Trace.Assert(Items.Count == yPositions.Count); From 6121cd3b67f31d0375d32725880a922395e1a112 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2017 12:30:39 +0900 Subject: [PATCH 53/53] Remove animating skipping and reorder file a bit --- osu.Game/Screens/Select/BeatmapCarousel.cs | 257 ++++++++++----------- 1 file changed, 125 insertions(+), 132 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 6dc7dd8922..ff1dd95eac 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -106,6 +106,12 @@ namespace osu.Game.Screens.Select }; } + [BackgroundDependencyLoader(permitNulls: true)] + private void load(OsuConfigManager config) + { + config.BindWith(OsuSetting.RandomSelectAlgorithm, RandomAlgorithm); + } + public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) { Schedule(() => @@ -255,6 +261,12 @@ namespace osu.Game.Screens.Select } } + private void select(CarouselItem item) + { + if (item == null) return; + item.State.Value = CarouselItemState.Selected; + } + private FilterCriteria activeCriteria = new FilterCriteria(); protected ScheduledDelegate FilterTask; @@ -285,15 +297,7 @@ namespace osu.Game.Screens.Select root.Filter(activeCriteria); itemsCache.Invalidate(); - - if (scroll) - { - Schedule(() => - { - updateItems(false); - updateScrollPosition(false); - }); - } + if (scroll) scrollPositionCache.Invalidate(); } FilterTask?.Cancel(); @@ -309,129 +313,6 @@ namespace osu.Game.Screens.Select public void ScrollToSelected() => scrollPositionCache.Invalidate(); - private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) - { - if (beatmapSet.Beatmaps.All(b => b.Hidden)) - return null; - - // todo: remove the need for this. - foreach (var b in beatmapSet.Beatmaps) - { - if (b.Metadata == null) - b.Metadata = beatmapSet.Metadata; - } - - var set = new CarouselBeatmapSet(beatmapSet); - - foreach (var c in set.Beatmaps) - { - c.State.ValueChanged += v => - { - if (v == CarouselItemState.Selected) - { - selectedBeatmapSet = set; - SelectionChanged?.Invoke(c.Beatmap); - - itemsCache.Invalidate(); - scrollPositionCache.Invalidate(); - } - }; - } - - return set; - } - - [BackgroundDependencyLoader(permitNulls: true)] - private void load(OsuConfigManager config) - { - config.BindWith(OsuSetting.RandomSelectAlgorithm, RandomAlgorithm); - } - - /// - /// Computes the target Y positions for every item in the carousel. - /// - /// The Y position of the currently selected item. - private void updateItems(bool animated = true) - { - Items = root.Drawables.ToList(); - - yPositions.Clear(); - - float currentY = DrawHeight / 2; - DrawableCarouselBeatmapSet lastSet = null; - - scrollTarget = null; - - foreach (DrawableCarouselItem d in Items) - { - if (d.IsPresent) - { - switch (d) - { - case DrawableCarouselBeatmapSet set: - lastSet = set; - - set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); - set.MoveToY(currentY, animated ? 750 : 0, Easing.OutExpo); - break; - case DrawableCarouselBeatmap beatmap: - if (beatmap.Item.State.Value == CarouselItemState.Selected) - scrollTarget = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; - - void performMove(float y, float? startY = null) - { - if (startY != null) beatmap.MoveTo(new Vector2(0, startY.Value)); - beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); - beatmap.MoveToY(y, animated ? 750 : 0, Easing.OutExpo); - } - - Debug.Assert(lastSet != null); - - float? setY = null; - if (!d.IsLoaded || beatmap.Alpha == 0) // can't use IsPresent due to DrawableCarouselItem override. - setY = lastSet.Y + lastSet.DrawHeight + 5; - - if (d.IsLoaded) - performMove(currentY, setY); - else - { - float y = currentY; - d.OnLoadComplete = _ => performMove(y, setY); - } - break; - } - } - - yPositions.Add(currentY); - - if (d.Item.Visible) - currentY += d.DrawHeight + 5; - } - - currentY += DrawHeight / 2; - scrollableContent.Height = currentY; - - if (selectedBeatmapSet != null && selectedBeatmapSet.State.Value != CarouselItemState.Selected) - { - selectedBeatmapSet = null; - SelectionChanged?.Invoke(null); - } - - itemsCache.Validate(); - } - - private void updateScrollPosition(bool animated = true) - { - if (scrollTarget != null) ScrollTo(scrollTarget.Value, animated); - scrollPositionCache.Validate(); - } - - private void select(CarouselItem item) - { - if (item == null) return; - item.State.Value = CarouselItemState.Selected; - } - protected override bool OnKeyDown(InputState state, KeyDownEventArgs args) { int direction = 0; @@ -531,6 +412,118 @@ namespace osu.Game.Screens.Select updateItem(p, halfHeight); } + private CarouselBeatmapSet createCarouselSet(BeatmapSetInfo beatmapSet) + { + if (beatmapSet.Beatmaps.All(b => b.Hidden)) + return null; + + // todo: remove the need for this. + foreach (var b in beatmapSet.Beatmaps) + { + if (b.Metadata == null) + b.Metadata = beatmapSet.Metadata; + } + + var set = new CarouselBeatmapSet(beatmapSet); + + foreach (var c in set.Beatmaps) + { + c.State.ValueChanged += v => + { + if (v == CarouselItemState.Selected) + { + selectedBeatmapSet = set; + SelectionChanged?.Invoke(c.Beatmap); + + itemsCache.Invalidate(); + scrollPositionCache.Invalidate(); + } + }; + } + + return set; + } + + /// + /// Computes the target Y positions for every item in the carousel. + /// + /// The Y position of the currently selected item. + private void updateItems() + { + Items = root.Drawables.ToList(); + + yPositions.Clear(); + + float currentY = DrawHeight / 2; + DrawableCarouselBeatmapSet lastSet = null; + + scrollTarget = null; + + foreach (DrawableCarouselItem d in Items) + { + if (d.IsPresent) + { + switch (d) + { + case DrawableCarouselBeatmapSet set: + lastSet = set; + + set.MoveToX(set.Item.State == CarouselItemState.Selected ? -100 : 0, 500, Easing.OutExpo); + set.MoveToY(currentY, 750, Easing.OutExpo); + break; + case DrawableCarouselBeatmap beatmap: + if (beatmap.Item.State.Value == CarouselItemState.Selected) + scrollTarget = currentY + beatmap.DrawHeight / 2 - DrawHeight / 2; + + void performMove(float y, float? startY = null) + { + if (startY != null) beatmap.MoveTo(new Vector2(0, startY.Value)); + beatmap.MoveToX(beatmap.Item.State == CarouselItemState.Selected ? -50 : 0, 500, Easing.OutExpo); + beatmap.MoveToY(y, 750, Easing.OutExpo); + } + + Debug.Assert(lastSet != null); + + float? setY = null; + if (!d.IsLoaded || beatmap.Alpha == 0) // can't use IsPresent due to DrawableCarouselItem override. + setY = lastSet.Y + lastSet.DrawHeight + 5; + + if (d.IsLoaded) + performMove(currentY, setY); + else + { + float y = currentY; + d.OnLoadComplete = _ => performMove(y, setY); + } + + break; + } + } + + yPositions.Add(currentY); + + if (d.Item.Visible) + currentY += d.DrawHeight + 5; + } + + currentY += DrawHeight / 2; + scrollableContent.Height = currentY; + + if (selectedBeatmapSet != null && selectedBeatmapSet.State.Value != CarouselItemState.Selected) + { + selectedBeatmapSet = null; + SelectionChanged?.Invoke(null); + } + + itemsCache.Validate(); + } + + private void updateScrollPosition() + { + if (scrollTarget != null) ScrollTo(scrollTarget.Value); + scrollPositionCache.Validate(); + } + /// /// Computes the x-offset of currently visible items. Makes the carousel appear round. ///