Merge pull request #1068 from peppy/fix-notification-threading

Fix cross-thread notification posting causing a hard crash

osu.Desktop.VisualTests/Tests/TestCaseNotificationOverlay.cs

@@ -12,17 +12,17 @@ using osu.Framework.Graphics.Containers;
 
 namespace osu.Desktop.VisualTests.Tests
 {
-    internal class TestCaseNotificationManager : TestCase
+    internal class TestCaseNotificationOverlay : TestCase
     {
         public override string Description => @"I handle notifications";
 
-        private readonly NotificationManager manager;
+        private readonly NotificationOverlay manager;
 
-        public TestCaseNotificationManager()
+        public TestCaseNotificationOverlay()
         {
             progressingNotifications.Clear();
 
-            Content.Add(manager = new NotificationManager
+            Content.Add(manager = new NotificationOverlay
             {
                 Anchor = Anchor.TopRight,
                 Origin = Anchor.TopRight,

osu.Desktop.VisualTests/osu.Desktop.VisualTests.csproj

@@ -198,7 +198,7 @@
     <Compile Include="Tests\TestCaseManiaPlayfield.cs" />
     <Compile Include="Tests\TestCaseMenuOverlays.cs" />
     <Compile Include="Tests\TestCaseMusicController.cs" />
-    <Compile Include="Tests\TestCaseNotificationManager.cs" />
+    <Compile Include="Tests\TestCaseNotificationOverlay.cs" />
     <Compile Include="Tests\TestCaseOnScreenDisplay.cs" />
     <Compile Include="Tests\TestCaseReplaySettingsOverlay.cs" />
     <Compile Include="Tests\TestCasePlayer.cs" />

osu.Desktop/Overlays/VersionManager.cs

@@ -25,16 +25,16 @@ namespace osu.Desktop.Overlays
     public class VersionManager : OverlayContainer
     {
         private UpdateManager updateManager;
-        private NotificationManager notificationManager;
+        private NotificationOverlay notificationOverlay;
 
         protected override bool HideOnEscape => false;
 
         public override bool HandleInput => false;
 
         [BackgroundDependencyLoader]
-        private void load(NotificationManager notification, OsuColour colours, TextureStore textures, OsuGameBase game)
+        private void load(NotificationOverlay notification, OsuColour colours, TextureStore textures, OsuGameBase game)
         {
-            notificationManager = notification;
+            notificationOverlay = notification;
 
             AutoSizeAxes = Axes.Both;
             Anchor = Anchor.BottomCentre;
@@ -116,7 +116,7 @@ namespace osu.Desktop.Overlays
                 if (notification == null)
                 {
                     notification = new UpdateProgressNotification { State = ProgressNotificationState.Active };
-                    Schedule(() => notificationManager.Post(notification));
+                    Schedule(() => notificationOverlay.Post(notification));
                 }
 
                 Schedule(() =>

osu.Game/Online/API/APIAccess.cs

@@ -117,7 +117,7 @@ namespace osu.Game.Online.API
                         if (!authentication.HasValidAccessToken && !authentication.AuthenticateWithLogin(Username, Password))
                         {
                             //todo: this fails even on network-related issues. we should probably handle those differently.
-                            //NotificationManager.ShowMessage("Login failed!");
+                            //NotificationOverlay.ShowMessage("Login failed!");
                             log.Add(@"Login failed!");
                             Password = null;
                             continue;
@@ -254,7 +254,7 @@ namespace osu.Game.Online.API
                 {
                     //OsuGame.Scheduler.Add(delegate
                     {
-                        //NotificationManager.ShowMessage($@"We just went {newState}!", newState == APIState.Online ? Color4.YellowGreen : Color4.OrangeRed, 5000);
+                        //NotificationOverlay.ShowMessage($@"We just went {newState}!", newState == APIState.Online ? Color4.YellowGreen : Color4.OrangeRed, 5000);
                         log.Add($@"We just went {newState}!");
                         Scheduler.Add(delegate
                         {

osu.Game/OsuGame.cs

@@ -37,7 +37,7 @@ namespace osu.Game
 
         private MusicController musicController;
 
-        private NotificationManager notificationManager;
+        private NotificationOverlay notificationOverlay;
 
         private DialogOverlay dialogOverlay;
 
@@ -132,7 +132,7 @@ namespace osu.Game
 
             if (s.Beatmap == null)
             {
-                notificationManager.Post(new SimpleNotification
+                notificationOverlay.Post(new SimpleNotification
                 {
                     Text = @"Tried to load a score for a beatmap we don't have!",
                     Icon = FontAwesome.fa_life_saver,
@@ -189,7 +189,7 @@ namespace osu.Game
                 Origin = Anchor.TopRight,
             }, overlayContent.Add);
 
-            LoadComponentAsync(notificationManager = new NotificationManager
+            LoadComponentAsync(notificationOverlay = new NotificationOverlay
             {
                 Depth = -3,
                 Anchor = Anchor.TopRight,
@@ -205,7 +205,7 @@ namespace osu.Game
             {
                 if (entry.Level < LogLevel.Important) return;
 
-                notificationManager.Post(new SimpleNotification
+                notificationOverlay.Post(new SimpleNotification
                 {
                     Text = $@"{entry.Level}: {entry.Message}"
                 });
@@ -216,7 +216,7 @@ namespace osu.Game
             dependencies.Cache(chat);
             dependencies.Cache(userProfile);
             dependencies.Cache(musicController);
-            dependencies.Cache(notificationManager);
+            dependencies.Cache(notificationOverlay);
             dependencies.Cache(dialogOverlay);
 
             // ensure both overlays aren't presented at the same time

osu.Game/Overlays/NotificationOverlay.cs

@@ -13,7 +13,7 @@ using osu.Game.Graphics.Containers;
 
 namespace osu.Game.Overlays
 {
-    public class NotificationManager : OsuFocusedOverlayContainer
+    public class NotificationOverlay : OsuFocusedOverlayContainer
     {
         private const float width = 320;
 
@@ -28,6 +28,8 @@ namespace osu.Game.Overlays
             Width = width;
             RelativeSizeAxes = Axes.Y;
 
+            AlwaysPresent = true;
+
             Children = new Drawable[]
             {
                 new Box
@@ -71,6 +73,8 @@ namespace osu.Game.Overlays
         private int runningDepth;
 
         public void Post(Notification notification)
+        {
+            Schedule(() =>
             {
                 State = Visibility.Visible;
 
@@ -83,6 +87,7 @@ namespace osu.Game.Overlays
 
                 var ourType = notification.GetType();
                 sections.Children.FirstOrDefault(s => s.AcceptTypes.Any(accept => accept.IsAssignableFrom(ourType)))?.Add(notification);
+            });
         }
 
         protected override void PopIn()

osu.Game/Overlays/Toolbar/ToolbarNotificationButton.cs

@@ -19,9 +19,9 @@ namespace osu.Game.Overlays.Toolbar
         }
 
         [BackgroundDependencyLoader]
-        private void load(NotificationManager notificationManager)
+        private void load(NotificationOverlay notificationOverlay)
         {
-            StateContainer = notificationManager;
+            StateContainer = notificationOverlay;
         }
     }
 }

osu.Game/Screens/Play/HUDOverlay.cs

@@ -61,7 +61,7 @@ namespace osu.Game.Screens.Play
         }
 
         [BackgroundDependencyLoader(true)]
-        private void load(OsuConfigManager config, NotificationManager notificationManager, OsuColour colours)
+        private void load(OsuConfigManager config, NotificationOverlay notificationOverlay, OsuColour colours)
         {
             showHud = config.GetBindable<bool>(OsuSetting.ShowInterface);
             showHud.ValueChanged += hudVisibility => content.FadeTo(hudVisibility ? 1 : 0, duration);
@@ -71,7 +71,7 @@ namespace osu.Game.Screens.Play
             {
                 hasShownNotificationOnce = true;
 
-                notificationManager?.Post(new SimpleNotification
+                notificationOverlay?.Post(new SimpleNotification
                 {
                     Text = @"The score overlay is currently disabled. You can toggle this by pressing Shift+Tab."
                 });

osu.Game/osu.Game.csproj

@@ -250,7 +250,7 @@
     <Compile Include="Configuration\OsuConfigManager.cs" />
     <Compile Include="Overlays\Notifications\IHasCompletionTarget.cs" />
     <Compile Include="Overlays\Notifications\Notification.cs" />
-    <Compile Include="Overlays\NotificationManager.cs" />
+    <Compile Include="Overlays\NotificationOverlay.cs" />
     <Compile Include="Overlays\Notifications\NotificationSection.cs" />
     <Compile Include="Overlays\Notifications\ProgressCompletionNotification.cs" />
     <Compile Include="Overlays\Notifications\ProgressNotification.cs" />