Skip to content

Conversation

@MhaWay
Copy link

@MhaWay MhaWay commented Jan 11, 2026

Add full in-game bootstrap flow so the first client can configure and upload settings.toml, generate the map, and auto-upload save.zip without manual file handling.
Arm map-init detection during the TOML-created path so colonist detection and saving always trigger after vanilla generation when in bootstrap mode.
Use the reconnecting connection for save.zip uploads.
Simplify UI strings (hardcoded English) and clean the workflow messaging for admins.
Build: succeeds locally; verified both scenarios:
TOML already exists → generate map → colonists detected → save uploaded.
TOML created in-session → generate map → colonists detected → save uploaded.

mibac138 and others added 27 commits December 17, 2025 12:39
The only remaining difference is that vanilla prefers to use a mod's
steam id, while we always use the package name.
Moved this responsibility from JoinData
- Server no longer crashes when save.zip is missing; enters bootstrap mode\n- Allow a single configurator client to connect while server isn't FullyStarted\n- Notify client early that server is in bootstrap\n- Add upload protocol (start/data/finish + sha256) to provision a ready-made save.zip\n- Atomically write save.zip then notify clients, disconnect, and stop server to allow external restart\n\nFiles touched: Server.cs, MultiplayerServer.cs, PlayerManager.cs, NetworkingLiteNet.cs, Packets.cs, state/packet implementations, plus small test updates.
- Bootstrap completion message now explicitly says the server will shut down and must be restarted manually\n- Ensure the standalone server process exits after bootstrap completes (avoid blocking on console loop)
- If settings.toml is missing, server enters bootstrap and waits for a configurator client to upload it\n- Add dedicated bootstrap settings upload packets (start/data/finish) with size/hash validation\n- Enforce settings.toml presence before accepting save.zip upload\n- Skip settings upload if settings.toml already exists\n- Avoid generating default settings.toml automatically on server start\n- Accept Server_Bootstrap packet also during ClientJoiningState
…ed nella state machine del client multiplayer. Migliora la robustezza della gestione degli stati di connessione.
…aPages and related methods). The bootstrap flow no longer forces automatic tile selection or Next clicks, leaving full control to the user/manual flow.
…ration in bootstrap. The save pipeline now starts without forcing vanilla dialog closure.
…n a random free port for bootstrap flows. Improves port management and prevents conflicts during automatic hosting.
…cted state. Prevents out-of-bounds errors and ensures all connection states are properly registered.
…ows explicit configuration of direct port for multiplayer hosting.
…ent connection state. Required for robust state machine handling.
- Arm AwaitingBootstrapMapInit flag in StartVanillaNewColonyFlow to ensure MapComponentUtility.FinalizeInit patch triggers in both TOML creation and pre-existing scenarios
- Fix save.zip upload to use reconnectingConn instead of stale connection after bootstrap reconnection
- Harden colonist detection with proper wait status message and debug logging
- Ensure bootstrap map initialization hook fires reliably for both workflow paths

This fixes Scenario 2 (TOML creation) where OnBootstrapMapInitialized was never called, blocking colonist detection and save upload.
Copilot AI review requested due to automatic review settings January 11, 2026 18:25
Copy link

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 comprehensive bootstrap flow that enables server administrators to configure and initialize a server entirely through in-game UI, eliminating the need for manual file handling. The changes span package updates, new protocol features, architectural improvements to networking and debugging, and extensive additions for handling server bootstrap configuration.

Changes:

  • Framework and package updates (upgraded to .NET 4.8 and updated multiple dependencies)
  • Added in-game bootstrap configuration UI for settings.toml and save.zip upload
  • Improved map initialization detection and save upload reliability during bootstrap
  • Enhanced debug UI with expandable panels, performance recording, and better metrics
  • Refactored connection handling and packet serialization to support new bootstrap flow

Reviewed changes

Copilot reviewed 194 out of 288 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Source/Common/Common.csproj Updated target framework to .NET 4.8 and upgraded package dependencies
Source/Common/CommandType.cs Added PlayerCount command type for player tracking
Source/Common/CommandHandler.cs Refactored to use new packet-based sending and improved property naming consistency
Source/Common/ChatCommands.cs Extracted ChatCmdManager class and updated messaging patterns
Source/Common/ByteWriter.cs Added generic WriteEnum method for proper enum serialization
Source/Common/ByteReader.cs Added ReadEnum method and converted to auto-property for Position
Source/Client/Windows/ServerBrowser.cs Refactored LAN discovery into separate LanListener class, added Steam relay initialization
Source/Client/Windows/SaveFileReader.cs Refactored save reading into static methods with nullable annotations
Source/Client/Windows/PendingPlayerWindow.cs New window for handling player join requests with Steam avatar display
Source/Client/Windows/PacketLogWindow.cs Refactored to use UINode instead of LogNode with stack trace attachment
Source/Client/Windows/ModCompatWindow.cs Fixed type casting in transpiler to use explicit pattern matching
Source/Client/Windows/JoinDataWindow.cs Updated to use SyncConfigs utility for config handling
Source/Client/Windows/HostWindow.cs Added programmatic hosting support and separate network manager initialization
Source/Client/Windows/GamePasswordWindow.cs Updated to use new ClientUsernamePacket
Source/Client/Windows/DesyncedWindow.cs Added ReadyToSave check before saving desync info
Source/Client/Windows/DebugTextWindow.cs Simplified layout calculation and improved scrolling behavior
Source/Client/Windows/ConnectingWindow.cs Enhanced download progress display with speed and ETA calculations
Source/Client/Windows/ChatWindow.cs Updated to use new packet types and improved network diagnostics
Source/Client/Windows/BootstrapStartedNewGamePatch.cs Harmony patch to detect when vanilla new game generation completes
Source/Client/Windows/BootstrapRootPlayUpdatePatch.cs Harmony patch for reliable map initialization detection during bootstrap
Source/Client/Windows/BootstrapRootPlayPatch.cs Harmony patch for Root_Play.Start to arm map init detection
Source/Client/Windows/BootstrapMapInitPatch.cs Harmony patch for FinalizeInit to trigger save pipeline
Source/Client/Windows/BootstrapCoordinator.cs GameComponent for reliable bootstrap coordination during long events
Source/Client/Windows/BootstrapConfiguratorWindow.cs Main bootstrap UI for TOML configuration and save.zip upload
Source/Client/Util/VersionChecker.cs New utility for checking and notifying about continuous release updates
Source/Client/Util/SyncConfigs.cs Extracted config sync logic from JoinData with HugsLib support
Source/Client/Util/SimpleProfiler.cs Updated for RimWorld 1.6 API changes (WorldPathFinder → WorldPathing)
Source/Client/Util/ShellOpenDirectory.cs Simplified string formatting using string interpolation
Source/Client/Util/RectExtensions.cs Added utility methods for UI layout (FitToText, margins, centering)
Source/Client/Util/MpUtil.cs Removed GetLocalIpAddress (moved to Endpoints class)
Source/Client/Util/MpUI.cs Extracted Checkbox method with disabled state support
Source/Client/Util/MpPatch.cs Added constructor overloads for MpPostfix and MpTranspiler
Source/Client/Util/HarmonyUtil.cs Optimized patch description generation with reduced allocations
Source/Client/Util/FileAssoc.cs New utility for Windows file association management (.rwmts files)
Source/Client/Util/FieldRef.cs Added FieldRef for WorldSelector.selected field access
Source/Client/Util/Extensions.cs Simplified MpComp extension and added IndexOfOccurrence for strings
Source/Client/Util/DeterministicHash.cs New deterministic hash combiners for sync integrity
Source/Client/Util/DebugInfoFile.cs New utility for generating comprehensive debug info ZIP files
Source/Client/Util/CollectionExtensions.cs Added ToDictionaryConsistent and optimized RemoveAll method
Source/Client/UI/PlayerCursors.cs Updated to use new ClientCursorPacket and WorldRendererUtility changes
Source/Client/UI/PingInfo.cs Changed planetTile to use PlanetTile struct instead of int
Source/Client/UI/MainMenuPatches.cs Added version display, debug file generation, and update notifications
Source/Client/UI/LocationPings.cs Updated to use new packet types for location pings
Source/Client/UI/Layouter.cs Added nullable annotation
Source/Client/UI/IngameUI.cs Integrated new SyncDebugPanel and reorganized upper UI widgets
Source/Client/UI/IngameDebug.cs Completely redesigned debug info display with better organization
Source/Client/UI/DebugPanel/SyncDebugPanel.cs New expandable debug panel with organized metrics and status badges
Source/Client/UI/DebugPanel/StatusBadge.cs Status badge helpers for debug panel (sync, performance, tick, VTR)
Source/Client/UI/DebugPanel/PerformanceRecorder.cs New performance recording system with detailed metrics and file output
Source/Client/UI/DebugPanel/PerformanceCalculator.cs Performance calculation utilities with stabilization period handling
Source/Client/UI/DebugPanel/DebugSection.cs Data structure for organizing debug panel sections
Source/Client/UI/DebugPanel/DebugLine.cs Data structure for debug panel line items
Source/Client/UI/CursorPatches.cs Updated to use Multiplayer.ThingsById instead of static ThingsById class
Source/Client/Syncing/SyncUtil.cs Updated to use FieldRefs for WorldSelector.selected access
Source/Client/Syncing/SyncFieldUtil.cs Improved buffered field change handling with better comments and logic
Source/Client/Syncing/Sync.cs Removed bufferedFields list (now tracked in SyncFieldUtil.bufferedChanges)
Source/Client/Syncing/Handler/SyncMethod.cs Refactored SyncTransformer to regular class with lowercase field names
Source/Client/Syncing/Handler/SyncField.cs Added PostApply overload with index parameter and improved documentation
Source/Client/Syncing/Handler/SyncDelegate.cs Updated to use lowercase field names in SyncTransformer
Source/Client/Syncing/Handler/SyncAction.cs Added ActionWrapper support for wrapping synced actions with custom logic
Source/Client/Syncing/Game/ThingFilterMarkers.cs Updated to use nameof for method names in patch attributes
Source/Client/Syncing/Game/SyncThingFilters.cs Updated to use nameof and collection expressions
Source/Client/Syncing/Game/SyncMethods.cs Added numerous new sync methods for RimWorld 1.6 features and bug fixes
Source/Client/Syncing/Game/SyncFields.FishingZone.cs New sync fields for fishing zone configuration
Source/Client/Syncing/Game/SyncActions.cs Added wrapper support for caravan action confirmations
Source/Client/Syncing/Dict/SyncDictMisc.cs Added FloatMenuContext and CellIndices serialization
Source/Client/Syncing/ApiSerialization.cs Added validation for Session type constructors
Source/Client/Saving/ReplayConnection.cs Updated OnClose signature to match base class changes

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

}
else
{
saveUploadStatus = "Failed to upload settings.toml: {0}: {1}";
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Error message contains format placeholders {0} and {1} but no formatting is applied. This will display the literal string with placeholders rather than the actual error information. Should use string interpolation or string.Format.

Suggested change
saveUploadStatus = "Failed to upload settings.toml: {0}: {1}";
saveUploadStatus = $"Failed to upload settings.toml: file missing: {savedReplayPath}";

Copilot uses AI. Check for mistakes.
Comment on lines 312 to 317
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 10).SetDebugOnly(); // Kill all non-slave pawns SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 10).SetDebugOnly(); // Kill all non-slave pawns
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 11).SetDebugOnly(); // Harm random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 11).SetDebugOnly(); // Harm random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 12).SetDebugOnly(); // Down random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 12).SetDebugOnly(); // Down random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 14).SetDebugOnly(); // Plague on random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 14).SetDebugOnly(); // Plague on random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 16).SetDebugOnly(); // Teleport to destination SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 16).SetDebugOnly(); // Teleport to destination
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 17).SetDebugOnly(); // +20% psyfocus SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 17).SetDebugOnly(); // +20% psyfocus
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Each line contains duplicate code where the same SyncMethod.Lambda call appears twice on the same line. This appears to be an accidental copy-paste error. Remove the duplicate portions after the comments.

Suggested change
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 10).SetDebugOnly(); // Kill all non-slave pawns SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 10).SetDebugOnly(); // Kill all non-slave pawns
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 11).SetDebugOnly(); // Harm random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 11).SetDebugOnly(); // Harm random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 12).SetDebugOnly(); // Down random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 12).SetDebugOnly(); // Down random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 14).SetDebugOnly(); // Plague on random pawn SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 14).SetDebugOnly(); // Plague on random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 16).SetDebugOnly(); // Teleport to destination SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 16).SetDebugOnly(); // Teleport to destination
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 17).SetDebugOnly(); // +20% psyfocus SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 17).SetDebugOnly(); // +20% psyfocus
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 10).SetDebugOnly(); // Kill all non-slave pawns
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 11).SetDebugOnly(); // Harm random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 12).SetDebugOnly(); // Down random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 14).SetDebugOnly(); // Plague on random pawn
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 16).SetDebugOnly(); // Teleport to destination
SyncMethod.Lambda(typeof(Caravan), nameof(Caravan.GetGizmos), 17).SetDebugOnly(); // +20% psyfocus

Copilot uses AI. Check for mistakes.
@MhaWay MhaWay changed the title Improve bootstrap flow: in-game TOML config, map init detection, and reliable save upload Improve bootstrap flow: in-game TOML config, map init detection, and reliable save upload for dedicated server Jan 11, 2026
@MhaWay MhaWay changed the base branch from master to dev January 11, 2026 19:50
@MhaWay MhaWay requested a review from Copilot January 11, 2026 19:51
Copy link

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

Copilot reviewed 36 out of 36 changed files in this pull request and generated 53 comments.


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

{
var r = Row();
TooltipHandler.TipRegion(r, "When clients automatically join (flags). Stored as a string in TOML.");
TextFieldLabeled(r, "When clients automatically join (flags). Stored as a string in TOML.", ref settings.autoJoinPoint);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The label text is duplicating the tooltip text verbatim. The label should be a short identifier like "Auto join point" while the tooltip provides the detailed explanation. Currently the long explanation text is used as both the label and tooltip, making the UI unnecessarily verbose.

Suggested change
TextFieldLabeled(r, "When clients automatically join (flags). Stored as a string in TOML.", ref settings.autoJoinPoint);
TextFieldLabeled(r, "Auto join point", ref settings.autoJoinPoint);

Copilot uses AI. Check for mistakes.

r = Row();
TooltipHandler.TipRegion(r, "Include desync traces to help debugging.");
CheckboxLabeled(r, "Include desync traces to help debugging.", ref settings.desyncTraces);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The label text is duplicating the tooltip text verbatim. The label should be a short identifier like "Desync traces" while the tooltip provides the detailed explanation.

Suggested change
CheckboxLabeled(r, "Include desync traces to help debugging.", ref settings.desyncTraces);
CheckboxLabeled(r, "Desync traces", ref settings.desyncTraces);

Copilot uses AI. Check for mistakes.
y += RowHeight;

r = Row();
TooltipHandler.TipRegion(r, "Dev mode scope.");
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The label text is duplicating the tooltip text verbatim. The label should be a short identifier like "Dev mode scope" while the tooltip provides the detailed explanation.

Suggested change
TooltipHandler.TipRegion(r, "Dev mode scope.");
TooltipHandler.TipRegion(r, "Configure where RimWorld developer mode is enabled (for example, on the server, on clients, or both).");

Copilot uses AI. Check for mistakes.
{
var r = Row();
TooltipHandler.TipRegion(r, "Arbiter is not supported in standalone server.");
CheckboxLabeled(r, "Arbiter is not supported in standalone server.", ref settings.arbiter);
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

The label text is duplicating the tooltip text verbatim. The label should be a short identifier like "Arbiter" while the tooltip provides the detailed explanation.

Suggested change
CheckboxLabeled(r, "Arbiter is not supported in standalone server.", ref settings.arbiter);
CheckboxLabeled(r, "Arbiter", ref settings.arbiter);

Copilot uses AI. Check for mistakes.
public class BootstrapConfiguratorWindow : Window
{
private readonly ConnectionBase connection;
private string serverAddress;
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Field 'serverAddress' can be 'readonly'.

Suggested change
private string serverAddress;
private readonly string serverAddress;

Copilot uses AI. Check for mistakes.
{
private readonly ConnectionBase connection;
private string serverAddress;
private int serverPort;
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Field 'serverPort' can be 'readonly'.

Suggested change
private int serverPort;
private readonly int serverPort;

Copilot uses AI. Check for mistakes.
private int reconnectCheckTimer;
private ConnectionBase reconnectingConn;

private ServerSettings settings;
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Field 'settings' can be 'readonly'.

Copilot uses AI. Check for mistakes.
Comment on lines +225 to +228
if (mapAsync?.cmds != null)
mapCmdsDict[id] = new List<ScheduledCommand>(mapAsync.cmds);
else
mapCmdsDict[id] = new List<ScheduledCommand>();
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Copilot uses AI. Check for mistakes.
Comment on lines +244 to +247
if (Multiplayer.AsyncWorldTime != null)
mapCmdsDict[ScheduledCommand.Global] = new List<ScheduledCommand>(Multiplayer.AsyncWorldTime.cmds);
else
mapCmdsDict[ScheduledCommand.Global] = new List<ScheduledCommand>();
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

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

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

@mibac138 mibac138 left a comment

Choose a reason for hiding this comment

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

This is a rather large PR. I have only taken a cursory look for now


public string directAddress = $"0.0.0.0:{MultiplayerServer.DefaultPort}";
public string directAddress = $"0.0.0.0:{MultiplayerServer.DefaultPort}";
public int directPort = MultiplayerServer.DefaultPort;

Choose a reason for hiding this comment

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

Looks like directPort is unused?

private bool isReconnecting;
private int reconnectCheckTimer;
private ConnectionBase reconnectingConn;

Choose a reason for hiding this comment

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

Please fix the formatting. The code is not indented properly in a few other files too.

[PacketDefinition(Packets.Client_BootstrapSettingsUploadStart)]
public record struct ClientBootstrapSettingsUploadStartPacket(string fileName, int length) : IPacket
{
public string fileName = fileName;

Choose a reason for hiding this comment

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

Isn't the name always settings.toml? How is this useful?

Comment on lines +546 to +558
const int chunk = 64 * 1024; // safe: packet will be fragmented by ConnectionBase
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);
connection.SendFragmented(new ClientBootstrapSettingsUploadDataPacket(part).Serialize());
sent += len;
var progress = bytes.Length == 0 ? 1f : (float)sent / bytes.Length;
OnMainThread.Enqueue(() => uploadProgress = Mathf.Clamp01(progress));
}

Choose a reason for hiding this comment

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

Why are you fragmenting the packet yourself? Connection should be able to handle packets up to 32MiB (MaxFragmentPacketTotalSize).

Comment on lines +22 to +29
// Restituisce una porta UDP libera
public static int GetFreeUdpPort()
{
var udp = new System.Net.Sockets.UdpClient(0);
int port = ((IPEndPoint)udp.Client.LocalEndPoint).Port;
udp.Close();
return port;
}

Choose a reason for hiding this comment

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

This can spuriously fail, as another program can have this port assigned by the time you bind it again. It's better to just use a port of 0 directly when starting a server. If you later need to retrieve the port assigned by the OS, you can access it through a NetManager. See ServerTest.MakeServer for an example

///
/// NOTE: This is currently a minimal placeholder to wire the new join-flow.
/// </summary>
public class BootstrapConfiguratorWindow : Window

Choose a reason for hiding this comment

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

This is a large file, I did not review it yet. Some high level notes:

  • the configuration is the same/very similar to the options when hosting a game, could that UI be reused? Even if not fully, at least partially?
  • the TOML preview should be behind a dev-only button at most, perhaps as an option after right clicking Copy TOML? or hidden behind a new button
  • Copy TOML is not useful for regular users, should be behind a dev option too

/// The client will send exactly one file: a pre-built save.zip (server format).
/// </summary>
[PacketDefinition(Packets.Client_BootstrapUploadStart)]
public record struct ClientBootstrapUploadStartPacket(string fileName, int length) : IPacket

Choose a reason for hiding this comment

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

The packet name is rather long and very similar to ...SettingsUploadPacket, making it hard to distinguish. Rename to ...BootstrapSaveUpload..., to hopefully make it easier to discern them

[PacketDefinition(Packets.Client_BootstrapSettingsUploadData, allowFragmented: true)]
public record struct ClientBootstrapSettingsUploadDataPacket(byte[] data) : IPacket
{
public byte[] data = data;

Choose a reason for hiding this comment

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

It'd be better to send this as ServerSettings instead. You can then use TomlSettings.Save on the server-side. This is the best option.
A worse option is to use settings.ExposeData in RebuildTomlPreview and generate the config that way. You should not hand-code the serialization logic as it will get out-of-sync at some point

}
}
/// <summary>
/// Avvia l'hosting programmaticamente per il flusso bootstrap.

Choose a reason for hiding this comment

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

Please keep the comments in English

Comment on lines +248 to +252
foreach (var p in Server.playerManager.Players.ToArray())
p.conn.Close(MpDisconnectReason.ServerClosed);

// Stop the server loop; an external supervisor should restart.
Server.running = false;

Choose a reason for hiding this comment

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

Suggested change
foreach (var p in Server.playerManager.Players.ToArray())
p.conn.Close(MpDisconnectReason.ServerClosed);
// Stop the server loop; an external supervisor should restart.
Server.running = false;
// Stop the server loop; an external supervisor should restart.
Server.running = false;
Server.TryStop();

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