Reducing allocations and error handling improvements (#2040)

NitroxClient/Communication/NetworkingLayer/LiteNetLib/LiteNetLibClient.cs

@@ -1,3 +1,4 @@
+using System.Buffers;
 using System.Threading;
 using System.Threading.Tasks;
 using LiteNetLib;
@@ -82,12 +83,18 @@ public void Stop()
     private void ReceivedNetworkData(NetPeer peer, NetDataReader reader, byte channel, DeliveryMethod deliveryMethod)
     {
         int packetDataLength = reader.GetInt();
-        byte[] packetData = new byte[packetDataLength];
-        reader.GetBytes(packetData, packetDataLength);
-
-        Packet packet = Packet.Deserialize(packetData);
-        packetReceiver.PacketReceived(packet);
-        networkDebugger?.PacketReceived(packet, packetDataLength);
+        byte[] packetData = ArrayPool<byte>.Shared.Rent(packetDataLength);
+        try
+        {
+            reader.GetBytes(packetData, packetDataLength);
+            Packet packet = Packet.Deserialize(packetData);
+            packetReceiver.PacketReceived(packet);
+            networkDebugger?.PacketReceived(packet, packetDataLength);
+        }
+        finally
+        {
+            ArrayPool<byte>.Shared.Return(packetData, true);
+        }
     }
 
     private void Connected(NetPeer peer)

NitroxModel/Helper/NatHelper.cs

@@ -11,48 +11,77 @@ namespace NitroxModel.Helper;
 
 public static class NatHelper
 {
-    public static async Task<IPAddress> GetExternalIpAsync()
+    public static async Task<IPAddress> GetExternalIpAsync() => await MonoNatHelper.GetFirstAsync(static async device =>
     {
-        return await MonoNatHelper.GetFirstAsync(async d => await d.GetExternalIPAsync().ConfigureAwait(false)).ConfigureAwait(false);
-    }
+        try
+        {
+            return await device.GetExternalIPAsync().ConfigureAwait(false);
+        }
+        catch (Exception)
+        {
+            return null;
+        }
+    }).ConfigureAwait(false);
 
     public static async Task<bool> DeletePortMappingAsync(ushort port, Protocol protocol)
     {
-        Mapping mapping = new(protocol, port, port);
-        return await MonoNatHelper.GetFirstAsync(async d =>
+        return await MonoNatHelper.GetFirstAsync(static async (device, mapping) =>
         {
             try
             {
-                return await d.DeletePortMapAsync(mapping).ConfigureAwait(false) != null;
+                return await device.DeletePortMapAsync(mapping).ConfigureAwait(false) != null;
             }
             catch (MappingException)
             {
                 return false;
             }
-        }).ConfigureAwait(false);
+        }, new Mapping(protocol, port, port)).ConfigureAwait(false);
     }
 
     public static async Task<Mapping> GetPortMappingAsync(ushort port, Protocol protocol)
     {
-        return await MonoNatHelper.GetFirstAsync(async d =>
+        return await MonoNatHelper.GetFirstAsync(static async (device, protocolAndPort) =>
         {
             try
             {
-                return await d.GetSpecificMappingAsync(protocol, port).ConfigureAwait(false);
+                return await device.GetSpecificMappingAsync(protocolAndPort.protocol, protocolAndPort.port).ConfigureAwait(false);
             }
             catch (Exception)
             {
                 return null;
             }
-        }).ConfigureAwait(false);
+        }, (port, protocol)).ConfigureAwait(false);
     }
 
-    public static async Task<bool> AddPortMappingAsync(ushort port, Protocol protocol)
+    public static async Task<ResultCodes> AddPortMappingAsync(ushort port, Protocol protocol)
     {
         Mapping mapping = new(protocol, port, port);
-        return await MonoNatHelper.GetFirstAsync(async d => await d.CreatePortMapAsync(mapping).ConfigureAwait(false) != null).ConfigureAwait(false);
+        return await MonoNatHelper.GetFirstAsync(static async (device, mapping) =>
+        {
+            try
+            {
+                return await device.CreatePortMapAsync(mapping).ConfigureAwait(false) != null ? ResultCodes.SUCCESS : ResultCodes.UNKNOWN_ERROR;
+            }
+            catch (MappingException ex)
+            {
+                return ExceptionToCode(ex);
+            }
+        }, mapping).ConfigureAwait(false);
     }
 
+    public enum ResultCodes
+    {
+        SUCCESS,
+        CONFLICT_IN_MAPPING_ENTRY,
+        UNKNOWN_ERROR
+    }
+
+    private static ResultCodes ExceptionToCode(MappingException exception) => exception.ErrorCode switch
+    {
+        ErrorCode.ConflictInMappingEntry => ResultCodes.CONFLICT_IN_MAPPING_ENTRY,
+        _ => ResultCodes.UNKNOWN_ERROR
+    };
+
     private static class MonoNatHelper
     {
         private static readonly ConcurrentDictionary<EndPoint, INatDevice> discoveredDevices = new();
@@ -116,15 +145,17 @@ void Handler(object sender, DeviceEventArgs args)
             return discoveredDevices.Values;
         }
 
-        public static async Task<TResult> GetFirstAsync<TResult>(Func<INatDevice, Task<TResult>> predicate)
+        public static async Task<TResult> GetFirstAsync<TResult>(Func<INatDevice, Task<TResult>> predicate) => await GetFirstAsync(static (device, p) => p(device), predicate);
+
+        public static async Task<TResult> GetFirstAsync<TResult, TExtraParam>(Func<INatDevice, TExtraParam, Task<TResult>> predicate, TExtraParam parameter)
         {
             // Start NAT discovery (if it hasn't started yet).
             Task<IEnumerable<INatDevice>> discoverTask = DiscoverAsync();
             if (discoverTask.IsCompleted && discoveredDevices.IsEmpty)
             {
                 return default;
             }
-                
+
             // Progressively handle devices until first not-null/false result or when discovery times out.
             ConcurrentDictionary<EndPoint, INatDevice> handledDevices = new();
             do
@@ -135,20 +166,20 @@ public static async Task<TResult> GetFirstAsync<TResult>(Func<INatDevice, Task<T
                     await Task.Delay(10).ConfigureAwait(false);
                     continue;
                 }
-                                
+
                 foreach (KeyValuePair<EndPoint, INatDevice> pair in unhandledDevices)
                 {
                     if (handledDevices.TryAdd(pair.Key, pair.Value))
                     {
-                        TResult result = await predicate(pair.Value);
+                        TResult result = await predicate(pair.Value, parameter);
                         if (result is true or not null)
                         {
                             return result;
                         }
                     }
                 }
             } while (!discoverTask.IsCompleted);
-            
+
             return default;
         }
     }

NitroxModel/Helper/NetHelper.cs

@@ -16,11 +16,11 @@ public static class NetHelper
     {
         private static readonly string[] privateNetworks =
             {
-                "10.0.0.0/8", 
-                "127.0.0.0/8", 
-                "172.16.0.0/12", 
-                "192.0.0.0/24 ", 
-                "192.168.0.0/16", 
+                "10.0.0.0/8",
+                "127.0.0.0/8",
+                "172.16.0.0/12",
+                "192.0.0.0/24 ",
+                "192.168.0.0/16",
                 "198.18.0.0/15",
             };
 
@@ -82,7 +82,7 @@ public static async Task<IPAddress> GetWanIpAsync()
 #if RELEASE
             if (ip == null || ip.IsPrivate())
             {
-                Regex regex = new(@"(?:[0-2]??[0-9]{1,2}\.){3}[0-2]??[0-9]+", RegexOptions.Compiled);
+                Regex regex = new(@"(?:[0-2]??[0-9]{1,2}\.){3}[0-2]??[0-9]+");
                 string[] sites =
                 {
                     "https://ipv4.icanhazip.com/",

NitroxPatcher/Patches/Persistent/GameSettings_SerializeInputSettings_Patch.cs

@@ -16,7 +16,6 @@ public static void Postfix(GameSettings.ISerializer serializer)
         {
             ClientConfig cfg = ClientConfig.Load(NitroxUser.AppDataPath);
             KeyBindingManager keyBindingManager = new();
-            string serializerFormat = "Input/Binding/{0}/{1}/{2}";
 
             foreach (GameInput.BindingSet bindingSet in Enum.GetValues(typeof(GameInput.BindingSet)))
             {

NitroxServer/Communication/LiteNetLib/LiteNetLibServer.cs

@@ -1,4 +1,5 @@
-using System.Threading;
+using System.Buffers;
+using System.Threading;
 using System.Threading.Tasks;
 using LiteNetLib;
 using LiteNetLib.Utils;
@@ -64,14 +65,18 @@ private async Task PortForwardAsync(ushort port)
             return;
         }
 
-        bool isMapped = await NatHelper.AddPortMappingAsync(port, Protocol.Udp);
-        if (isMapped)
+        NatHelper.ResultCodes mappingResult = await NatHelper.AddPortMappingAsync(port, Protocol.Udp);
+        switch (mappingResult)
         {
-            Log.Info($"Server port {port} UDP has been automatically opened on your router (port is closed when server closes)");
-        }
-        else
-        {
-            Log.Warn($"Failed to automatically port forward {port} UDP through UPnP. If using Hamachi or manually port-forwarding, please disregard this warning. To disable this feature you can go into the server settings.");
+            case NatHelper.ResultCodes.SUCCESS:
+                Log.Info($"Server port {port} UDP has been automatically opened on your router (port is closed when server closes)");
+                break;
+            case NatHelper.ResultCodes.CONFLICT_IN_MAPPING_ENTRY:
+                Log.Warn($"Port forward for {port} UDP failed. It appears to already be port forwarded or it conflicts with another port forward rule.");
+                break;
+            case NatHelper.ResultCodes.UNKNOWN_ERROR:
+                Log.Warn($"Failed to port forward {port} UDP through UPnP. If using Hamachi or you've manually port-forwarded, please disregard this warning. To disable this feature you can go into the server settings.");
+                break;
         }
     }
 
@@ -121,12 +126,18 @@ private void PeerDisconnected(NetPeer peer, DisconnectInfo disconnectInfo)
     private void NetworkDataReceived(NetPeer peer, NetDataReader reader, byte channel, DeliveryMethod deliveryMethod)
     {
         int packetDataLength = reader.GetInt();
-        byte[] packetData = new byte[packetDataLength];
-        reader.GetBytes(packetData, packetDataLength);
-
-        Packet packet = Packet.Deserialize(packetData);
-        NitroxConnection connection = GetConnection(peer.Id);
-        ProcessIncomingData(connection, packet);
+        byte[] packetData = ArrayPool<byte>.Shared.Rent(packetDataLength);
+        try
+        {
+            reader.GetBytes(packetData, packetDataLength);
+            Packet packet = Packet.Deserialize(packetData);
+            NitroxConnection connection = GetConnection(peer.Id);
+            ProcessIncomingData(connection, packet);
+        }
+        finally
+        {
+            ArrayPool<byte>.Shared.Return(packetData, true);
+        }
     }
 
     private NitroxConnection GetConnection(int remoteIdentifier)