Merge pull request #19718 from peppy/fix-playlist-imported-items

Fix playlist overlay showing new imports when they don't match collection filter

osu.Game.Tests/Resources/TestResources.cs

@@ -128,6 +128,8 @@ namespace osu.Game.Tests.Resources
 
                     var rulesetInfo = getRuleset();
 
+                    string hash = Guid.NewGuid().ToString().ComputeMD5Hash();
+
                     yield return new BeatmapInfo
                     {
                         OnlineID = beatmapId,
@@ -136,7 +138,8 @@ namespace osu.Game.Tests.Resources
                         Length = length,
                         BeatmapSet = beatmapSet,
                         BPM = bpm,
-                        Hash = Guid.NewGuid().ToString().ComputeMD5Hash(),
+                        Hash = hash,
+                        MD5Hash = hash,
                         Ruleset = rulesetInfo,
                         Metadata = metadata.DeepClone(),
                         Difficulty = new BeatmapDifficulty

osu.Game.Tests/Visual/UserInterface/TestScenePlaylistOverlay.cs

@@ -1,18 +1,20 @@
 // 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 System.Collections.Generic;
-
 using System.Linq;
 using NUnit.Framework;
-using osu.Framework.Bindables;
+using osu.Framework.Allocation;
+using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Platform;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Collections;
 using osu.Game.Database;
-using osu.Game.Graphics.Containers;
 using osu.Game.Overlays.Music;
+using osu.Game.Rulesets;
 using osu.Game.Tests.Resources;
 using osuTK;
 using osuTK.Input;
@@ -21,13 +23,25 @@ namespace osu.Game.Tests.Visual.UserInterface
 {
     public class TestScenePlaylistOverlay : OsuManualInputManagerTestScene
     {
-        private readonly BindableList<Live<BeatmapSetInfo>> beatmapSets = new BindableList<Live<BeatmapSetInfo>>();
+        protected override bool UseFreshStoragePerRun => true;
 
-        private PlaylistOverlay playlistOverlay;
+        private PlaylistOverlay playlistOverlay = null!;
 
-        private Live<BeatmapSetInfo> first;
+        private BeatmapManager beatmapManager = null!;
 
-        private const int item_count = 100;
+        private const int item_count = 20;
+
+        private List<BeatmapSetInfo> beatmapSets => beatmapManager.GetAllUsableBeatmapSets();
+
+        [BackgroundDependencyLoader]
+        private void load(GameHost host)
+        {
+            Dependencies.Cache(new RealmRulesetStore(Realm));
+            Dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, Realm, null, Audio, Resources, host, Beatmap.Default));
+            Dependencies.Cache(Realm);
+
+            beatmapManager.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely();
+        }
 
         [SetUp]
         public void Setup() => Schedule(() =>
@@ -46,16 +60,12 @@ namespace osu.Game.Tests.Visual.UserInterface
                 }
             };
 
-            beatmapSets.Clear();
-
             for (int i = 0; i < item_count; i++)
             {
-                beatmapSets.Add(TestResources.CreateTestBeatmapSetInfo().ToLiveUnmanaged());
+                beatmapManager.Import(TestResources.CreateTestBeatmapSetInfo());
             }
 
-            first = beatmapSets.First();
+            beatmapSets.First().ToLive(Realm);
-
-            playlistOverlay.BeatmapSets.BindTo(beatmapSets);
         });
 
         [Test]
@@ -70,9 +80,13 @@ namespace osu.Game.Tests.Visual.UserInterface
 
             AddUntilStep("wait for animations to complete", () => !playlistOverlay.Transforms.Any());
 
+            PlaylistItem firstItem = null!;
+
             AddStep("hold 1st item handle", () =>
             {
-                var handle = this.ChildrenOfType<OsuRearrangeableListItem<Live<BeatmapSetInfo>>.PlaylistItemHandle>().First();
+                firstItem = this.ChildrenOfType<PlaylistItem>().First();
+                var handle = firstItem.ChildrenOfType<PlaylistItem.PlaylistItemHandle>().First();
+
                 InputManager.MoveMouseTo(handle.ScreenSpaceDrawQuad.Centre);
                 InputManager.PressButton(MouseButton.Left);
             });
@@ -83,7 +97,7 @@ namespace osu.Game.Tests.Visual.UserInterface
                 InputManager.MoveMouseTo(item.ScreenSpaceDrawQuad.BottomLeft);
             });
 
-            AddAssert("song 1 is 5th", () => beatmapSets[4].Equals(first));
+            AddAssert("first is moved", () => playlistOverlay.ChildrenOfType<Playlist>().Single().Items.ElementAt(4).Value.Equals(firstItem.Model.Value));
 
             AddStep("release handle", () => InputManager.ReleaseButton(MouseButton.Left));
         }
@@ -101,6 +115,68 @@ namespace osu.Game.Tests.Visual.UserInterface
                 () => playlistOverlay.ChildrenOfType<PlaylistItem>()
                                      .Where(item => item.MatchingFilter)
                                      .All(item => item.FilterTerms.Any(term => term.ToString().Contains("10"))));
+
+            AddStep("Import new non-matching beatmap", () =>
+            {
+                var testBeatmapSetInfo = TestResources.CreateTestBeatmapSetInfo(1);
+                testBeatmapSetInfo.Beatmaps.Single().Metadata.Title = "no guid";
+                beatmapManager.Import(testBeatmapSetInfo);
+            });
+
+            AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh()));
+
+            AddAssert("results filtered correctly",
+                () => playlistOverlay.ChildrenOfType<PlaylistItem>()
+                                     .Where(item => item.MatchingFilter)
+                                     .All(item => item.FilterTerms.Any(term => term.ToString().Contains("10"))));
+        }
+
+        [Test]
+        public void TestCollectionFiltering()
+        {
+            NowPlayingCollectionDropdown collectionDropdown() => playlistOverlay.ChildrenOfType<NowPlayingCollectionDropdown>().Single();
+
+            AddStep("Add collection", () =>
+            {
+                Dependencies.Get<RealmAccess>().Write(r =>
+                {
+                    r.RemoveAll<BeatmapCollection>();
+                    r.Add(new BeatmapCollection("wang"));
+                });
+            });
+
+            AddUntilStep("wait for dropdown to have new collection", () => collectionDropdown().Items.Count() == 2);
+
+            AddStep("Filter to collection", () =>
+            {
+                collectionDropdown().Current.Value = collectionDropdown().Items.Last();
+            });
+
+            AddUntilStep("No items present", () => !playlistOverlay.ChildrenOfType<PlaylistItem>().Any(i => i.MatchingFilter));
+
+            AddStep("Import new non-matching beatmap", () =>
+            {
+                beatmapManager.Import(TestResources.CreateTestBeatmapSetInfo(1));
+            });
+
+            AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh()));
+
+            AddUntilStep("No items matching", () => !playlistOverlay.ChildrenOfType<PlaylistItem>().Any(i => i.MatchingFilter));
+
+            BeatmapSetInfo collectionAddedBeatmapSet = null!;
+
+            AddStep("Import new matching beatmap", () =>
+            {
+                collectionAddedBeatmapSet = TestResources.CreateTestBeatmapSetInfo(1);
+
+                beatmapManager.Import(collectionAddedBeatmapSet);
+                Realm.Write(r => r.All<BeatmapCollection>().First().BeatmapMD5Hashes.Add(collectionAddedBeatmapSet.Beatmaps.First().MD5Hash));
+            });
+
+            AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh()));
+
+            AddUntilStep("Only matching item",
+                () => playlistOverlay.ChildrenOfType<PlaylistItem>().Where(i => i.MatchingFilter).Select(i => i.Model.ID), () => Is.EquivalentTo(new[] { collectionAddedBeatmapSet.ID }));
         }
     }
 }

osu.Game/Overlays/Music/Playlist.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 System;
 using System.Linq;
 using osu.Framework.Bindables;
@@ -17,10 +15,12 @@ namespace osu.Game.Overlays.Music
 {
     public class Playlist : OsuRearrangeableListContainer<Live<BeatmapSetInfo>>
     {
-        public Action<Live<BeatmapSetInfo>> RequestSelection;
+        public Action<Live<BeatmapSetInfo>>? RequestSelection;
 
         public readonly Bindable<Live<BeatmapSetInfo>> SelectedSet = new Bindable<Live<BeatmapSetInfo>>();
 
+        private FilterCriteria currentCriteria = new FilterCriteria();
+
         public new MarginPadding Padding
         {
             get => base.Padding;
@@ -31,26 +31,22 @@ namespace osu.Game.Overlays.Music
         {
             var items = (SearchContainer<RearrangeableListItem<Live<BeatmapSetInfo>>>)ListContainer;
 
-            string[] currentCollectionHashes = criteria.Collection?.PerformRead(c => c.BeatmapMD5Hashes.ToArray());
+            string[]? currentCollectionHashes = criteria.Collection?.PerformRead(c => c.BeatmapMD5Hashes.ToArray());
 
             foreach (var item in items.OfType<PlaylistItem>())
             {
-                if (currentCollectionHashes == null)
+                item.InSelectedCollection = currentCollectionHashes == null || item.Model.Value.Beatmaps.Select(b => b.MD5Hash).Any(currentCollectionHashes.Contains);
-                    item.InSelectedCollection = true;
-                else
-                {
-                    item.InSelectedCollection = item.Model.Value.Beatmaps.Select(b => b.MD5Hash)
-                                                    .Any(currentCollectionHashes.Contains);
-                }
             }
 
             items.SearchTerm = criteria.SearchText;
+            currentCriteria = criteria;
         }
 
-        public Live<BeatmapSetInfo> FirstVisibleSet => Items.FirstOrDefault(i => ((PlaylistItem)ItemMap[i]).MatchingFilter);
+        public Live<BeatmapSetInfo>? FirstVisibleSet => Items.FirstOrDefault(i => ((PlaylistItem)ItemMap[i]).MatchingFilter);
 
         protected override OsuRearrangeableListItem<Live<BeatmapSetInfo>> CreateOsuDrawable(Live<BeatmapSetInfo> item) => new PlaylistItem(item)
         {
+            InSelectedCollection = currentCriteria.Collection?.PerformRead(c => item.Value.Beatmaps.Select(b => b.MD5Hash).Any(c.BeatmapMD5Hashes.Contains)) != false,
             SelectedSet = { BindTarget = SelectedSet },
             RequestSelection = set => RequestSelection?.Invoke(set)
         };

osu.Game/Overlays/Music/PlaylistOverlay.cs

@@ -26,8 +26,6 @@ namespace osu.Game.Overlays.Music
         private const float transition_duration = 600;
         private const float playlist_height = 510;
 
-        public IBindableList<Live<BeatmapSetInfo>> BeatmapSets => beatmapSets;
-
         private readonly BindableList<Live<BeatmapSetInfo>> beatmapSets = new BindableList<Live<BeatmapSetInfo>>();
 
         private readonly Bindable<WorkingBeatmap> beatmap = new Bindable<WorkingBeatmap>();
@@ -104,9 +102,7 @@ namespace osu.Game.Overlays.Music
         {
             base.LoadComplete();
 
-            // tests might bind externally, in which case we don't want to involve realm.
+            beatmapSubscription = realm.RegisterForNotifications(r => r.All<BeatmapSetInfo>().Where(s => !s.DeletePending), beatmapsChanged);
-            if (beatmapSets.Count == 0)
-                beatmapSubscription = realm.RegisterForNotifications(r => r.All<BeatmapSetInfo>().Where(s => !s.DeletePending), beatmapsChanged);
 
             list.Items.BindTo(beatmapSets);
             beatmap.BindValueChanged(working => list.SelectedSet.Value = working.NewValue.BeatmapSetInfo.ToLive(realm), true);