Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions osu.Game/Online/HubClientConnector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class HubClientConnector : PersistentEndpointClientConnector, IHubClientC
private readonly string endpoint;
private readonly string versionHash;
private readonly bool preferMessagePack;
private readonly IAPIProvider api;

/// <summary>
/// The current connection opened by this connector.
Expand All @@ -47,7 +46,6 @@ public HubClientConnector(string clientName, string endpoint, IAPIProvider api,
{
ClientName = clientName;
this.endpoint = endpoint;
this.api = api;
this.versionHash = versionHash;
this.preferMessagePack = preferMessagePack;

Expand All @@ -70,7 +68,7 @@ protected override Task<PersistentEndpointClient> BuildConnectionAsync(Cancellat
options.Proxy.Credentials = CredentialCache.DefaultCredentials;
}

options.Headers.Add("Authorization", $"Bearer {api.AccessToken}");
options.Headers.Add("Authorization", $"Bearer {API.AccessToken}");
options.Headers.Add("OsuVersionHash", versionHash);
});

Expand Down Expand Up @@ -102,6 +100,12 @@ protected override Task<PersistentEndpointClient> BuildConnectionAsync(Cancellat
return Task.FromResult((PersistentEndpointClient)new HubClient(newConnection));
}

async Task IHubClientConnector.Disconnect()
{
await Disconnect().ConfigureAwait(false);
API.Logout();
}

protected override string ClientName { get; }
}
}
5 changes: 5 additions & 0 deletions osu.Game/Online/IHubClientConnector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ public interface IHubClientConnector : IDisposable
/// </summary>
public Action<HubConnection>? ConfigureConnection { get; set; }

/// <summary>
/// Forcefully disconnects the client from the server.
/// </summary>
Task Disconnect();

/// <summary>
/// Reconnect if already connected.
/// </summary>
Expand Down
18 changes: 18 additions & 0 deletions osu.Game/Online/IStatefulUserHubClient.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
2 changes: 1 addition & 1 deletion osu.Game/Online/Multiplayer/IMultiplayerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 17 additions & 4 deletions osu.Game/Online/Multiplayer/MultiplayerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
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;
Expand Down Expand Up @@ -88,6 +87,11 @@ public abstract partial class MultiplayerClient : Component, IMultiplayerClient,
/// </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;
peppy marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Whether the <see cref="MultiplayerClient"/> is currently connected.
/// This is NOT thread safe and usage should be scheduled.
Expand Down Expand Up @@ -155,10 +159,7 @@ private void load()
{
// 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();
}
}));
}

Expand Down Expand Up @@ -357,6 +358,8 @@ public async Task ToggleSpectate()

public abstract Task ChangeBeatmapAvailability(BeatmapAvailability newBeatmapAvailability);

public abstract Task DisconnectInternal();

/// <summary>
/// Change the local user's mods in the currently joined room.
/// </summary>
Expand Down Expand Up @@ -876,5 +879,15 @@ private Task runOnUpdateThreadAsync(Action action, CancellationToken cancellatio

return tcs.Task;
}

Task IStatefulUserHubClient.DisconnectRequested()
{
Schedule(() =>
{
Disconnecting?.Invoke();
DisconnectInternal();
});
return Task.CompletedTask;
}
}
}
9 changes: 9 additions & 0 deletions osu.Game/Online/Multiplayer/OnlineMultiplayerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ private void load(IAPIProvider api)
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);
Expand Down Expand Up @@ -255,6 +256,14 @@ public override Task RemovePlaylistItem(long playlistItemId)
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);
Expand Down
120 changes: 120 additions & 0 deletions osu.Game/Online/OnlineStatusNotifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// 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
{
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!;
private bool forcedDisconnection;

public OnlineStatusNotifier(Func<IScreen> getCurrentScreen)
peppy marked this conversation as resolved.
Show resolved Hide resolved
{
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;
}

private void notifyAboutForcedDisconnection()
{
if (forcedDisconnection)
return;

forcedDisconnection = 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 LoadComplete()
{
base.LoadComplete();

apiState.BindValueChanged(_ =>
{
if (apiState.Value == APIState.Online)
forcedDisconnection = false;

Scheduler.AddOnce(updateState);
});
multiplayerState.BindValueChanged(_ => Scheduler.AddOnce(updateState));
spectatorState.BindValueChanged(_ => Scheduler.AddOnce(updateState));
}

private void updateState()
{
if (forcedDisconnection)
return;

if (apiState.Value == APIState.Offline && getCurrentScreen() is OnlinePlayScreen)
{
notificationOverlay?.Post(new SimpleErrorNotification
{
Icon = FontAwesome.Solid.ExclamationCircle,
Text = "API connection was lost. Can't continue with online play."
});
return;
}

if (!multiplayerClient.IsConnected.Value && multiplayerClient.Room != null)
{
notificationOverlay?.Post(new SimpleErrorNotification
{
Icon = FontAwesome.Solid.ExclamationCircle,
Text = "Connection to the multiplayer server was lost. Exiting multiplayer."
});
}

// TODO: handle spectator server failure somehow?
}

protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);

if (spectatorClient.IsNotNull())
spectatorClient.Disconnecting -= notifyAboutForcedDisconnection;

if (multiplayerClient.IsNotNull())
multiplayerClient.Disconnecting -= notifyAboutForcedDisconnection;
}
}
}
2 changes: 2 additions & 0 deletions osu.Game/Online/PersistentEndpointClientConnector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ private async Task onConnectionClosed(Exception? ex, CancellationToken cancellat
await Task.Run(connect, default).ConfigureAwait(false);
}

protected Task Disconnect() => disconnect(true);

private async Task disconnect(bool takeLock)
{
cancelExistingConnect();
Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Online/Spectator/ISpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions osu.Game/Online/Spectator/OnlineSpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ private void load(IAPIProvider api)
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);
Expand Down Expand Up @@ -113,5 +114,15 @@ protected override Task StopWatchingUserInternal(int userId)

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);
}
}
}
17 changes: 17 additions & 0 deletions osu.Game/Online/Spectator/SpectatorClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ public abstract partial class SpectatorClient : Component, ISpectatorClient
/// </summary>
public virtual 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>
Expand Down Expand Up @@ -174,6 +179,12 @@ Task ISpectatorClient.UserScoreProcessed(int userId, long scoreId)
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`.
Expand Down Expand Up @@ -291,6 +302,12 @@ public void StopWatchingUser(int userId)

protected abstract Task StopWatchingUserInternal(int userId);

protected virtual Task DisconnectInternal()
{
Disconnecting?.Invoke();
return Task.CompletedTask;
}

protected override void Update()
{
base.Update();
Expand Down
1 change: 1 addition & 0 deletions osu.Game/OsuGame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@ protected override void LoadComplete()
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 };
Expand Down
4 changes: 3 additions & 1 deletion osu.Game/Screens/OnlinePlay/Components/RoomManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ public virtual void PartRoom()
if (JoinedRoom.Value == null)
return;

api.Queue(new PartRoomRequest(joinedRoom.Value));
if (api.State.Value == APIState.Online)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this check is it actually feasible that this fires and causes issues?

Copy link
Collaborator Author

@bdach bdach Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I hit this once in testing.

That said I'll probably look at this again, after I'm somewhat sure this pair of PRs is going in the correct direction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, I think there should be no issues with this. Server-side flows are supposed to correctly clean up the room anyway, so this request not being realised should have no impact on the server.

api.Queue(new PartRoomRequest(joinedRoom.Value));

joinedRoom.Value = null;
}

Expand Down
5 changes: 1 addition & 4 deletions osu.Game/Screens/OnlinePlay/OnlinePlayScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,15 @@ private void load()
screenStack = new OnlinePlaySubScreenStack { RelativeSizeAxes = Axes.Both },
new Header(ScreenTitle, screenStack),
RoomManager,
ongoingOperationTracker
ongoingOperationTracker,
}
};
}

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()
Expand Down
Loading
Loading