Merge pull request #11817 from peppy/fix-collection-dropdown-cross-thread-manipulation

Fix potential crash from cross-thread drawable manipulation in CollectionFilterDropdown

osu.Game/Collections/CollectionFilterDropdown.cs

@@ -29,6 +29,14 @@ namespace osu.Game.Collections
         /// </summary>
         protected virtual bool ShowManageCollectionsItem => true;
 
+        private readonly BindableWithCurrent<CollectionFilterMenuItem> current = new BindableWithCurrent<CollectionFilterMenuItem>();
+
+        public new Bindable<CollectionFilterMenuItem> Current
+        {
+            get => current.Current;
+            set => current.Current = value;
+        }
+
         private readonly IBindableList<BeatmapCollection> collections = new BindableList<BeatmapCollection>();
         private readonly IBindableList<BeatmapInfo> beatmaps = new BindableList<BeatmapInfo>();
         private readonly BindableList<CollectionFilterMenuItem> filters = new BindableList<CollectionFilterMenuItem>();
@@ -36,25 +44,28 @@ namespace osu.Game.Collections
         [Resolved(CanBeNull = true)]
         private ManageCollectionsDialog manageCollectionsDialog { get; set; }
 
+        [Resolved(CanBeNull = true)]
+        private CollectionManager collectionManager { get; set; }
+
         public CollectionFilterDropdown()
         {
             ItemSource = filters;
-        }
+            Current.Value = new AllBeatmapsCollectionFilterMenuItem();
-
-        [BackgroundDependencyLoader(permitNulls: true)]
-        private void load([CanBeNull] CollectionManager collectionManager)
-        {
-            if (collectionManager != null)
-                collections.BindTo(collectionManager.Collections);
-
-            collections.CollectionChanged += (_, __) => collectionsChanged();
-            collectionsChanged();
         }
 
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
+            if (collectionManager != null)
+                collections.BindTo(collectionManager.Collections);
+
+            // Dropdown has logic which triggers a change on the bindable with every change to the contained items.
+            // This is not desirable here, as it leads to multiple filter operations running even though nothing has changed.
+            // An extra bindable is enough to subvert this behaviour.
+            base.Current = Current;
+
+            collections.BindCollectionChanged((_, __) => collectionsChanged(), true);
             Current.BindValueChanged(filterChanged, true);
         }
 

osu.Game/Collections/CollectionFilterMenuItem.cs

@@ -1,6 +1,7 @@
 // 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 JetBrains.Annotations;
 using osu.Framework.Bindables;
 
@@ -9,7 +10,7 @@ namespace osu.Game.Collections
     /// <summary>
     /// A <see cref="BeatmapCollection"/> filter.
     /// </summary>
-    public class CollectionFilterMenuItem
+    public class CollectionFilterMenuItem : IEquatable<CollectionFilterMenuItem>
     {
         /// <summary>
         /// The collection to filter beatmaps from.
@@ -33,6 +34,11 @@ namespace osu.Game.Collections
             Collection = collection;
             CollectionName = Collection?.Name.GetBoundCopy() ?? new Bindable<string>("All beatmaps");
         }
+
+        public bool Equals(CollectionFilterMenuItem other)
+            => other != null && CollectionName.Value == other.CollectionName.Value;
+
+        public override int GetHashCode() => CollectionName.Value.GetHashCode();
     }
 
     public class AllBeatmapsCollectionFilterMenuItem : CollectionFilterMenuItem

osu.Game/Screens/Select/FilterControl.cs

@@ -44,7 +44,7 @@ namespace osu.Game.Screens.Select
                 Sort = sortMode.Value,
                 AllowConvertedBeatmaps = showConverted.Value,
                 Ruleset = ruleset.Value,
-                Collection = collectionDropdown?.Current.Value.Collection
+                Collection = collectionDropdown?.Current.Value?.Collection
             };
 
             if (!minimumStars.IsDefault)