diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs index eb610a40f1..3638c8eeec 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs @@ -22,12 +22,16 @@ namespace osu.Game.Tests.Visual.SongSelectV2 { private BeatmapSetInfo baseTestBeatmap = null!; + private const int initial_filter_count = 3; + [SetUpSteps] public void SetUpSteps() { RemoveAllBeatmaps(); CreateCarousel(); + WaitForFiltering(); AddBeatmaps(1, 3); + WaitForFiltering(); AddStep("generate and add test beatmap", () => { baseTestBeatmap = TestResources.CreateTestBeatmapSetInfo(3); @@ -42,8 +46,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2 b.Metadata = metadata; BeatmapSets.Add(baseTestBeatmap); }); - WaitForFiltering(); + + AddAssert("filter count correct", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count)); } [Test] @@ -81,12 +86,18 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddUntilStep("is scrolled to end", () => Carousel.ChildrenOfType().Single().IsScrolledToEnd()); - updateBeatmap(b => b.Metadata = new BeatmapMetadata + updateBeatmap(b => { - Artist = "updated test", - Title = $"beatmap {RNG.Next().ToString()}" + // hash will be updated when important metadata changes, such as title, difficulty, author etc. + b.Hash = "new hash"; + b.Metadata = new BeatmapMetadata + { + Artist = "updated test", + Title = $"beatmap {RNG.Next().ToString()}" + }; }); + assertDidFilter(); WaitForFiltering(); AddAssert("scroll is still at end", () => Carousel.ChildrenOfType().Single().IsScrolledToEnd()); @@ -113,8 +124,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddStep("find panel", () => panel = Carousel.ChildrenOfType().Single(p => p.ChildrenOfType().Any(t => t.Text.ToString() == "beatmap"))); - updateBeatmap(b => b.Metadata = metadata); + updateBeatmap(b => + { + b.Metadata = metadata; + // hash will be updated when important metadata changes, such as title, difficulty, author etc. + b.Hash = "new hash"; + }); + assertDidFilter(); WaitForFiltering(); AddAssert("drawables unchanged", () => Carousel.ChildrenOfType(), () => Is.EqualTo(originalDrawables)); @@ -123,7 +140,41 @@ namespace osu.Game.Tests.Visual.SongSelectV2 } [Test] - public void TestSelectionHeld() + public void TestOnlineStatusUpdated() + { + List originalDrawables = new List(); + + AddStep("store drawable references", () => + { + originalDrawables.Clear(); + originalDrawables.AddRange(Carousel.ChildrenOfType().ToList()); + }); + + updateBeatmap(b => b.Status = BeatmapOnlineStatus.Graveyard); + + assertDidFilter(); + WaitForFiltering(); + + AddAssert("drawables unchanged", () => Carousel.ChildrenOfType(), () => Is.EqualTo(originalDrawables)); + } + + [Test] + public void TestNoUpdateTriggeredOnUserTagsChange() + { + var metadata = new BeatmapMetadata + { + Artist = "updated test", + Title = "new beatmap title", + UserTags = { "hi" } + }; + + updateBeatmap(b => b.Metadata = metadata); + assertDidNotFilter(); + } + + [TestCase(false)] + [TestCase(true)] + public void TestSelectionHeld(bool hashChanged) { SelectNextSet(); @@ -131,7 +182,17 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); - updateBeatmap(); + updateBeatmap(b => + { + if (hashChanged) + b.Hash = "new hash"; + }); + + if (hashChanged) + assertDidFilter(); + else + assertDidNotFilter(); + WaitForFiltering(); AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); @@ -148,6 +209,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.DifficultyName = "new name"); + assertDidFilter(); WaitForFiltering(); AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); @@ -164,6 +226,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); updateBeatmap(b => b.OnlineID = b.OnlineID + 1); + assertDidFilter(); WaitForFiltering(); AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0])); @@ -339,6 +402,10 @@ namespace osu.Game.Tests.Visual.SongSelectV2 AddAssert("Order didn't change", () => Carousel.PostFilterBeatmaps.Select(b => b.ID), () => Is.EqualTo(originalOrder)); } + private void assertDidFilter() => AddAssert("did filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count + 1)); + + private void assertDidNotFilter() => AddAssert("did not filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count)); + private void updateBeatmap(Action? updateBeatmap = null, Action? updateSet = null) { AddStep("update beatmap with different reference", () => diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs index 69bdb97617..895f148965 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs @@ -23,7 +23,9 @@ using osu.Game.Screens.Ranking; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Leaderboards; using osu.Game.Screens.SelectV2; +using osu.Game.Tests.Resources; using osuTK.Input; +using BeatmapCarousel = osu.Game.Screens.SelectV2.BeatmapCarousel; using FooterButtonMods = osu.Game.Screens.SelectV2.FooterButtonMods; using FooterButtonOptions = osu.Game.Screens.SelectV2.FooterButtonOptions; using FooterButtonRandom = osu.Game.Screens.SelectV2.FooterButtonRandom; @@ -302,6 +304,28 @@ namespace osu.Game.Tests.Visual.SongSelectV2 }); } + /// + /// Last played and rank achieved may have changed, so we want to make sure filtering runs on resume to song select. + /// + [Test] + public void TestFilteringRunsAfterReturningFromGameplay() + { + AddStep("import actual beatmap", () => Beatmaps.Import(TestResources.GetQuickTestBeatmapForImport())); + LoadSongSelect(); + + AddUntilStep("wait for filtered", () => SongSelect.ChildrenOfType().Single().FilterCount, () => Is.EqualTo(1)); + + AddStep("enter gameplay", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + AddUntilStep("wait for fail", () => ((Player)Stack.CurrentScreen).GameplayState.HasFailed); + + AddStep("exit gameplay", () => InputManager.Key(Key.Escape)); + AddStep("exit gameplay", () => InputManager.Key(Key.Escape)); + + AddUntilStep("wait for filtered", () => SongSelect.ChildrenOfType().Single().FilterCount, () => Is.EqualTo(2)); + } + [Test] public void TestAutoplayShortcut() { diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index 7b5aea08b6..b81df0a7eb 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using System.Threading; @@ -79,7 +80,7 @@ namespace osu.Game.Graphics.Carousel /// /// The number of times filter operations have been triggered. /// - internal int FilterCount { get; private set; } + public int FilterCount { get; private set; } /// /// The number of displayable items currently being tracked (before filtering). @@ -210,6 +211,12 @@ namespace osu.Game.Graphics.Carousel return filterTask; } + /// + /// Called when changes in any way. + /// + /// Whether a re-filter is required. + protected virtual bool HandleItemsChanged(NotifyCollectionChangedEventArgs args) => true; + /// /// Fired after a filter operation completed. /// @@ -301,7 +308,11 @@ namespace osu.Game.Graphics.Carousel RelativeSizeAxes = Axes.Both, }; - Items.BindCollectionChanged((_, _) => filterAfterItemsChanged.Invalidate()); + Items.BindCollectionChanged((_, args) => + { + if (HandleItemsChanged(args)) + filterAfterItemsChanged.Invalidate(); + }); } [BackgroundDependencyLoader] diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index da841aa361..a4e957a1bf 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -143,6 +143,9 @@ namespace osu.Game.Screens.SelectV2 switch (changed.Action) { case NotifyCollectionChangedAction.Add: + if (!newItems!.Any()) + return; + Items.AddRange(newItems!.SelectMany(s => s.Beatmaps)); break; @@ -353,6 +356,57 @@ namespace osu.Game.Screens.SelectV2 } } + protected override bool HandleItemsChanged(NotifyCollectionChangedEventArgs args) + { + switch (args.Action) + { + case NotifyCollectionChangedAction.Add: + case NotifyCollectionChangedAction.Remove: + case NotifyCollectionChangedAction.Move: + case NotifyCollectionChangedAction.Reset: + return true; + + case NotifyCollectionChangedAction.Replace: + var oldBeatmaps = args.OldItems!.OfType().ToList(); + var newBeatmaps = args.NewItems!.OfType().ToList(); + + for (int i = 0; i < oldBeatmaps.Count; i++) + { + var oldBeatmap = oldBeatmaps[i]; + var newBeatmap = newBeatmaps[i]; + + // Ignore changes which don't concern us. + // + // Here are some examples of things that can go wrong: + // - Background difficulty calculation runs and causes a realm update. + // We use `BeatmapDifficultyCache` and don't want to know about these. + // - Background user tag population runs and causes a realm update. + // We don't display user tags so want to ignore this. + bool equalForDisplayPurposes = + // covers metadata changes + oldBeatmap.Hash == newBeatmap.Hash && + // sanity check + oldBeatmap.OnlineID == newBeatmap.OnlineID && + // displayed on panel + oldBeatmap.Status == newBeatmap.Status && + // displayed on panel + oldBeatmap.DifficultyName == newBeatmap.DifficultyName && + // hidden changed, needs re-filter + oldBeatmap.Hidden == newBeatmap.Hidden && + // might be used for grouping, returning from gameplay + oldBeatmap.LastPlayed == newBeatmap.LastPlayed; + + if (equalForDisplayPurposes) + return false; + } + + return true; + + default: + throw new ArgumentOutOfRangeException(); + } + } + protected override void HandleFilterCompleted() { base.HandleFilterCompleted();