Merge pull request #16313 from bdach/fix-listing-terminal-breakage

Fix beatmap listing overlay not expiring content from previous searches

osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs

@@ -107,19 +107,31 @@ namespace osu.Game.Tests.Visual.Online
             AddUntilStep("is hidden", () => overlay.State.Value == Visibility.Hidden);
         }
 
+        [Test]
+        public void TestCorrectOldContentExpiration()
+        {
+            AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
+
+            AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
+            assertAllCardsOfType<BeatmapCardNormal>(100);
+
+            AddStep("show more results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 30).ToArray()));
+            assertAllCardsOfType<BeatmapCardNormal>(30);
+        }
+
         [Test]
         public void TestCardSizeSwitching()
         {
             AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
 
             AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
-            assertAllCardsOfType<BeatmapCardNormal>();
+            assertAllCardsOfType<BeatmapCardNormal>(100);
 
             setCardSize(BeatmapCardSize.Extra);
-            assertAllCardsOfType<BeatmapCardExtra>();
+            assertAllCardsOfType<BeatmapCardExtra>(100);
 
             setCardSize(BeatmapCardSize.Normal);
-            assertAllCardsOfType<BeatmapCardNormal>();
+            assertAllCardsOfType<BeatmapCardNormal>(100);
 
             AddStep("fetch for 0 beatmaps", () => fetchFor());
             AddUntilStep("placeholder shown", () => overlay.ChildrenOfType<BeatmapListingOverlay.NotFoundDrawable>().SingleOrDefault()?.IsPresent == true);
@@ -323,13 +335,12 @@ namespace osu.Game.Tests.Visual.Online
 
         private void setCardSize(BeatmapCardSize cardSize) => AddStep($"set card size to {cardSize}", () => overlay.ChildrenOfType<BeatmapListingCardSizeTabControl>().Single().Current.Value = cardSize);
 
-        private void assertAllCardsOfType<T>()
+        private void assertAllCardsOfType<T>(int expectedCount)
             where T : BeatmapCard =>
             AddUntilStep($"all loaded beatmap cards are {typeof(T)}", () =>
             {
                 int loadedCorrectCount = this.ChildrenOfType<BeatmapCard>().Count(card => card.IsLoaded && card.GetType() == typeof(T));
-                int totalCount = this.ChildrenOfType<BeatmapCard>().Count();
+                return loadedCorrectCount > 0 && loadedCorrectCount == expectedCount;
-                return loadedCorrectCount > 0 && loadedCorrectCount == totalCount;
             });
     }
 }

osu.Game/Overlays/BeatmapListingOverlay.cs

@@ -79,7 +79,6 @@ namespace osu.Game.Overlays
                                 Padding = new MarginPadding { Horizontal = 20 },
                                 Children = new Drawable[]
                                 {
-                                    foundContent = new FillFlowContainer<BeatmapCard>(),
                                     notFoundContent = new NotFoundDrawable(),
                                     supporterRequiredContent = new SupporterRequiredDrawable(),
                                 }
@@ -140,7 +139,7 @@ namespace osu.Game.Overlays
             if (searchResult.Type == BeatmapListingFilterControl.SearchResultType.SupporterOnlyFilters)
             {
                 supporterRequiredContent.UpdateText(searchResult.SupporterOnlyFiltersUsed);
-                addContentToPlaceholder(supporterRequiredContent);
+                addContentToResultsArea(supporterRequiredContent);
                 return;
             }
 
@@ -151,13 +150,13 @@ namespace osu.Game.Overlays
                 //No matches case
                 if (!newCards.Any())
                 {
-                    addContentToPlaceholder(notFoundContent);
+                    addContentToResultsArea(notFoundContent);
                     return;
                 }
 
                 var content = createCardContainerFor(newCards);
 
-                panelLoadTask = LoadComponentAsync(foundContent = content, addContentToPlaceholder, (cancellationToken = new CancellationTokenSource()).Token);
+                panelLoadTask = LoadComponentAsync(foundContent = content, addContentToResultsArea, (cancellationToken = new CancellationTokenSource()).Token);
             }
             else
             {
@@ -192,7 +191,7 @@ namespace osu.Game.Overlays
             return content;
         }
 
-        private void addContentToPlaceholder(Drawable content)
+        private void addContentToResultsArea(Drawable content)
         {
             Loading.Hide();
             lastFetchDisplayedTime = Time.Current;
@@ -204,37 +203,27 @@ namespace osu.Game.Overlays
 
             if (lastContent != null)
             {
-                lastContent.FadeOut(100, Easing.OutQuint);
+                lastContent.FadeOut();
-
+                if (!isPlaceholderContent(lastContent))
-                // Consider the case when the new content is smaller than the last content.
+                    lastContent.Expire();
-                // If the auto-size computation is delayed until fade out completes, the background remain high for too long making the resulting transition to the smaller height look weird.
-                // At the same time, if the last content's height is bypassed immediately, there is a period where the new content is at Alpha = 0 when the auto-sized height will be 0.
-                // To resolve both of these issues, the bypass is delayed until a point when the content transitions (fade-in and fade-out) overlap and it looks good to do so.
-                var sequence = lastContent.Delay(25).Schedule(() => lastContent.BypassAutoSizeAxes = Axes.Y);
-
-                if (lastContent == foundContent)
-                {
-                    sequence.Then().Schedule(() =>
-                    {
-                        foundContent.Expire();
-                        foundContent = null;
-                    });
-                }
             }
 
             if (!content.IsAlive)
                 panelTarget.Add(content);
 
-            content.FadeInFromZero(200, Easing.OutQuint);
+            content.FadeInFromZero();
             currentContent = content;
-            // currentContent may be one of the placeholders, and still have BypassAutoSizeAxes set to Y from the last fade-out.
-            // restore to the initial state.
-            currentContent.BypassAutoSizeAxes = Axes.None;
         }
 
+        /// <summary>
+        /// Whether <paramref name="drawable"/> is a static placeholder reused multiple times by this overlay.
+        /// </summary>
+        private bool isPlaceholderContent(Drawable drawable)
+            => drawable == notFoundContent || drawable == supporterRequiredContent;
+
         private void onCardSizeChanged()
         {
-            if (foundContent == null || !foundContent.Any())
+            if (foundContent?.IsAlive != true || !foundContent.Any())
                 return;
 
             Loading.Show();
@@ -259,10 +248,6 @@ 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;
@@ -307,10 +292,6 @@ 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()