Split standalone save trigger foundation by MhaWay · Pull Request #874 · rwmt/Multiplayer

MhaWay · 2026-04-11T15:40:47Z

Summary

split out the standalone save-trigger foundation from the broader standalone save work
add standalone protocol and session identification
add typed autosave and join-point reason plumbing
keep this PR focused on the initial standalone save trigger flow

Validation

dotnet build Source/Multiplayer.sln -c Release
dotnet test Source/Tests/Tests.csproj -c Release --no-build

Stack

base PR for the standalone save stack targeting upstream/dev
subsequent standalone save PRs build on top of this one

Copilot

Pull request overview

This PR lays the groundwork for a “standalone save trigger” flow by extending the handshake to identify standalone servers and by plumbing a typed autosave/join-point request reason from client to server, so standalone clients can request join point creation for save- and travel-related events.

Changes:

Extend protocol handshake (Server_ProtocolOk) to include whether the server is standalone; persist this on the client session.
Introduce ClientAutosavingPacket + JoinPointRequestReason and route autosave/save/world-travel triggers through it to server-side join point creation.
Adjust standalone-specific join/join-point behavior and add a synthetic autosave timer for standalone connections.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Source/Common/Networking/Packet/ProtocolPacket.cs	Adds `isStandaloneServer` field to protocol OK packet for client session identification.
Source/Common/Networking/Packet/AutosavingPacket.cs	Introduces typed autosaving packet with a reason enum.
Source/Common/JoinPointRequestReason.cs	Adds enum describing why a join point is requested (save, world travel, etc.).
Source/Common/Networking/State/ServerPlayingState.cs	Updates autosaving handler to typed packet + reason; loosens world upload policy for standalone.
Source/Common/Networking/State/ServerJoiningState.cs	Adjusts join-point creation policy for standalone joins; sends standalone flag during handshake.
Source/Common/PlayerManager.cs	Treats first non-arbiter joiner as “primary” faction on standalone servers.
Source/Client/Networking/State/ClientJoiningState.cs	Stores `isStandaloneServer` from protocol OK on the client session.
Source/Client/Session/MultiplayerSession.cs	Adds session flag + helper property for “connected to standalone server”.
Source/Client/Session/Autosaving.cs	Sends typed autosaving packet after save; changes save routine to return `bool` and tweaks file replacement logic.
Source/Client/Windows/SaveGameWindow.cs	Sends autosaving packet on manual save; special-cases standalone to trigger server-side save flow.
Source/Client/Patches/VTRSyncPatch.cs	On standalone, triggers a join point when leaving a map (world travel).
Source/Client/ConstantTicker.cs	Adds standalone-only synthetic autosave timer.
Source/Client/AsyncTime/AsyncWorldTimeComp.cs	Adjusts join-point creation upload behavior during `CreateJoinPoint`.
Source/Tests/PacketTest.cs	Updates packet roundtrip expectations for the extended protocol OK packet.
Source/Tests/packet-serializations/ServerProtocolOkPacket.verified.txt	Updates verified serialization output for the extended protocol OK packet.

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

Copilot · 2026-04-11T15:48:28Z

Source/Client/AsyncTime/AsyncWorldTimeComp.cs

    private static void CreateJoinPointAndSendIfHost()
    {
        Multiplayer.session.dataSnapshot = SaveLoad.CreateGameDataSnapshot(SaveLoad.SaveAndReload(), Multiplayer.GameComp.multifaction);

-        if (!TickPatch.Simulating && !Multiplayer.IsReplay &&
-            (Multiplayer.LocalServer != null || Multiplayer.arbiterInstance))
+        if (!TickPatch.Simulating && !Multiplayer.IsReplay)
            SaveLoad.SendGameData(Multiplayer.session.dataSnapshot, true);
    }


CreateJoinPointAndSendIfHost now uploads world data from every non-simulating, non-replay client. On non-standalone servers the server will still reject Client_WorldDataUpload from non-host/non-arbiter, but the upload cost (fragmenting + network/bandwidth + compression work) is still paid by all clients, which can be very expensive for large saves. Please gate SendGameData so only the designated uploader sends (e.g., arbiter/local host, plus standalone clients), or introduce a server-directed "uploader" selection instead of broadcasting uploads from everyone.

Related to my comment #874 (comment)

Copilot · 2026-04-11T15:48:28Z

Source/Client/Session/Autosaving.cs

+                dst.Delete();
+
+            tmp.MoveTo(dst.FullName);



SaveGameToFile_Overwrite deletes the destination save and then moves the temp file into place. This is not atomic and can permanently lose an existing save if the process crashes or MoveTo fails after dst.Delete(). Prefer an atomic replace strategy (e.g., File.Replace or equivalent) so the old save is preserved unless the new one is successfully written.

Suggested change

dst.Delete();

tmp.MoveTo(dst.FullName);

File.Replace(tmp.FullName, dst.FullName, null);

else

tmp.MoveTo(dst.FullName);

Related #874 (comment)

Copilot · 2026-04-11T15:48:28Z

Source/Client/Session/Autosaving.cs

        {
            Log.Error($"Exception saving multiplayer game as {fileNameNoExtension}: {e}");
            Messages.Message("MpGameSaveFailed".Translate(), MessageTypeDefOf.SilentInput, false);
+            return false;


SaveGameToFile_Overwrite now returns bool and swallows exceptions internally. Callers that relied on exceptions to detect failure (e.g., code wrapping this call in try/catch) will no longer observe failures unless they check the return value, and may continue follow-up steps assuming a save exists. Consider either re-throwing (and letting callers handle) or ensure all call sites are updated to handle the false return explicitly.

Suggested change

return false;

throw;

Copilot · 2026-04-11T15:48:29Z

Source/Common/Networking/Packet/ProtocolPacket.cs

 [PacketDefinition(Packets.Server_ProtocolOk)]
-public record struct ServerProtocolOkPacket(bool hasPassword) : IPacket
+public record struct ServerProtocolOkPacket(bool hasPassword, bool isStandaloneServer = false) : IPacket
 {
    public bool hasPassword = hasPassword;
+    public bool isStandaloneServer = isStandaloneServer;

    public void Bind(PacketBuffer buf)
    {
        buf.Bind(ref hasPassword);
+        buf.Bind(ref isStandaloneServer);
    }


This PR changes the on-wire layout of existing packet IDs (Server_ProtocolOk adds a new bound bool, and Client_Autosaving now has a payload). To avoid old clients/servers passing the protocol check and then failing during packet deserialization, MpVersion.Protocol should be bumped in the same PR as these layout changes so mismatched versions fail fast during the handshake.

Yes, please bump the protocol version

Copilot · 2026-04-11T15:48:29Z

Source/Client/Windows/SaveGameWindow.cs

+                if (!Autosaving.SaveGameToFile_Overwrite(curText, currentReplay))
+                    return;
+
+                Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));


ClientAutosavingPacket is sent after saving even when currentReplay is true. In replay sessions the connection ignores non-command packets, so this send is a no-op and adds confusing coupling between replay saving and network join-point logic. Consider skipping the send when currentReplay is true (or when Multiplayer.Client is null) to keep replay saves purely local.

Suggested change

Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));

if (!currentReplay && Multiplayer.Client != null)

Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));

mibac138 · 2026-04-11T20:01:50Z

Source/Client/AsyncTime/AsyncWorldTimeComp.cs


-        if (!TickPatch.Simulating && !Multiplayer.IsReplay &&
-            (Multiplayer.LocalServer != null || Multiplayer.arbiterInstance))
+        if (!TickPatch.Simulating && !Multiplayer.IsReplay)


Instead of having every player send the game data back to the server, could you have the server send info which player is expected to send the game data back? WorldData.cs Server.commands.Send(CommandType.CreateJoinPoint, ScheduledCommand.NoFaction, ScheduledCommand.Global, Array.Empty<byte>()); - the Array.Empty could be changed to include playerId or perhaps a value of true could be sent to the desired player and false to the rest?

mibac138 · 2026-04-11T20:02:00Z

Source/Client/Session/Autosaving.cs

+            if (dst.Exists)
+                dst.Delete();
+
+            tmp.MoveTo(dst.FullName);


I'm not particularly against this change, but I'm curious why is this better? Edit: it probably isn't #874 (comment)

Had errors on overwrite tmp, this fixed

mibac138 · 2026-04-11T20:03:39Z

Source/Client/Patches/VTRSyncPatch.cs

                    }
+
+                    // On standalone, trigger a join point when leaving a map
+                    if (Multiplayer.session?.ConnectedToStandaloneServer == true)


Isn't it going to be pretty annoying (for the players) to save the game every time anyone changes the map? As far as I remember, the game pauses for everyone during join point creation

Tecnically just for the one saving and for the maps it is on. The idea is to let players save their maps, but im still trying to test

mibac138 · 2026-04-11T20:06:24Z

Source/Client/ConstantTicker.cs


        private static void TickAutosave()
        {
+            // Only standalone connections use the synthetic autosave timer.


I don't understand this comment. autosaveCounter is also used by other code in this method?

mibac138 · 2026-04-11T20:07:08Z

Source/Client/ConstantTicker.cs

+                var session = Multiplayer.session;
+                session.autosaveCounter++;
+
+                if (session.autosaveCounter > 5 * TicksPerMinute)


Why is this fixed to 5 minutes instead of being adjustable like the rest of the code?

mibac138 · 2026-04-11T20:16:54Z

Source/Common/Networking/State/ServerPlayingState.cs

+            var forceJoinPoint = packet.reason == JoinPointRequestReason.Save;
+
+            // On standalone, any playing client can trigger a join point (always, regardless of settings)
+            // On hosted, only the host can trigger and only if the Autosave flag is set
+            if (Server.IsStandaloneServer ||
+                (Player.IsHost && Server.settings.autoJoinPoint.HasFlag(AutoJoinPointFlags.Autosave)))
+                Server.worldData.TryStartJoinPointCreation(forceJoinPoint);


Unless I'm missing something, this change along with the changes to CreateJoinPointAndSendIfHost and HandleWorldDataUpload, mean that anyone can request a join point to be created, which makes everyone on the server upload a joinpoint to the server, and then the server just accepts the world data from every player. I don't think this is a good workflow. I haven't really thought deeply about this, but I think I'd prefer the server to choose one player to send the game data, or in some other way determine a single player to save.

I actually found a todo somewhere about the implementation to load map joinpoints independently and thought it might lighten the loading.. so the idea is that Each player can load their own active maps

The intent here was not to have every client produce and upload a joinpoint. The idea is that only the player who explicitly triggers a save, or another deliberate action tied to their current context, should send the joinpoint for that map.

The main goal is to avoid unnecessary interruptions or loading work for players who are active on separate maps. I tried to keep this as continuity-friendly as possible: if the server arbitrarily designates a client to provide the joinpoint, that creates awkward edge cases where the chosen player may be in the middle of leaving, changing state, or otherwise not be the best source for that save. Tying joinpoint creation to an explicit player action keeps the flow more predictable and reduces the impact on everyone else.

mibac138 · 2026-04-11T20:18:22Z

Source/Client/Windows/SaveGameWindow.cs

+                if (!currentReplay && Multiplayer.session?.isStandaloneServer == true)
+                {
+                    Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));
+                    Messages.Message("MpGameSaved".Translate(curText), MessageTypeDefOf.SilentInput, false);
+                    Multiplayer.session.lastSaveAt = Time.realtimeSinceStartup;
+                    return;
+                }


This means that you can only save the game by sending it to the standalone server. I think I'd be nice to have a way to save the game locally even on a standalone server?

Split standalone save trigger foundation

94ad756

Copilot AI review requested due to automatic review settings April 11, 2026 15:40

Copilot started reviewing on behalf of MhaWay April 11, 2026 15:41 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

This was referenced Apr 11, 2026

Standalone snapshot persistence #875

Open

Standalone async reconnect fixes #876

Open

Standalone prepublish maintenance #877

Open

notfood added enhancement New feature or request. 1.6 Fixes or bugs relating to 1.6 (Not Odyssey). standalone server Fix or bugs relating to the standalone server. labels Apr 11, 2026

notfood added this to 1.6 and Odyssey Apr 11, 2026

notfood moved this to In review in 1.6 and Odyssey Apr 11, 2026

mibac138 reviewed Apr 11, 2026

View reviewed changes

	Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));
	if (!currentReplay && Multiplayer.Client != null)
	Multiplayer.Client.Send(new ClientAutosavingPacket(JoinPointRequestReason.Save));

Uh oh!

Conversation

MhaWay commented Apr 11, 2026

Summary

Validation

Stack

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mibac138 Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mibac138 Apr 11, 2026 •

edited

Loading