Fix ManageCollectionsDialog and remove weird placeholder logic

osu.Game.Tests/Visual/Collections/TestSceneManageCollectionsDialog.cs

@@ -109,10 +109,15 @@ namespace osu.Game.Tests.Visual.Collections
                 InputManager.Click(MouseButton.Left);
             });
 
-            // Done directly via the collection since InputManager methods cannot add text to textbox...
+            assertCollectionCount(0);
-            AddStep("change collection name", () => placeholderItem.Model.PerformWrite(c => c.Name = "a"));
+
+            AddStep("change collection name", () =>
+            {
+                placeholderItem.ChildrenOfType<TextBox>().First().Text = "test text";
+                InputManager.Key(Key.Enter);
+            });
+
             assertCollectionCount(1);
-            AddAssert("collection now exists", () => placeholderItem.Model.IsManaged);
 
             AddAssert("last item is placeholder", () => !dialog.ChildrenOfType<DrawableCollectionListItem>().Last().Model.IsManaged);
         }
@@ -257,6 +262,7 @@ namespace osu.Game.Tests.Visual.Collections
         public void TestCollectionRenamedOnTextChange()
         {
             BeatmapCollection first = null!;
+            DrawableCollectionListItem firstItem = null!;
 
             AddStep("add two collections", () =>
             {
@@ -272,12 +278,23 @@ namespace osu.Game.Tests.Visual.Collections
 
             assertCollectionCount(2);
 
-            AddStep("change first collection name", () => dialog.ChildrenOfType<TextBox>().First().Text = "First");
+            AddStep("focus first collection", () =>
+            {
+                InputManager.MoveMouseTo(firstItem = dialog.ChildrenOfType<DrawableCollectionListItem>().First());
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddStep("change first collection name", () =>
+            {
+                firstItem.ChildrenOfType<TextBox>().First().Text = "First";
+                InputManager.Key(Key.Enter);
+            });
+
             AddUntilStep("collection has new name", () => first.Name == "First");
         }
 
         private void assertCollectionCount(int count)
-            => AddUntilStep($"{count} collections shown", () => dialog.ChildrenOfType<DrawableCollectionListItem>().Count(i => i.IsCreated.Value) == count);
+            => AddUntilStep($"{count} collections shown", () => dialog.ChildrenOfType<DrawableCollectionListItem>().Count() == count + 1); // +1 for placeholder
 
         private void assertCollectionName(int index, string name)
             => AddUntilStep($"item {index + 1} has correct name", () => dialog.ChildrenOfType<DrawableCollectionListItem>().ElementAt(index).ChildrenOfType<TextBox>().First().Text == name);

osu.Game/Collections/CollectionFilterDropdown.cs

@@ -53,21 +53,27 @@ namespace osu.Game.Collections
 
             realm.RegisterForNotifications(r => r.All<BeatmapCollection>(), collectionsChanged);
 
-            Current.BindValueChanged(currentChanged);
+            Current.BindValueChanged(selectionChanged);
         }
 
         private void collectionsChanged(IRealmCollection<BeatmapCollection> collections, ChangeSet? changes, Exception error)
         {
             var selectedItem = SelectedItem?.Value?.Collection;
 
+            var allBeatmaps = new AllBeatmapsCollectionFilterMenuItem();
+
             filters.Clear();
-            filters.Add(new AllBeatmapsCollectionFilterMenuItem());
+            filters.Add(allBeatmaps);
             filters.AddRange(collections.Select(c => new CollectionFilterMenuItem(c.ToLive(realm))));
 
             if (ShowManageCollectionsItem)
                 filters.Add(new ManageCollectionsFilterMenuItem());
 
-            Current.Value = filters.SingleOrDefault(f => f.Collection != null && f.Collection.ID == selectedItem?.ID) ?? filters[0];
+            // This current update and schedule is required to work around dropdown headers not updating text even when the selected item
+            // changes. It's not great but honestly the whole dropdown menu structure isn't great. This needs to be fixed, but I'll issue
+            // a warning that it's going to be a frustrating journey.
+            Current.Value = allBeatmaps;
+            Schedule(() => Current.Value = filters.SingleOrDefault(f => f.Collection != null && f.Collection.ID == selectedItem?.ID) ?? filters[0]);
 
             // Trigger a re-filter if the current item was in the change set.
             if (selectedItem != null && changes != null)
@@ -80,7 +86,9 @@ namespace osu.Game.Collections
             }
         }
 
-        private void currentChanged(ValueChangedEvent<CollectionFilterMenuItem> filter)
+        private Live<BeatmapCollection>? lastFiltered;
+
+        private void selectionChanged(ValueChangedEvent<CollectionFilterMenuItem> filter)
         {
             // May be null during .Clear().
             if (filter.NewValue == null)
@@ -95,15 +103,20 @@ namespace osu.Game.Collections
                 return;
             }
 
+            var newCollection = filter.NewValue?.Collection;
+
             // This dropdown be weird.
             // We only care about filtering if the actual collection has changed.
-            if (filter.OldValue?.Collection != null || filter.NewValue?.Collection != null)
+            if (newCollection != lastFiltered)
+            {
                 RequestFilter?.Invoke();
+                lastFiltered = newCollection;
+            }
         }
 
         protected override LocalisableString GenerateItemText(CollectionFilterMenuItem item) => item.CollectionName;
 
-        protected sealed override DropdownHeader CreateHeader() => CreateCollectionHeader().With(d => d.SelectedItem.BindTarget = Current);
+        protected sealed override DropdownHeader CreateHeader() => CreateCollectionHeader();
 
         protected sealed override DropdownMenu CreateMenu() => CreateCollectionMenu();
 
@@ -113,8 +126,6 @@ namespace osu.Game.Collections
 
         public class CollectionDropdownHeader : OsuDropdownHeader
         {
-            public readonly Bindable<CollectionFilterMenuItem> SelectedItem = new Bindable<CollectionFilterMenuItem>();
-
             public CollectionDropdownHeader()
             {
                 Height = 25;
@@ -130,14 +141,14 @@ namespace osu.Game.Collections
                 MaxHeight = 200;
             }
 
-            protected override DrawableDropdownMenuItem CreateDrawableDropdownMenuItem(MenuItem item) => new CollectionDropdownMenuItem(item)
+            protected override DrawableDropdownMenuItem CreateDrawableDropdownMenuItem(MenuItem item) => new CollectionDropdownDrawableMenuItem(item)
             {
                 BackgroundColourHover = HoverColour,
                 BackgroundColourSelected = SelectionColour
             };
         }
 
-        protected class CollectionDropdownMenuItem : OsuDropdownMenu.DrawableOsuDropdownMenuItem
+        protected class CollectionDropdownDrawableMenuItem : OsuDropdownMenu.DrawableOsuDropdownMenuItem
         {
             protected new CollectionFilterMenuItem Item => ((DropdownMenuItem<CollectionFilterMenuItem>)base.Item).Value;
 
@@ -148,7 +159,7 @@ namespace osu.Game.Collections
             [Resolved]
             private IBindable<WorkingBeatmap> beatmap { get; set; } = null!;
 
-            public CollectionDropdownMenuItem(MenuItem item)
+            public CollectionDropdownDrawableMenuItem(MenuItem item)
                 : base(item)
             {
             }

osu.Game/Collections/DeleteCollectionDialog.cs

@@ -4,16 +4,17 @@
 using System;
 using Humanizer;
 using osu.Framework.Graphics.Sprites;
+using osu.Game.Database;
 using osu.Game.Overlays.Dialog;
 
 namespace osu.Game.Collections
 {
     public class DeleteCollectionDialog : PopupDialog
     {
-        public DeleteCollectionDialog(BeatmapCollection collection, Action deleteAction)
+        public DeleteCollectionDialog(Live<BeatmapCollection> collection, Action deleteAction)
         {
             HeaderText = "Confirm deletion of";
-            BodyText = $"{collection.Name} ({"beatmap".ToQuantity(collection.BeatmapMD5Hashes.Count)})";
+            BodyText = collection.PerformRead(c => $"{c.Name} ({"beatmap".ToQuantity(c.BeatmapMD5Hashes.Count)})");
 
             Icon = FontAwesome.Regular.TrashAlt;
 

osu.Game/Collections/DrawableCollectionList.cs

@@ -1,33 +1,51 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.Diagnostics;
 using System.Linq;
+using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Game.Database;
 using osu.Game.Graphics.Containers;
 using osuTK;
+using Realms;
 
 namespace osu.Game.Collections
 {
     /// <summary>
     /// Visualises a list of <see cref="BeatmapCollection"/>s.
     /// </summary>
-    public class DrawableCollectionList : OsuRearrangeableListContainer<BeatmapCollection>
+    public class DrawableCollectionList : OsuRearrangeableListContainer<Live<BeatmapCollection>>
     {
         private Scroll scroll = null!;
 
         protected override ScrollContainer<Drawable> CreateScrollContainer() => scroll = new Scroll();
 
-        protected override FillFlowContainer<RearrangeableListItem<BeatmapCollection>> CreateListFillFlowContainer() => new Flow
+        [Resolved]
+        private RealmAccess realm { get; set; } = null!;
+
+        protected override FillFlowContainer<RearrangeableListItem<Live<BeatmapCollection>>> CreateListFillFlowContainer() => new Flow
         {
             DragActive = { BindTarget = DragActive }
         };
 
-        // TODO: source from realm
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
 
-        protected override OsuRearrangeableListItem<BeatmapCollection> CreateOsuDrawable(BeatmapCollection item)
+            realm.RegisterForNotifications(r => r.All<BeatmapCollection>(), collectionsChanged);
+        }
+
+        private void collectionsChanged(IRealmCollection<BeatmapCollection> collections, ChangeSet? changes, Exception error)
+        {
+            Items.Clear();
+            Items.AddRange(collections.AsEnumerable().Select(c => c.ToLive(realm)));
+        }
+
+        protected override OsuRearrangeableListItem<Live<BeatmapCollection>> CreateOsuDrawable(Live<BeatmapCollection> item)
         {
             if (item.ID == scroll.PlaceholderItem.Model.ID)
                 return scroll.ReplacePlaceholder();
@@ -97,7 +115,7 @@ namespace osu.Game.Collections
                 var previous = PlaceholderItem;
 
                 placeholderContainer.Clear(false);
-                placeholderContainer.Add(PlaceholderItem = new DrawableCollectionListItem(new BeatmapCollection(), false));
+                placeholderContainer.Add(PlaceholderItem = new DrawableCollectionListItem(new BeatmapCollection().ToLiveUnmanaged(), false));
 
                 return previous;
             }
@@ -106,7 +124,7 @@ namespace osu.Game.Collections
         /// <summary>
         /// The flow of <see cref="DrawableCollectionListItem"/>. Disables layout easing unless a drag is in progress.
         /// </summary>
-        private class Flow : FillFlowContainer<RearrangeableListItem<BeatmapCollection>>
+        private class Flow : FillFlowContainer<RearrangeableListItem<Live<BeatmapCollection>>>
         {
             public readonly IBindable<bool> DragActive = new Bindable<bool>();
 

osu.Game/Collections/DrawableCollectionListItem.cs

@@ -3,12 +3,12 @@
 
 using System;
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Input.Events;
 using osu.Game.Database;
 using osu.Game.Graphics;
@@ -23,52 +23,37 @@ namespace osu.Game.Collections
     /// <summary>
     /// Visualises a <see cref="BeatmapCollection"/> inside a <see cref="DrawableCollectionList"/>.
     /// </summary>
-    public class DrawableCollectionListItem : OsuRearrangeableListItem<BeatmapCollection>
+    public class DrawableCollectionListItem : OsuRearrangeableListItem<Live<BeatmapCollection>>
     {
         private const float item_height = 35;
         private const float button_width = item_height * 0.75f;
 
-        /// <summary>
-        /// Whether the <see cref="BeatmapCollection"/> currently exists inside realm.
-        /// </summary>
-        public IBindable<bool> IsCreated => isCreated;
-
-        private readonly Bindable<bool> isCreated = new Bindable<bool>();
-
         /// <summary>
         /// Creates a new <see cref="DrawableCollectionListItem"/>.
         /// </summary>
         /// <param name="item">The <see cref="BeatmapCollection"/>.</param>
         /// <param name="isCreated">Whether <paramref name="item"/> currently exists inside realm.</param>
-        public DrawableCollectionListItem(BeatmapCollection item, bool isCreated)
+        public DrawableCollectionListItem(Live<BeatmapCollection> item, bool isCreated)
             : base(item)
         {
-            this.isCreated.Value = isCreated;
+            ShowDragHandle.Value = item.IsManaged;
-
-            ShowDragHandle.BindTo(this.isCreated);
         }
 
-        protected override Drawable CreateContent() => new ItemContent(Model)
+        protected override Drawable CreateContent() => new ItemContent(Model);
-        {
-            IsCreated = { BindTarget = isCreated }
-        };
 
         /// <summary>
         /// The main content of the <see cref="DrawableCollectionListItem"/>.
         /// </summary>
         private class ItemContent : CircularContainer
         {
-            public readonly Bindable<bool> IsCreated = new Bindable<bool>();
+            private readonly Live<BeatmapCollection> collection;
 
-            private readonly BeatmapCollection collection;
-
-            private Container textBoxPaddingContainer = null!;
             private ItemTextBox textBox = null!;
 
             [Resolved]
             private RealmAccess realm { get; set; } = null!;
 
-            public ItemContent(BeatmapCollection collection)
+            public ItemContent(Live<BeatmapCollection> collection)
             {
                 this.collection = collection;
 
@@ -80,19 +65,20 @@ namespace osu.Game.Collections
             [BackgroundDependencyLoader]
             private void load()
             {
-                Children = new Drawable[]
+                Children = new[]
                 {
-                    new DeleteButton(collection)
+                    collection.IsManaged
-                    {
+                        ? new DeleteButton(collection)
-                        Anchor = Anchor.CentreRight,
+                        {
-                        Origin = Anchor.CentreRight,
+                            Anchor = Anchor.CentreRight,
-                        IsCreated = { BindTarget = IsCreated },
+                            Origin = Anchor.CentreRight,
-                        IsTextBoxHovered = v => textBox.ReceivePositionalInputAt(v)
+                            IsTextBoxHovered = v => textBox.ReceivePositionalInputAt(v)
-                    },
+                        }
-                    textBoxPaddingContainer = new Container
+                        : Empty(),
+                    new Container
                     {
                         RelativeSizeAxes = Axes.Both,
-                        Padding = new MarginPadding { Right = button_width },
+                        Padding = new MarginPadding { Right = collection.IsManaged ? button_width : 0 },
                         Children = new Drawable[]
                         {
                             textBox = new ItemTextBox
@@ -100,7 +86,7 @@ namespace osu.Game.Collections
                                 RelativeSizeAxes = Axes.Both,
                                 Size = Vector2.One,
                                 CornerRadius = item_height / 2,
-                                PlaceholderText = IsCreated.Value ? string.Empty : "Create a new collection"
+                                PlaceholderText = collection.IsManaged ? string.Empty : "Create a new collection"
                             },
                         }
                     },
@@ -112,28 +98,18 @@ namespace osu.Game.Collections
                 base.LoadComplete();
 
                 // Bind late, as the collection name may change externally while still loading.
-                textBox.Current.Value = collection.Name;
+                textBox.Current.Value = collection.PerformRead(c => c.IsValid ? c.Name : string.Empty);
-                textBox.Current.BindValueChanged(_ => createNewCollection(), true);
+                textBox.OnCommit += onCommit;
-
-                IsCreated.BindValueChanged(created => textBoxPaddingContainer.Padding = new MarginPadding { Right = created.NewValue ? button_width : 0 }, true);
             }
 
-            private void createNewCollection()
+            private void onCommit(TextBox sender, bool newText)
             {
-                if (IsCreated.Value)
+                if (collection.IsManaged)
-                    return;
+                    collection.PerformWrite(c => c.Name = textBox.Current.Value);
+                else if (!string.IsNullOrEmpty(textBox.Current.Value))
+                    realm.Write(r => r.Add(new BeatmapCollection(textBox.Current.Value)));
 
-                if (string.IsNullOrEmpty(textBox.Current.Value))
+                textBox.Text = string.Empty;
-                    return;
-
-                // Add the new collection and disable our placeholder. If all text is removed, the placeholder should not show back again.
-                realm.Write(r => r.Add(collection));
-                textBox.PlaceholderText = string.Empty;
-
-                // When this item changes from placeholder to non-placeholder (via changing containers), its textbox will lose focus, so it needs to be re-focused.
-                Schedule(() => GetContainingInputManager().ChangeFocus(textBox));
-
-                IsCreated.Value = true;
             }
         }
 
@@ -151,22 +127,17 @@ namespace osu.Game.Collections
 
         public class DeleteButton : CompositeDrawable
         {
-            public readonly IBindable<bool> IsCreated = new Bindable<bool>();
-
             public Func<Vector2, bool> IsTextBoxHovered = null!;
 
             [Resolved]
             private IDialogOverlay? dialogOverlay { get; set; }
 
-            [Resolved]
+            private readonly Live<BeatmapCollection> collection;
-            private RealmAccess realmAccess { get; set; } = null!;
-
-            private readonly BeatmapCollection collection;
 
             private Drawable fadeContainer = null!;
             private Drawable background = null!;
 
-            public DeleteButton(BeatmapCollection collection)
+            public DeleteButton(Live<BeatmapCollection> collection)
             {
                 this.collection = collection;
                 RelativeSizeAxes = Axes.Y;
@@ -200,12 +171,6 @@ namespace osu.Game.Collections
                 };
             }
 
-            protected override void LoadComplete()
-            {
-                base.LoadComplete();
-                IsCreated.BindValueChanged(created => Alpha = created.NewValue ? 1 : 0, true);
-            }
-
             public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) && !IsTextBoxHovered(screenSpacePos);
 
             protected override bool OnHover(HoverEvent e)
@@ -223,7 +188,7 @@ namespace osu.Game.Collections
             {
                 background.FlashColour(Color4.White, 150);
 
-                if (collection.BeatmapMD5Hashes.Count == 0)
+                if (collection.PerformRead(c => c.BeatmapMD5Hashes.Count) == 0)
                     deleteCollection();
                 else
                     dialogOverlay?.Push(new DeleteCollectionDialog(collection, deleteCollection));
@@ -231,7 +196,7 @@ namespace osu.Game.Collections
                 return true;
             }
 
-            private void deleteCollection() => realmAccess.Write(r => r.Remove(collection));
+            private void deleteCollection() => collection.PerformWrite(c => c.Realm.Remove(c));
         }
     }
 }

osu.Game/Collections/ManageCollectionsDialog.cs

@@ -1,8 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-#nullable disable
-
 using osu.Framework.Allocation;
 using osu.Framework.Audio;
 using osu.Framework.Graphics;
@@ -23,7 +21,7 @@ namespace osu.Game.Collections
         private const double enter_duration = 500;
         private const double exit_duration = 200;
 
-        private AudioFilter lowPassFilter;
+        private AudioFilter lowPassFilter = null!;
 
         public ManageCollectionsDialog()
         {