Rewrite PlaylistList as CompositeDrawable and remove all backwards PlaylistList references

Now handles drag at a PlaylistList.ItemsScrollContainer level (private class), and PlaylistList itself is no longer a Container so it only supports adding BeatmapSets. Sorry for the rewrite x.x.

osu.Game/Overlays/Music/PlaylistItem.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using OpenTK.Graphics;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
@@ -18,7 +17,7 @@ using OpenTK;
 
 namespace osu.Game.Overlays.Music
 {
-    internal class PlaylistItem : Container, IFilterable
+    internal class PlaylistItem : Container, IFilterable, IDraggable
     {
         private const float fade_duration = 100;
 
@@ -31,11 +30,12 @@ namespace osu.Game.Overlays.Music
         private UnicodeBindableString titleBind;
         private UnicodeBindableString artistBind;
 
-        private readonly FillFlowContainer<PlaylistItem> playlist;
         public readonly BeatmapSetInfo BeatmapSetInfo;
 
         public Action<BeatmapSetInfo> OnSelect;
 
+        public bool IsDraggable => handle.IsHovered;
+
         private bool selected;
         public bool Selected
         {
@@ -51,9 +51,8 @@ namespace osu.Game.Overlays.Music
             }
         }
 
-        public PlaylistItem(FillFlowContainer<PlaylistItem> playlist, BeatmapSetInfo setInfo)
+        public PlaylistItem(BeatmapSetInfo setInfo)
         {
-            this.playlist = playlist;
             BeatmapSetInfo = setInfo;
 
             RelativeSizeAxes = Axes.X;
@@ -72,9 +71,9 @@ namespace osu.Game.Overlays.Music
 
             Children = new Drawable[]
             {
-                handle = new PlaylistItemHandle(playlist)
+                handle = new PlaylistItemHandle
                 {
-                    Colour = colours.Gray5,
+                    Colour = colours.Gray5
                 },
                 text = new OsuTextFlowContainer
                 {
@@ -149,11 +148,9 @@ namespace osu.Game.Overlays.Music
 
         private class PlaylistItemHandle : SpriteIcon
         {
-            private readonly FillFlowContainer<PlaylistItem> playlist;
 
-            public PlaylistItemHandle(FillFlowContainer<PlaylistItem> playlist)
+            public PlaylistItemHandle()
             {
-                this.playlist = playlist;
                 Anchor = Anchor.TopLeft;
                 Origin = Anchor.TopLeft;
                 Size = new Vector2(12);
@@ -161,47 +158,14 @@ namespace osu.Game.Overlays.Music
                 Alpha = 0f;
                 Margin = new MarginPadding { Left = 5, Top = 2 };
             }
-
-            protected override bool OnDragStart(InputState state) => true;
-
-            protected override bool OnDrag(InputState state)
-            {
-                int src = (int)Parent.Depth;
-
-                var matchingItem = playlist.Children.LastOrDefault(c => c.Position.Y < state.Mouse.Position.Y + Parent.Position.Y);
-                if (matchingItem == null)
-                    return true;
-
-                int dst = (int)matchingItem.Depth;
-
-                // Due to the position predicate above, there is an edge case to consider when an item is moved upwards:
-                // At the point where the two items cross there will be two items sharing the same condition, and the items will jump back
-                // and forth between the two positions because of this. This is accentuated if the items span differing line heights.
-                // The easiest way to avoid this is to ensure the movement direction matches the expected mouse delta
-
-                if (state.Mouse.Delta.Y <= 0 && dst > src)
-                    return true;
-
-                if (state.Mouse.Delta.Y >= 0 && dst < src)
-                    return true;
-
-                if (src == dst)
-                    return true;
-
-                if (src < dst)
-                {
-                    for (int i = src + 1; i <= dst; i++)
-                        playlist.ChangeChildDepth(playlist[i], i - 1);
-                }
-                else
-                {
-                    for (int i = dst; i < src; i++)
-                        playlist.ChangeChildDepth(playlist[i], i + 1);
-                }
-
-                playlist.ChangeChildDepth(Parent as PlaylistItem, dst);
-                return true;
-            }
         }
     }
+
+    public interface IDraggable : IDrawable
+    {
+        /// <summary>
+        /// Whether this <see cref="IDraggable"/> can be dragged in its current state.
+        /// </summary>
+        bool IsDraggable { get; }
+    }
 }

osu.Game/Overlays/Music/PlaylistList.cs

@@ -4,125 +4,205 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Input;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.Containers;
+using OpenTK;
 
 namespace osu.Game.Overlays.Music
 {
-    internal class PlaylistList : Container
+    internal class PlaylistList : CompositeDrawable
     {
-        private readonly FillFlowContainer<PlaylistItem> items;
-
-        public IEnumerable<BeatmapSetInfo> BeatmapSets
-        {
-            set
-            {
-                items.Children = value.Select((item, index) => new PlaylistItem(items, item) { OnSelect = itemSelected, Depth = index }).ToList();
-            }
-        }
-
-        public BeatmapSetInfo FirstVisibleSet => items.Children.FirstOrDefault(i => i.MatchingFilter)?.BeatmapSetInfo;
-
-        private void itemSelected(BeatmapSetInfo b)
-        {
-            OnSelect?.Invoke(b);
-        }
-
         public Action<BeatmapSetInfo> OnSelect;
 
-        private readonly SearchContainer search;
+        private readonly ItemsScrollContainer items;
-
-        public void Filter(string searchTerm) => search.SearchTerm = searchTerm;
-
-        public BeatmapSetInfo SelectedItem
-        {
-            get { return items.Children.FirstOrDefault(i => i.Selected)?.BeatmapSetInfo; }
-            set
-            {
-                foreach (PlaylistItem s in items.Children)
-                    s.Selected = s.BeatmapSetInfo.ID == value?.ID;
-            }
-        }
-
-        public BeatmapSetInfo NextItem
-        {
-            get
-            {
-                var available = items.Children;
-                return (available.SkipWhile(i => !i.Selected).Skip(1).FirstOrDefault() ?? available.FirstOrDefault())?.BeatmapSetInfo;
-            }
-        }
-
-        public BeatmapSetInfo PreviousItem
-        {
-            get
-            {
-                var available = items.Children;
-                return (available.TakeWhile(i => !i.Selected).LastOrDefault() ?? available.LastOrDefault())?.BeatmapSetInfo;
-            }
-        }
 
         public PlaylistList()
         {
-            Children = new Drawable[]
+            InternalChild = items = new ItemsScrollContainer
             {
-                new OsuScrollContainer
+                RelativeSizeAxes = Axes.Both,
-                {
+                OnSelect = set => OnSelect?.Invoke(set)
-                    RelativeSizeAxes = Axes.Both,
-                    Children = new Drawable[]
-                    {
-                        search = new SearchContainer
-                        {
-                            RelativeSizeAxes = Axes.X,
-                            AutoSizeAxes = Axes.Y,
-                            Children = new Drawable[]
-                            {
-                                items = new ItemSearchContainer
-                                {
-                                    RelativeSizeAxes = Axes.X,
-                                    AutoSizeAxes = Axes.Y,
-                                },
-                            }
-                        }
-                    },
-                },
             };
         }
 
-        public void AddBeatmapSet(BeatmapSetInfo beatmapSet)
+        public new MarginPadding Padding
         {
-            items.Add(new PlaylistItem(items, beatmapSet) { OnSelect = itemSelected, Depth = items.Count });
+            get { return base.Padding; }
+            set { base.Padding = value; }
         }
 
-        public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet)
+        public IEnumerable<BeatmapSetInfo> BeatmapSets { set { items.Sets = value; } }
+
+        public BeatmapSetInfo FirstVisibleSet => items.FirstVisibleSet;
+        public BeatmapSetInfo NextSet => items.NextSet;
+        public BeatmapSetInfo PreviousSet => items.PreviousSet;
+
+        public BeatmapSetInfo SelectedSet
         {
-            PlaylistItem itemToRemove = items.Children.FirstOrDefault(item => item.BeatmapSetInfo.ID == beatmapSet.ID);
+            get { return items.SelectedSet; }
-            if (itemToRemove != null) items.Remove(itemToRemove);
+            set { items.SelectedSet = value; }
         }
 
-        private class ItemSearchContainer : FillFlowContainer<PlaylistItem>, IHasFilterableChildren
+        public void AddBeatmapSet(BeatmapSetInfo beatmapSet) => items.AddBeatmapSet(beatmapSet);
+        public bool RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => items.RemoveBeatmapSet(beatmapSet);
+
+        public void Filter(string searchTerm) => items.SearchTerm = searchTerm;
+
+        private class ItemsScrollContainer : OsuScrollContainer
         {
-            public string[] FilterTerms => new string[] { };
+            public Action<BeatmapSetInfo> OnSelect;
-            public bool MatchingFilter
+
+            private readonly SearchContainer search;
+            private readonly FillFlowContainer<PlaylistItem> items;
+
+            private PlaylistItem draggedItem;
+
+            public ItemsScrollContainer()
+            {
+                Children = new Drawable[]
+                {
+                    search = new SearchContainer
+                    {
+                        RelativeSizeAxes = Axes.X,
+                        AutoSizeAxes = Axes.Y,
+                        Children = new Drawable[]
+                        {
+                            items = new ItemSearchContainer
+                            {
+                                RelativeSizeAxes = Axes.X,
+                                AutoSizeAxes = Axes.Y,
+                            },
+                        }
+                    }
+                };
+            }
+
+            public IEnumerable<BeatmapSetInfo> Sets
             {
                 set
                 {
-                    if (value)
+                    items.Clear();
-                        InvalidateLayout();
+                    value.ForEach(AddBeatmapSet);
                 }
             }
 
-            // Compare with reversed ChildID and Depth
+            public string SearchTerm
-            protected override int Compare(Drawable x, Drawable y) => base.Compare(y, x);
-
-            public IEnumerable<IFilterable> FilterableChildren => Children;
-
-            public ItemSearchContainer()
             {
-                LayoutDuration = 200;
+                get { return search.SearchTerm; }
-                LayoutEasing = Easing.OutQuint;
+                set { search.SearchTerm = value; }
+            }
+
+            public void AddBeatmapSet(BeatmapSetInfo beatmapSet)
+            {
+                items.Add(new PlaylistItem(beatmapSet)
+                {
+                    OnSelect = set => OnSelect?.Invoke(set),
+                    Depth = items.Count
+                });
+            }
+
+            public bool RemoveBeatmapSet(BeatmapSetInfo beatmapSet)
+            {
+                var itemToRemove = items.FirstOrDefault(i => i.BeatmapSetInfo.ID == beatmapSet.ID);
+                if (itemToRemove == null)
+                    return false;
+                return items.Remove(itemToRemove);
+            }
+
+            public BeatmapSetInfo SelectedSet
+            {
+                get { return items.FirstOrDefault(i => i.Selected)?.BeatmapSetInfo; }
+                set
+                {
+                    foreach (PlaylistItem s in items.Children)
+                        s.Selected = s.BeatmapSetInfo.ID == value?.ID;
+                }
+            }
+
+            public BeatmapSetInfo FirstVisibleSet => items.FirstOrDefault(i => i.MatchingFilter)?.BeatmapSetInfo;
+            public BeatmapSetInfo NextSet => (items.SkipWhile(i => !i.Selected).Skip(1).FirstOrDefault() ?? items.FirstOrDefault())?.BeatmapSetInfo;
+            public BeatmapSetInfo PreviousSet => (items.TakeWhile(i => !i.Selected).LastOrDefault() ?? items.LastOrDefault())?.BeatmapSetInfo;
+
+            protected override bool OnDragStart(InputState state)
+            {
+                draggedItem = items.FirstOrDefault(d => d.IsDraggable);
+                return draggedItem != null || base.OnDragStart(state);
+            }
+
+            protected override bool OnDrag(InputState state)
+            {
+                if (draggedItem == null)
+                    return base.OnDrag(state);
+
+                // Mouse position in the position space of the items container
+                Vector2 itemsPos = items.ToLocalSpace(state.Mouse.NativeState.Position);
+
+                int src = (int)draggedItem.Depth;
+
+                var matchingItem = items.LastOrDefault(c => c.Position.Y < itemsPos.Y);
+                if (matchingItem == null)
+                    return true;
+
+                int dst = (int)matchingItem.Depth;
+
+                // Due to the position predicate above, there is an edge case to consider when an item is moved upwards:
+                // At the point where the two items cross there will be two items sharing the same condition, and the items will jump back
+                // and forth between the two positions because of this. This is accentuated if the items span differing line heights.
+                // The easiest way to avoid this is to ensure the movement direction matches the expected mouse delta
+
+                if (state.Mouse.Delta.Y <= 0 && dst > src)
+                    return true;
+
+                if (state.Mouse.Delta.Y >= 0 && dst < src)
+                    return true;
+
+                if (src == dst)
+                    return true;
+
+                if (src < dst)
+                {
+                    for (int i = src + 1; i <= dst; i++)
+                        items.ChangeChildDepth(items[i], i - 1);
+                }
+                else
+                {
+                    for (int i = dst; i < src; i++)
+                        items.ChangeChildDepth(items[i], i + 1);
+                }
+
+                items.ChangeChildDepth(draggedItem, dst);
+
+                return true;
+            }
+
+            protected override bool OnDragEnd(InputState state) => draggedItem != null || base.OnDragEnd(state);
+
+            private class ItemSearchContainer : FillFlowContainer<PlaylistItem>, IHasFilterableChildren
+            {
+                public string[] FilterTerms => new string[] { };
+                public bool MatchingFilter
+                {
+                    set
+                    {
+                        if (value)
+                            InvalidateLayout();
+                    }
+                }
+
+                // Compare with reversed ChildID and Depth
+                protected override int Compare(Drawable x, Drawable y) => base.Compare(y, x);
+
+                public IEnumerable<IFilterable> FilterableChildren => Children;
+
+                public ItemSearchContainer()
+                {
+                    LayoutDuration = 200;
+                    LayoutEasing = Easing.OutQuint;
+                }
             }
         }
     }

osu.Game/Overlays/Music/PlaylistOverlay.cs

@@ -92,7 +92,7 @@ namespace osu.Game.Overlays.Music
         protected override void LoadComplete()
         {
             base.LoadComplete();
-            beatmapBacking.ValueChanged += b => list.SelectedItem = b?.BeatmapSetInfo;
+            beatmapBacking.ValueChanged += b => list.SelectedSet = b?.BeatmapSetInfo;
             beatmapBacking.TriggerChange();
         }
 
@@ -126,23 +126,23 @@ namespace osu.Game.Overlays.Music
 
         public void PlayPrevious()
         {
-            var playable = list.PreviousItem;
+            var playable = list.PreviousSet;
 
             if (playable != null)
             {
                 playSpecified(playable.Beatmaps[0]);
-                list.SelectedItem = playable;
+                list.SelectedSet = playable;
             }
         }
 
         public void PlayNext()
         {
-            var playable = list.NextItem;
+            var playable = list.NextSet;
 
             if (playable != null)
             {
                 playSpecified(playable.Beatmaps[0]);
-                list.SelectedItem = playable;
+                list.SelectedSet = playable;
             }
         }