From 0504fe15ed44d2b3c803df91b814062ad629050e Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:36:40 +0000 Subject: [PATCH] Let the instance decide the components of a download The scenario no longer sends its components with the download request. The instance strips the download by its own config and sends the components it used with the first chunk. If they differ from the ones the scenario started the download with, because the config changed in between, the scenario restarts the download. A pending download is now marked with sync_in_progress instead of clearing dirty while spectating, and it is set for every download rather than only when inventories are synced. Uploads are skipped while it is set, so a player who leaves during a download with inventories off no longer uploads their stale state. SyncComponent names the components in the Lua annotations. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/instance.ts | 20 ++++++---- .../player_sync/module/download_inventory.lua | 10 ++++- .../module/gui/dialog_failed_download.lua | 1 + plugins/player_sync/module/player_sync.lua | 27 ++++++++----- plugins/player_sync/module/serialize.lua | 28 ++++++++++++- plugins/player_sync/test/instance.js | 40 ++++++++++++++----- 6 files changed, 96 insertions(+), 30 deletions(-) diff --git a/plugins/player_sync/instance.ts b/plugins/player_sync/instance.ts index 008cb573..b4e6b978 100644 --- a/plugins/player_sync/instance.ts +++ b/plugins/player_sync/instance.ts @@ -5,7 +5,7 @@ import type { InstancePluginContext } from "@clusterio/host"; import { AcquireRequest, AcquireResponse, ReleaseRequest, UploadRequest, DownloadRequest, DownloadResponse, IpcPlayerData, } from "./messages.js"; -import { enabledComponents, scenarioComponents, stripPlayerData } from "./components.js"; +import { type Component, enabledComponents, stripPlayerData } from "./components.js"; type IpcPlayerName = { player_name: string @@ -14,8 +14,11 @@ type IpcPlayerName = { type IpcDownloadRequest = { player_name: string, recipe_notifications?: string, - // Components the download was started with, all when missing - components?: Record | unknown[], +} + +/** Components as an escaped JSON object for the scenario. */ +function componentsJson(enabled: Set) { + return lib.escapeString(JSON.stringify(Object.fromEntries([...enabled].map(name => [name, true])))); } type IpcAcquireResponse = { @@ -201,16 +204,20 @@ export default async function(context: InstancePluginContext) { return; } - stripPlayerData(response.playerData, scenarioComponents(request.components)); + const enabled = enabledComponents(instance.config); + stripPlayerData(response.playerData, enabled); await applyRecipeNotificationDelta(response.playerData, request.recipe_notifications, logger); const chunkSize = instance.config.get("player_sync.rcon_chunk_size"); const chunks = chunkify(chunkSize, JSON.stringify(response.playerData)); logger.verbose(`Sending inventory for ${playerName} in ${chunks.length} chunks`); + // The scenario restarts the download if it was started with other components + const components = componentsJson(enabled); for (let i = 0; i < chunks.length; i++) { const chunk = lib.escapeString(chunks[i]); + const extra = i === 0 ? `,'${components}'` : ""; await instance.sendRcon( - `/sc player_sync.download_inventory('${playerName}','${chunk}',${i + 1},${chunks.length})`, + `/sc player_sync.download_inventory('${playerName}','${chunk}',${i + 1},${chunks.length}${extra})`, true, plugin.name, ); @@ -245,8 +252,7 @@ export default async function(context: InstancePluginContext) { /** Tell the scenario which components are synced. */ async function sendComponents() { - const enabled = Object.fromEntries([...enabledComponents(instance.config)].map(name => [name, true])); - const json = lib.escapeString(JSON.stringify(enabled)); + const json = componentsJson(enabledComponents(instance.config)); await instance.sendRcon(`/sc player_sync.set_components("${json}")`, true, plugin.name); } diff --git a/plugins/player_sync/module/download_inventory.lua b/plugins/player_sync/module/download_inventory.lua index 18f96828..d6f6e54d 100644 --- a/plugins/player_sync/module/download_inventory.lua +++ b/plugins/player_sync/module/download_inventory.lua @@ -3,9 +3,11 @@ local ensure_character = require("modules/player_sync/ensure_character") local restore_position = require("modules/player_sync/restore_position") local get_script_data = require("modules/player_sync/get_script_data") local serialize = require("modules/player_sync/serialize") +local compat = require("modules/clusterio/compat") -local function download_inventory(player_name, data, number, total) +--- @param components string? JSON of the components the instance sent the data with, given with the first part +local function download_inventory(player_name, data, number, total, components) local player = game.get_player(player_name) if player == nil then rcon.print("Player " .. player_name .. " does not exist") @@ -20,6 +22,11 @@ local function download_inventory(player_name, data, number, total) end local player_record = script_data.players[player_name] + -- The components changed after the download was started + local sent_components = number == 1 and components and compat.json_to_table(components) + if sent_components and not serialize.same_components(record.components, sent_components) then + record.restart = true + end if record.restart then rcon.print("Restarting outdated download") player_sync.initiate_inventory_download(player, player_record, record.generation) @@ -44,6 +51,7 @@ local function download_inventory(player_name, data, number, total) script_data.active_downloads[player_name] = nil player_record.dirty = player.connected + player_record.sync_in_progress = false player_record.sync = true player_record.components = record.components player_record.generation = record.generation diff --git a/plugins/player_sync/module/gui/dialog_failed_download.lua b/plugins/player_sync/module/gui/dialog_failed_download.lua index 4859e40b..1884d007 100644 --- a/plugins/player_sync/module/gui/dialog_failed_download.lua +++ b/plugins/player_sync/module/gui/dialog_failed_download.lua @@ -131,6 +131,7 @@ dialog_failed_download.events = { player.gui.screen.dialog_failed_download.destroy() player_sync.players[player.name].dirty = true player_sync.players[player.name].sync = false + player_sync.players[player.name].sync_in_progress = false player_sync.players[player.name].local_controller = nil elseif event.element.name == "player_sync_failed_download_retry" then diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index fed3783d..6285c8fe 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -31,9 +31,9 @@ local function start_sync(player_record, components) end -- Remember the controller the player had before sync code replaced it, restored when the controller is not synced -local function save_local_controller(player, player_record) +local function save_local_controller(player, player_record, components) if not player_record.local_controller then - player_record.local_controller = serialize.serialize_local_controller(player) + player_record.local_controller = serialize.serialize_local_controller(player, components) end end @@ -42,6 +42,7 @@ local function create_player(player, dirty) local script_data = get_script_data(true) -- no early return to support loading from single player script_data.players[player.name] = { dirty = dirty, -- Player inventory has changes that should be persisted + sync_in_progress = false, -- Player data is being downloaded, the current state should not be persisted sync = false, -- Player inventory is synced from the controller generation = 0, } @@ -115,7 +116,8 @@ function player_sync.deserialize_player(player, finished_record) local script_data = get_script_data() local player_record = script_data.players[player.name] - local local_controller = player_record.local_controller or serialize.serialize_local_controller(player) + local local_controller = player_record.local_controller + or serialize.serialize_local_controller(player, finished_record.components) -- Stash temporary inventory if it exists, without inventory sync the current inventory is the one to keep local stashed_corpse @@ -315,17 +317,18 @@ function player_sync.sync_player(acquire_response) local player_record = script_data.players[acquire_response.player_name] if acquire_response.status ~= "acquired" then - if player_record.sync and not player_record.dirty then + if player_record.sync and (not player_record.dirty or player_record.sync_in_progress) then if serialize.syncs(player_record.components, "inventories") and player.controller_type == defines.controllers.character then - save_local_controller(player, player_record) + save_local_controller(player, player_record, player_record.components) local character = player.character player.set_controller({ type = defines.controllers.spectator }) if character then character.destroy() end end player_record.sync = false end + player_record.sync_in_progress = false dialog_failed_download.create(player, acquire_response) return end @@ -351,6 +354,7 @@ function player_sync.sync_player(acquire_response) start_sync(player_record, script_data.components) player_record.dirty = true + player_record.sync_in_progress = false end end @@ -366,10 +370,12 @@ function player_sync.finish_download(player, finished_record) -- Treat this as a temporary inventory so the incomplete state is not uploaded over the synced data player_record.dirty = true player_record.sync = false + player_record.sync_in_progress = false return end player_record.dirty = true + player_record.sync_in_progress = false start_sync(player_record, finished_record.components) player_record.generation = finished_record.generation end @@ -426,7 +432,7 @@ player_sync.events[defines.events.on_pre_player_left_game] = function(event) return end - if not player_record.dirty or not player_record.sync then + if not player_record.dirty or not player_record.sync or player_record.sync_in_progress then -- Nothing to upload, release the acquisition so other instances are not blocked clusterio_api.send_json("player_sync_release", { player_name = player.name }) return @@ -493,9 +499,11 @@ function player_sync.initiate_inventory_download(player, player_record, generati clusterio_api.send_json("player_sync_download", { player_name = player.name, recipe_notifications = recipe_notifications, - components = record.components, }) + -- Indicate the current state shouldn't be persisted + player_record.sync_in_progress = true + -- If this is a synced player turn them into a spectator while the -- player data is downloading if player_record.sync and serialize.syncs(record.components, "inventories") then @@ -511,13 +519,10 @@ function player_sync.initiate_inventory_download(player, player_record, generati record.vehicle = player.vehicle end - save_local_controller(player, player_record) + save_local_controller(player, player_record, record.components) local character = player.character player.set_controller({ type = defines.controllers.spectator }) if character ~= nil then character.destroy() end - - -- Indicate this state shouldn't be persisted - player_record.dirty = false end progress_dialog.display(player, 0, 1) diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index 4a20b3b9..7f9b3b2a 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -963,21 +963,45 @@ function serialize.deserialize_game_view_settings(player, serialized) end end +--- @alias SyncComponent +--- | "controller" +--- | "force" +--- | "appearance" +--- | "inventories" +--- | "logistics" +--- | "quick_bar" +--- | "settings" +--- | "recipe_notifications" + --- Components which are synced, nil when all of them are ---- @alias SyncComponents table? +--- @alias SyncComponents table? --- @param components SyncComponents ---- @param name string +--- @param name SyncComponent --- @return boolean local function syncs(components, name) return components == nil or components[name] == true end serialize.syncs = syncs +--- @type SyncComponent[] local component_names = { "controller", "force", "appearance", "inventories", "logistics", "quick_bar", "settings", "recipe_notifications", } +--- True if the same components are synced in both +--- @param a SyncComponents +--- @param b SyncComponents +--- @return boolean +function serialize.same_components(a, b) + for _, name in ipairs(component_names) do + if syncs(a, name) ~= syncs(b, name) then + return false + end + end + return true +end + --- True if a component synced in new_components was not synced in old_components --- @param old_components SyncComponents --- @param new_components SyncComponents diff --git a/plugins/player_sync/test/instance.js b/plugins/player_sync/test/instance.js index 93689029..269e87dc 100644 --- a/plugins/player_sync/test/instance.js +++ b/plugins/player_sync/test/instance.js @@ -104,9 +104,10 @@ describe("player_sync", function() { await lib.wait(1); } } - function downloadCommand(playerData) { + function downloadCommand(playerData, enabled = Object.keys(components)) { const json = lib.escapeString(JSON.stringify(playerData)); - return `/sc player_sync.download_inventory('test','${json}',1,1)`; + const sent = lib.escapeString(JSON.stringify(Object.fromEntries(enabled.map(c => [c, true])))); + return `/sc player_sync.download_inventory('test','${json}',1,1,'${sent}')`; } it("should send an empty download when the controller has no data", async function() { @@ -129,13 +130,16 @@ describe("player_sync", function() { const expected = { generation: 1, name: "test", recipe_notifications: await encode({ add: ["a"] }) }; assert.deepEqual(instance.server.rconCommands, [downloadCommand(expected)]); }); - it("should leave out components the download was not started with", async function() { + it("should leave out components which are not synced", async function() { + for (const component of Object.keys(components)) { + instance.mockConfigEntries.set(`player_sync.sync_${component}`, component === "appearance"); + } respondWith({ generation: 1, name: "test", force: "enemy", tag: "tag", recipe_notifications: "x" }); - await download({ player_name: "test", components: { appearance: true } }); + await download({ player_name: "test" }); const expected = { generation: 1, name: "test", tag: "tag" }; - assert.deepEqual(instance.server.rconCommands, [downloadCommand(expected)]); + assert.deepEqual(instance.server.rconCommands, [downloadCommand(expected, ["appearance"])]); }); - it("should ignore config changes made while the controller responds", async function() { + it("should send the components it stripped by when the config changes during the download", async function() { instance.connector.once("send", message => { instance.mockConfigEntries.set("player_sync.sync_force", false); instance.connector.emit("message", new lib.MessageResponse( @@ -143,9 +147,27 @@ describe("player_sync", function() { new DownloadResponse({ generation: 1, name: "test", force: "enemy", tag: "tag" }), )); }); - await download({ player_name: "test", components: { force: true } }); - const expected = { generation: 1, name: "test", force: "enemy" }; - assert.deepEqual(instance.server.rconCommands, [downloadCommand(expected)]); + await download({ player_name: "test" }); + const expected = { generation: 1, name: "test", tag: "tag" }; + const enabled = Object.keys(components).filter(c => c !== "force"); + assert.deepEqual(instance.server.rconCommands, [downloadCommand(expected, enabled)]); + }); + it("should only send the components with the first chunk", async function() { + instance.mockConfigEntries.set("player_sync.rcon_chunk_size", 10); + try { + respondWith({ generation: 1, name: "test" }); + await download({ player_name: "test" }); + for (let i = 0; i < 50 && instance.server.rconCommands.length < 3; i++) { + await lib.wait(1); + } + const [first, ...rest] = instance.server.rconCommands; + assert.match(first, /,1,3,'.+'\)$/); + for (const command of rest) { + assert.match(command, /,\d,3\)$/); + } + } finally { + instance.mockConfigEntries.set("player_sync.rcon_chunk_size", 100000); + } }); });