Merge pull request #1777 from peppy/fix-notification-scheduled-events

Fix notification overlay layout and scheduled tasks being delayed

osu.Game.Tests/Visual/TestCaseNotificationOverlay.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.MathUtils;
 using osu.Game.Overlays;
@@ -19,11 +20,12 @@ public class TestCaseNotificationOverlay : OsuTestCase
 
         public override IReadOnlyList<Type> RequiredTypes => new[]
         {
-            typeof(Notification),
+            typeof(NotificationSection),
+            typeof(SimpleNotification),
             typeof(ProgressNotification),
             typeof(ProgressCompletionNotification),
-            typeof(SimpleNotification),
             typeof(IHasCompletionTarget),
+            typeof(Notification)
         };
 
         public TestCaseNotificationOverlay()
@@ -40,17 +42,44 @@ public TestCaseNotificationOverlay()
 
             Content.Add(displayedCount);
 
+            void setState(Visibility state) => AddStep(state.ToString(), () => manager.State = state);
+            void checkProgressingCount(int expected) => AddAssert($"progressing count is {expected}", () => progressingNotifications.Count == expected);
+
             manager.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count}"; };
 
-            AddStep(@"toggle", manager.ToggleVisibility);
+
+            setState(Visibility.Visible);
             AddStep(@"simple #1", sendHelloNotification);
             AddStep(@"simple #2", sendAmazingNotification);
             AddStep(@"progress #1", sendUploadProgress);
             AddStep(@"progress #2", sendDownloadProgress);
-            AddStep(@"barrage", () => sendBarrage());
+
+            checkProgressingCount(2);
+
+            setState(Visibility.Hidden);
+
+            AddRepeatStep(@"add many simple", sendManyNotifications, 3);
+            AddWaitStep(5);
+
+            checkProgressingCount(0);
+
+            AddStep(@"progress #3", sendUploadProgress);
+
+            checkProgressingCount(1);
+
+            AddAssert("Displayed count is 33", () => manager.UnreadCount.Value == 33);
+
+            AddWaitStep(10);
+
+            checkProgressingCount(0);
+
+
+            setState(Visibility.Visible);
+
+            //AddStep(@"barrage", () => sendBarrage());
         }
 
-        private void sendBarrage(int remaining = 100)
+        private void sendBarrage(int remaining = 10)
         {
             switch (RNG.Next(0, 4))
             {
@@ -80,7 +109,7 @@ protected override void Update()
 
             if (progressingNotifications.Count(n => n.State == ProgressNotificationState.Active) < 3)
             {
-                var p = progressingNotifications.FirstOrDefault(n => n.IsAlive && n.State == ProgressNotificationState.Queued);
+                var p = progressingNotifications.FirstOrDefault(n => n.State == ProgressNotificationState.Queued);
                 if (p != null)
                     p.State = ProgressNotificationState.Active;
             }
@@ -88,7 +117,7 @@ protected override void Update()
             foreach (var n in progressingNotifications.FindAll(n => n.State == ProgressNotificationState.Active))
             {
                 if (n.Progress < 1)
-                    n.Progress += (float)(Time.Elapsed / 2000) * RNG.NextSingle();
+                    n.Progress += (float)(Time.Elapsed / 400) * RNG.NextSingle();
                 else
                     n.State = ProgressNotificationState.Completed;
             }
@@ -125,5 +154,11 @@ private void sendHelloNotification()
         {
             manager.Post(new SimpleNotification { Text = @"Welcome to osu!. Enjoy your stay!" });
         }
+
+        private void sendManyNotifications()
+        {
+            for (int i = 0; i < 10; i++)
+                manager.Post(new SimpleNotification { Text = @"Spam incoming!!" });
+        }
     }
 }

osu.Game/Overlays/Notifications/NotificationSection.cs

@@ -15,7 +15,7 @@
 
 namespace osu.Game.Overlays.Notifications
 {
-    public class NotificationSection : FillFlowContainer
+    public class NotificationSection : AlwaysUpdateFillFlowContainer<Drawable>
     {
         private OsuSpriteText titleText;
         private OsuSpriteText countText;
@@ -33,6 +33,7 @@ public class NotificationSection : FillFlowContainer
         public IEnumerable<Type> AcceptTypes;
 
         private string clearText;
+
         public string ClearText
         {
             get { return clearText; }
@@ -110,7 +111,7 @@ private void load(OsuColour colours)
                         },
                     },
                 },
-                notifications = new FillFlowContainer<Notification>
+                notifications = new AlwaysUpdateFillFlowContainer<Notification>
                 {
                     AutoSizeAxes = Axes.Y,
                     RelativeSizeAxes = Axes.X,
@@ -159,4 +160,13 @@ public void MarkAllRead()
             notifications?.Children.ForEach(n => n.Read = true);
         }
     }
+
+    public class AlwaysUpdateFillFlowContainer<T> : FillFlowContainer<T>
+        where T : Drawable
+    {
+        // this is required to ensure correct layout and scheduling on children.
+        // the layout portion of this is being tracked as a framework issue (https://github.com/ppy/osu-framework/issues/1297).
+        protected override bool RequiresChildrenUpdate => true;
+    }
+
 }