From 02fa1c21b74dc42a329839a119137f046ae69563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 15:02:59 +0100 Subject: [PATCH 1/2] Adjust existing placeholder test to demonstrate failure case --- .../Online/TestSceneBeatmapListingOverlay.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs index 80f7d0ca29..966f2f3610 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs @@ -7,6 +7,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; +using osu.Game.Beatmaps.Drawables.Cards; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Online.API.Requests; @@ -110,17 +111,25 @@ namespace osu.Game.Tests.Visual.Online public void TestNoBeatmapsPlaceholder() { AddStep("fetch for 0 beatmaps", () => fetchFor()); - AddUntilStep("placeholder shown", () => overlay.ChildrenOfType().SingleOrDefault()?.IsPresent == true); + placeholderShown(); - AddStep("fetch for 1 beatmap", () => fetchFor(CreateAPIBeatmapSet(Ruleset.Value))); + AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray())); + AddUntilStep("wait for loaded", () => this.ChildrenOfType().Count() == 100); AddUntilStep("placeholder hidden", () => !overlay.ChildrenOfType().Any(d => d.IsPresent)); AddStep("fetch for 0 beatmaps", () => fetchFor()); - AddUntilStep("placeholder shown", () => overlay.ChildrenOfType().SingleOrDefault()?.IsPresent == true); + placeholderShown(); // fetch once more to ensure nothing happens in displaying placeholder again when it already is present. AddStep("fetch for 0 beatmaps again", () => fetchFor()); - AddUntilStep("placeholder shown", () => overlay.ChildrenOfType().SingleOrDefault()?.IsPresent == true); + placeholderShown(); + + void placeholderShown() => + AddUntilStep("placeholder shown", () => + { + var notFoundDrawable = overlay.ChildrenOfType().SingleOrDefault(); + return notFoundDrawable != null && notFoundDrawable.IsPresent && notFoundDrawable.Parent.DrawHeight > 0; + }); } [Test] From 25e38560ce302889df60029473403ed5a9e91739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Dec 2021 15:05:23 +0100 Subject: [PATCH 2/2] Fix placeholder drawables on beatmap listing not always hiding correctly `BeatmapListingOverlay.addContentToPlaceholder()`, in order to make transitions between different beatmap listing content (whether it is actual cards, or placeholders for no beatmaps found/supporter-specific filters chosen), would set `BypassAutoSizeAxes = Y` on content as it is fading out, to make the transition smoother. The property in question was supposed to be getting restored to `None` on the next show. In testing scenarios, it sometimes turned out that this wasn't the case, therefore making the placeholders effectively not show - while they were present and fully opaque, they would be the only child of an auto-sized container with `BypassAutoSizeAxes = Y`, so the parent auto-sized to a zero height, which logically follows from the premise, but is not what was desired. This in turn was caused by the fact that the `BypassAutoSizeAxes = Y` set was scheduled, and sometimes it would be scheduled in such a way that the drawable would cease to be present on the next frame due to its alpha being past the cutoff point of 0.0001. Therefore the scheduled set would not execute until the *next* time the placeholder was shown, therefore causing the bug. Fix by ensuring that the placeholder drawables are always present if their schedulers have any tasks enqueued, on top of the usual checks of alpha and scale performed via the base implementation. --- osu.Game/Overlays/BeatmapListingOverlay.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Overlays/BeatmapListingOverlay.cs b/osu.Game/Overlays/BeatmapListingOverlay.cs index c4d1d62250..d07f7c8f8f 100644 --- a/osu.Game/Overlays/BeatmapListingOverlay.cs +++ b/osu.Game/Overlays/BeatmapListingOverlay.cs @@ -217,6 +217,10 @@ namespace osu.Game.Overlays public class NotFoundDrawable : CompositeDrawable { + // required for scheduled tasks to complete correctly + // (see `addContentToPlaceholder()` and the scheduled `BypassAutoSizeAxes` set during fade-out in outer class above) + public override bool IsPresent => base.IsPresent || Scheduler.HasPendingTasks; + public NotFoundDrawable() { RelativeSizeAxes = Axes.X; @@ -261,6 +265,10 @@ namespace osu.Game.Overlays // (https://github.com/ppy/osu-framework/issues/4530) public class SupporterRequiredDrawable : CompositeDrawable { + // required for scheduled tasks to complete correctly + // (see `addContentToPlaceholder()` and the scheduled `BypassAutoSizeAxes` set during fade-out in outer class above) + public override bool IsPresent => base.IsPresent || Scheduler.HasPendingTasks; + private LinkFlowContainer supporterRequiredText; public SupporterRequiredDrawable()