Server Bootstrap runtime compat 1.6 by MhaWay · Pull Request #1 · MhaWay/Multiplayer

MhaWay · 2026-03-27T09:31:55Z

Summary

This PR introduces the runtime compatibility work needed for the bootstrap/server flow on .NET Framework 4.8.

Includes

net48 runtime compatibility fixes
shared TOML settings serialization cleanup
server startup/runtime fixes needed for bootstrap and normal startup

Notes

This is PR 1 of a 3-PR stack.
Next PR base: bootstrap-runtime-compat-1.6.

- Add IPacket interface with PacketBuffer/PacketReader/PacketWriter for strongly-typed packet serialization/deserialization - Add PacketDefinitionAttribute, SerializedPacket, PacketTypeInfo<T> - Add FragmentedPacket record for structured fragment handling - Add ByteReader.ReadEnum<T>() and ByteWriter.WriteEnum<T>() helpers - Refactor PacketHandlerAttribute: replace IsFragmentedAttribute with allowFragmented parameter, add TypedPacketHandlerAttribute, PacketHandlerClassAttribute, FragmentedPacketHandlerAttribute - Add PacketHandlerInvoker delegate type replacing FastInvokeHandler for packet handler invocation - Refactor MpConnectionState: encapsulate handler lookup via GetPacketHandler(), add RegisterTypedPacketHandler and RegisterFragmentedPacketHandler with DynamicMethod IL emit - Add typed packet support to AsyncConnectionState: TypedPacket<T>(), TypedPacketOrNull<T>(), TypedPacketAwaitable<T>() - Update ConnectionBase: add Send<T> for typed packets, Send(SerializedPacket), SendFragmented(SerializedPacket), use WriteEnum/WriteRaw in GetDisconnectBytes - Migrate all [IsFragmented] attributes to [PacketHandler(allowFragmented)]

- Rewrite SendFragmented() with smaller chunk sizes (1KB vs 64KB), fragment IDs for tracking, and expected parts/size metadata headers - Rewrite fragment receive handling with HandleReceiveFragment(): fragment reassembly by ID, size validation, concurrent fragment limit - Add ExecuteMessageHandler() with error handling and hex dump on failure - Rename FragmentSize -> MaxSinglePacketSize, MaxPacketSize -> MaxFragmentPacketTotalSize, add MaxFragmentPacketSize - Add packet consumption check in HandleReceiveRaw - Add fragment progress handler support via PacketHandlerInfo.FragmentHandler - Update ClientLoadingState: add WorldExpectedSize/WorldReceivedSize tracking, download speed calculation, fragment handler for progress, game load timing - Update ConnectingWindow: download progress bar with speed, ETA, and percentage display

- Add ServerDisconnectPacket record struct with BindEnum(reason) + BindRemaining(data) - Add Server_Disconnect packet type to Packets enum - Add IsServer()/IsClient() extension methods to ConnectionStateEnum - Refactor ConnectionBase.Close() from abstract to concrete: sends ServerDisconnectPacket for server-side states, then calls OnClose() - Add abstract OnClose() to ConnectionBase, override in all subclasses (LiteNetConnection, NetworkingInMemory, NetworkingSteam, ReplayConnection) - Add typed HandleDisconnected handler in ClientBaseState - Add SessionDisconnectInfo.From() factory method mapping MpDisconnectReason to UI strings - Update IConnectionStatusListener.Disconnected() to accept SessionDisconnectInfo - Remove ProcessDisconnectPacket() and disconnectInfo field from MultiplayerSession - Update all callers: NetworkingLiteNet, NetworkingSteam, ClientUtil, ConnectingWindow

Copilot

Pull request overview

This PR implements .NET Framework 4.8 runtime compatibility for the server/bootstrap flow by removing incompatible dependencies, tightening networking/runtime behavior, and introducing new packet/serialization infrastructure to support disconnect propagation and typed packet handling.

Changes:

Removed Tomlyn-based server settings handling and replaced it with a shared, manual TOML reader/writer to avoid netstandard 2.1 dependency issues on net48.
Introduced a typed packet system (IPacket, packet definitions, typed handlers) and added a server-to-client disconnect packet.
Applied several net48/runtime-compat fixes across networking/reflection/collections and improved robustness around LiteNetLib event polling and packet handling.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
Source/Server/TomlSettings.cs	Removed Tomlyn-based TOML settings implementation.
Source/Server/Server.csproj	Removed Tomlyn package reference for net48 compatibility.
Source/Server/Server.cs	Uses shared TOML settings loader, sets CWD, adds LAN IP detection helper, and adjusts string splitting overloads for net48.
Source/Common/Util/TomlSettingsCommon.cs	Adds manual TOML key/value parser + serializer for `ServerSettings`.
Source/Common/Util/MpReflection.cs	Replaces `GetValueOrDefault` with `TryGetValue` for net48 compatibility.
Source/Common/Syncing/Logger/LoggingByteWriter.cs	Normalizes file header/encoding (BOM).
Source/Common/Syncing/Logger/LoggingByteReader.cs	Normalizes file header/encoding (BOM).
Source/Common/ServerSettings.cs	Initializes `gameName` to a non-null default.
Source/Common/Networking/Packets.cs	Adds `Server_Disconnect` packet id for all states.
Source/Common/Networking/PacketReadException.cs	Adds inner-exception constructor for better error propagation.
Source/Common/Networking/PacketHandlerAttribute.cs	Adds typed handler + class handler attributes and changes handler invocation delegate type.
Source/Common/Networking/Packet/IPacket.cs	Introduces typed packet abstractions, serialization helpers, and packet buffer binding API.
Source/Common/Networking/Packet/DisconnectPacket.cs	Adds typed `ServerDisconnectPacket`.
Source/Common/Networking/MpConnectionState.cs	Updates handler registration to support typed handlers and optional inheritance behavior.
Source/Common/Networking/LiteNetConnection.cs	Refactors `Close` to `OnClose` and changes disconnect behavior.
Source/Common/Networking/ConnectionStateEnum.cs	Adds `IsClient`/`IsServer` helpers.
Source/Common/Networking/ConnectionBase.cs	Adds typed send APIs, fragmented-send size checks, better receive consumption, and wraps handler execution with richer exceptions.
Source/Common/Networking/AsyncConnectionState.cs	Adds typed packet await helpers and refactors `PacketAwaitable` plumbing.
Source/Common/MultiplayerServer.cs	Adjusts string split overloads for net48.
Source/Common/LiteNetManager.cs	Makes polling safer during disconnect/shutdown and avoids collection modification during iteration.
Source/Common/ByteWriter.cs	Normalizes file header/encoding (BOM).
Source/Common/ByteReader.cs	Normalizes file header/encoding (BOM).
Source/Common/ActionQueue.cs	Replaces `Queue<T>` with `List<T>` for cross-runtime type loading compatibility.
Source/Client/Windows/ConnectingWindow.cs	Updates disconnect callback signature and translation formatting.
Source/Client/Session/SessionDisconnectInfo.cs	Centralizes disconnect message construction from reason + reader and adds logging.
Source/Client/Session/MultiplayerSession.cs	Removes stored disconnect info state and passes disconnect info through callbacks.
Source/Client/Saving/ReplayConnection.cs	Updates to `OnClose` override.
Source/Client/Networking/State/ClientLoadingState.cs	Enables inherited handlers for base disconnect handling; adds download/load timing.
Source/Client/Networking/State/ClientBaseState.cs	Adds typed disconnect handler to map server disconnect packet to UI/stop flow.
Source/Client/Networking/NetworkingSteam.cs	Refactors disconnect flow to use `SessionDisconnectInfo` and updates special disconnect handling.
Source/Client/Networking/NetworkingLiteNet.cs	Refactors disconnect parsing and fallback handling to use `SessionDisconnectInfo`.
Source/Client/Networking/NetworkingInMemory.cs	Updates to `OnClose` override.
Source/Client/Networking/ConnectionStatusListeners.cs	Changes listener API to pass `SessionDisconnectInfo`.
Source/Client/Networking/ClientUtil.cs	Uses structured disconnect info for local packet handling failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T09:44:57Z

Source/Common/Networking/MpConnectionState.cs

+            var bindingFlags = BindingFlags.Instance | BindingFlags.Public;
+            if (typeAttr?.inheritHandlers != true) bindingFlags |= BindingFlags.DeclaredOnly;
+
+            foreach (var method in type.GetMethods(bindingFlags))
            {
                var attr = method.GetAttribute<PacketHandlerAttribute>();
                if (attr != null) RegisterPacketHandler(state, method, attr);

                var attr2 = method.GetAttribute<FragmentedPacketHandlerAttribute>();
                if (attr2 != null) RegisterFragmentedPacketHandler(state, method, attr2);
+
+                var attr3 = method.GetAttribute<TypedPacketHandlerAttribute>();
+                if (attr3 != null) RegisterTypedPacketHandler(state, method);
            }


Because handler scanning defaults to DeclaredOnly unless [PacketHandlerClass(inheritHandlers: true)] is present, any shared handlers placed on a base state class (e.g., the new typed disconnect handler on ClientBaseState) will not be registered for most states. Either default to including inherited methods, or ensure every state that should inherit base handlers is annotated appropriately; otherwise Server_Disconnect may be treated as an unhandled packet.

Copilot · 2026-03-27T09:44:58Z

Source/Client/Networking/NetworkingSteam.cs


-        public override void Close(MpDisconnectReason reason, byte[] data)
+        protected override void OnClose()
        {


SteamBaseConn.OnClose() is now a no-op, but both SteamClientConn and SteamServerConn still have logic to handle Packets.Special_Steam_Disconnect. If the client calls Close(), the server may no longer be notified promptly (potentially leaving ghost players until a timeout). Consider either sending Special_Steam_Disconnect from OnClose() (client->server), or explicitly closing the P2P session via Steamworks APIs.

Suggested change

{

{

// Ensure the Steam P2P session is explicitly closed so the remote side

// is promptly notified of the disconnect (avoids ghost players).

SteamNetworking.CloseP2PSessionWithUser(remoteId);

Copilot · 2026-03-27T09:44:58Z

Source/Common/ActionQueue.cs

                while (tempQueue.Count > 0)
-                    tempQueue.Dequeue().Invoke();
+                {
+                    var action = tempQueue[0];
+                    tempQueue.RemoveAt(0);
+                    action.Invoke();
+                }


ActionQueue.RunQueue now removes from the front of a List<Action> via RemoveAt(0) in a loop, which is O(n) per dequeue and can become O(n²) for larger bursts. If this runs per-frame on the main thread, it could cause spikes; consider using a moving head index (and occasionally trimming) or removing from the end instead.

Copilot · 2026-03-27T09:44:58Z

Source/Common/Util/TomlSettingsCommon.cs

+            foreach (var line in File.ReadAllLines(filename))
+            {
+                var trimmed = line.Trim();
+                if (trimmed.Length == 0 || trimmed.StartsWith("#"))
+                    continue;
+
+                var eqIdx = trimmed.IndexOf('=');
+                if (eqIdx < 0)
+                    continue;
+
+                var key = trimmed.Substring(0, eqIdx).Trim();
+                var val = trimmed.Substring(eqIdx + 1).Trim();
+                data[key] = val;
+            }


ParseFile doesn't strip inline comments (e.g., maxPlayers = 8 # comment) or handle TOML # after values. Since this is replacing Tomlyn, existing/hand-edited configs with inline comments will now fail to parse for numeric/enum fields. Consider trimming inline comments outside quoted strings before storing val.

Copilot · 2026-03-27T09:44:58Z

Source/Common/Util/TomlSettingsCommon.cs

+
+            if (type == typeof(bool))
+                return (T)(object)(raw.Equals("true", StringComparison.OrdinalIgnoreCase));
+


Boolean parsing treats any non-true value as false (raw.Equals("true", ...)), so invalid values like lan = tru or lan = 1 silently become false. This makes misconfigurations hard to notice compared to the previous Tomlyn-based parsing. Consider explicitly handling false and throwing (or at least logging) on unrecognized boolean literals.

Copilot · 2026-03-27T09:44:59Z

Source/Server/Server.cs

+static string GetLocalIpAddress()
+{
+    try
+    {
+        using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.IP))
+        {
+            socket.Connect("8.8.8.8", 65530);
+            var endPoint = socket.LocalEndPoint as IPEndPoint;
+            return endPoint.Address.ToString();
+        }
+    }
+    catch
+    {
+        return Dns.GetHostEntry(Dns.GetHostName()).AddressList.FirstOrDefault(i => i.AddressFamily == AddressFamily.InterNetwork)?.ToString();
+    }


GetLocalIpAddress is declared as returning string, but the fallback path can return null (FirstOrDefault(...)?). With nullable enabled this is a mismatch, and it also forces callers to rely on ?? to handle a value that the signature claims can't be null. Consider changing the return type to string? or always returning a non-null string (e.g., "127.0.0.1").

Copilot · 2026-03-27T09:44:59Z

Source/Common/LiteNetManager.cs

+
+        private static bool IsQueueEmptyRace(InvalidOperationException e)
+        {
+            return e.Message == "Queue empty." || e.Message == "Coda vuota.";


SafePollEvents detects the LiteNetLib race by matching InvalidOperationException.Message strings (currently English + Italian). This is brittle (message text can change between versions/locales) and risks either missing the race or swallowing unrelated exceptions. Prefer a more robust check (e.g., verifying the throwing method via stack trace, or catching only around the known dequeue site if possible).

Suggested change

return e.Message == "Queue empty." || e.Message == "Coda vuota.";

var stackTrace = e.StackTrace;

if (string.IsNullOrEmpty(stackTrace))

return false;

// Detect the known LiteNetLib event-queue race more robustly by checking

// for Queue<T>.Dequeue inside LiteNetLib rather than relying on

// localized exception messages.

return stackTrace.IndexOf("System.Collections.Generic.Queue`1.Dequeue", StringComparison.Ordinal) >= 0

&& stackTrace.IndexOf("LiteNetLib", StringComparison.Ordinal) >= 0;

Copilot · 2026-03-27T09:45:00Z

Source/Client/Session/SessionDisconnectInfo.cs

+        var disconnectInfo = new SessionDisconnectInfo();
+        string titleKey = null;
+        string descKey = null;
+


With nullable enabled, titleKey/descKey are declared as non-nullable string but initialized to null, which will produce warnings and makes the nullability contract misleading. Use string? for these locals (and potentially for the struct fields too if they can legitimately be unset).

Copilot · 2026-03-27T09:45:00Z

Source/Client/Session/SessionDisconnectInfo.cs

+
+        Log.Message($"Processed disconnect packet ({reason}). Title: {disconnectInfo.titleTranslated} ({titleKey}), " +
+                    $"description: {disconnectInfo.descTranslated} ({descKey})");


SessionDisconnectInfo.From logs a Log.Message on every disconnect, including expected ones. This may spam the RimWorld log for normal user flows. Consider downgrading to a verbose/debug log, or gating it behind a debug flag.

Suggested change

Log.Message($"Processed disconnect packet ({reason}). Title: {disconnectInfo.titleTranslated} ({titleKey}), " +

$"description: {disconnectInfo.descTranslated} ({descKey})");

#if DEBUG

Log.Message($"Processed disconnect packet ({reason}). Title: {disconnectInfo.titleTranslated} ({titleKey}), " +

$"description: {disconnectInfo.descTranslated} ({descKey})");

#endif

Copilot · 2026-03-27T09:45:00Z

Source/Common/Networking/MpConnectionState.cs

+            var typeAttr = type.GetAttribute<PacketHandlerClassAttribute>();
+            if (typeAttr == null)
+                ServerLog.Log($"Packet handler {type.FullName} does not have a PacketHandlerClass attribute");
+
+            var bindingFlags = BindingFlags.Instance | BindingFlags.Public;
+            if (typeAttr?.inheritHandlers != true) bindingFlags |= BindingFlags.DeclaredOnly;
+


SetImplementation now logs when [PacketHandlerClass] is missing, but most existing state types (client + server + tests) don't appear to have this attribute. This will generate noisy logs at startup even though the default behavior (DeclaredOnly) matches the old scanning behavior. Consider removing this log, or making the attribute required/enforced consistently across all state implementations.

MhaWay added 4 commits March 26, 2026 23:07

Bootstrap: fix net48 runtime compatibility

9498281

Copilot AI review requested due to automatic review settings March 27, 2026 09:31

Copilot started reviewing on behalf of MhaWay March 27, 2026 09:32 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Server Bootstrap runtime compat 1.6#1

Server Bootstrap runtime compat 1.6#1
MhaWay wants to merge 4 commits intobase-1.6from
bootstrap-runtime-compat-1.6

MhaWay commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        {
+        {
+            // Ensure the Steam P2P session is explicitly closed so the remote side
+            // is promptly notified of the disconnect (avoids ghost players).
+            SteamNetworking.CloseP2PSessionWithUser(remoteId);


		if (type == typeof(bool))
		return (T)(object)(raw.Equals("true", StringComparison.OrdinalIgnoreCase));

-            return e.Message == "Queue empty." || e.Message == "Coda vuota.";
+            var stackTrace = e.StackTrace;
+            if (string.IsNullOrEmpty(stackTrace))
+                return false;
+            // Detect the known LiteNetLib event-queue race more robustly by checking
+            // for Queue<T>.Dequeue inside LiteNetLib rather than relying on
+            // localized exception messages.
+            return stackTrace.IndexOf("System.Collections.Generic.Queue`1.Dequeue", StringComparison.Ordinal) >= 0
+                   && stackTrace.IndexOf("LiteNetLib", StringComparison.Ordinal) >= 0;


		Log.Message($"Processed disconnect packet ({reason}). Title: {disconnectInfo.titleTranslated} ({titleKey}), " +
		$"description: {disconnectInfo.descTranslated} ({descKey})");

Conversation

MhaWay commented Mar 27, 2026

Summary

Includes

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants