Merge pull request #434 from Tom94/song-select-refactorr

Song select refactor

osu-framework

@@ -1 +1 @@
-Subproject commit 798409058a421307b5a92aeea4cd60a065f5a0d4
+Subproject commit 854977e3fa0c41eec7637641ec5fec9ee65d73b9

osu.Game/Beatmaps/Drawables/Panel.cs

@@ -18,10 +18,6 @@ namespace osu.Game.Beatmaps.Drawables
 
         public override bool RemoveWhenNotAlive => false;
 
-        public bool IsOnScreen;
-
-        public override bool IsAlive => IsOnScreen && base.IsAlive;
-
         private Container nestedContainer;
 
         protected override Container<Drawable> Content => nestedContainer;

osu.Game/Screens/Select/CarouselContainer.cs

@@ -16,6 +16,7 @@ using osu.Framework.Input;
 using OpenTK.Input;
 using System.Collections;
 using osu.Framework.MathUtils;
+using System.Diagnostics;
 
 namespace osu.Game.Screens.Select
 {
@@ -23,73 +24,33 @@ namespace osu.Game.Screens.Select
     {
         private Container<Panel> scrollableContent;
         private List<BeatmapGroup> groups = new List<BeatmapGroup>();
+        private List<Panel> panels = new List<Panel>();
 
         public BeatmapGroup SelectedGroup { get; private set; }
         public BeatmapPanel SelectedPanel { get; private set; }
 
         private List<float> yPositions = new List<float>();
-        private CarouselLifetimeList<Panel> lifetime;
 
         public CarouselContainer()
         {
             DistanceDecayJump = 0.01;
 
-            Add(scrollableContent = new Container<Panel>(lifetime = new CarouselLifetimeList<Panel>(DepthComparer))
+            Add(scrollableContent = new Container<Panel>
             {
                 RelativeSizeAxes = Axes.X,
             });
         }
 
-        internal class CarouselLifetimeList<T> : LifetimeList<Panel>
-        {
-            public CarouselLifetimeList(IComparer<Panel> comparer)
-                : base(comparer)
-            {
-            }
-
-            public int StartIndex;
-            public int EndIndex;
-
-            public override bool Update(FrameTimeInfo time)
-            {
-                bool anyAliveChanged = false;
-
-                //check existing items to make sure they haven't died.
-                foreach (var item in AliveItems.ToArray())
-                {
-                    item.UpdateTime(time);
-                    if (!item.IsAlive)
-                    {
-                        //todo: make this more efficient
-                        int i = IndexOf(item);
-                        anyAliveChanged |= CheckItem(item, ref i);
-                    }
-                }
-
-                //handle custom range
-                for (int i = StartIndex; i < EndIndex; i++)
-                {
-                    var item = this[i];
-                    item.UpdateTime(time);
-                    anyAliveChanged |= CheckItem(item, ref i);
-                }
-
-                return anyAliveChanged;
-            }
-        }
-
         public void AddGroup(BeatmapGroup group)
         {
-            group.State = BeatmapGroupState.Collapsed;
             groups.Add(group);
 
-            group.Header.Depth = -scrollableContent.Children.Count();
+            panels.Add(group.Header);
-            scrollableContent.Add(group.Header);
+            group.Header.UpdateClock(Clock);
-
             foreach (BeatmapPanel panel in group.BeatmapPanels)
             {
-                panel.Depth = -scrollableContent.Children.Count();
+                panels.Add(panel);
-                scrollableContent.Add(panel);
+                panel.UpdateClock(Clock);
             }
 
             computeYPositions();
@@ -98,6 +59,9 @@ namespace osu.Game.Screens.Select
         public void RemoveGroup(BeatmapGroup group)
         {
             groups.Remove(group);
+            foreach (var p in group.BeatmapPanels)
+                panels.Remove(p);
+
             scrollableContent.Remove(group.Header);
             scrollableContent.Remove(group.BeatmapPanels);
 
@@ -107,7 +71,7 @@ namespace osu.Game.Screens.Select
         private void movePanel(Panel panel, bool advance, bool animated, ref float currentY)
         {
             yPositions.Add(currentY);
-            panel.MoveToY(currentY, animated && (panel.IsOnScreen || panel.State != PanelSelectedState.Hidden) ? 750 : 0, EasingTypes.OutExpo);
+            panel.MoveToY(currentY, animated ? 750 : 0, EasingTypes.OutExpo);
 
             if (advance)
                 currentY += panel.DrawHeight + 5;
@@ -172,14 +136,16 @@ namespace osu.Game.Screens.Select
                 var panel = group.BeatmapPanels.FirstOrDefault(p => p.Beatmap.Equals(beatmap));
                 if (panel != null)
                 {
-                    SelectGroup(group, panel, animated);
+                    selectGroup(group, panel, animated);
                     return;
                 }
             }
         }
 
-        public void SelectGroup(BeatmapGroup group, BeatmapPanel panel, bool animated = true)
+        private void selectGroup(BeatmapGroup group, BeatmapPanel panel, bool animated = true)
         {
+            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;
 
@@ -194,19 +160,20 @@ namespace osu.Game.Screens.Select
 
         public void Sort(FilterControl.SortMode mode)
         {
+            List<BeatmapGroup> sortedGroups = new List<BeatmapGroup>(groups);
             switch (mode)
             {
                 case FilterControl.SortMode.Artist:
-                    groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Artist, y.BeatmapSet.Metadata.Artist));
+                    sortedGroups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Artist, y.BeatmapSet.Metadata.Artist));
                     break;
                 case FilterControl.SortMode.Title:
-                    groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Title, y.BeatmapSet.Metadata.Title));
+                    sortedGroups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Title, y.BeatmapSet.Metadata.Title));
                     break;
                 case FilterControl.SortMode.Author:
-                    groups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Author, y.BeatmapSet.Metadata.Author));
+                    sortedGroups.Sort((x, y) => string.Compare(x.BeatmapSet.Metadata.Author, y.BeatmapSet.Metadata.Author));
                     break;
                 case FilterControl.SortMode.Difficulty:
-                    groups.Sort((x, y) =>
+                    sortedGroups.Sort((x, y) =>
                     {
                         float xAverage = 0, yAverage = 0;
                         int counter = 0;
@@ -232,22 +199,23 @@ namespace osu.Game.Screens.Select
                 default:
                     throw new NotImplementedException();
             }
+
             scrollableContent.Clear(false);
-            lifetime.Clear();
+            panels.Clear();
-            foreach (BeatmapGroup group in groups)
+            groups.Clear();
-            {
-                group.Header.Depth = -scrollableContent.Children.Count();
-                scrollableContent.Add(group.Header);
-
-                foreach (BeatmapPanel panel in group.BeatmapPanels)
-                {
-                    panel.Depth = -scrollableContent.Children.Count();
-                    scrollableContent.Add(panel);
-                }
-            }
 
+            foreach (BeatmapGroup group in sortedGroups)
+                AddGroup(group);
         }
 
+        /// <summary>
+        /// Computes the x-offset of currently visible panels. Makes the carousel appear round.
+        /// </summary>
+        /// <param name="dist">
+        /// Vertical distance from the center of the carousel container
+        /// ranging from -1 to 1.
+        /// </param>
+        /// <param name="halfHeight">Half the height of the carousel container.</param>
         private static float offsetX(float dist, float halfHeight)
         {
             // The radius of the circle the carousel moves on.
@@ -286,34 +254,46 @@ namespace osu.Game.Screens.Select
         {
             base.Update();
 
-            // Determine which items stopped being on screen for future removal from the lifetimelist.
             float drawHeight = DrawHeight;
-            float halfHeight = drawHeight / 2;
 
-            foreach (Panel p in lifetime.AliveItems)
+            // Remove all panels that should no longer be on-screen
+            scrollableContent.RemoveAll(delegate (Panel p)
             {
                 float panelPosY = p.Position.Y;
-                p.IsOnScreen = panelPosY >= Current - p.DrawHeight && panelPosY <= Current + drawHeight;
+                bool remove = panelPosY < Current - p.DrawHeight || panelPosY > Current + drawHeight || !p.IsPresent;
-                updatePanel(p, halfHeight);
+                return remove;
-            }
+            });
+
+            // Find index range of all panels that should be on-screen
+            Trace.Assert(panels.Count == yPositions.Count);
 
-            // Determine range of indices for items that are now definitely on screen to be added
-            // to the lifetimelist in the future.
             int firstIndex = yPositions.BinarySearch(Current - Panel.MAX_HEIGHT);
             if (firstIndex < 0) firstIndex = ~firstIndex;
             int lastIndex = yPositions.BinarySearch(Current + drawHeight);
             if (lastIndex < 0) lastIndex = ~lastIndex;
 
-            lifetime.StartIndex = firstIndex;
+            // Add those panels within the previously found index range that should be displayed.
-            lifetime.EndIndex = lastIndex;
-
             for (int i = firstIndex; i < lastIndex; ++i)
             {
-                Panel p = lifetime[i];
+                Panel panel = panels[i];
-                if (p.State != PanelSelectedState.Hidden)
+                if (panel.State == PanelSelectedState.Hidden)
-                    p.IsOnScreen = true; //we don't want to update the on-screen state of hidden pannels as they have incorrect (stacked) y values.
+                    continue;
-                updatePanel(p, halfHeight);
+
+                // Only add if we're not already part of the content.
+                if (!scrollableContent.Contains(panel))
+                {
+                    // Makes sure headers are always _below_ panels,
+                    // and depth flows downward.
+                    panel.Depth = i + (panel is BeatmapSetHeader ? panels.Count : 0);
+                    scrollableContent.Add(panel);
+                }
             }
+
+            // Update externally controlled state of currently visible panels
+            // (e.g. x-offset and opacity).
+            float halfHeight = drawHeight / 2;
+            foreach (Panel p in scrollableContent.Children)
+                updatePanel(p, halfHeight);
         }
 
         protected override bool OnKeyDown(InputState state, KeyDownEventArgs args)
@@ -355,7 +335,7 @@ namespace osu.Game.Screens.Select
                 if (i >= 0 && i < SelectedGroup.BeatmapPanels.Count)
                 {
                     //changing difficulty panel, not set.
-                    SelectGroup(SelectedGroup, SelectedGroup.BeatmapPanels[i]);
+                    selectGroup(SelectedGroup, SelectedGroup.BeatmapPanels[i]);
                     return;
                 }
             }
@@ -378,11 +358,14 @@ namespace osu.Game.Screens.Select
         {
             if (groups.Count < 1)
                 return;
+
             BeatmapGroup group = groups[RNG.Next(groups.Count)];
             BeatmapPanel panel = group?.BeatmapPanels.First();
+
             if (panel == null)
                 return;
-            SelectGroup(group, panel);
+
+            selectGroup(group, panel);
         }
 
         public IEnumerator<BeatmapGroup> GetEnumerator() => groups.GetEnumerator();

osu.Game/Screens/Select/PlaySongSelect.cs

@@ -396,6 +396,7 @@ namespace osu.Game.Screens.Select
             {
                 beatmapGroups.Add(group);
 
+                group.State = BeatmapGroupState.Collapsed;
                 carousel.AddGroup(group);
 
                 filterChanged(false, false);
@@ -403,11 +404,7 @@ namespace osu.Game.Screens.Select
                 if (Beatmap == null || select)
                     carousel.SelectBeatmap(beatmapSet.Beatmaps.First());
                 else
-                {
+                    carousel.SelectBeatmap(Beatmap.BeatmapInfo);
-                    var panel = group.BeatmapPanels.FirstOrDefault(p => p.Beatmap.Equals(Beatmap.BeatmapInfo));
-                    if (panel != null)
-                        carousel.SelectGroup(group, panel);
-                }
             }));
         }