From d9e9c2897644ab04fd07c9e9da6bf7b35156e205 Mon Sep 17 00:00:00 2001
From: Vinzenz Schroeter <vinzenz.f.s@gmail.com>
Date: Thu, 2 May 2024 19:07:04 +0200
Subject: [PATCH] remove locking from other connections too

---
 .../ClientScreenServerConnection.cs           |  2 -
 .../Interactivity/ControlsServerConnection.cs |  2 +-
 .../Interactivity/PlayerInfoConnection.cs     | 53 ++++++++++---------
 .../WebsocketServerConnection.cs              |  5 +-
 4 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs b/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs
index 55985d2..38efa55 100644
--- a/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs
+++ b/tanks-backend/TanksServer/Interactivity/ClientScreenServerConnection.cs
@@ -27,8 +27,6 @@ internal sealed class ClientScreenServerConnection(
         ? null
         : new PlayerScreenData(logger, player);
 
-    protected override ValueTask HandleMessageLockedAsync(Memory<byte> buffer) => throw new UnreachableException();
-
     protected override ValueTask HandleMessageAsync(Memory<byte> _)
     {
         if (_wantsFrameOnTick != 0)
diff --git a/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs b/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs
index ac8e9b6..35f57f4 100644
--- a/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs
+++ b/tanks-backend/TanksServer/Interactivity/ControlsServerConnection.cs
@@ -23,7 +23,7 @@ internal sealed class ControlsServerConnection(
         Shoot = 0x05
     }
 
-    protected override ValueTask HandleMessageLockedAsync(Memory<byte> buffer)
+    protected override ValueTask HandleMessageAsync(Memory<byte> buffer)
     {
         var type = (MessageType)buffer.Span[0];
         var control = (InputType)buffer.Span[1];
diff --git a/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs b/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs
index 64a544e..b35e759 100644
--- a/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs
+++ b/tanks-backend/TanksServer/Interactivity/PlayerInfoConnection.cs
@@ -11,37 +11,37 @@ internal sealed class PlayerInfoConnection(
     MapEntityManager entityManager
 ) : WebsocketServerConnection(logger, new ByteChannelWebSocket(rawSocket, logger, 0))
 {
-    private bool _wantsInfoOnTick = true;
-    private byte[] _lastMessage = [];
+    private int _wantsInfoOnTick = 1;
+    private byte[]? _lastMessage = null;
+    private byte[]? _nextMessage = null;
 
-    protected override ValueTask HandleMessageLockedAsync(Memory<byte> buffer)
+    protected override ValueTask HandleMessageAsync(Memory<byte> buffer)
     {
-        var response = GetMessageToSend();
-        if (response == null)
-        {
-            Logger.LogTrace("cannot respond directly, increasing wanted frames");
-            _wantsInfoOnTick = true;
-            return ValueTask.CompletedTask;
-        }
+        var next = Interlocked.Exchange(ref _nextMessage, null);
+        if (next != null)
+            return SendAndDisposeAsync(next);
 
-        Logger.LogTrace("responding directly");
-        return Socket.SendTextAsync(response);
+        _wantsInfoOnTick = 1;
+        return ValueTask.CompletedTask;
     }
 
-    public ValueTask OnGameTickAsync() => LockedAsync(() =>
+    public async ValueTask OnGameTickAsync()
     {
-        if (!_wantsInfoOnTick)
-            return ValueTask.CompletedTask;
+        await Task.Yield();
 
         var response = GetMessageToSend();
-        if (response == null)
-            return ValueTask.CompletedTask;
+        var wantsNow = Interlocked.Exchange(ref _wantsInfoOnTick, 0) != 0;
 
-        Logger.LogTrace("responding indirectly");
-        return Socket.SendTextAsync(response);
-    });
+        if (wantsNow)
+        {
+            await SendAndDisposeAsync(response);
+            return;
+        }
 
-    private byte[]? GetMessageToSend()
+        Interlocked.Exchange(ref _nextMessage, response);
+    }
+
+    private byte[] GetMessageToSend()
     {
         var tank = entityManager.GetCurrentTankOfPlayer(player);
 
@@ -53,11 +53,14 @@ internal sealed class PlayerInfoConnection(
         }
 
         var info = new PlayerInfo(player.Name, player.Scores, player.Controls.ToDisplayString(), tankInfo);
-        var response = JsonSerializer.SerializeToUtf8Bytes(info, AppSerializerContext.Default.PlayerInfo);
 
-        if (response.SequenceEqual(_lastMessage))
-            return null;
+        // TODO: switch to async version with pre-allocated buffer / IMemoryOwner
+        return JsonSerializer.SerializeToUtf8Bytes(info, AppSerializerContext.Default.PlayerInfo);
+    }
 
-        return _lastMessage = response;
+    private async ValueTask SendAndDisposeAsync(byte[] data)
+    {
+        await Socket.SendTextAsync(data);
+        Interlocked.Exchange(ref _lastMessage, data);
     }
 }
diff --git a/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs b/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs
index 3610981..23b698c 100644
--- a/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs
+++ b/tanks-backend/TanksServer/Interactivity/WebsocketServerConnection.cs
@@ -22,10 +22,7 @@ internal abstract class WebsocketServerConnection(
         Logger.LogTrace("done receiving");
     }
 
-    protected virtual ValueTask HandleMessageAsync(Memory<byte> buffer)
-        => LockedAsync(() => HandleMessageLockedAsync(buffer));
-
-    protected abstract ValueTask HandleMessageLockedAsync(Memory<byte> buffer);
+    protected abstract ValueTask HandleMessageAsync(Memory<byte> buffer);
 
     protected async ValueTask LockedAsync(Func<ValueTask> action)
     {