From b4b2f12116ba9a7c5038661f50a7c9e23d6ac6b5 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 14 Dec 2017 16:59:08 +0900
Subject: [PATCH] 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<BeatmapInfo>(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<CarouselItem>().ToList());
-                        Items = root.Drawables.Value.ToList();
-
-                        yPositionsCache.Invalidate();
-                        BeatmapSetsChanged?.Invoke();
+                        updateItems();
                     });
                 });
             }
         }
 
+        /// <summary>
+        /// Call after altering <see cref="BeatmapSets"/> in any way.
+        /// </summary>
+        private void updateItems()
+        {
+            scrollableContent.Clear(false);
+
+            root = new CarouselGroup(beatmapSets.OfType<CarouselItem>().ToList());
+            Items = root.Drawables.Value.ToList();
+
+            yPositionsCache.Invalidate();
+            BeatmapSetsChanged?.Invoke();
+        }
+
         private readonly List<float> yPositions = new List<float>();
         private Cached yPositionsCache = new Cached();
 
         private readonly Container<DrawableCarouselItem> scrollableContent;
 
-
-
         public Bindable<RandomSelectAlgorithm> RandomAlgorithm = new Bindable<RandomSelectAlgorithm>();
         private readonly List<CarouselBeatmapSet> previouslyVisitedRandomSets = new List<CarouselBeatmapSet>();
+        private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
 
         protected List<DrawableCarouselItem> Items = new List<DrawableCarouselItem>();
         private CarouselGroup root = new CarouselGroup();
 
-        private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
-
         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();
-        }
-
         /// <summary>
         /// Computes the target Y positions for every item in the carousel.
         /// </summary>
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