Cleanup and basic TestCase

osu.Game.Tests/Visual/TestCaseBeatmapCarousel.cs

@@ -0,0 +1,132 @@
+// Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
+// 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<Type> 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<BeatmapInfo>(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,
+                        }
+                    },
+                }),
+            };
+        }
+    }
+}

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<Type> 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);

osu.Game.Tests/osu.Game.Tests.csproj

@@ -88,6 +88,7 @@
     <Compile Include="Beatmaps\IO\ImportBeatmapTest.cs" />
     <Compile Include="Resources\Resource.cs" />
     <Compile Include="Beatmaps\Formats\LegacyBeatmapDecoderTest.cs" />
+    <Compile Include="Visual\TestCaseBeatmapCarousel.cs" />
     <Compile Include="Visual\TestCaseBeatmapDetailArea.cs" />
     <Compile Include="Visual\TestCaseBeatmapDetails.cs" />
     <Compile Include="Visual\TestCaseBeatmapInfoWedge.cs" />

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -26,20 +26,35 @@ namespace osu.Game.Screens.Select
 {
     public class BeatmapCarousel : OsuScrollContainer
     {
+        /// <summary>
+        /// Triggered when the <see cref="Beatmaps"/> loaded change and are completely loaded.
+        /// </summary>
+        public Action BeatmapsChanged;
+
+        /// <summary>
+        /// The currently selected beatmap.
+        /// </summary>
         public BeatmapInfo SelectedBeatmap => selectedBeatmap?.Beatmap;
 
-        public override bool HandleInput => AllowSelection;
+        /// <summary>
+        /// Raised when the <see cref="SelectedBeatmap"/> is changed.
+        /// </summary>
+        public Action<BeatmapInfo> SelectionChanged;
 
-        public Action BeatmapsChanged;
+        public override bool HandleInput => AllowSelection;
 
         public IEnumerable<BeatmapSetInfo> Beatmaps
         {
             get { return carouselSets.Select(g => g.BeatmapSet); }
             set
             {
-                scrollableContent.Clear(false);
+                Schedule(() =>
-                items.Clear();
+                {
-                carouselSets.Clear();
+                    scrollableContent.Clear(false);
+                    items.Clear();
+                    carouselSets.Clear();
+                    yPositionsCache.Invalidate();
+                });
 
                 List<CarouselBeatmapSet> newSets = null;
 
@@ -51,8 +66,7 @@ namespace osu.Game.Screens.Select
                 {
                     Schedule(() =>
                     {
-                        foreach (var g in newSets)
+                        carouselSets.AddRange(newSets);
-                            addBeatmapSet(g);
 
                         root = new CarouselGroup(newSets.OfType<CarouselItem>().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
+                //else
-                    addBeatmapSet(newSet);
+                //    addBeatmapSet(newSet);
             }
 
             if (hadSelection && newSet == null)
@@ -158,8 +172,6 @@ namespace osu.Game.Screens.Select
             }
         }
 
-        public Action<BeatmapInfo> 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<RandomSelectAlgorithm>(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;

osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs

@@ -52,11 +52,4 @@ namespace osu.Game.Screens.Select.Carousel
             }*/
         }
     }
-
-    public enum CarouselItemState
-    {
-        Hidden,
-        NotSelected,
-        Selected,
-    }
 }

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,
+    }
 }

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;
+            if (songSelect != null)
-            EditRequested = songSelect.Edit;
+            {
-            HideRequested = manager.Hide;
+                StartRequested = songSelect.Start;
+                EditRequested = songSelect.Edit;
+            }
+
+            if (manager != null)
+                HideRequested = manager.Hide;
         }
 
         public DrawableCarouselBeatmap(CarouselBeatmap panel)

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),

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,

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();
         }