From fa8a723ff99b8aaf7e4e361b87c8ec01e0dcaf66 Mon Sep 17 00:00:00 2001 From: Vinzenz Schroeter Date: Sat, 4 May 2024 14:25:37 +0200 Subject: [PATCH] deduplicate connection logic --- tank-frontend/src/ClientScreen.tsx | 21 +++++- tank-frontend/src/PlayerInfo.tsx | 4 +- tank-frontend/src/serverCalls.tsx | 4 +- .../ClientScreenServerConnection.cs | 75 +++++-------------- .../Interactivity/ControlsServerConnection.cs | 6 +- .../DroppablePackageRequestConnection.cs | 50 +++++++++++++ .../Interactivity/PlayerInfoConnection.cs | 53 ++++--------- .../Interactivity/WebsocketServer.cs | 2 +- .../WebsocketServerConnection.cs | 6 +- 9 files changed, 108 insertions(+), 113 deletions(-) create mode 100644 tanks-backend/TanksServer/Interactivity/DroppablePackageRequestConnection.cs diff --git a/tank-frontend/src/ClientScreen.tsx b/tank-frontend/src/ClientScreen.tsx index df0e871..ba78edd 100644 --- a/tank-frontend/src/ClientScreen.tsx +++ b/tank-frontend/src/ClientScreen.tsx @@ -1,7 +1,8 @@ -import {useEffect, useRef} from 'react'; +import {useEffect, useRef, useState} from 'react'; import './ClientScreen.css'; import {hslToString, Theme} from './theme.ts'; import {makeApiUrl, useMyWebSocket} from './serverCalls.tsx'; +import {ReadyState} from 'react-use-websocket'; const pixelsPerRow = 352; const pixelsPerCol = 160; @@ -101,6 +102,7 @@ export default function ClientScreen({theme, player}: { player: string | null }) { const canvasRef = useRef(null); + const [shouldSendMessage, setShouldSendMessage] = useState(false); const url = makeApiUrl('/screen', 'ws'); if (player && player !== '') @@ -109,13 +111,24 @@ export default function ClientScreen({theme, player}: { const { lastMessage, sendMessage, - getWebSocket - } = useMyWebSocket(url.toString(), {}); + getWebSocket, + readyState + } = useMyWebSocket(url.toString(), { + onOpen: _ => setShouldSendMessage(true) + }); const socket = getWebSocket(); if (socket) (socket as WebSocket).binaryType = 'arraybuffer'; + useEffect(() => { + if (!shouldSendMessage || readyState !== ReadyState.OPEN) + return; + setShouldSendMessage(false); + sendMessage(''); + }, [readyState, shouldSendMessage]); + + useEffect(() => { if (lastMessage === null) return; @@ -155,7 +168,7 @@ export default function ClientScreen({theme, player}: { if (ignore) return; - sendMessage(''); + setShouldSendMessage(true); }; start(); diff --git a/tank-frontend/src/PlayerInfo.tsx b/tank-frontend/src/PlayerInfo.tsx index 17e67d1..2916d04 100644 --- a/tank-frontend/src/PlayerInfo.tsx +++ b/tank-frontend/src/PlayerInfo.tsx @@ -28,7 +28,6 @@ type TankInfo = { readonly moving: boolean; } - type PlayerInfoMessage = { readonly name: string; readonly scores: Scores; @@ -48,7 +47,8 @@ export default function PlayerInfo({player}: { player: string }) { readyState, sendMessage } = useMyWebSocket(url.toString(), { - onMessage: () => setShouldSendMessage(true) + onMessage: () => setShouldSendMessage(true), + onOpen: _ => setShouldSendMessage(true) }); useEffect(() => { diff --git a/tank-frontend/src/serverCalls.tsx b/tank-frontend/src/serverCalls.tsx index d2cc9f5..6e003fb 100644 --- a/tank-frontend/src/serverCalls.tsx +++ b/tank-frontend/src/serverCalls.tsx @@ -19,10 +19,10 @@ export type Player = { readonly scores: Scores; }; -export function useMyWebSocket(url: string, options: Options) { +export function useMyWebSocket(url: string, options: Options = {}) { return useWebSocket(url, { shouldReconnect: () => true, - reconnectAttempts: 5, + reconnectAttempts: 2, onReconnectStop: () => alert('server connection failed. please reload.'), ...options }); diff --git a/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs b/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs index 6a0616d..6208cbf 100644 --- a/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs +++ b/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs @@ -1,21 +1,16 @@ using System.Buffers; -using System.Diagnostics; using System.Net.WebSockets; using DisplayCommands; -using DotNext.Threading; using TanksServer.Graphics; namespace TanksServer.Interactivity; internal sealed class ClientScreenServerConnection - : WebsocketServerConnection, IDisposable + : DroppablePackageRequestConnection { private readonly BufferPool _bufferPool; private readonly PlayerScreenData? _playerDataBuilder; private readonly Player? _player; - private readonly AsyncAutoResetEvent _nextPackageEvent = new(false, 1); - private int _runningMessageHandlers = 0; - private Package? _next; public ClientScreenServerConnection( WebSocket webSocket, @@ -32,47 +27,11 @@ internal sealed class ClientScreenServerConnection : new PlayerScreenData(logger, player); } - protected override ValueTask HandleMessageAsync(Memory _) - { - if (Interlocked.Increment(ref _runningMessageHandlers) == 1) - return Core(); - - Interlocked.Decrement(ref _runningMessageHandlers); - return ValueTask.CompletedTask; - - async ValueTask Core() - { - await _nextPackageEvent.WaitAsync(); - var package = Interlocked.Exchange(ref _next, null); - if (package == null) - throw new UnreachableException("package should be set here"); - await SendAndDisposeAsync(package); - Interlocked.Decrement(ref _runningMessageHandlers); - } - } - public async Task OnGameTickAsync(PixelGrid pixels, GamePixelGrid gamePixelGrid) { await Task.Yield(); - var next = BuildNextPackage(pixels, gamePixelGrid); - var oldNext = Interlocked.Exchange(ref _next, next); - - _nextPackageEvent.Set(); - - oldNext?.Dispose(); - } - - public override ValueTask RemovedAsync() - { - _player?.DecrementConnectionCount(); - return ValueTask.CompletedTask; - } - - public void Dispose() - { - _nextPackageEvent.Dispose(); - Interlocked.Exchange(ref _next, null)?.Dispose(); + SetNextPackage(next); } private Package BuildNextPackage(PixelGrid pixels, GamePixelGrid gamePixelGrid) @@ -80,19 +39,17 @@ internal sealed class ClientScreenServerConnection var nextPixels = _bufferPool.Rent(pixels.Data.Length); pixels.Data.CopyTo(nextPixels.Memory); - IMemoryOwner? nextPlayerData = null; - if (_playerDataBuilder != null) - { - var data = _playerDataBuilder.Build(gamePixelGrid); - nextPlayerData = _bufferPool.Rent(data.Length); - data.CopyTo(nextPlayerData.Memory); - } + if (_playerDataBuilder == null) + return new Package(nextPixels, null); - var next = new Package(nextPixels, nextPlayerData); - return next; + var data = _playerDataBuilder.Build(gamePixelGrid); + var nextPlayerData = _bufferPool.Rent(data.Length); + data.CopyTo(nextPlayerData.Memory); + + return new Package(nextPixels, nextPlayerData); } - private async ValueTask SendAndDisposeAsync(Package package) + protected override async ValueTask SendPackageAsync(Package package) { try { @@ -104,13 +61,15 @@ internal sealed class ClientScreenServerConnection { Logger.LogWarning(ex, "send failed"); } - finally - { - package.Dispose(); - } } - private sealed record class Package( + public override void Dispose() + { + base.Dispose(); + _player?.DecrementConnectionCount(); + } + + internal sealed record class Package( IMemoryOwner Pixels, IMemoryOwner? PlayerData ) : IDisposable diff --git a/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs b/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs index edceb3d..3059dad 100644 --- a/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs +++ b/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs @@ -69,9 +69,5 @@ internal sealed class ControlsServerConnection : WebsocketServerConnection return ValueTask.CompletedTask; } - public override ValueTask RemovedAsync() - { - _player.DecrementConnectionCount(); - return ValueTask.CompletedTask; - } + public override void Dispose() => _player.DecrementConnectionCount(); } diff --git a/tanks-backend/TanksServer/Interactivity/DroppablePackageRequestConnection.cs b/tanks-backend/TanksServer/Interactivity/DroppablePackageRequestConnection.cs new file mode 100644 index 0000000..19956a9 --- /dev/null +++ b/tanks-backend/TanksServer/Interactivity/DroppablePackageRequestConnection.cs @@ -0,0 +1,50 @@ +using System.Diagnostics; +using DotNext.Threading; + +namespace TanksServer.Interactivity; + +internal abstract class DroppablePackageRequestConnection( + ILogger logger, + ByteChannelWebSocket socket +) : WebsocketServerConnection(logger, socket), IDisposable + where TPackage : class, IDisposable +{ + private readonly AsyncAutoResetEvent _nextPackageEvent = new(false, 1); + private int _runningMessageHandlers = 0; + private TPackage? _next; + + protected override ValueTask HandleMessageAsync(Memory _) + { + if (Interlocked.Increment(ref _runningMessageHandlers) == 1) + return Core(); + + // client has requested multiple frames, ignoring duplicate requests + Interlocked.Decrement(ref _runningMessageHandlers); + return ValueTask.CompletedTask; + + async ValueTask Core() + { + await _nextPackageEvent.WaitAsync(); + var package = Interlocked.Exchange(ref _next, null); + if (package == null) + throw new UnreachableException("package should be set here"); + await SendPackageAsync(package); + Interlocked.Decrement(ref _runningMessageHandlers); + } + } + + protected void SetNextPackage(TPackage next) + { + var oldNext = Interlocked.Exchange(ref _next, next); + _nextPackageEvent.Set(); + oldNext?.Dispose(); + } + + protected abstract ValueTask SendPackageAsync(TPackage package); + + public override void Dispose() + { + _nextPackageEvent.Dispose(); + Interlocked.Exchange(ref _next, null)?.Dispose(); + } +} diff --git a/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs b/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs index 29ac8b6..04661c3 100644 --- a/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs +++ b/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs @@ -6,18 +6,14 @@ using TanksServer.GameLogic; namespace TanksServer.Interactivity; -// MemoryStream is IDisposable but does not need to be disposed -#pragma warning disable CA1001 -internal sealed class PlayerInfoConnection : WebsocketServerConnection -#pragma warning restore CA1001 +internal sealed class PlayerInfoConnection + : DroppablePackageRequestConnection> { private readonly Player _player; private readonly MapEntityManager _entityManager; private readonly BufferPool _bufferPool; private readonly MemoryStream _tempStream = new(); - private int _wantsInfoOnTick = 1; private IMemoryOwner? _lastMessage = null; - private IMemoryOwner? _nextMessage = null; public PlayerInfoConnection( Player player, @@ -33,47 +29,22 @@ internal sealed class PlayerInfoConnection : WebsocketServerConnection _player.IncrementConnectionCount(); } - protected override ValueTask HandleMessageAsync(Memory buffer) - { - var next = Interlocked.Exchange(ref _nextMessage, null); - if (next != null) - return SendAndDisposeAsync(next); - - _wantsInfoOnTick = 1; - return ValueTask.CompletedTask; - } - public async Task OnGameTickAsync() { await Task.Yield(); var response = await GenerateMessageAsync(); - - var shouldDropPacket = _lastMessage != null && response.Memory.Span.SequenceEqual(_lastMessage.Memory.Span); - if (shouldDropPacket) - { - response.Dispose(); - return; - } - - var wantsNow = Interlocked.Exchange(ref _wantsInfoOnTick, 0) != 0; - - if (wantsNow) - { - await SendAndDisposeAsync(response); - return; - } - - Interlocked.Exchange(ref _nextMessage, response); + if (response != null) + SetNextPackage(response); } - public override ValueTask RemovedAsync() + public override void Dispose() { + base.Dispose(); _player.DecrementConnectionCount(); - return ValueTask.CompletedTask; } - private async ValueTask> GenerateMessageAsync() + private async ValueTask?> GenerateMessageAsync() { var tank = _entityManager.GetCurrentTankOfPlayer(_player); @@ -97,12 +68,18 @@ internal sealed class PlayerInfoConnection : WebsocketServerConnection var messageLength = (int)_tempStream.Position; var owner = _bufferPool.Rent(messageLength); + _tempStream.Position = 0; await _tempStream.ReadExactlyAsync(owner.Memory); - return owner; + + if (_lastMessage == null || !owner.Memory.Span.SequenceEqual(_lastMessage.Memory.Span)) + return owner; + + owner.Dispose(); + return null; } - private async ValueTask SendAndDisposeAsync(IMemoryOwner data) + protected override async ValueTask SendPackageAsync(IMemoryOwner data) { await Socket.SendTextAsync(data.Memory); Interlocked.Exchange(ref _lastMessage, data)?.Dispose(); diff --git a/tanks-backend/TanksServer/Interactivity/WebsocketServer.cs b/tanks-backend/TanksServer/Interactivity/WebsocketServer.cs index 19f7e7a..0c33e4c 100644 --- a/tanks-backend/TanksServer/Interactivity/WebsocketServer.cs +++ b/tanks-backend/TanksServer/Interactivity/WebsocketServer.cs @@ -37,7 +37,7 @@ internal abstract class WebsocketServer( await connection.ReceiveAsync(); _ = _connections.TryRemove(connection, out _); - await connection.RemovedAsync(); + connection.Dispose(); } public Task StartAsync(CancellationToken cancellationToken) => Task.CompletedTask; diff --git a/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs b/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs index 6be215c..893b7a5 100644 --- a/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs +++ b/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs @@ -3,7 +3,7 @@ namespace TanksServer.Interactivity; internal abstract class WebsocketServerConnection( ILogger logger, ByteChannelWebSocket socket -) +): IDisposable { protected readonly ByteChannelWebSocket Socket = socket; protected readonly ILogger Logger = logger; @@ -21,7 +21,7 @@ internal abstract class WebsocketServerConnection( Logger.LogTrace("done receiving"); } - public abstract ValueTask RemovedAsync(); - protected abstract ValueTask HandleMessageAsync(Memory buffer); + + public abstract void Dispose(); }