Redesign classes and generally improve code

osu.Desktop/Updater/SquirrelUpdateManager.cs

@@ -22,33 +22,25 @@ namespace osu.Desktop.Updater
 {
     public class SquirrelUpdateManager : osu.Game.Updater.UpdateManager
     {
-        public override bool CanCheckForUpdate => true;
-
         private UpdateManager updateManager;
         private NotificationOverlay notificationOverlay;
-        private OsuGameBase gameBase;
 
         public Task PrepareUpdateAsync() => UpdateManager.RestartAppWhenExited();
 
         private static readonly Logger logger = Logger.GetLogger("updater");
 
         [BackgroundDependencyLoader]
-        private void load(NotificationOverlay notification, OsuGameBase game)
+        private void load(NotificationOverlay notification)
         {
-            gameBase = game;
             notificationOverlay = notification;
 
             Splat.Locator.CurrentMutable.Register(() => new SquirrelLogger(), typeof(Splat.ILogger));
-            CheckForUpdate();
+            Schedule(() => Task.Run(CheckForUpdateAsync));
         }
 
-        public override void CheckForUpdate()
+        protected override async Task InternalCheckForUpdateAsync() => await checkForUpdateAsync();
-        {
-            if (gameBase.IsDeployedBuild)
-                Schedule(() => Task.Run(() => checkForUpdateAsync()));
-        }
 
-        private async void checkForUpdateAsync(bool useDeltaPatching = true, UpdateProgressNotification notification = null)
+        private async Task checkForUpdateAsync(bool useDeltaPatching = true, UpdateProgressNotification notification = null)
         {
             // should we schedule a retry on completion of this check?
             bool scheduleRecheck = true;
@@ -90,7 +82,7 @@ namespace osu.Desktop.Updater
 
                         // could fail if deltas are unavailable for full update path (https://github.com/Squirrel/Squirrel.Windows/issues/959)
                         // try again without deltas.
-                        checkForUpdateAsync(false, notification);
+                        await checkForUpdateAsync(false, notification);
                         scheduleRecheck = false;
                     }
                     else
@@ -109,7 +101,7 @@ namespace osu.Desktop.Updater
                 if (scheduleRecheck)
                 {
                     // check again in 30 minutes.
-                    Scheduler.AddDelayed(() => checkForUpdateAsync(), 60000 * 30);
+                    Scheduler.AddDelayed(async () => await checkForUpdateAsync(), 60000 * 30);
                 }
             }
         }

osu.Game/Overlays/Settings/Sections/General/UpdateSettings.cs

@@ -1,6 +1,7 @@
 // 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.
 
+using System.Threading.Tasks;
 using osu.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Platform;
@@ -28,15 +29,12 @@ namespace osu.Game.Overlays.Settings.Sections.General
             });
 
             // We should only display the button for UpdateManagers that do check for updates
-            if (updateManager?.CanCheckForUpdate == true)
+            Add(new SettingsButton
             {
-                Add(new SettingsButton
+                Text = "Check for updates",
-                {
+                Action = () => Schedule(() => Task.Run(updateManager.CheckForUpdateAsync)),
-                    Text = "Check for updates",
+                Enabled = { Value = updateManager.CanCheckForUpdate }
-                    Action = updateManager.CheckForUpdate,
+            });
-                    Enabled = { Value = game.IsDeployedBuild }
-                });
-            }
 
             if (RuntimeInfo.IsDesktop)
             {

osu.Game/Updater/SimpleUpdateManager.cs

@@ -19,31 +19,20 @@ namespace osu.Game.Updater
     /// </summary>
     public class SimpleUpdateManager : UpdateManager
     {
-        public override bool CanCheckForUpdate => true;
-
         private string version;
 
         [Resolved]
         private GameHost host { get; set; }
 
-        private OsuGameBase gameBase;
-
         [BackgroundDependencyLoader]
         private void load(OsuGameBase game)
         {
-            gameBase = game;
             version = game.Version;
 
-            CheckForUpdate();
+            Schedule(() => Task.Run(CheckForUpdateAsync));
         }
 
-        public override void CheckForUpdate()
+        protected override async Task InternalCheckForUpdateAsync()
-        {
-            if (gameBase.IsDeployedBuild)
-                Schedule(() => Task.Run(checkForUpdateAsync));
-        }
-
-        private async void checkForUpdateAsync()
         {
             try
             {

osu.Game/Updater/UpdateManager.cs

@@ -1,10 +1,10 @@
 // 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.
 
+using System.Threading.Tasks;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
-using osu.Framework.Logging;
 using osu.Game.Configuration;
 using osu.Game.Graphics;
 using osu.Game.Overlays;
@@ -18,9 +18,11 @@ namespace osu.Game.Updater
     public class UpdateManager : CompositeDrawable
     {
         /// <summary>
-        /// Whether this UpdateManager is capable of checking for updates.
+        /// Whether this UpdateManager should be or is capable of checking for updates.
         /// </summary>
-        public virtual bool CanCheckForUpdate => false;
+        public bool CanCheckForUpdate => game.IsDeployedBuild;
+
+        private string lastVersion;
 
         [Resolved]
         private OsuConfigManager config { get; set; }
@@ -35,24 +37,37 @@ namespace osu.Game.Updater
         {
             base.LoadComplete();
 
-            var version = game.Version;
+            Schedule(() => Task.Run(CheckForUpdateAsync));
-            var lastVersion = config.Get<string>(OsuSetting.Version);
 
-            if (game.IsDeployedBuild && version != lastVersion)
+            // debug / local compilations will reset to a non-release string.
+            // can be useful to check when an install has transitioned between release and otherwise (see OsuConfigManager's migrations).
+            config.Set(OsuSetting.Version, game.Version);
+        }
+
+        public async Task CheckForUpdateAsync()
+        {
+            if (!CanCheckForUpdate)
+                return;
+
+            await InternalCheckForUpdateAsync();
+        }
+
+        protected virtual Task InternalCheckForUpdateAsync()
+        {
+            // Query last version only *once*, so the user can re-check for updates, in case they closed the notification or else.
+            lastVersion ??= config.Get<string>(OsuSetting.Version);
+
+            var version = game.Version;
+
+            if (version != lastVersion)
             {
                 // only show a notification if we've previously saved a version to the config file (ie. not the first run).
                 if (!string.IsNullOrEmpty(lastVersion))
                     Notifications.Post(new UpdateCompleteNotification(version));
             }
 
-            // debug / local compilations will reset to a non-release string.
+            // we aren't doing any async in this method, so we return a completed task instead.
-            // can be useful to check when an install has transitioned between release and otherwise (see OsuConfigManager's migrations).
+            return Task.CompletedTask;
-            config.Set(OsuSetting.Version, version);
-        }
-
-        public virtual void CheckForUpdate()
-        {
-            Logger.Log("CheckForUpdate was called on the base class (UpdateManager)", LoggingTarget.Information);
         }
 
         private class UpdateCompleteNotification : SimpleNotification