Skip to content

Commit

Permalink
feat: Implement player destroy notification propagation
Browse files Browse the repository at this point in the history
Fixes handle invalidation on player destroy which caused an error when attempting to reconnect a player
  • Loading branch information
angelobreuer committed Aug 16, 2023
1 parent 802062d commit b25d599
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 43 deletions.
3 changes: 2 additions & 1 deletion src/Lavalink4NET.Tests/Players/LavalinkPlayerHandleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,6 @@ private static ILavalinkApiClient CreateApiClientMock()
SessionProvider: Mock.Of<ILavalinkSessionProvider>(x
=> x.GetSessionAsync(123UL, It.IsAny<CancellationToken>())
== ValueTask.FromResult(new LavalinkPlayerSession(CreateApiClientMock(), "abc", "abc"))),
SystemClock: new SystemClock());
SystemClock: new SystemClock(),
LifecycleNotifier: null);
}
30 changes: 3 additions & 27 deletions src/Lavalink4NET.Tests/Players/LavalinkPlayerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,32 +121,6 @@ public async Task TestPlayerIsDisposedIfDisconnectOnDestroyIsTrueOnChannelDiscon
Assert.Equal(PlayerState.Destroyed, player.State);
}

[Fact]
public async Task TestPlayerIsNotDisposedIfDisconnectOnDestroyIsFalseOnChannelDisconnectAsync()
{
// Arrange
var playerModel = new PlayerInformationModel(
GuildId: 0UL,
CurrentTrack: null,
Volume: 1F,
IsPaused: false,
VoiceState: CreateVoiceState(),
Filters: new PlayerFilterMapModel());

var playerProperties = CreateProperties(
playerModel: playerModel,
options: new LavalinkPlayerOptions { DisconnectOnDestroy = false, });

var player = new LavalinkPlayer(playerProperties);
var playerListener = player as ILavalinkPlayerListener;

// Act
await playerListener.NotifyChannelUpdateAsync(voiceChannelId: null);

// Assert
Assert.NotEqual(PlayerState.Destroyed, player.State);
}

[Fact]
public async Task TestVoiceChannelIdIsUpdatedAfterPlayerMoveAsync()
{
Expand Down Expand Up @@ -877,7 +851,9 @@ private static PlayerProperties<CustomTracingLavalinkPlayer, LavalinkPlayerOptio
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down
28 changes: 21 additions & 7 deletions src/Lavalink4NET.Tests/Players/QueuedLavalinkPlayerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public async Task TestPlayerIsStartedOnSkipAsync()
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -140,7 +142,9 @@ public async Task TestPlayerStartsSecondTrackIfNoneIsPlayingButCountIsTwoOnSkipA
ServiceProvider: null,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock(),
SessionProvider: sessionProvider),
SessionProvider: sessionProvider,
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -209,7 +213,9 @@ public async Task TestPlayerStopsOnSkipIfQueueIsEmptyAsync()
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -281,7 +287,9 @@ public async Task TestPlayerPlaysNextAfterTrackEndAsync()
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -358,7 +366,9 @@ public async Task TestPlayerRepeatsTrackIfRepeatModeIsTrackAsync()
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -437,7 +447,9 @@ public async Task TestPlayerRepeatsTrackIfRepeatModeIsTrackOnSkipAsync()
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down Expand Up @@ -507,7 +519,9 @@ public async Task TestPlayerStopsTrackIfRepeatModeIsTrackButNoTrackIsPlayingOnSk
ServiceProvider: null,
SessionProvider: sessionProvider,
DiscordClient: discordClientMock.Object,
SystemClock: new SystemClock()),
SystemClock: new SystemClock(),
LifecycleNotifier: null),
Lifecycle: Mock.Of<IPlayerLifecycle>(),
ApiClient: apiClientMock.Object,
InitialState: playerModel,
Label: "Player",
Expand Down
7 changes: 7 additions & 0 deletions src/Lavalink4NET/Players/IPlayerLifecycle.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Lavalink4NET.Players;

using System;

public interface IPlayerLifecycle : IAsyncDisposable
{
}
9 changes: 9 additions & 0 deletions src/Lavalink4NET/Players/IPlayerLifecycleNotifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace Lavalink4NET.Players;

using System.Threading;
using System.Threading.Tasks;

internal interface IPlayerLifecycleNotifier
{
ValueTask NotifyDisposeAsync(ulong guildId, CancellationToken cancellationToken = default);
}
2 changes: 2 additions & 0 deletions src/Lavalink4NET/Players/IPlayerProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ public interface IPlayerProperties<out TPlayer, out TOptions>
ulong VoiceChannelId { get; }

string SessionId { get; }

IPlayerLifecycle Lifecycle { get; }
}
6 changes: 6 additions & 0 deletions src/Lavalink4NET/Players/LavalinkPlayer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class LavalinkPlayer : ILavalinkPlayer, ILavalinkPlayerListener
private readonly ILogger<LavalinkPlayer> _logger;
private readonly ISystemClock _systemClock;
private readonly bool _disconnectOnStop;
private readonly IPlayerLifecycle _playerLifecycle;
private string? _currentTrackState;
private int _disposed;
private DateTimeOffset _syncedAt;
Expand All @@ -44,6 +45,8 @@ public LavalinkPlayer(IPlayerProperties<LavalinkPlayer, LavalinkPlayerOptions> p
_systemClock = properties.SystemClock;
_logger = properties.Logger;
_syncedAt = properties.SystemClock.UtcNow;
_playerLifecycle = properties.Lifecycle;

_unstretchedRelativePosition = default;
_connectedOnce = false;

Expand Down Expand Up @@ -411,6 +414,9 @@ protected virtual async ValueTask DisposeAsyncCore()
return;
}

// Dispose the lifecycle to notify the player is being destroyed
await using var _ = _playerLifecycle.ConfigureAwait(false);

_logger.PlayerDestroyed(_label);

await ApiClient
Expand Down
28 changes: 28 additions & 0 deletions src/Lavalink4NET/Players/LavalinkPlayerHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,17 @@ private async ValueTask<TPlayer> CreatePlayerAsync(CancellationToken cancellatio

var label = _options.Value.Label ?? $"{typeof(TPlayer).Name}@{_guildId} on {playerSession.Label}";

var lifecycle = _playerContext.LifecycleNotifier is null
? EmptyPlayerLifecycle.Instance as IPlayerLifecycle
: new PlayerLifecycle(_guildId, _playerContext.LifecycleNotifier);

var properties = new PlayerProperties<TPlayer, TOptions>(
Context: _playerContext,
VoiceChannelId: _voiceState.Value.VoiceChannelId!.Value,
InitialState: initialState,
Label: label,
SessionId: playerSession.SessionId,
Lifecycle: lifecycle,
ApiClient: playerSession.ApiClient,
Options: _options,
Logger: _logger);
Expand All @@ -213,6 +218,29 @@ private async ValueTask<TPlayer> CreatePlayerAsync(CancellationToken cancellatio
}
}

file sealed class EmptyPlayerLifecycle : IPlayerLifecycle
{
public static EmptyPlayerLifecycle Instance { get; } = new();

public ValueTask DisposeAsync() => default;
}

file sealed class PlayerLifecycle : IPlayerLifecycle
{
private readonly ulong _guildId;
private readonly IPlayerLifecycleNotifier _lifecycleNotifier;

public PlayerLifecycle(ulong guildId, IPlayerLifecycleNotifier lifecycleNotifier)
{
ArgumentNullException.ThrowIfNull(lifecycleNotifier);

_guildId = guildId;
_lifecycleNotifier = lifecycleNotifier;
}

public ValueTask DisposeAsync() => _lifecycleNotifier.NotifyDisposeAsync(_guildId);
}

file static class Diagnostics
{
private static long _playerHandles;
Expand Down
3 changes: 2 additions & 1 deletion src/Lavalink4NET/Players/PlayerContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ internal sealed record class PlayerContext(
IServiceProvider? ServiceProvider,
ILavalinkSessionProvider SessionProvider,
IDiscordClientWrapper DiscordClient,
ISystemClock SystemClock);
ISystemClock SystemClock,
IPlayerLifecycleNotifier? LifecycleNotifier);
32 changes: 25 additions & 7 deletions src/Lavalink4NET/Players/PlayerManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -14,7 +15,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

internal sealed class PlayerManager : IPlayerManager, IDisposable
internal sealed class PlayerManager : IPlayerManager, IDisposable, IPlayerLifecycleNotifier
{
private readonly ConcurrentDictionary<ulong, ILavalinkPlayerHandle> _handles;
private readonly ILogger<PlayerManager> _logger;
Expand Down Expand Up @@ -44,7 +45,8 @@ public PlayerManager(
ServiceProvider: serviceProvider,
SessionProvider: sessionProvider,
DiscordClient: discordClient,
SystemClock: systemClock);
SystemClock: systemClock,
LifecycleNotifier: this);

DiscordClient.VoiceStateUpdated += OnVoiceStateUpdated;
DiscordClient.VoiceServerUpdated += OnVoiceServerUpdated;
Expand Down Expand Up @@ -113,12 +115,15 @@ LavalinkPlayerHandle<TPlayer, TOptions> Create(ulong guildId)

var handle = _handles.GetOrAdd(guildId, Create);

var selfDeaf = options.Value.SelfDeaf;
var selfMute = options.Value.SelfMute;
if (handle.Player?.VoiceChannelId != voiceChannelId)
{
var selfDeaf = options.Value.SelfDeaf;
var selfMute = options.Value.SelfMute;

await DiscordClient
.SendVoiceUpdateAsync(guildId, voiceChannelId, selfDeaf: selfDeaf, selfMute: selfMute, cancellationToken: cancellationToken)
.ConfigureAwait(false);
await DiscordClient
.SendVoiceUpdateAsync(guildId, voiceChannelId, selfDeaf: selfDeaf, selfMute: selfMute, cancellationToken: cancellationToken)
.ConfigureAwait(false);
}

var player = await handle
.GetPlayerAsync(cancellationToken)
Expand Down Expand Up @@ -265,6 +270,19 @@ await DiscordClient

return await CheckPreconditionsAsync(player, preconditions, cancellationToken).ConfigureAwait(false);
}

ValueTask IPlayerLifecycleNotifier.NotifyDisposeAsync(ulong guildId, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

if (_handles.TryRemove(guildId, out var playerHandle))
{
Debug.Assert(playerHandle.Player is not null);
Debug.Assert(playerHandle.Player is { State: PlayerState.Destroyed, });
}

return default;
}
}

internal static partial class Logging
Expand Down
1 change: 1 addition & 0 deletions src/Lavalink4NET/Players/PlayerProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal sealed record class PlayerProperties<TPlayer, TOptions>(
string Label,
ulong VoiceChannelId,
string SessionId,
IPlayerLifecycle Lifecycle,
ILavalinkApiClient ApiClient,
IOptions<TOptions> Options,
ILogger<TPlayer> Logger)
Expand Down

0 comments on commit b25d599

Please sign in to comment.