Skip to content

Server Bootstrap: add client bootstrap configurator flow#3

Open
MhaWay wants to merge 4 commits intobootstrap-protocol-state-1.6from
bootstrap-client-flow-1.6
Open

Server Bootstrap: add client bootstrap configurator flow#3
MhaWay wants to merge 4 commits intobootstrap-protocol-state-1.6from
bootstrap-client-flow-1.6

Conversation

@MhaWay
Copy link
Copy Markdown
Owner

@MhaWay MhaWay commented Mar 27, 2026

Summary

This PR adds the client-side bootstrap configurator flow on top of the server bootstrap protocol/state work.

Includes

  • bootstrap configurator UI
  • map generation and reconnect flow
  • save upload flow
  • final client-side bootstrap hardening/refactor

Notes

This is PR 3 of a 3-PR stack.
Depends on: bootstrap-protocol-state-1.6.

MhaWay added 4 commits March 27, 2026 08:48
Add client-side bootstrap flow for configuring and uploading
server settings and save files to a headless server.

New files:
- ClientBootstrapState: connection state during bootstrap config
- BootstrapConfiguratorWindow: full UI for settings upload, vanilla
  new colony flow, save generation, reconnection, and save upload
- BootstrapCoordinator: GameComponent for reliable tick-based detection
- 4 Harmony patches: BootstrapMapInitPatch, BootstrapRootPlayPatch,
  BootstrapRootPlayUpdatePatch, BootstrapStartedNewGamePatch

Modified files:
- ClientJoiningState: HandleBootstrap handler + bootstrap redirect
- MultiplayerSession: serverIsInBootstrap, serverBootstrapSettingsMissing
- Autosaving: atomic file replace, null session check for bootstrap
- HostWindow: HostProgrammatically static method for bootstrap hosting
- ServerSettingsUI: DrawGameplaySettingsOnly wrapper
Copilot AI review requested due to automatic review settings March 27, 2026 09:34
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

Adds the client-side “bootstrap configurator” flow for servers started in bootstrap mode, building on the existing server bootstrap protocol/state work.

Changes:

  • Introduces a new client bootstrap connection state and UI window to configure/upload settings.toml and save.zip.
  • Adds bootstrap map-generation, autosave creation, reconnect, and save upload flow on the client.
  • Updates packet handler registration (via [PacketHandlerClass]) and hardens a few reconnect/offline behaviors.

Reviewed changes

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

Show a summary per file
File Description
Source/Common/PlayerManager.cs Allows pre-connect when server is in bootstrap mode even if not fully started.
Source/Common/Networking/State/ServerSteamState.cs Adds packet handler class attribute for state handler registration.
Source/Common/Networking/State/ServerPlayingState.cs Adds packet handler class attribute for state handler registration.
Source/Common/Networking/State/ServerLoadingState.cs Adds packet handler class attribute for state handler registration.
Source/Common/Networking/State/ServerJoiningState.cs Adds packet handler class attribute for state handler registration.
Source/Common/Networking/State/ServerBootstrapState.cs Adds handler-class attribute and a cursor packet drain handler during bootstrap.
Source/Client/Windows/ServerSettingsUI.cs Exposes gameplay-only settings drawing helper for bootstrap UI reuse.
Source/Client/Windows/HostWindow.cs Adds a programmatic hosting entry point used by bootstrap flow.
Source/Client/Windows/BootstrapConfiguratorWindow.cs Adds the main bootstrap configurator window + reconnection polling/visibility logic.
Source/Client/Windows/BootstrapConfiguratorWindow.SettingsUi.cs Implements settings tabs, TOML preview, and settings upload.
Source/Client/Windows/BootstrapConfiguratorWindow.BootstrapFlow.cs Implements map generation, autosave creation, reconnect, and save upload pipeline.
Source/Client/Session/MultiplayerSession.cs Tracks bootstrap mode + “settings missing” flags from handshake.
Source/Client/Session/Autosaving.cs Makes saving work when offline/bootstrap by ensuring replay dir exists and avoiding null session usage.
Source/Client/Patches/BootstrapStartedNewGamePatch.cs Adds a backup signal to trigger bootstrap “map initialized” handling.
Source/Client/Patches/BootstrapRootPlayUpdatePatch.cs Adds periodic arming of bootstrap map-init detection during Root_Play updates.
Source/Client/Patches/BootstrapRootPlayPatch.cs Adds bootstrap map-init arming at Root_Play.Start.
Source/Client/Patches/BootstrapMapInitPatch.cs Hooks map FinalizeInit to trigger bootstrap “map initialized” logic.
Source/Client/OnMainThread.cs Hardens LiteNetLib PollEvents exception during reconnect/disconnect teardown.
Source/Client/Networking/State/ClientSteamState.cs Adds packet handler class attribute for handler registration.
Source/Client/Networking/State/ClientPlayingState.cs Adds packet handler class attribute for handler registration.
Source/Client/Networking/State/ClientJoiningState.cs Handles bootstrap handshake packet and routes to bootstrap UI/state.
Source/Client/Networking/State/ClientBootstrapState.cs New client state for bootstrap-mode servers and bootstrap completion messaging.
Source/Client/Networking/State/ClientBaseState.cs Adds handler-class attribute and makes disconnect handler virtual for overrides.
Source/Client/MultiplayerStatic.cs Registers the new ClientBootstrap state implementation.
Source/Client/Comp/BootstrapCoordinator.cs Adds a GameComponent ticker to reliably drive bootstrap save trigger logic.
Source/Client/AsyncTime/SetMapTime.cs Adds null-guards for tick manager / async time component access.

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

Comment on lines +146 to +172
new System.Threading.Thread(() =>
{
try
{
connection.Send(new ClientBootstrapSettingsPacket(settings));

OnMainThread.Enqueue(() =>
{
isUploadingToml = false;
settingsUploaded = true;
statusText = "Server settings configured correctly. Proceed with map generation.";
step = Step.GenerateMap;

if (Multiplayer.session != null)
Multiplayer.session.serverBootstrapSettingsMissing = false;
});
}
catch (Exception e)
{
Log.Error($"Bootstrap settings upload failed: {e}");
OnMainThread.Enqueue(() =>
{
isUploadingToml = false;
statusText = $"Failed to upload settings: {e.GetType().Name}: {e.Message}";
});
}
}) { IsBackground = true, Name = "MP Bootstrap settings upload" }.Start();
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.

The settings upload runs on a raw background thread and calls connection.Send(...) while the UI can still mutate settings. ConnectionBase.Send<T> serializes immediately and isn't synchronized, so this risks racing with UI updates and with other sends from the main thread. Consider snapshotting/copying settings on the main thread before starting the worker, and marshal the actual Send call onto the main thread (or protect sending with a connection-level lock).

Suggested change
new System.Threading.Thread(() =>
{
try
{
connection.Send(new ClientBootstrapSettingsPacket(settings));
OnMainThread.Enqueue(() =>
{
isUploadingToml = false;
settingsUploaded = true;
statusText = "Server settings configured correctly. Proceed with map generation.";
step = Step.GenerateMap;
if (Multiplayer.session != null)
Multiplayer.session.serverBootstrapSettingsMissing = false;
});
}
catch (Exception e)
{
Log.Error($"Bootstrap settings upload failed: {e}");
OnMainThread.Enqueue(() =>
{
isUploadingToml = false;
statusText = $"Failed to upload settings: {e.GetType().Name}: {e.Message}";
});
}
}) { IsBackground = true, Name = "MP Bootstrap settings upload" }.Start();
// Snapshot the current settings into a packet on the main thread
var packet = new ClientBootstrapSettingsPacket(settings);
// Perform the send and subsequent state updates on the main thread to avoid threading issues
OnMainThread.Enqueue(() =>
{
try
{
connection.Send(packet);
isUploadingToml = false;
settingsUploaded = true;
statusText = "Server settings configured correctly. Proceed with map generation.";
step = Step.GenerateMap;
if (Multiplayer.session != null)
Multiplayer.session.serverBootstrapSettingsMissing = false;
}
catch (Exception e)
{
Log.Error($"Bootstrap settings upload failed: {e}");
isUploadingToml = false;
statusText = $"Failed to upload settings: {e.GetType().Name}: {e.Message}";
}
});

Copilot uses AI. Check for mistakes.
Comment on lines +423 to +437
new System.Threading.Thread(() =>
{
try
{
targetConn.Send(new ClientBootstrapSaveStartPacket("save.zip", bytes.Length));

const int chunk = 256 * 1024;
var sent = 0;
while (sent < bytes.Length)
{
var len = Math.Min(chunk, bytes.Length - sent);
var part = new byte[len];
Buffer.BlockCopy(bytes, sent, part, 0, len);
targetConn.SendFragmented(new ClientBootstrapSaveDataPacket(part).Serialize());
sent += len;
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.

The save upload sends many SendFragmented(...) calls from a background thread while the main thread can still send other packets (e.g. cursors/keepalives). ConnectionBase.SendFragmented is explicitly not thread-safe ("All fragmented packets need to be sent from the same thread"), and sendFragId is unsynchronized, so this risks corrupted fragmentation state or crashes. Consider doing all connection sends on a single thread (typically the main thread via OnMainThread.Enqueue), or add explicit locking/pausing of other outgoing traffic during upload.

Copilot uses AI. Check for mistakes.
Comment on lines +152 to +154
// Title
Widgets.Label(inRect.Down(0), "Server Bootstrap Configuration");
Text.Anchor = TextAnchor.UpperLeft;
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.

This window introduces multiple user-facing hardcoded English strings (e.g. title/status text) instead of using the translation system (Translate()) used elsewhere in client UI. Please add translation keys for these strings so they can be localized consistently.

Copilot uses AI. Check for mistakes.
Comment on lines +300 to +303
var localServer = new MultiplayerServer(settings);
Multiplayer.LocalServer = localServer;
HostUtil.HostServer(settings, false);
return true;
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.

HostProgrammatically creates a MultiplayerServer but does not perform the same network initialization/validation as TryStartLocalServer (e.g. liteNet.StartNet() and bind failure checks). If settings.direct/settings.lan are true, the server may not actually listen/bind, and the method still returns true. Consider reusing TryStartLocalServer (or factoring its logic into a shared helper) and returning success/failure based on actual startup results.

Suggested change
var localServer = new MultiplayerServer(settings);
Multiplayer.LocalServer = localServer;
HostUtil.HostServer(settings, false);
return true;
try
{
var localServer = new MultiplayerServer(settings);
Multiplayer.LocalServer = localServer;
HostUtil.HostServer(settings, false);
return true;
}
catch (Exception)
{
Multiplayer.LocalServer = null;
return false;
}

Copilot uses AI. Check for mistakes.
{
// Drain the packet so the "not fully consumed" check passes
data.ReadByte(); // seq
data.ReadByte(); // map
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.

HandleCursor only reads seq and map, but the Client_Cursor packet contains additional fields when map < byte.MaxValue (icon, x/z, drag box). If a cursor packet is sent with an in-map cursor during bootstrap, the packet will not be fully consumed and will trigger the "Packet was not fully consumed" error path. Consider consuming the remaining fields conditionally (mirroring ServerPlayingState.HandleCursor) or explicitly seeking to the end of the reader.

Suggested change
data.ReadByte(); // map
data.ReadByte(); // map
// Consume any remaining fields (icon, coordinates, drag box, or future extensions)
while (data.Position < data.Length)
{
data.ReadByte();
}

Copilot uses AI. Check for mistakes.
Comment on lines +95 to +99
public BootstrapConfiguratorWindow(ConnectionBase connection)
{
this.connection = connection;
Instance = this;

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.

BootstrapConfiguratorWindow.Instance is set in the constructor but never cleared. Since several Harmony patches and BootstrapCoordinator poll Instance, a closed/removed window can leave a stale reference and keep bootstrap logic running unexpectedly. Override PostClose()/Close() to clear Instance and reset static bootstrap flags (e.g. AwaitingBootstrapMapInit) when the flow ends.

Copilot uses AI. Check for mistakes.
Comment on lines +403 to +407
byte[] bytes;
try
{
bytes = File.ReadAllBytes(savedReplayPath);
}
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.

File.ReadAllBytes(savedReplayPath) loads the entire save.zip into memory at once. RimWorld saves can be large, so this can cause high memory usage or OOM during bootstrap. Consider streaming the file and uploading in chunks (while computing SHA-256 incrementally) instead of buffering the full byte array.

Copilot uses AI. Check for mistakes.
{
Multiplayer.session?.netClient?.PollEvents();
}
catch (InvalidOperationException e) when (e.Message == "Queue empty." || e.Message == "Coda vuota.")
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.

The InvalidOperationException filter relies on exact, localized exception messages (only English/Italian are handled). On other locales the same LiteNetLib queue-race can still crash, and message matching is generally brittle. Consider centralizing this check (there is already LiteNetManager.IsQueueEmptyRace) and/or using a more robust condition (e.g. catching the specific LiteNetLib call site) instead of comparing localized strings.

Suggested change
catch (InvalidOperationException e) when (e.Message == "Queue empty." || e.Message == "Coda vuota.")
catch (InvalidOperationException e) when (LiteNetManager.IsQueueEmptyRace(e))

Copilot uses AI. Check for mistakes.

// Poll LiteNet during reconnection
if (isReconnecting && Multiplayer.session?.netClient != null)
Multiplayer.session.netClient.PollEvents();
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.

Multiplayer.session.netClient.PollEvents() can throw InvalidOperationException ("Queue empty."/localized) during reconnect/disconnect teardown (as noted in OnMainThread.Update). Here it is called without a guard, so the bootstrap reconnection path can still crash the client. Wrap this call with the same catch logic (or route polling through a shared safe helper).

Suggested change
Multiplayer.session.netClient.PollEvents();
{
try
{
Multiplayer.session.netClient.PollEvents();
}
catch (InvalidOperationException)
{
// PollEvents can throw during reconnect/disconnect teardown; ignore to avoid crashing the client.
}
}

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