Merge branch 'master' into beatmap-info

osu.Game/Online/HubClient.cs

@@ -0,0 +1,28 @@
+// 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;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.SignalR.Client;
+
+namespace osu.Game.Online
+{
+    public class HubClient : PersistentEndpointClient
+    {
+        public readonly HubConnection Connection;
+
+        public HubClient(HubConnection connection)
+        {
+            Connection = connection;
+            Connection.Closed += InvokeClosed;
+        }
+
+        public override Task ConnectAsync(CancellationToken cancellationToken) => Connection.StartAsync(cancellationToken);
+
+        public override async ValueTask DisposeAsync()
+        {
+            await base.DisposeAsync().ConfigureAwait(false);
+            await Connection.DisposeAsync().ConfigureAwait(false);
+        }
+    }
+}

osu.Game/Online/HubClientConnector.cs

@@ -10,13 +10,11 @@ using Microsoft.AspNetCore.SignalR.Client;
 using Microsoft.Extensions.DependencyInjection;
 using Newtonsoft.Json;
 using osu.Framework;
-using osu.Framework.Bindables;
-using osu.Framework.Logging;
 using osu.Game.Online.API;
 
 namespace osu.Game.Online
 {
-    public class HubClientConnector : IHubClientConnector
+    public class HubClientConnector : PersistentEndpointClientConnector, IHubClientConnector
     {
         public const string SERVER_SHUTDOWN_MESSAGE = "Server is shutting down.";
 
@@ -25,7 +23,6 @@ namespace osu.Game.Online
         /// </summary>
         public Action<HubConnection>? ConfigureConnection { get; set; }
 
-        private readonly string clientName;
         private readonly string endpoint;
         private readonly string versionHash;
         private readonly bool preferMessagePack;
@@ -34,18 +31,7 @@ namespace osu.Game.Online
         /// <summary>
         /// The current connection opened by this connector.
         /// </summary>
-        public HubConnection? CurrentConnection { get; private set; }
-
-        /// <summary>
-        /// Whether this is connected to the hub, use <see cref="CurrentConnection"/> to access the connection, if this is <c>true</c>.
-        /// </summary>
-        public IBindable<bool> IsConnected => isConnected;
-
-        private readonly Bindable<bool> isConnected = new Bindable<bool>();
-        private readonly SemaphoreSlim connectionLock = new SemaphoreSlim(1);
-        private CancellationTokenSource connectCancelSource = new CancellationTokenSource();
-
-        private readonly IBindable<APIState> apiState = new Bindable<APIState>();
+        public new HubConnection? CurrentConnection => ((HubClient?)base.CurrentConnection)?.Connection;
 
         /// <summary>
         /// Constructs a new <see cref="HubClientConnector"/>.
@@ -56,99 +42,16 @@ namespace osu.Game.Online
         /// <param name="versionHash">The hash representing the current game version, used for verification purposes.</param>
         /// <param name="preferMessagePack">Whether to use MessagePack for serialisation if available on this platform.</param>
         public HubClientConnector(string clientName, string endpoint, IAPIProvider api, string versionHash, bool preferMessagePack = true)
+            : base(api)
         {
-            this.clientName = clientName;
+            ClientName = clientName;
             this.endpoint = endpoint;
             this.api = api;
             this.versionHash = versionHash;
             this.preferMessagePack = preferMessagePack;
-
-            apiState.BindTo(api.State);
-            apiState.BindValueChanged(_ => Task.Run(connectIfPossible), true);
         }
 
-        public Task Reconnect()
-        {
-            Logger.Log($"{clientName} reconnecting...", LoggingTarget.Network);
-            return Task.Run(connectIfPossible);
-        }
-
-        private async Task connectIfPossible()
-        {
-            switch (apiState.Value)
-            {
-                case APIState.Failing:
-                case APIState.Offline:
-                    await disconnect(true);
-                    break;
-
-                case APIState.Online:
-                    await connect();
-                    break;
-            }
-        }
-
-        private async Task connect()
-        {
-            cancelExistingConnect();
-
-            if (!await connectionLock.WaitAsync(10000).ConfigureAwait(false))
-                throw new TimeoutException("Could not obtain a lock to connect. A previous attempt is likely stuck.");
-
-            try
-            {
-                while (apiState.Value == APIState.Online)
-                {
-                    // ensure any previous connection was disposed.
-                    // this will also create a new cancellation token source.
-                    await disconnect(false).ConfigureAwait(false);
-
-                    // this token will be valid for the scope of this connection.
-                    // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere.
-                    var cancellationToken = connectCancelSource.Token;
-
-                    cancellationToken.ThrowIfCancellationRequested();
-
-                    Logger.Log($"{clientName} connecting...", LoggingTarget.Network);
-
-                    try
-                    {
-                        // importantly, rebuild the connection each attempt to get an updated access token.
-                        CurrentConnection = buildConnection(cancellationToken);
-
-                        await CurrentConnection.StartAsync(cancellationToken).ConfigureAwait(false);
-
-                        Logger.Log($"{clientName} connected!", LoggingTarget.Network);
-                        isConnected.Value = true;
-                        return;
-                    }
-                    catch (OperationCanceledException)
-                    {
-                        //connection process was cancelled.
-                        throw;
-                    }
-                    catch (Exception e)
-                    {
-                        await handleErrorAndDelay(e, cancellationToken).ConfigureAwait(false);
-                    }
-                }
-            }
-            finally
-            {
-                connectionLock.Release();
-            }
-        }
-
-        /// <summary>
-        /// Handles an exception and delays an async flow.
-        /// </summary>
-        private async Task handleErrorAndDelay(Exception exception, CancellationToken cancellationToken)
-        {
-            Logger.Log($"{clientName} connect attempt failed: {exception.Message}", LoggingTarget.Network);
-            await Task.Delay(5000, cancellationToken).ConfigureAwait(false);
-        }
-
-        private HubConnection buildConnection(CancellationToken cancellationToken)
+        protected override Task<PersistentEndpointClient> BuildConnectionAsync(CancellationToken cancellationToken)
         {
             var builder = new HubConnectionBuilder()
                 .WithUrl(endpoint, options =>
@@ -188,59 +91,9 @@ namespace osu.Game.Online
 
             ConfigureConnection?.Invoke(newConnection);
 
-            newConnection.Closed += ex => onConnectionClosed(ex, cancellationToken);
-            return newConnection;
+            return Task.FromResult((PersistentEndpointClient)new HubClient(newConnection));
         }
 
-        private async Task onConnectionClosed(Exception? ex, CancellationToken cancellationToken)
-        {
-            isConnected.Value = false;
-
-            if (ex != null)
-                await handleErrorAndDelay(ex, cancellationToken).ConfigureAwait(false);
-            else
-                Logger.Log($"{clientName} disconnected", LoggingTarget.Network);
-
-            // make sure a disconnect wasn't triggered (and this is still the active connection).
-            if (!cancellationToken.IsCancellationRequested)
-                await Task.Run(connect, default).ConfigureAwait(false);
-        }
-
-        private async Task disconnect(bool takeLock)
-        {
-            cancelExistingConnect();
-
-            if (takeLock)
-            {
-                if (!await connectionLock.WaitAsync(10000).ConfigureAwait(false))
-                    throw new TimeoutException("Could not obtain a lock to disconnect. A previous attempt is likely stuck.");
-            }
-
-            try
-            {
-                if (CurrentConnection != null)
-                    await CurrentConnection.DisposeAsync().ConfigureAwait(false);
-            }
-            finally
-            {
-                CurrentConnection = null;
-                if (takeLock)
-                    connectionLock.Release();
-            }
-        }
-
-        private void cancelExistingConnect()
-        {
-            connectCancelSource.Cancel();
-            connectCancelSource = new CancellationTokenSource();
-        }
-
-        public override string ToString() => $"Connector for {clientName} ({(IsConnected.Value ? "connected" : "not connected")}";
-
-        public void Dispose()
-        {
-            apiState.UnbindAll();
-            cancelExistingConnect();
-        }
+        protected override string ClientName { get; }
     }
 }

osu.Game/Online/PersistentEndpointClient.cs

@@ -0,0 +1,35 @@
+// 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;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace osu.Game.Online
+{
+    public abstract class PersistentEndpointClient : IAsyncDisposable
+    {
+        /// <summary>
+        /// An event notifying the <see cref="PersistentEndpointClientConnector"/> that the connection has been closed
+        /// </summary>
+        public event Func<Exception?, Task>? Closed;
+
+        /// <summary>
+        /// Notifies the <see cref="PersistentEndpointClientConnector"/> that the connection has been closed.
+        /// </summary>
+        /// <param name="exception">The exception that the connection closed with.</param>
+        protected Task InvokeClosed(Exception? exception) => Closed?.Invoke(exception) ?? Task.CompletedTask;
+
+        /// <summary>
+        /// Connects the client to the remote service to begin processing messages.
+        /// </summary>
+        /// <param name="cancellationToken">A cancellation token to stop processing messages.</param>
+        public abstract Task ConnectAsync(CancellationToken cancellationToken);
+
+        public virtual ValueTask DisposeAsync()
+        {
+            Closed = null;
+            return new ValueTask(Task.CompletedTask);
+        }
+    }
+}

osu.Game/Online/PersistentEndpointClientConnector.cs

@@ -0,0 +1,198 @@
+// 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;
+using System.Threading;
+using System.Threading.Tasks;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions.TypeExtensions;
+using osu.Framework.Logging;
+using osu.Game.Online.API;
+
+namespace osu.Game.Online
+{
+    public abstract class PersistentEndpointClientConnector : IDisposable
+    {
+        /// <summary>
+        /// Whether the managed connection is currently connected. When <c>true</c> use <see cref="CurrentConnection"/> to access the connection.
+        /// </summary>
+        public IBindable<bool> IsConnected => isConnected;
+
+        /// <summary>
+        /// The current connection opened by this connector.
+        /// </summary>
+        public PersistentEndpointClient? CurrentConnection { get; private set; }
+
+        private readonly Bindable<bool> isConnected = new Bindable<bool>();
+        private readonly SemaphoreSlim connectionLock = new SemaphoreSlim(1);
+        private CancellationTokenSource connectCancelSource = new CancellationTokenSource();
+
+        private readonly IBindable<APIState> apiState = new Bindable<APIState>();
+
+        /// <summary>
+        /// Constructs a new <see cref="PersistentEndpointClientConnector"/>.
+        /// </summary>
+        /// <param name="api"> An API provider used to react to connection state changes.</param>
+        protected PersistentEndpointClientConnector(IAPIProvider api)
+        {
+            apiState.BindTo(api.State);
+            apiState.BindValueChanged(_ => Task.Run(connectIfPossible), true);
+        }
+
+        public Task Reconnect()
+        {
+            Logger.Log($"{ClientName} reconnecting...", LoggingTarget.Network);
+            return Task.Run(connectIfPossible);
+        }
+
+        private async Task connectIfPossible()
+        {
+            switch (apiState.Value)
+            {
+                case APIState.Failing:
+                case APIState.Offline:
+                    await disconnect(true);
+                    break;
+
+                case APIState.Online:
+                    await connect();
+                    break;
+            }
+        }
+
+        private async Task connect()
+        {
+            cancelExistingConnect();
+
+            if (!await connectionLock.WaitAsync(10000).ConfigureAwait(false))
+                throw new TimeoutException("Could not obtain a lock to connect. A previous attempt is likely stuck.");
+
+            try
+            {
+                while (apiState.Value == APIState.Online)
+                {
+                    // ensure any previous connection was disposed.
+                    // this will also create a new cancellation token source.
+                    await disconnect(false).ConfigureAwait(false);
+
+                    // this token will be valid for the scope of this connection.
+                    // if cancelled, we can be sure that a disconnect or reconnect is handled elsewhere.
+                    var cancellationToken = connectCancelSource.Token;
+
+                    cancellationToken.ThrowIfCancellationRequested();
+
+                    Logger.Log($"{ClientName} connecting...", LoggingTarget.Network);
+
+                    try
+                    {
+                        // importantly, rebuild the connection each attempt to get an updated access token.
+                        CurrentConnection = await BuildConnectionAsync(cancellationToken).ConfigureAwait(false);
+                        CurrentConnection.Closed += ex => onConnectionClosed(ex, cancellationToken);
+
+                        cancellationToken.ThrowIfCancellationRequested();
+
+                        await CurrentConnection.ConnectAsync(cancellationToken).ConfigureAwait(false);
+
+                        Logger.Log($"{ClientName} connected!", LoggingTarget.Network);
+                        isConnected.Value = true;
+                        return;
+                    }
+                    catch (OperationCanceledException)
+                    {
+                        //connection process was cancelled.
+                        throw;
+                    }
+                    catch (Exception e)
+                    {
+                        await handleErrorAndDelay(e, cancellationToken).ConfigureAwait(false);
+                    }
+                }
+            }
+            finally
+            {
+                connectionLock.Release();
+            }
+        }
+
+        /// <summary>
+        /// Handles an exception and delays an async flow.
+        /// </summary>
+        private async Task handleErrorAndDelay(Exception exception, CancellationToken cancellationToken)
+        {
+            Logger.Log($"{ClientName} connect attempt failed: {exception.Message}", LoggingTarget.Network);
+            await Task.Delay(5000, cancellationToken).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Creates a new <see cref="PersistentEndpointClient"/>.
+        /// </summary>
+        /// <param name="cancellationToken">A cancellation token to stop the process.</param>
+        protected abstract Task<PersistentEndpointClient> BuildConnectionAsync(CancellationToken cancellationToken);
+
+        private async Task onConnectionClosed(Exception? ex, CancellationToken cancellationToken)
+        {
+            isConnected.Value = false;
+
+            if (ex != null)
+                await handleErrorAndDelay(ex, cancellationToken).ConfigureAwait(false);
+            else
+                Logger.Log($"{ClientName} disconnected", LoggingTarget.Network);
+
+            // make sure a disconnect wasn't triggered (and this is still the active connection).
+            if (!cancellationToken.IsCancellationRequested)
+                await Task.Run(connect, default).ConfigureAwait(false);
+        }
+
+        private async Task disconnect(bool takeLock)
+        {
+            cancelExistingConnect();
+
+            if (takeLock)
+            {
+                if (!await connectionLock.WaitAsync(10000).ConfigureAwait(false))
+                    throw new TimeoutException("Could not obtain a lock to disconnect. A previous attempt is likely stuck.");
+            }
+
+            try
+            {
+                if (CurrentConnection != null)
+                    await CurrentConnection.DisposeAsync().ConfigureAwait(false);
+            }
+            finally
+            {
+                CurrentConnection = null;
+                if (takeLock)
+                    connectionLock.Release();
+            }
+        }
+
+        private void cancelExistingConnect()
+        {
+            connectCancelSource.Cancel();
+            connectCancelSource = new CancellationTokenSource();
+        }
+
+        protected virtual string ClientName => GetType().ReadableName();
+
+        public override string ToString() => $"{ClientName} ({(IsConnected.Value ? "connected" : "not connected")})";
+
+        private bool isDisposed;
+
+        protected virtual void Dispose(bool isDisposing)
+        {
+            if (isDisposed)
+                return;
+
+            apiState.UnbindAll();
+            cancelExistingConnect();
+
+            isDisposed = true;
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+    }
+}

osu.Game/Overlays/Notifications/ProgressNotification.cs

@@ -148,7 +148,7 @@ namespace osu.Game.Overlays.Notifications
             }
         }
 
-        private bool completionSent;
+        private int completionSent;
 
         /// <summary>
         /// Attempt to post a completion notification.
@@ -162,11 +162,11 @@ namespace osu.Game.Overlays.Notifications
             if (CompletionTarget == null)
                 return;
 
-            if (completionSent)
+            // Thread-safe barrier, as this may be called by a web request and also scheduled to the update thread at the same time.
+            if (Interlocked.Exchange(ref completionSent, 1) == 1)
                 return;
 
             CompletionTarget.Invoke(CreateCompletionNotification());
-            completionSent = true;
 
             Close(false);
         }

osu.Game/Screens/Play/PlayerSettings/PlayerCheckbox.cs

@@ -1,15 +1,19 @@
 // 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.Framework.Graphics;
 using osu.Game.Graphics;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Overlays.Settings;
 
 namespace osu.Game.Screens.Play.PlayerSettings
 {
-    public class PlayerCheckbox : OsuCheckbox
+    public class PlayerCheckbox : SettingsCheckbox
+    {
+        protected override Drawable CreateControl() => new PlayerCheckboxControl();
+
+        public class PlayerCheckboxControl : OsuCheckbox
         {
             [BackgroundDependencyLoader]
             private void load(OsuColour colours)
@@ -19,4 +23,5 @@ namespace osu.Game.Screens.Play.PlayerSettings
                 Nub.GlowColour = colours.YellowDark;
             }
         }
+    }
 }

osu.Game/Screens/Play/PlayerSettings/VisualSettings.cs

@@ -1,12 +1,9 @@
 // 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.Framework.Graphics;
 using osu.Game.Configuration;
-using osu.Game.Graphics.Sprites;
 using osu.Game.Localisation;
 
 namespace osu.Game.Screens.Play.PlayerSettings
@@ -24,26 +21,16 @@ namespace osu.Game.Screens.Play.PlayerSettings
         {
             Children = new Drawable[]
             {
-                new OsuSpriteText
-                {
-                    Text = GameplaySettingsStrings.BackgroundDim
-                },
                 dimSliderBar = new PlayerSliderBar<double>
                 {
+                    LabelText = GameplaySettingsStrings.BackgroundDim,
                     DisplayAsPercentage = true
                 },
-                new OsuSpriteText
-                {
-                    Text = GameplaySettingsStrings.BackgroundBlur
-                },
                 blurSliderBar = new PlayerSliderBar<double>
                 {
+                    LabelText = GameplaySettingsStrings.BackgroundBlur,
                     DisplayAsPercentage = true
                 },
-                new OsuSpriteText
-                {
-                    Text = "Toggles:"
-                },
                 showStoryboardToggle = new PlayerCheckbox { LabelText = GraphicsSettingsStrings.StoryboardVideo },
                 beatmapSkinsToggle = new PlayerCheckbox { LabelText = SkinSettingsStrings.BeatmapSkins },
                 beatmapColorsToggle = new PlayerCheckbox { LabelText = SkinSettingsStrings.BeatmapColours },