Skip to content

Server Bootstrap: add server bootstrap protocol and state flow#2

Open
MhaWay wants to merge 3 commits intobootstrap-runtime-compat-1.6from
bootstrap-protocol-state-1.6
Open

Server Bootstrap: add server bootstrap protocol and state flow#2
MhaWay wants to merge 3 commits intobootstrap-runtime-compat-1.6from
bootstrap-protocol-state-1.6

Conversation

@MhaWay
Copy link
Copy Markdown
Owner

@MhaWay MhaWay commented Mar 27, 2026

Summary

This PR adds the server-side bootstrap protocol and state handling on top of the runtime compatibility base.

Includes

  • bootstrap server mode/state flow
  • bootstrap packet handling
  • settings/save upload handling
  • shared server/bootstrap protocol changes

Notes

This is PR 2 of a 3-PR stack.
Depends on: bootstrap-runtime-compat-1.6
Next PR base: bootstrap-protocol-state-1.6.

MhaWay added 3 commits March 26, 2026 23:09
- Add ServerSettingsUI static class with DrawNetworkingSettings() and DrawGameplaySettings()
- Add BufferSet class for managing text field buffers across UI calls
- Move networking settings (max players, password, direct/LAN/steam, sync configs) to ServerSettingsUI
- Move gameplay settings (autosave, multifaction, async time, time control, etc.) to ServerSettingsUI
- Move helper methods (DoTimeControl, DoPauseOnLetter, DrawJoinPointOptions, CustomButton, LeftLabel, DoRow) to ServerSettingsUI
- HostWindow now delegates to ServerSettingsUI via BufferSet pattern
- Steam checkbox now always shown (disabled when unavailable) instead of hidden
- Add BootstrapMode.cs: loop that keeps server alive while waiting for client configuration
- Add ServerBootstrapState: handles settings upload (via TomlSettingsCommon), save.zip upload
  with SHA256 verification, then disconnects clients with BootstrapCompleted and stops
- Add ServerBootstrapPacket: sent during handshake to signal bootstrap mode to client
- Add bootstrap upload packets: ClientBootstrapSettingsPacket, ClientBootstrapSaveStartPacket,
  ClientBootstrapSaveDataPacket (fragmented), ClientBootstrapSaveEndPacket
- Add ServerSettingsPacketBinder for serializing ServerSettings over the wire
- Add TomlSettingsCommon for saving ServerSettings to TOML from Common project
- Add Tomlyn dependency to Common.csproj
- Add ConnectionStateEnum: ClientBootstrap, ServerBootstrap
- Add 7 bootstrap packet entries to Packets enum
- Add MpDisconnectReason.BootstrapCompleted
- Add SendToPlaying<T> generic overload to MultiplayerServer
- Register ServerBootstrapState implementation in MultiplayerServer static constructor
- Modify ServerJoiningState: send ServerBootstrapPacket during handshake, redirect to
  ServerBootstrap state when server is in bootstrap mode
- Modify Server.cs: detect missing save.zip/settings.toml, enable bootstrap mode,
  call BootstrapMode.WaitForClient
- Handle BootstrapCompleted in SessionDisconnectInfo.From()
- Add MpBootstrapCompleted/MpBootstrapCompletedDesc translation keys
Copilot AI review requested due to automatic review settings March 27, 2026 09:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a server-side “bootstrap mode” to allow starting a server without settings.toml and/or save.zip, keeping the connection alive so a client can upload initial configuration and world data. It also adds new protocol packets/state transitions to support the bootstrap flow, plus UI refactoring to share server-settings drawing code.

Changes:

  • Add bootstrap startup flow to the dedicated server entrypoint and a new ServerBootstrapState for handling settings/save uploads.
  • Add new bootstrap packets / disconnect reason / connection states, and wire server state mapping.
  • Refactor client hosting settings UI into a shared ServerSettingsUI helper.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
Source/Server/Server.cs Detects missing settings/save and enables bootstrap mode; waits for a client upload flow.
Source/Server/BootstrapMode.cs Adds a minimal “keep-alive” loop while waiting for bootstrap completion.
Source/Common/Util/TomlSettingsCommon.cs Adds Serialize() and reuses it in Save().
Source/Common/ServerSettings.cs Fixes serialization keys for asyncTime and multifaction.
Source/Common/Networking/State/ServerJoiningState.cs Diverts connections into ServerBootstrap and sends a bootstrap handshake packet.
Source/Common/Networking/State/ServerBootstrapState.cs Implements server-side bootstrap upload handling and shutdown signaling.
Source/Common/Networking/Packets.cs Adds bootstrap packet ids.
Source/Common/Networking/Packet/BootstrapUploadPackets.cs Defines bootstrap upload packet structs + settings binder.
Source/Common/Networking/Packet/BootstrapPacket.cs Defines ServerBootstrapPacket.
Source/Common/Networking/MpDisconnectReason.cs Adds BootstrapCompleted reason.
Source/Common/Networking/ConnectionStateEnum.cs Adds ClientBootstrap / ServerBootstrap enum values.
Source/Common/MultiplayerServer.cs Registers ServerBootstrapState, adds BootstrapMode, adds typed SendToPlaying<T>.
Source/Common/Common.csproj Adds Tomlyn package reference.
Source/Client/Windows/ServerSettingsUI.cs New shared UI renderer for server settings fields.
Source/Client/Windows/HostWindow.cs Refactors to use ServerSettingsUI.
Source/Client/Session/SessionDisconnectInfo.cs Adds UI messaging for BootstrapCompleted.

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

Comment on lines +10 to +46
[PacketDefinition(Packets.Client_BootstrapSettingsUploadStart)]
public record struct ClientBootstrapSettingsPacket(ServerSettings settings) : IPacket
{
public ServerSettings settings = settings;

public void Bind(PacketBuffer buf)
{
ServerSettingsPacketBinder.Bind(buf, ref settings);
}
}

/// <summary>
/// Upload start metadata for bootstrap configuration.
/// The client will send exactly one file: a pre-built save.zip (server format).
/// </summary>
[PacketDefinition(Packets.Client_BootstrapUploadStart)]
public record struct ClientBootstrapSaveStartPacket(string fileName, int length) : IPacket
{
public string fileName = fileName;
public int length = length;

public void Bind(PacketBuffer buf)
{
buf.Bind(ref fileName);
buf.Bind(ref length);
}
}

/// <summary>
/// Upload raw bytes for the save.zip.
/// This packet is expected to be delivered fragmented due to size.
/// </summary>
[PacketDefinition(Packets.Client_BootstrapUploadData, allowFragmented: true)]
public record struct ClientBootstrapSaveDataPacket(byte[] data) : IPacket
{
public byte[] data = data;

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same compilation issue as ServerBootstrapPacket: these positional record struct(...) declarations redeclare members with identical names (e.g., public string fileName = fileName;), which will conflict with the compiler-generated members for the primary constructor parameters. Convert these to non-positional structs/record structs with explicit fields + constructors so the code compiles and Bind can use ref fields.

Copilot uses AI. Check for mistakes.
Comment on lines +120 to +134
// Accumulate fragmented upload data
if (pendingZipBytes == null)
{
pendingZipBytes = packet.data;
}
else
{
// Append new chunk to existing data
var oldLen = pendingZipBytes.Length;
var newChunk = packet.data;
var combined = new byte[oldLen + newChunk.Length];
Buffer.BlockCopy(pendingZipBytes, 0, combined, 0, oldLen);
Buffer.BlockCopy(newChunk, 0, combined, oldLen, newChunk.Length);
pendingZipBytes = combined;
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HandleSaveData repeatedly concatenates byte arrays, making upload assembly O(n²) in time and allocations (and can cause large GC pressure/OOM for big saves). Accumulate into a pre-sized buffer (using pendingLength) or stream to a temp file/MemoryStream with capacity, and reject/abort if received bytes exceed the declared length or a sane maximum.

Copilot uses AI. Check for mistakes.
Comment on lines +171 to +178
ServerLog.Log($"Bootstrap: wrote '{targetPath}'. Configuration complete; disconnecting clients and stopping.");

// Notify and disconnect all clients.
Server.SendToPlaying(new ServerDisconnectPacket { reason = MpDisconnectReason.BootstrapCompleted, data = Array.Empty<byte>() });

// Stop the server loop; an external supervisor should restart.
Server.running = false;
Server.TryStop();
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bootstrap completion currently calls Server.SendToPlaying(new ServerDisconnectPacket { reason = BootstrapCompleted, ... }), but in bootstrap mode clients are not in PlayingPlayers, so they likely won’t receive the reason-specific disconnect. Instead, disconnect all connected/joined players (e.g., iterate Server.JoinedPlayers and call player.Disconnect(MpDisconnectReason.BootstrapCompleted)), then stop the server so clients see the correct UI message.

Copilot uses AI. Check for mistakes.

public void Bind(PacketBuffer buf)
{
buf.BindBytes(ref sha256Hash, maxLength: 32);
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buf.BindBytes(ref sha256Hash, maxLength: 32) does not currently enforce maxLength (PacketReader/Writer ignore it), so a malicious client could send an oversized hash buffer and force large allocations. Add explicit validation in Bind/handler (e.g., check sha256Hash.Length == 32 when provided) before using it.

Suggested change
buf.BindBytes(ref sha256Hash, maxLength: 32);
buf.BindBytes(ref sha256Hash, maxLength: 32);
// Enforce exact SHA-256 hash length to prevent oversized allocations.
if (sha256Hash == null || sha256Hash.Length != 32)
throw new ArgumentException("Invalid SHA-256 hash length in ClientBootstrapSaveEndPacket; expected 32 bytes.", nameof(sha256Hash));

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +50
var bootstrap = !File.Exists(settingsFile);

var consoleSource = new ConsoleSource();

LoadSave(server, saveFile);
if (!bootstrap && File.Exists(saveFile))
{
LoadSave(server, saveFile);
}
else
{
bootstrap = true;
ServerLog.Log($"Bootstrap mode: '{saveFile}' not found. Server will start without a loaded save.");
ServerLog.Log("Waiting for a client to upload world data.");
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bootstrap is derived from missing settings, but later the save-load branch treats any bootstrap==true as “save missing” and logs that even when save.zip exists (e.g., settings missing + save present). Consider tracking settingsMissing and saveMissing separately, and base BootstrapMode strictly on whether a save was loaded (per the property docstring), so logs and behavior stay consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +65
// Let the client know early when the server is in bootstrap mode so it can switch
// to server-configuration flow while keeping the connection open.
var settingsMissing = false;
if (Server.BootstrapMode)
settingsMissing = !File.Exists(Path.Combine(System.AppContext.BaseDirectory, "settings.toml"));

Player.conn.Send(new ServerBootstrapPacket(Server.BootstrapMode, settingsMissing));
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServerBootstrapPacket is sent on every successful protocol handshake (even when Server.BootstrapMode is false). In this PR there is no client-side handler for Packets.Server_Bootstrap, so clients will throw “No handler for packet … in state ClientJoining” and disconnect. Either add a client handler/state in this PR, or only send this packet when bootstrap mode is enabled and the client version/protocol supports it.

Suggested change
// Let the client know early when the server is in bootstrap mode so it can switch
// to server-configuration flow while keeping the connection open.
var settingsMissing = false;
if (Server.BootstrapMode)
settingsMissing = !File.Exists(Path.Combine(System.AppContext.BaseDirectory, "settings.toml"));
Player.conn.Send(new ServerBootstrapPacket(Server.BootstrapMode, settingsMissing));
// In bootstrap mode, let the client know early so it can switch
// to server-configuration flow while keeping the connection open.
if (Server.BootstrapMode)
{
var settingsMissing = !File.Exists(Path.Combine(System.AppContext.BaseDirectory, "settings.toml"));
Player.conn.Send(new ServerBootstrapPacket(true, settingsMissing));
}

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +13
public record struct ServerBootstrapPacket(bool bootstrap, bool settingsMissing = false) : IPacket
{
public bool bootstrap = bootstrap;
public bool settingsMissing = settingsMissing;

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These packet types use a positional record struct(...) and then declare fields with the same names (e.g., public bool bootstrap = bootstrap;). Record structs already generate members for positional parameters, so this will not compile due to duplicate member names. Use a non-positional record struct/struct and define the fields explicitly (or use different names) so Bind can take ref fields.

Suggested change
public record struct ServerBootstrapPacket(bool bootstrap, bool settingsMissing = false) : IPacket
{
public bool bootstrap = bootstrap;
public bool settingsMissing = settingsMissing;
public record struct ServerBootstrapPacket : IPacket
{
public bool bootstrap;
public bool settingsMissing;
public ServerBootstrapPacket(bool bootstrap, bool settingsMissing = false)
{
this.bootstrap = bootstrap;
this.settingsMissing = settingsMissing;
}

Copilot uses AI. Check for mistakes.
<PackageReference Include="LiteNetLib" Version="0.9.5.2" />
<PackageReference Include="Lib.Harmony" Version="2.3.6" ExcludeAssets="runtime" />
<PackageReference Include="System.IO.Compression" Version="4.3.0" />
<PackageReference Include="Tomlyn" Version="0.16.2" />
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common.csproj adds a Tomlyn package reference, but the codebase explicitly says it does not use Tomlyn (and there are no using Tomlyn references). This introduces an unused dependency (and may cause net48 compatibility issues depending on Tomlyn’s TFMs). Remove the package reference unless it’s required in this PR, or update the implementation/docs accordingly.

Suggested change
<PackageReference Include="Tomlyn" Version="0.16.2" />

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +75
// Bootstrap
Client_BootstrapSettingsUploadStart,
Client_BootstrapSettingsUploadData,
Client_BootstrapSettingsUploadFinish,
Client_BootstrapUploadStart,
Client_BootstrapUploadData,
Client_BootstrapUploadFinish,
Server_Bootstrap,
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client_BootstrapSettingsUploadData / Client_BootstrapSettingsUploadFinish are declared in Packets but are not used anywhere in this PR. Given the tight 6-bit packet id space, consider removing unused packet ids until the multi-packet settings upload is implemented, to avoid consuming ids prematurely.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants