diff --git a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs index 206c32725e..eb8877738f 100644 --- a/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs +++ b/osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs @@ -8,6 +8,7 @@ using NUnit.Framework; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Extensions; +using osu.Game.Graphics.Sprites; using osu.Game.Screens.Select.Filter; using osu.Game.Screens.SelectV2; using osu.Game.Tests.Resources; @@ -63,6 +64,8 @@ namespace osu.Game.Tests.Visual.SongSelectV2 [Test] public void TestBeatmapSetMetadataUpdated() { + PanelBeatmapSet panel = null!; + var metadata = new BeatmapMetadata { Artist = "updated test", @@ -77,10 +80,15 @@ namespace osu.Game.Tests.Visual.SongSelectV2 originalDrawables.AddRange(Carousel.ChildrenOfType().ToList()); }); + AddStep("find panel", () => panel = Carousel.ChildrenOfType().Single(p => p.ChildrenOfType().Any(t => t.Text.ToString() == "beatmap"))); + updateBeatmap(b => b.Metadata = metadata); WaitForFiltering(); - AddAssert("drawables changed", () => Carousel.ChildrenOfType(), () => Is.Not.EqualTo(originalDrawables)); + + AddAssert("drawables unchanged", () => Carousel.ChildrenOfType(), () => Is.EqualTo(originalDrawables)); + + AddAssert("title updated", () => panel.ChildrenOfType().Any(t => t.Text.ToString() == metadata.Title)); } [Test] diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index cf386307ca..1ee4326ccb 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -756,12 +756,7 @@ namespace osu.Game.Graphics.Carousel continue; } - // The case where we're intending to display this panel, but it's already displayed. - // Note that we **must compare the model here** as the CarouselItems may be fresh instances due to a filter operation. - // - // Reference equality is used here instead of CheckModelEquality intentionally. In order to switch to `CheckModelEquality`, - // we need a way to signal to the drawable panels that there is an update. - var existing = toDisplay.FirstOrDefault(i => ReferenceEquals(i.Model, carouselPanel.Item!.Model)); + var existing = toDisplay.FirstOrDefault(i => CheckModelEquality(i.Model, carouselPanel.Item!.Model)); if (existing != null) { diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 5642685b23..a9e87d3587 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -473,6 +473,12 @@ namespace osu.Game.Screens.SelectV2 if (x is BeatmapInfo beatmapX && y is BeatmapInfo beatmapY) return beatmapX.Equals(beatmapY); + if (x is GroupDefinition groupX && y is GroupDefinition groupY) + return groupX.Equals(groupY); + + if (x is StarDifficultyGroupDefinition starX && y is StarDifficultyGroupDefinition starY) + return starX.Equals(starY); + return base.CheckModelEquality(x, y); } diff --git a/osu.Game/Screens/SelectV2/Panel.cs b/osu.Game/Screens/SelectV2/Panel.cs index 5400dcb2ce..de559be4a9 100644 --- a/osu.Game/Screens/SelectV2/Panel.cs +++ b/osu.Game/Screens/SelectV2/Panel.cs @@ -216,7 +216,18 @@ namespace osu.Game.Screens.SelectV2 protected override void PrepareForUse() { base.PrepareForUse(); - this.FadeInFromZero(DURATION, Easing.OutQuint); + + this.FadeIn(DURATION, Easing.OutQuint); + } + + protected override void FreeAfterUse() + { + base.FreeAfterUse(); + + Hide(); + + // Important to set this to null to handle reuse scenarios correctly, see `Item` implementation. + item = null; } protected override bool OnClick(ClickEvent e) @@ -283,7 +294,30 @@ namespace osu.Game.Screens.SelectV2 #region ICarouselPanel - public CarouselItem? Item { get; set; } + private CarouselItem? item; + + public CarouselItem? Item + { + get => item; + set + { + if (ReferenceEquals(item, value)) + return; + + // If a new item is set and we already have an item, this is a case of reuse. + // To keep things simple, assume that we need to do a full refresh. + // + // In the future, this could be more contextual and check whether the associated model has actually changed. + if (item != null && value != null) + { + item = value; + PrepareForUse(); + } + else + item = value; + } + } + public BindableBool Selected { get; } = new BindableBool(); public BindableBool Expanded { get; } = new BindableBool(); public BindableBool KeyboardSelected { get; } = new BindableBool();