Merge pull request #25522 from bdach/no-concurrent-connections

Implement flow allowing disconnection from online services when another client instance for same user is detected

osu.Game/Online/HubClientConnector.cs

@@ -27,7 +27,6 @@ namespace osu.Game.Online
         private readonly string endpoint;
         private readonly string versionHash;
         private readonly bool preferMessagePack;
-        private readonly IAPIProvider api;
 
         /// <summary>
         /// The current connection opened by this connector.
@@ -47,7 +46,6 @@ namespace osu.Game.Online
         {
             ClientName = clientName;
             this.endpoint = endpoint;
-            this.api = api;
             this.versionHash = versionHash;
             this.preferMessagePack = preferMessagePack;
 
@@ -70,7 +68,7 @@ namespace osu.Game.Online
                             options.Proxy.Credentials = CredentialCache.DefaultCredentials;
                     }
 
-                    options.Headers.Add("Authorization", $"Bearer {api.AccessToken}");
+                    options.Headers.Add("Authorization", $"Bearer {API.AccessToken}");
                     options.Headers.Add("OsuVersionHash", versionHash);
                 });
 
@@ -102,6 +100,12 @@ namespace osu.Game.Online
             return Task.FromResult((PersistentEndpointClient)new HubClient(newConnection));
         }
 
+        async Task IHubClientConnector.Disconnect()
+        {
+            await Disconnect().ConfigureAwait(false);
+            API.Logout();
+        }
+
         protected override string ClientName { get; }
     }
 }

osu.Game/Online/IHubClientConnector.cs

@@ -30,6 +30,11 @@ namespace osu.Game.Online
         /// </summary>
         public Action<HubConnection>? ConfigureConnection { get; set; }
 
+        /// <summary>
+        /// Forcefully disconnects the client from the server.
+        /// </summary>
+        Task Disconnect();
+
         /// <summary>
         /// Reconnect if already connected.
         /// </summary>

osu.Game/Online/IStatefulUserHubClient.cs

@@ -0,0 +1,18 @@
+// 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;
+
+namespace osu.Game.Online
+{
+    /// <summary>
+    /// Common interface for clients of "stateful user hubs", i.e. server-side hubs
+    /// that preserve user state.
+    /// In the case of such hubs, concurrency constraints are enforced (only one client
+    /// can be connected at a time).
+    /// </summary>
+    public interface IStatefulUserHubClient
+    {
+        Task DisconnectRequested();
+    }
+}

osu.Game/Online/Multiplayer/IMultiplayerClient.cs

@@ -13,7 +13,7 @@ namespace osu.Game.Online.Multiplayer
     /// <summary>
     /// An interface defining a multiplayer client instance.
     /// </summary>
-    public interface IMultiplayerClient
+    public interface IMultiplayerClient : IStatefulUserHubClient
     {
         /// <summary>
         /// Signals that the room has changed state.

osu.Game/Online/Multiplayer/MultiplayerClient.cs

@@ -12,7 +12,6 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Development;
 using osu.Framework.Graphics;
-using osu.Framework.Logging;
 using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests.Responses;
@@ -88,6 +87,11 @@ namespace osu.Game.Online.Multiplayer
         /// </summary>
         public event Action? ResultsReady;
 
+        /// <summary>
+        /// Invoked just prior to disconnection requested by the server via <see cref="IStatefulUserHubClient.DisconnectRequested"/>.
+        /// </summary>
+        public event Action? Disconnecting;
+
         /// <summary>
         /// Whether the <see cref="MultiplayerClient"/> is currently connected.
         /// This is NOT thread safe and usage should be scheduled.
@@ -155,10 +159,7 @@ namespace osu.Game.Online.Multiplayer
             {
                 // clean up local room state on server disconnect.
                 if (!connected.NewValue && Room != null)
-                {
-                    Logger.Log("Clearing room due to multiplayer server connection loss.", LoggingTarget.Runtime, LogLevel.Important);
                     LeaveRoom();
-                }
             }));
         }
 
@@ -357,6 +358,8 @@ namespace osu.Game.Online.Multiplayer
 
         public abstract Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability);
 
+        public abstract Task DisconnectInternal();
+
         /// <summary>
         /// Change the local user's mods in the currently joined room.
         /// </summary>
@@ -876,5 +879,15 @@ namespace osu.Game.Online.Multiplayer
 
             return tcs.Task;
         }
+
+        Task IStatefulUserHubClient.DisconnectRequested()
+        {
+            Schedule(() =>
+            {
+                Disconnecting?.Invoke();
+                DisconnectInternal();
+            });
+            return Task.CompletedTask;
+        }
     }
 }

osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs

@@ -68,6 +68,7 @@ namespace osu.Game.Online.Multiplayer
                     connection.On<MultiplayerPlaylistItem>(nameof(IMultiplayerClient.PlaylistItemAdded), ((IMultiplayerClient)this).PlaylistItemAdded);
                     connection.On<long>(nameof(IMultiplayerClient.PlaylistItemRemoved), ((IMultiplayerClient)this).PlaylistItemRemoved);
                     connection.On<MultiplayerPlaylistItem>(nameof(IMultiplayerClient.PlaylistItemChanged), ((IMultiplayerClient)this).PlaylistItemChanged);
+                    connection.On(nameof(IStatefulUserHubClient.DisconnectRequested), ((IMultiplayerClient)this).DisconnectRequested);
                 };
 
                 IsConnected.BindTo(connector.IsConnected);
@@ -255,6 +256,14 @@ namespace osu.Game.Online.Multiplayer
             return connection.InvokeAsync(nameof(IMultiplayerServer.RemovePlaylistItem), playlistItemId);
         }
 
+        public override Task DisconnectInternal()
+        {
+            if (connector == null)
+                return Task.CompletedTask;
+
+            return connector.Disconnect();
+        }
+
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);

osu.Game/Online/OnlineStatusNotifier.cs

@@ -0,0 +1,136 @@
+// 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 osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions.ObjectExtensions;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Sprites;
+using osu.Framework.Screens;
+using osu.Game.Online.API;
+using osu.Game.Online.Multiplayer;
+using osu.Game.Online.Spectator;
+using osu.Game.Overlays;
+using osu.Game.Overlays.Notifications;
+using osu.Game.Screens.OnlinePlay;
+
+namespace osu.Game.Online
+{
+    /// <summary>
+    /// Handles various scenarios where connection is lost and we need to let the user know what and why.
+    /// </summary>
+    public partial class OnlineStatusNotifier : Component
+    {
+        private readonly Func<IScreen> getCurrentScreen;
+
+        [Resolved]
+        private MultiplayerClient multiplayerClient { get; set; } = null!;
+
+        [Resolved]
+        private SpectatorClient spectatorClient { get; set; } = null!;
+
+        [Resolved]
+        private INotificationOverlay? notificationOverlay { get; set; }
+
+        private IBindable<APIState> apiState = null!;
+        private IBindable<bool> multiplayerState = null!;
+        private IBindable<bool> spectatorState = null!;
+
+        /// <summary>
+        /// This flag will be set to <c>true</c> when the user has been notified so we don't show more than one notification.
+        /// </summary>
+        private bool userNotified;
+
+        public OnlineStatusNotifier(Func<IScreen> getCurrentScreen)
+        {
+            this.getCurrentScreen = getCurrentScreen;
+        }
+
+        [BackgroundDependencyLoader]
+        private void load(IAPIProvider api)
+        {
+            apiState = api.State.GetBoundCopy();
+            multiplayerState = multiplayerClient.IsConnected.GetBoundCopy();
+            spectatorState = spectatorClient.IsConnected.GetBoundCopy();
+
+            multiplayerClient.Disconnecting += notifyAboutForcedDisconnection;
+            spectatorClient.Disconnecting += notifyAboutForcedDisconnection;
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            apiState.BindValueChanged(state =>
+            {
+                if (state.NewValue == APIState.Online)
+                {
+                    userNotified = false;
+                    return;
+                }
+
+                if (userNotified) return;
+
+                if (state.NewValue == APIState.Offline && getCurrentScreen() is OnlinePlayScreen)
+                {
+                    userNotified = true;
+                    notificationOverlay?.Post(new SimpleErrorNotification
+                    {
+                        Icon = FontAwesome.Solid.ExclamationCircle,
+                        Text = "Connection to API was lost. Can't continue with online play."
+                    });
+                }
+            });
+
+            multiplayerState.BindValueChanged(connected => Schedule(() =>
+            {
+                if (connected.NewValue)
+                {
+                    userNotified = false;
+                    return;
+                }
+
+                if (userNotified) return;
+
+                if (multiplayerClient.Room != null)
+                {
+                    userNotified = true;
+                    notificationOverlay?.Post(new SimpleErrorNotification
+                    {
+                        Icon = FontAwesome.Solid.ExclamationCircle,
+                        Text = "Connection to the multiplayer server was lost. Exiting multiplayer."
+                    });
+                }
+            }));
+
+            spectatorState.BindValueChanged(_ =>
+            {
+                // TODO: handle spectator server failure somehow?
+            });
+        }
+
+        private void notifyAboutForcedDisconnection()
+        {
+            if (userNotified) return;
+
+            userNotified = true;
+            notificationOverlay?.Post(new SimpleErrorNotification
+            {
+                Icon = FontAwesome.Solid.ExclamationCircle,
+                Text = "You have been logged out on this device due to a login to your account on another device."
+            });
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (spectatorClient.IsNotNull())
+                spectatorClient.Disconnecting -= notifyAboutForcedDisconnection;
+
+            if (multiplayerClient.IsNotNull())
+                multiplayerClient.Disconnecting -= notifyAboutForcedDisconnection;
+        }
+    }
+}

osu.Game/Online/PersistentEndpointClientConnector.cs

@@ -159,6 +159,8 @@ namespace osu.Game.Online
                 await Task.Run(connect, default).ConfigureAwait(false);
         }
 
+        protected Task Disconnect() => disconnect(true);
+
         private async Task disconnect(bool takeLock)
         {
             cancelExistingConnect();

osu.Game/Online/Spectator/ISpectatorClient.cs

@@ -8,7 +8,7 @@ namespace osu.Game.Online.Spectator
     /// <summary>
     /// An interface defining a spectator client instance.
     /// </summary>
-    public interface ISpectatorClient
+    public interface ISpectatorClient : IStatefulUserHubClient
     {
         /// <summary>
         /// Signals that a user has begun a new play session.

osu.Game/Online/Spectator/OnlineSpectatorClient.cs

@@ -42,6 +42,7 @@ namespace osu.Game.Online.Spectator
                     connection.On<int, FrameDataBundle>(nameof(ISpectatorClient.UserSentFrames), ((ISpectatorClient)this).UserSentFrames);
                     connection.On<int, SpectatorState>(nameof(ISpectatorClient.UserFinishedPlaying), ((ISpectatorClient)this).UserFinishedPlaying);
                     connection.On<int, long>(nameof(ISpectatorClient.UserScoreProcessed), ((ISpectatorClient)this).UserScoreProcessed);
+                    connection.On(nameof(IStatefulUserHubClient.DisconnectRequested), ((IStatefulUserHubClient)this).DisconnectRequested);
                 };
 
                 IsConnected.BindTo(connector.IsConnected);
@@ -113,5 +114,15 @@ namespace osu.Game.Online.Spectator
 
             return connection.InvokeAsync(nameof(ISpectatorServer.EndWatchingUser), userId);
         }
+
+        protected override async Task DisconnectInternal()
+        {
+            await base.DisconnectInternal().ConfigureAwait(false);
+
+            if (connector == null)
+                return;
+
+            await connector.Disconnect().ConfigureAwait(false);
+        }
     }
 }

osu.Game/Online/Spectator/SpectatorClient.cs

@@ -70,6 +70,11 @@ namespace osu.Game.Online.Spectator
         /// </summary>
         public event Action<int, long>? OnUserScoreProcessed;
 
+        /// <summary>
+        /// Invoked just prior to disconnection requested by the server via <see cref="IStatefulUserHubClient.DisconnectRequested"/>.
+        /// </summary>
+        public event Action? Disconnecting;
+
         /// <summary>
         /// A dictionary containing all users currently being watched, with the number of watching components for each user.
         /// </summary>
@@ -174,6 +179,12 @@ namespace osu.Game.Online.Spectator
             return Task.CompletedTask;
         }
 
+        Task IStatefulUserHubClient.DisconnectRequested()
+        {
+            Schedule(() => DisconnectInternal());
+            return Task.CompletedTask;
+        }
+
         public void BeginPlaying(long? scoreToken, GameplayState state, Score score)
         {
             // This schedule is only here to match the one below in `EndPlaying`.
@@ -291,6 +302,12 @@ namespace osu.Game.Online.Spectator
 
         protected abstract Task StopWatchingUserInternal(int userId);
 
+        protected virtual Task DisconnectInternal()
+        {
+            Disconnecting?.Invoke();
+            return Task.CompletedTask;
+        }
+
         protected override void Update()
         {
             base.Update();

osu.Game/OsuGame.cs

@@ -1077,6 +1077,7 @@ namespace osu.Game
             Add(difficultyRecommender);
             Add(externalLinkOpener = new ExternalLinkOpener());
             Add(new MusicKeyBindingHandler());
+            Add(new OnlineStatusNotifier(() => ScreenStack.CurrentScreen));
 
             // side overlays which cancel each other.
             var singleDisplaySideOverlays = new OverlayContainer[] { Settings, Notifications, FirstRunOverlay };

osu.Game/Screens/OnlinePlay/Components/RoomManager.cs

@@ -98,7 +98,9 @@ namespace osu.Game.Screens.OnlinePlay.Components
             if (JoinedRoom.Value == null)
                 return;
 
-            api.Queue(new PartRoomRequest(joinedRoom.Value));
+            if (api.State.Value == APIState.Online)
+                api.Queue(new PartRoomRequest(joinedRoom.Value));
+
             joinedRoom.Value = null;
         }
 

osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs

@@ -71,7 +71,7 @@ namespace osu.Game.Screens.OnlinePlay
                     screenStack = new OnlinePlaySubScreenStack { RelativeSizeAxes = Axes.Both },
                     new Header(ScreenTitle, screenStack),
                     RoomManager,
-                    ongoingOperationTracker
+                    ongoingOperationTracker,
                 }
             };
         }
@@ -79,10 +79,7 @@ namespace osu.Game.Screens.OnlinePlay
         private void onlineStateChanged(ValueChangedEvent<APIState> state) => Schedule(() =>
         {
             if (state.NewValue != APIState.Online)
-            {
-                Logger.Log("API connection was lost, can't continue with online play", LoggingTarget.Network, LogLevel.Important);
                 Schedule(forcefullyExit);
-            }
         });
 
         protected override void LoadComplete()

osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs

@@ -658,5 +658,11 @@ namespace osu.Game.Tests.Visual.Multiplayer
             PlayedAt = item.PlayedAt,
             StarRating = item.Beatmap.StarRating,
         };
+
+        public override Task DisconnectInternal()
+        {
+            isConnected.Value = false;
+            return Task.CompletedTask;
+        }
     }
 }

osu.Game/Tests/Visual/Spectator/TestSpectatorClient.cs

@@ -33,7 +33,8 @@ namespace osu.Game.Tests.Visual.Spectator
 
         public int FrameSendAttempts { get; private set; }
 
-        public override IBindable<bool> IsConnected { get; } = new Bindable<bool>(true);
+        public override IBindable<bool> IsConnected => isConnected;
+        private readonly BindableBool isConnected = new BindableBool(true);
 
         public IReadOnlyDictionary<int, ReplayFrame> LastReceivedUserFrames => lastReceivedUserFrames;
 
@@ -179,5 +180,11 @@ namespace osu.Game.Tests.Visual.Spectator
                 State = SpectatedUserState.Playing
             });
         }
+
+        protected override async Task DisconnectInternal()
+        {
+            await base.DisconnectInternal().ConfigureAwait(false);
+            isConnected.Value = false;
+        }
     }
 }