From 9a663715cdb2fc5fee9e163b1d42ab8acfaf14e7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jun 2019 19:05:33 +0900 Subject: [PATCH 1/3] Make get methods of ProgressNotification return correct values immediately Previously they were only updated after the resultant schedule ran. This would not bode well when the overlay containing them is not present. --- .../Notifications/ProgressNotification.cs | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/osu.Game/Overlays/Notifications/ProgressNotification.cs b/osu.Game/Overlays/Notifications/ProgressNotification.cs index c8e081d29f..99836705c4 100644 --- a/osu.Game/Overlays/Notifications/ProgressNotification.cs +++ b/osu.Game/Overlays/Notifications/ProgressNotification.cs @@ -23,10 +23,16 @@ namespace osu.Game.Overlays.Notifications public string CompletionText { get; set; } = "Task has completed!"; + private float progress; + public float Progress { - get => progressBar.Progress; - set => Schedule(() => progressBar.Progress = value); + get => progress; + set + { + progress = value; + Scheduler.AddOnce(() => progressBar.Progress = progress); + } } protected override void LoadComplete() @@ -34,59 +40,56 @@ namespace osu.Game.Overlays.Notifications base.LoadComplete(); //we may have received changes before we were displayed. - State = state; + updateState(); } private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); public CancellationToken CancellationToken => cancellationTokenSource.Token; - public virtual ProgressNotificationState State + public ProgressNotificationState State { get => state; - set => - Schedule(() => - { - bool stateChanged = state != value; - state = value; + set + { + if (state == value) return; - if (IsLoaded) - { - switch (state) - { - case ProgressNotificationState.Queued: - Light.Colour = colourQueued; - Light.Pulsate = false; - progressBar.Active = false; - break; + state = value; - case ProgressNotificationState.Active: - Light.Colour = colourActive; - Light.Pulsate = true; - progressBar.Active = true; - break; + if (IsLoaded) + Schedule(updateState); + } + } - case ProgressNotificationState.Cancelled: - cancellationTokenSource.Cancel(); + private void updateState() + { + switch (state) + { + case ProgressNotificationState.Queued: + Light.Colour = colourQueued; + Light.Pulsate = false; + progressBar.Active = false; + break; - Light.Colour = colourCancelled; - Light.Pulsate = false; - progressBar.Active = false; - break; - } - } + case ProgressNotificationState.Active: + Light.Colour = colourActive; + Light.Pulsate = true; + progressBar.Active = true; + break; - if (stateChanged) - { - switch (state) - { - case ProgressNotificationState.Completed: - NotificationContent.MoveToY(-DrawSize.Y / 2, 200, Easing.OutQuint); - this.FadeOut(200).Finally(d => Completed()); - break; - } - } - }); + case ProgressNotificationState.Cancelled: + cancellationTokenSource.Cancel(); + + Light.Colour = colourCancelled; + Light.Pulsate = false; + progressBar.Active = false; + break; + + case ProgressNotificationState.Completed: + NotificationContent.MoveToY(-DrawSize.Y / 2, 200, Easing.OutQuint); + this.FadeOut(200).Finally(d => Completed()); + break; + } } private ProgressNotificationState state; From 21a1fd738b549d6e9c9d7487b63024377b4d28f3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jun 2019 19:06:21 +0900 Subject: [PATCH 2/3] Remove the necessity for NotificationOverlay to always be present Now it will only become present when there is a pending notification. --- .../TestSceneNotificationOverlay.cs | 218 ++++++++++++------ osu.Game/Overlays/NotificationOverlay.cs | 9 +- 2 files changed, 153 insertions(+), 74 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs index 6b7427cef5..7cab21f157 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs @@ -18,9 +18,6 @@ namespace osu.Game.Tests.Visual.UserInterface [TestFixture] public class TestSceneNotificationOverlay : OsuTestScene { - private readonly NotificationOverlay manager; - private readonly List progressingNotifications = new List(); - public override IReadOnlyList RequiredTypes => new[] { typeof(NotificationSection), @@ -31,25 +28,33 @@ namespace osu.Game.Tests.Visual.UserInterface typeof(Notification) }; - public TestSceneNotificationOverlay() + private NotificationOverlay notificationOverlay; + + private readonly List progressingNotifications = new List(); + + private SpriteText displayedCount; + + [SetUp] + public void SetUp() => Schedule(() => { progressingNotifications.Clear(); - Content.Add(manager = new NotificationOverlay + Content.Children = new Drawable[] { - Anchor = Anchor.TopRight, - Origin = Anchor.TopRight - }); + notificationOverlay = new NotificationOverlay + { + Anchor = Anchor.TopRight, + Origin = Anchor.TopRight + }, + displayedCount = new OsuSpriteText() + }; - SpriteText displayedCount = new OsuSpriteText(); - - Content.Add(displayedCount); - - void setState(Visibility state) => AddStep(state.ToString(), () => manager.State.Value = state); - void checkProgressingCount(int expected) => AddAssert($"progressing count is {expected}", () => progressingNotifications.Count == expected); - - manager.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count.NewValue}"; }; + notificationOverlay.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count.NewValue}"; }; + }); + [Test] + public void TestBasicFlow() + { setState(Visibility.Visible); AddStep(@"simple #1", sendHelloNotification); AddStep(@"simple #2", sendAmazingNotification); @@ -61,6 +66,7 @@ namespace osu.Game.Tests.Visual.UserInterface setState(Visibility.Hidden); AddRepeatStep(@"add many simple", sendManyNotifications, 3); + AddWaitStep("wait some", 5); checkProgressingCount(0); @@ -69,17 +75,122 @@ namespace osu.Game.Tests.Visual.UserInterface checkProgressingCount(1); - AddAssert("Displayed count is 33", () => manager.UnreadCount.Value == 33); + checkDisplayedCount(33); AddWaitStep("wait some", 10); checkProgressingCount(0); - - setState(Visibility.Visible); - - //AddStep(@"barrage", () => sendBarrage()); } + [Test] + public void TestImportantWhileClosed() + { + AddStep(@"simple #1", sendHelloNotification); + + AddAssert("Is visible", () => notificationOverlay.State.Value == Visibility.Visible); + + checkDisplayedCount(1); + + AddStep(@"progress #1", sendUploadProgress); + AddStep(@"progress #2", sendDownloadProgress); + + checkDisplayedCount(3); + + checkProgressingCount(2); + } + + [Test] + public void TestUnimportantWhileClosed() + { + AddStep(@"background #1", sendBackgroundNotification); + + AddAssert("Is not visible", () => notificationOverlay.State.Value == Visibility.Hidden); + + checkDisplayedCount(1); + + AddStep(@"background progress #1", sendBackgroundUploadProgress); + + AddWaitStep("wait some", 5); + + checkProgressingCount(0); + + checkDisplayedCount(3); + + AddStep(@"simple #1", sendHelloNotification); + + checkDisplayedCount(3); + } + + [Test] + public void TestSpam() + { + setState(Visibility.Visible); + AddStep("send barrage", () => sendBarrage()); + } + + protected override void Update() + { + base.Update(); + + progressingNotifications.RemoveAll(n => n.State == ProgressNotificationState.Completed); + + if (progressingNotifications.Count(n => n.State == ProgressNotificationState.Active) < 3) + { + var p = progressingNotifications.Find(n => n.State == ProgressNotificationState.Queued); + + if (p != null) + p.State = ProgressNotificationState.Active; + } + + foreach (var n in progressingNotifications.FindAll(n => n.State == ProgressNotificationState.Active)) + { + if (n.Progress < 1) + n.Progress += (float)(Time.Elapsed / 400) * RNG.NextSingle(); + else + n.State = ProgressNotificationState.Completed; + } + } + + private void checkDisplayedCount(int expected) => + AddAssert($"Displayed count is {expected}", () => notificationOverlay.UnreadCount.Value == expected); + + private void sendDownloadProgress() + { + var n = new ProgressNotification + { + Text = @"Downloading Haitai...", + CompletionText = "Downloaded Haitai!", + }; + notificationOverlay.Post(n); + progressingNotifications.Add(n); + } + + private void sendUploadProgress() + { + var n = new ProgressNotification + { + Text = @"Uploading to BSS...", + CompletionText = "Uploaded to BSS!", + }; + notificationOverlay.Post(n); + progressingNotifications.Add(n); + } + + private void sendBackgroundUploadProgress() + { + var n = new BackgroundProgressNotification + { + Text = @"Uploading to BSS...", + CompletionText = "Uploaded to BSS!", + }; + notificationOverlay.Post(n); + progressingNotifications.Add(n); + } + + private void setState(Visibility state) => AddStep(state.ToString(), () => notificationOverlay.State.Value = state); + + private void checkProgressingCount(int expected) => AddAssert($"progressing count is {expected}", () => progressingNotifications.Count == expected); + private void sendBarrage(int remaining = 10) { switch (RNG.Next(0, 4)) @@ -105,64 +216,35 @@ namespace osu.Game.Tests.Visual.UserInterface Scheduler.AddDelayed(() => sendBarrage(remaining - 1), 80); } - protected override void Update() - { - base.Update(); - - progressingNotifications.RemoveAll(n => n.State == ProgressNotificationState.Completed); - - if (progressingNotifications.Count(n => n.State == ProgressNotificationState.Active) < 3) - { - var p = progressingNotifications.Find(n => n.State == ProgressNotificationState.Queued); - if (p != null) - p.State = ProgressNotificationState.Active; - } - - foreach (var n in progressingNotifications.FindAll(n => n.State == ProgressNotificationState.Active)) - { - if (n.Progress < 1) - n.Progress += (float)(Time.Elapsed / 400) * RNG.NextSingle(); - else - n.State = ProgressNotificationState.Completed; - } - } - - private void sendDownloadProgress() - { - var n = new ProgressNotification - { - Text = @"Downloading Haitai...", - CompletionText = "Downloaded Haitai!", - }; - manager.Post(n); - progressingNotifications.Add(n); - } - - private void sendUploadProgress() - { - var n = new ProgressNotification - { - Text = @"Uploading to BSS...", - CompletionText = "Uploaded to BSS!", - }; - manager.Post(n); - progressingNotifications.Add(n); - } - private void sendAmazingNotification() { - manager.Post(new SimpleNotification { Text = @"You are amazing" }); + notificationOverlay.Post(new SimpleNotification { Text = @"You are amazing" }); } private void sendHelloNotification() { - manager.Post(new SimpleNotification { Text = @"Welcome to osu!. Enjoy your stay!" }); + notificationOverlay.Post(new SimpleNotification { Text = @"Welcome to osu!. Enjoy your stay!" }); + } + + private void sendBackgroundNotification() + { + notificationOverlay.Post(new BackgroundNotification { Text = @"Welcome to osu!. Enjoy your stay!" }); } private void sendManyNotifications() { for (int i = 0; i < 10; i++) - manager.Post(new SimpleNotification { Text = @"Spam incoming!!" }); + notificationOverlay.Post(new SimpleNotification { Text = @"Spam incoming!!" }); + } + + private class BackgroundNotification : SimpleNotification + { + public override bool IsImportant => false; + } + + private class BackgroundProgressNotification : ProgressNotification + { + public override bool IsImportant => false; } } } diff --git a/osu.Game/Overlays/NotificationOverlay.cs b/osu.Game/Overlays/NotificationOverlay.cs index 2e4c504645..320aa707e2 100644 --- a/osu.Game/Overlays/NotificationOverlay.cs +++ b/osu.Game/Overlays/NotificationOverlay.cs @@ -35,8 +35,6 @@ namespace osu.Game.Overlays Width = width; RelativeSizeAxes = Axes.Y; - AlwaysPresent = true; - Children = new Drawable[] { new Box @@ -100,9 +98,6 @@ namespace osu.Game.Overlays OverlayActivationMode.BindValueChanged(_ => updateProcessingMode(), true); } - private int totalCount => sections.Select(c => c.DisplayedCount).Sum(); - private int unreadCount => sections.Select(c => c.UnreadCount).Sum(); - public readonly BindableInt UnreadCount = new BindableInt(); private int runningDepth; @@ -111,6 +106,8 @@ namespace osu.Game.Overlays private readonly Scheduler postScheduler = new Scheduler(); + public override bool IsPresent => base.IsPresent || postScheduler.HasPendingTasks; + private bool processingPosts = true; public void Post(Notification notification) => postScheduler.Add(() => @@ -160,7 +157,7 @@ namespace osu.Game.Overlays private void updateCounts() { - UnreadCount.Value = unreadCount; + UnreadCount.Value = sections.Select(c => c.UnreadCount).Sum(); } private void markAllRead() From 8c383ddc2735996b09874303f506a8f71e01dfae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jun 2019 19:23:27 +0900 Subject: [PATCH 3/3] Fix dodgy test scheduling to other tests --- .../UserInterface/TestSceneNotificationOverlay.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs index 7cab21f157..d8a4514df1 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs @@ -94,9 +94,8 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep(@"progress #1", sendUploadProgress); AddStep(@"progress #2", sendDownloadProgress); - checkDisplayedCount(3); - checkProgressingCount(2); + checkDisplayedCount(3); } [Test] @@ -114,7 +113,7 @@ namespace osu.Game.Tests.Visual.UserInterface checkProgressingCount(0); - checkDisplayedCount(3); + checkDisplayedCount(2); AddStep(@"simple #1", sendHelloNotification); @@ -125,7 +124,7 @@ namespace osu.Game.Tests.Visual.UserInterface public void TestSpam() { setState(Visibility.Visible); - AddStep("send barrage", () => sendBarrage()); + AddRepeatStep("send barrage", sendBarrage, 10); } protected override void Update() @@ -191,7 +190,7 @@ namespace osu.Game.Tests.Visual.UserInterface private void checkProgressingCount(int expected) => AddAssert($"progressing count is {expected}", () => progressingNotifications.Count == expected); - private void sendBarrage(int remaining = 10) + private void sendBarrage() { switch (RNG.Next(0, 4)) { @@ -211,9 +210,6 @@ namespace osu.Game.Tests.Visual.UserInterface sendDownloadProgress(); break; } - - if (remaining > 0) - Scheduler.AddDelayed(() => sendBarrage(remaining - 1), 80); } private void sendAmazingNotification()