From b25d5993a6488acded9f3e3bcdc42c7b842837ff Mon Sep 17 00:00:00 2001 From: Angelo Breuer Date: Wed, 16 Aug 2023 11:06:57 +0200 Subject: [PATCH] feat: Implement player destroy notification propagation Fixes handle invalidation on player destroy which caused an error when attempting to reconnect a player --- .../Players/LavalinkPlayerHandleTests.cs | 3 +- .../Players/LavalinkPlayerTests.cs | 30 ++--------------- .../Players/QueuedLavalinkPlayerTests.cs | 28 ++++++++++++---- src/Lavalink4NET/Players/IPlayerLifecycle.cs | 7 ++++ .../Players/IPlayerLifecycleNotifier.cs | 9 ++++++ src/Lavalink4NET/Players/IPlayerProperties.cs | 2 ++ src/Lavalink4NET/Players/LavalinkPlayer.cs | 6 ++++ .../Players/LavalinkPlayerHandle.cs | 28 ++++++++++++++++ src/Lavalink4NET/Players/PlayerContext.cs | 3 +- src/Lavalink4NET/Players/PlayerManager.cs | 32 +++++++++++++++---- src/Lavalink4NET/Players/PlayerProperties.cs | 1 + 11 files changed, 106 insertions(+), 43 deletions(-) create mode 100644 src/Lavalink4NET/Players/IPlayerLifecycle.cs create mode 100644 src/Lavalink4NET/Players/IPlayerLifecycleNotifier.cs diff --git a/src/Lavalink4NET.Tests/Players/LavalinkPlayerHandleTests.cs b/src/Lavalink4NET.Tests/Players/LavalinkPlayerHandleTests.cs index 74363cf7..e55cc4bf 100644 --- a/src/Lavalink4NET.Tests/Players/LavalinkPlayerHandleTests.cs +++ b/src/Lavalink4NET.Tests/Players/LavalinkPlayerHandleTests.cs @@ -91,5 +91,6 @@ private static ILavalinkApiClient CreateApiClientMock() SessionProvider: Mock.Of(x => x.GetSessionAsync(123UL, It.IsAny()) == ValueTask.FromResult(new LavalinkPlayerSession(CreateApiClientMock(), "abc", "abc"))), - SystemClock: new SystemClock()); + SystemClock: new SystemClock(), + LifecycleNotifier: null); } diff --git a/src/Lavalink4NET.Tests/Players/LavalinkPlayerTests.cs b/src/Lavalink4NET.Tests/Players/LavalinkPlayerTests.cs index 571ce85f..f1c335f7 100644 --- a/src/Lavalink4NET.Tests/Players/LavalinkPlayerTests.cs +++ b/src/Lavalink4NET.Tests/Players/LavalinkPlayerTests.cs @@ -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() { @@ -877,7 +851,9 @@ private static PlayerProperties(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", diff --git a/src/Lavalink4NET.Tests/Players/QueuedLavalinkPlayerTests.cs b/src/Lavalink4NET.Tests/Players/QueuedLavalinkPlayerTests.cs index 46824ddc..d0ecefc5 100644 --- a/src/Lavalink4NET.Tests/Players/QueuedLavalinkPlayerTests.cs +++ b/src/Lavalink4NET.Tests/Players/QueuedLavalinkPlayerTests.cs @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", @@ -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(), ApiClient: apiClientMock.Object, InitialState: playerModel, Label: "Player", diff --git a/src/Lavalink4NET/Players/IPlayerLifecycle.cs b/src/Lavalink4NET/Players/IPlayerLifecycle.cs new file mode 100644 index 00000000..83729696 --- /dev/null +++ b/src/Lavalink4NET/Players/IPlayerLifecycle.cs @@ -0,0 +1,7 @@ +namespace Lavalink4NET.Players; + +using System; + +public interface IPlayerLifecycle : IAsyncDisposable +{ +} diff --git a/src/Lavalink4NET/Players/IPlayerLifecycleNotifier.cs b/src/Lavalink4NET/Players/IPlayerLifecycleNotifier.cs new file mode 100644 index 00000000..6d3a0e8c --- /dev/null +++ b/src/Lavalink4NET/Players/IPlayerLifecycleNotifier.cs @@ -0,0 +1,9 @@ +namespace Lavalink4NET.Players; + +using System.Threading; +using System.Threading.Tasks; + +internal interface IPlayerLifecycleNotifier +{ + ValueTask NotifyDisposeAsync(ulong guildId, CancellationToken cancellationToken = default); +} diff --git a/src/Lavalink4NET/Players/IPlayerProperties.cs b/src/Lavalink4NET/Players/IPlayerProperties.cs index 756840e9..acaf1943 100644 --- a/src/Lavalink4NET/Players/IPlayerProperties.cs +++ b/src/Lavalink4NET/Players/IPlayerProperties.cs @@ -31,4 +31,6 @@ public interface IPlayerProperties ulong VoiceChannelId { get; } string SessionId { get; } + + IPlayerLifecycle Lifecycle { get; } } diff --git a/src/Lavalink4NET/Players/LavalinkPlayer.cs b/src/Lavalink4NET/Players/LavalinkPlayer.cs index 3318110a..1e775c88 100644 --- a/src/Lavalink4NET/Players/LavalinkPlayer.cs +++ b/src/Lavalink4NET/Players/LavalinkPlayer.cs @@ -23,6 +23,7 @@ public class LavalinkPlayer : ILavalinkPlayer, ILavalinkPlayerListener private readonly ILogger _logger; private readonly ISystemClock _systemClock; private readonly bool _disconnectOnStop; + private readonly IPlayerLifecycle _playerLifecycle; private string? _currentTrackState; private int _disposed; private DateTimeOffset _syncedAt; @@ -44,6 +45,8 @@ public LavalinkPlayer(IPlayerProperties p _systemClock = properties.SystemClock; _logger = properties.Logger; _syncedAt = properties.SystemClock.UtcNow; + _playerLifecycle = properties.Lifecycle; + _unstretchedRelativePosition = default; _connectedOnce = false; @@ -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 diff --git a/src/Lavalink4NET/Players/LavalinkPlayerHandle.cs b/src/Lavalink4NET/Players/LavalinkPlayerHandle.cs index 9527dfd1..52eb644b 100644 --- a/src/Lavalink4NET/Players/LavalinkPlayerHandle.cs +++ b/src/Lavalink4NET/Players/LavalinkPlayerHandle.cs @@ -199,12 +199,17 @@ private async ValueTask 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( Context: _playerContext, VoiceChannelId: _voiceState.Value.VoiceChannelId!.Value, InitialState: initialState, Label: label, SessionId: playerSession.SessionId, + Lifecycle: lifecycle, ApiClient: playerSession.ApiClient, Options: _options, Logger: _logger); @@ -213,6 +218,29 @@ private async ValueTask 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; diff --git a/src/Lavalink4NET/Players/PlayerContext.cs b/src/Lavalink4NET/Players/PlayerContext.cs index 3adeb75b..f3d35708 100644 --- a/src/Lavalink4NET/Players/PlayerContext.cs +++ b/src/Lavalink4NET/Players/PlayerContext.cs @@ -8,4 +8,5 @@ internal sealed record class PlayerContext( IServiceProvider? ServiceProvider, ILavalinkSessionProvider SessionProvider, IDiscordClientWrapper DiscordClient, - ISystemClock SystemClock); + ISystemClock SystemClock, + IPlayerLifecycleNotifier? LifecycleNotifier); diff --git a/src/Lavalink4NET/Players/PlayerManager.cs b/src/Lavalink4NET/Players/PlayerManager.cs index 59bc8ce7..82fce52f 100644 --- a/src/Lavalink4NET/Players/PlayerManager.cs +++ b/src/Lavalink4NET/Players/PlayerManager.cs @@ -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; @@ -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 _handles; private readonly ILogger _logger; @@ -44,7 +45,8 @@ public PlayerManager( ServiceProvider: serviceProvider, SessionProvider: sessionProvider, DiscordClient: discordClient, - SystemClock: systemClock); + SystemClock: systemClock, + LifecycleNotifier: this); DiscordClient.VoiceStateUpdated += OnVoiceStateUpdated; DiscordClient.VoiceServerUpdated += OnVoiceServerUpdated; @@ -113,12 +115,15 @@ LavalinkPlayerHandle 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) @@ -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 diff --git a/src/Lavalink4NET/Players/PlayerProperties.cs b/src/Lavalink4NET/Players/PlayerProperties.cs index 8c42b337..3e407939 100644 --- a/src/Lavalink4NET/Players/PlayerProperties.cs +++ b/src/Lavalink4NET/Players/PlayerProperties.cs @@ -14,6 +14,7 @@ internal sealed record class PlayerProperties( string Label, ulong VoiceChannelId, string SessionId, + IPlayerLifecycle Lifecycle, ILavalinkApiClient ApiClient, IOptions Options, ILogger Logger)