Merge pull request #20144 from peppy/fix-notification-overlay-presence

Fix completion toasts sometimes not displaying

osu.Game.Tests/Visual/Gameplay/TestScenePlayerLoader.cs

@@ -301,7 +301,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             AddAssert("check for notification", () => notificationOverlay.UnreadCount.Value, () => Is.EqualTo(1));
 
-            clickNotificationIfAny();
+            clickNotification();
 
             AddAssert("check " + volumeName, assert);
 
@@ -370,8 +370,12 @@ namespace osu.Game.Tests.Visual.Gameplay
                 batteryInfo.SetChargeLevel(chargeLevel);
             }));
             AddUntilStep("wait for player", () => player?.LoadState == LoadState.Ready);
-            AddAssert($"notification {(shouldWarn ? "triggered" : "not triggered")}", () => notificationOverlay.UnreadCount.Value == (shouldWarn ? 1 : 0));
+
-            clickNotificationIfAny();
+            if (shouldWarn)
+                clickNotification();
+            else
+                AddAssert("notification not triggered", () => notificationOverlay.UnreadCount.Value == 0);
+
             AddUntilStep("wait for player load", () => player.IsLoaded);
         }
 
@@ -436,9 +440,13 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddUntilStep("skip button not visible", () => !checkSkipButtonVisible());
         }
 
-        private void clickNotificationIfAny()
+        private void clickNotification()
         {
-            AddStep("click notification", () => notificationOverlay.ChildrenOfType<Notification>().FirstOrDefault()?.TriggerClick());
+            Notification notification = null;
+
+            AddUntilStep("wait for notification", () => (notification = notificationOverlay.ChildrenOfType<Notification>().FirstOrDefault()) != null);
+            AddStep("open notification overlay", () => notificationOverlay.Show());
+            AddStep("click notification", () => notification.TriggerClick());
         }
 
         private EpilepsyWarning getWarning() => loader.ChildrenOfType<EpilepsyWarning>().SingleOrDefault();

osu.Game.Tests/Visual/Navigation/TestSceneStartupImport.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.Linq;
 using NUnit.Framework;
 using osu.Framework.Testing;
@@ -13,7 +11,7 @@ namespace osu.Game.Tests.Visual.Navigation
 {
     public class TestSceneStartupImport : OsuGameTestScene
     {
-        private string importFilename;
+        private string? importFilename;
 
         protected override TestOsuGame CreateTestGame() => new TestOsuGame(LocalStorage, API, new[] { importFilename });
 

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

@@ -7,6 +7,7 @@ using NUnit.Framework;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Testing;
 using osu.Framework.Utils;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Overlays;
@@ -23,9 +24,12 @@ namespace osu.Game.Tests.Visual.UserInterface
 
         private SpriteText displayedCount = null!;
 
+        public double TimeToCompleteProgress { get; set; } = 2000;
+
         [SetUp]
         public void SetUp() => Schedule(() =>
         {
+            TimeToCompleteProgress = 2000;
             progressingNotifications.Clear();
 
             Content.Children = new Drawable[]
@@ -41,10 +45,36 @@ namespace osu.Game.Tests.Visual.UserInterface
             notificationOverlay.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count.NewValue}"; };
         });
 
+        [Test]
+        public void TestPresence()
+        {
+            AddAssert("tray not present", () => !notificationOverlay.ChildrenOfType<NotificationOverlayToastTray>().Single().IsPresent);
+            AddAssert("overlay not present", () => !notificationOverlay.IsPresent);
+
+            AddStep(@"post notification", sendBackgroundNotification);
+
+            AddUntilStep("wait tray not present", () => !notificationOverlay.ChildrenOfType<NotificationOverlayToastTray>().Single().IsPresent);
+            AddUntilStep("wait overlay not present", () => !notificationOverlay.IsPresent);
+        }
+
+        [Test]
+        public void TestPresenceWithManualDismiss()
+        {
+            AddAssert("tray not present", () => !notificationOverlay.ChildrenOfType<NotificationOverlayToastTray>().Single().IsPresent);
+            AddAssert("overlay not present", () => !notificationOverlay.IsPresent);
+
+            AddStep(@"post notification", sendBackgroundNotification);
+            AddStep("click notification", () => notificationOverlay.ChildrenOfType<Notification>().Single().TriggerClick());
+
+            AddUntilStep("wait tray not present", () => !notificationOverlay.ChildrenOfType<NotificationOverlayToastTray>().Single().IsPresent);
+            AddUntilStep("wait overlay not present", () => !notificationOverlay.IsPresent);
+        }
+
         [Test]
         public void TestCompleteProgress()
         {
             ProgressNotification notification = null!;
+
             AddStep("add progress notification", () =>
             {
                 notification = new ProgressNotification
@@ -57,6 +87,31 @@ namespace osu.Game.Tests.Visual.UserInterface
             });
 
             AddUntilStep("wait completion", () => notification.State == ProgressNotificationState.Completed);
+
+            AddAssert("Completion toast shown", () => notificationOverlay.ToastCount == 1);
+            AddUntilStep("wait forwarded", () => notificationOverlay.ToastCount == 0);
+        }
+
+        [Test]
+        public void TestCompleteProgressSlow()
+        {
+            ProgressNotification notification = null!;
+
+            AddStep("Set progress slow", () => TimeToCompleteProgress *= 2);
+            AddStep("add progress notification", () =>
+            {
+                notification = new ProgressNotification
+                {
+                    Text = @"Uploading to BSS...",
+                    CompletionText = "Uploaded to BSS!",
+                };
+                notificationOverlay.Post(notification);
+                progressingNotifications.Add(notification);
+            });
+
+            AddUntilStep("wait completion", () => notification.State == ProgressNotificationState.Completed);
+
+            AddAssert("Completion toast shown", () => notificationOverlay.ToastCount == 1);
         }
 
         [Test]
@@ -177,7 +232,7 @@ namespace osu.Game.Tests.Visual.UserInterface
             foreach (var n in progressingNotifications.FindAll(n => n.State == ProgressNotificationState.Active))
             {
                 if (n.Progress < 1)
-                    n.Progress += (float)(Time.Elapsed / 2000);
+                    n.Progress += (float)(Time.Elapsed / TimeToCompleteProgress);
                 else
                     n.State = ProgressNotificationState.Completed;
             }

osu.Game/Overlays/NotificationOverlay.cs

@@ -71,7 +71,6 @@ namespace osu.Game.Overlays
                 },
                 mainContent = new Container
                 {
-                    AlwaysPresent = true,
                     RelativeSizeAxes = Axes.Both,
                     Children = new Drawable[]
                     {
@@ -137,7 +136,9 @@ namespace osu.Game.Overlays
 
         private readonly Scheduler postScheduler = new Scheduler();
 
-        public override bool IsPresent => base.IsPresent || postScheduler.HasPendingTasks;
+        public override bool IsPresent =>
+            // Delegate presence as we need to consider the toast tray in addition to the main overlay.
+            State.Value == Visibility.Visible || mainContent.IsPresent || toastTray.IsPresent || postScheduler.HasPendingTasks;
 
         private bool processingPosts = true;
 

osu.Game/Overlays/NotificationOverlayToastTray.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using osu.Framework.Allocation;
@@ -23,6 +24,8 @@ namespace osu.Game.Overlays
     /// </summary>
     public class NotificationOverlayToastTray : CompositeDrawable
     {
+        public override bool IsPresent => toastContentBackground.Height > 0 || toastFlow.Count > 0;
+
         public bool IsDisplayingToasts => toastFlow.Count > 0;
 
         private FillFlowContainer<Notification> toastFlow = null!;
@@ -33,8 +36,12 @@ namespace osu.Game.Overlays
 
         public Action<Notification>? ForwardNotificationToPermanentStore { get; set; }
 
-        public int UnreadCount => toastFlow.Count(n => !n.WasClosed && !n.Read)
+        public int UnreadCount => allDisplayedNotifications.Count(n => !n.WasClosed && !n.Read);
-                                  + InternalChildren.OfType<Notification>().Count(n => !n.WasClosed && !n.Read);
+
+        /// <summary>
+        /// Notifications contained in the toast flow, or in a detached state while they animate during forwarding to the main overlay.
+        /// </summary>
+        private IEnumerable<Notification> allDisplayedNotifications => toastFlow.Concat(InternalChildren.OfType<Notification>());
 
         private int runningDepth;
 
@@ -55,6 +62,7 @@ namespace osu.Game.Overlays
                         colourProvider.Background6.Opacity(0.7f),
                         colourProvider.Background6.Opacity(0.5f)),
                     RelativeSizeAxes = Axes.Both,
+                    Height = 0,
                 }.WithEffect(new BlurEffect
                 {
                     PadExtent = true,
@@ -66,7 +74,7 @@ namespace osu.Game.Overlays
                     postEffectDrawable.AutoSizeAxes = Axes.None;
                     postEffectDrawable.RelativeSizeAxes = Axes.X;
                 })),
-                toastFlow = new AlwaysUpdateFillFlowContainer<Notification>
+                toastFlow = new FillFlowContainer<Notification>
                 {
                     LayoutDuration = 150,
                     LayoutEasing = Easing.OutQuart,
@@ -143,8 +151,8 @@ namespace osu.Game.Overlays
         {
             base.Update();
 
-            float height = toastFlow.DrawHeight + 120;
+            float height = toastFlow.Count > 0 ? toastFlow.DrawHeight + 120 : 0;
-            float alpha = IsDisplayingToasts ? MathHelper.Clamp(toastFlow.DrawHeight / 40, 0, 1) * toastFlow.Children.Max(n => n.Alpha) : 0;
+            float alpha = toastFlow.Count > 0 ? MathHelper.Clamp(toastFlow.DrawHeight / 41, 0, 1) * toastFlow.Children.Max(n => n.Alpha) : 0;
 
             toastContentBackground.Height = (float)Interpolation.DampContinuously(toastContentBackground.Height, height, 10, Clock.ElapsedFrameTime);
             toastContentBackground.Alpha = (float)Interpolation.DampContinuously(toastContentBackground.Alpha, alpha, 10, Clock.ElapsedFrameTime);

osu.Game/Overlays/Notifications/ProgressCompletionNotification.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 osu.Framework.Allocation;
 using osu.Game.Graphics;
 using osu.Framework.Graphics.Colour;

osu.Game/Overlays/Notifications/ProgressNotification.cs

@@ -71,7 +71,7 @@ namespace osu.Game.Overlays.Notifications
             base.LoadComplete();
 
             // we may have received changes before we were displayed.
-            updateState();
+            Scheduler.AddOnce(updateState);
         }
 
         private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
@@ -87,8 +87,8 @@ namespace osu.Game.Overlays.Notifications
 
                 state = value;
 
-                if (IsLoaded)
+                Scheduler.AddOnce(updateState);
-                    Schedule(updateState);
+                attemptPostCompletion();
             }
         }
 
@@ -141,11 +141,33 @@ namespace osu.Game.Overlays.Notifications
 
                 case ProgressNotificationState.Completed:
                     loadingSpinner.Hide();
-                    Completed();
+                    attemptPostCompletion();
+                    base.Close();
                     break;
             }
         }
 
+        private bool completionSent;
+
+        /// <summary>
+        /// Attempt to post a completion notification.
+        /// </summary>
+        private void attemptPostCompletion()
+        {
+            if (state != ProgressNotificationState.Completed) return;
+
+            // This notification may not have been posted yet (and thus may not have a target to post the completion to).
+            // Completion posting will be re-attempted in a scheduled invocation.
+            if (CompletionTarget == null)
+                return;
+
+            if (completionSent)
+                return;
+
+            CompletionTarget.Invoke(CreateCompletionNotification());
+            completionSent = true;
+        }
+
         private ProgressNotificationState state;
 
         protected virtual Notification CreateCompletionNotification() => new ProgressCompletionNotification
@@ -154,14 +176,10 @@ namespace osu.Game.Overlays.Notifications
             Text = CompletionText
         };
 
-        protected void Completed()
-        {
-            CompletionTarget?.Invoke(CreateCompletionNotification());
-            base.Close();
-        }
-
         public override bool DisplayOnTop => false;
 
+        public override bool IsImportant => false;
+
         private readonly ProgressBar progressBar;
         private Color4 colourQueued;
         private Color4 colourActive;