Simplify CollectionFilterDropdown filter flow weirdness

osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs

@@ -176,6 +176,8 @@ namespace osu.Game.Tests.Visual.SongSelect
         [Test]
         public void TestManageCollectionsFilterIsNotSelected()
         {
+            bool received = false;
+
             addExpandHeaderStep();
 
             AddStep("add collection", () => Realm.Write(r => r.Add(new BeatmapCollection(name: "1", new List<string> { "abc" }))));
@@ -187,6 +189,12 @@ namespace osu.Game.Tests.Visual.SongSelect
 
             addExpandHeaderStep();
 
+            AddStep("watch for filter requests", () =>
+            {
+                received = false;
+                control.ChildrenOfType<CollectionDropdown>().First().RequestFilter = () => received = true;
+            });
+
             AddStep("click manage collections filter", () =>
             {
                 InputManager.MoveMouseTo(getCollectionDropdownItems().Last());
@@ -194,6 +202,8 @@ namespace osu.Game.Tests.Visual.SongSelect
             });
 
             AddAssert("collection filter still selected", () => control.CreateCriteria().CollectionBeatmapMD5Hashes.Any());
+
+            AddAssert("filter request not fired", () => !received);
         }
 
         private BeatmapCollection getFirstCollection() => Realm.Run(r => r.All<BeatmapCollection>().First());

osu.Game/Collections/CollectionFilterDropdown.cs

@@ -30,15 +30,8 @@ namespace osu.Game.Collections
         /// </summary>
         protected virtual bool ShowManageCollectionsItem => true;
 
-        private readonly BindableWithCurrent<CollectionFilterMenuItem> current = new BindableWithCurrent<CollectionFilterMenuItem>();
+        public Action? RequestFilter { private get; set; }
 
-        public new Bindable<CollectionFilterMenuItem> Current
-        {
-            get => current.Current;
-            set => current.Current = value;
-        }
-
-        private readonly IBindableList<string> beatmaps = new BindableList<string>();
         private readonly BindableList<CollectionFilterMenuItem> filters = new BindableList<CollectionFilterMenuItem>();
 
         [Resolved]
@@ -50,6 +43,7 @@ namespace osu.Game.Collections
         public CollectionFilterDropdown()
         {
             ItemSource = filters;
+
             Current.Value = new AllBeatmapsCollectionFilterMenuItem();
         }
 
@@ -59,17 +53,9 @@ namespace osu.Game.Collections
 
             realm.RegisterForNotifications(r => r.All<BeatmapCollection>(), collectionsChanged);
 
-            // Dropdown has logic which triggers a change on the bindable with every change to the contained items.
+            Current.BindValueChanged(currentChanged);
-            // 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;
-
-            Current.BindValueChanged(currentChanged, true);
         }
 
-        /// <summary>
-        /// Occurs when a collection has been added or removed.
-        /// </summary>
         private void collectionsChanged(IRealmCollection<BeatmapCollection> collections, ChangeSet? changes, Exception error)
         {
             var selectedItem = SelectedItem?.Value?.Collection;
@@ -89,32 +75,35 @@ namespace osu.Game.Collections
                 foreach (int index in changes.ModifiedIndices)
                 {
                     if (collections[index].ID == selectedItem.ID)
-                    {
+                        RequestFilter?.Invoke();
-                        // The filtered beatmaps have changed, without the filter having changed itself. So a change in filter must be notified.
-                        // Note that this does NOT propagate to bound bindables, so the FilterControl must bind directly to the value change event of this bindable.
-                        Current.TriggerChange();
-                    }
                 }
             }
         }
 
         private void currentChanged(ValueChangedEvent<CollectionFilterMenuItem> filter)
         {
+            // May be null during .Clear().
+            if (filter.NewValue == null)
+                return;
+
             // Never select the manage collection filter - rollback to the previous filter.
             // This is done after the above since it is important that bindable is unbound from OldValue, which is lost after forcing it back to the old value.
             if (filter.NewValue is ManageCollectionsFilterMenuItem)
             {
                 Current.Value = filter.OldValue;
                 manageCollectionsDialog?.Show();
+                return;
             }
+
+            // This dropdown be weird.
+            // We only care about filtering if the actual collection has changed.
+            if (filter.OldValue?.Collection != null || filter.NewValue?.Collection != null)
+                RequestFilter?.Invoke();
         }
 
         protected override LocalisableString GenerateItemText(CollectionFilterMenuItem item) => item.CollectionName;
 
-        protected sealed override DropdownHeader CreateHeader() => CreateCollectionHeader().With(d =>
+        protected sealed override DropdownHeader CreateHeader() => CreateCollectionHeader().With(d => d.SelectedItem.BindTarget = Current);
-        {
-            d.SelectedItem.BindTarget = Current;
-        });
 
         protected sealed override DropdownMenu CreateMenu() => CreateCollectionMenu();
 

osu.Game/Screens/Select/FilterControl.cs

@@ -39,6 +39,10 @@ namespace osu.Game.Screens.Select
 
         private Bindable<GroupMode> groupMode;
 
+        private SeekLimitedSearchTextBox searchTextBox;
+
+        private CollectionFilterDropdown collectionDropdown;
+
         public FilterCriteria CreateCriteria()
         {
             string query = searchTextBox.Text;
@@ -49,7 +53,7 @@ namespace osu.Game.Screens.Select
                 Sort = sortMode.Value,
                 AllowConvertedBeatmaps = showConverted.Value,
                 Ruleset = ruleset.Value,
-                CollectionBeatmapMD5Hashes = collectionDropdown?.Current.Value?.Collection?.PerformRead(c => c.BeatmapMD5Hashes)
+                CollectionBeatmapMD5Hashes = collectionDropdown.Current.Value?.Collection?.PerformRead(c => c.BeatmapMD5Hashes)
             };
 
             if (!minimumStars.IsDefault)
@@ -64,10 +68,6 @@ namespace osu.Game.Screens.Select
             return criteria;
         }
 
-        private SeekLimitedSearchTextBox searchTextBox;
-
-        private CollectionFilterDropdown collectionDropdown;
-
         public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) =>
             base.ReceivePositionalInputAt(screenSpacePos) || sortTabs.ReceivePositionalInputAt(screenSpacePos);
 
@@ -183,6 +183,7 @@ namespace osu.Game.Screens.Select
                                     {
                                         Anchor = Anchor.TopRight,
                                         Origin = Anchor.TopRight,
+                                        RequestFilter = updateCriteria,
                                         RelativeSizeAxes = Axes.X,
                                         Y = 4,
                                         Width = 0.5f,
@@ -209,15 +210,6 @@ namespace osu.Game.Screens.Select
             groupMode.BindValueChanged(_ => updateCriteria());
             sortMode.BindValueChanged(_ => updateCriteria());
 
-            collectionDropdown.Current.ValueChanged += val =>
-            {
-                if (val.NewValue == null)
-                    // may be null briefly while menu is repopulated.
-                    return;
-
-                updateCriteria();
-            };
-
             searchTextBox.Current.ValueChanged += _ => updateCriteria();
 
             updateCriteria();