From 39df22be650319c5f5eaf640d0abd39b8ddb6bac Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:50:37 +0000 Subject: [PATCH] Close the remaining ways an accepted import fails to restore Imports no longer accept the cutscene controller, which needs waypoints to restore. Logistic filters and the basic item stack fields now have the types and limits Factorio enforces, and the inventory summary skips malformed items and counts repeated exported items once instead of expanding them. Lua's JSON loses precision past 2^50, so a generation there could no longer be counted up and uploads were discarded. Imports now start over from generation 1 when the stored or imported generation is at that limit. Instances holding a copy from another epoch take the stored generation instead of keeping their own higher one when there is no data to download. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/controller.ts | 5 +- plugins/player_sync/messages.ts | 40 +++++++++++-- plugins/player_sync/module/player_sync.lua | 7 ++- plugins/player_sync/player_data.ts | 18 +++--- plugins/player_sync/test/controller.js | 48 ++++++++++++++- plugins/player_sync/test/plugin.js | 69 ++++++++++++++++++++-- 6 files changed, 167 insertions(+), 20 deletions(-) diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 419c1e0e..93ba3aa5 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -295,7 +295,10 @@ export default async function(context: ControllerPluginContext) { // A new epoch makes every instance drop its copy, including ones with an unconfirmed upload let stored = storedVersion(playerName); playerData.name = playerName; - playerData.generation = Math.max(playerData.generation, stored.generation + 1); + // Start over from generations at or past the limit, instances that know epochs download the import anyway + let imported = playerData.generation < msg.generationLimit ? playerData.generation : 0; + let next = stored.generation < msg.generationLimit ? stored.generation + 1 : 1; + playerData.generation = Math.max(imported, next); playerData.epoch = stored.epoch + 1; playerDatastore.set(playerName, playerData); tombstones.delete(playerName); diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index ea780235..7663a794 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -45,11 +45,21 @@ const jsonPlayerData = Type.Object({ function jsonList(item: T) { return Type.Union([Type.Array(item), Type.Object({}, { maxProperties: 0 })]); } +// Item stack format of packages/host/modules/clusterio/serialize.lua, the nested parts are left to Lua const jsonInventory = Type.Object({ b: Type.Optional(Type.Integer({ minimum: 0 })), i: jsonList(Type.Object({ s: Type.Optional(Type.Integer({ minimum: 1 })), r: Type.Optional(Type.Integer({ minimum: 0 })), + f: Type.Optional(Type.Union([Type.String(), Type.Object({})])), + e: Type.Optional(Type.String()), + n: Type.Optional(Type.String()), + c: Type.Optional(Type.Integer({ minimum: 1 })), + q: Type.Optional(Type.String()), + h: Type.Optional(Type.Number({ minimum: 0 })), + p: Type.Optional(Type.Number({ minimum: 0 })), + d: Type.Optional(Type.Number({ minimum: 0 })), + a: Type.Optional(Type.Number({ minimum: 0 })), })), }); const jsonInventories = Type.Record(Type.String(), jsonInventory); @@ -83,6 +93,26 @@ const jsonLogisticSlot = Type.Object({ min: Type.Optional(Type.Integer({ minimum: 0 })), max: Type.Optional(Type.Integer({ minimum: 0 })), }); +const jsonComparator = StringEnum(["=", ">", "<", "≥", ">=", "≤", "<=", "≠", "!="]); +// Limits enforced by LuaLogisticSection.filters +const jsonLogisticFilter = Type.Object({ + value: Type.Optional(Type.Union([ + Type.String(), + Type.Object({ + type: Type.Optional(StringEnum([ + "item", "fluid", "virtual", "entity", "recipe", "space-location", "asteroid-chunk", "quality", + ])), + name: Type.String(), + quality: Type.Optional(Type.String()), + comparator: Type.Optional(jsonComparator), + }), + ])), + min: Type.Optional(Type.Integer({ minimum: -(2 ** 31), maximum: 2 ** 31 - 1 })), + max: Type.Optional(Type.Integer({ minimum: 0, maximum: 2 ** 32 - 1 })), + minimum_delivery_count: Type.Optional(Type.Integer({ minimum: 0, maximum: 2 ** 32 - 1 })), + // Exported as null, the prototype does not convert to JSON + import_from: Type.Optional(Type.Union([Type.String(), Type.Null()])), +}); const jsonPersonalLogisticSlots = Type.Union([ Type.Object({ enabled: Type.Boolean(), @@ -91,7 +121,7 @@ const jsonPersonalLogisticSlots = Type.Union([ group: Type.String(), active: Type.Boolean(), multiplier: Type.Number({ minimum: 0 }), - filters: jsonList(Type.Object({ value: Type.Optional(Type.Object({})) })), + filters: jsonList(jsonLogisticFilter), })), }), // From before 2.0, slot index to request @@ -105,7 +135,7 @@ const jsonQuickBarSlot = Type.Union([ t: Type.Literal("f"), n: Type.String(), q: Type.Optional(Type.String()), - c: Type.Optional(StringEnum(["=", ">", "<", "≥", ">=", "≤", "<=", "≠", "!="])), + c: Type.Optional(jsonComparator), }), Type.Object({ t: Type.Literal("i"), @@ -130,12 +160,14 @@ const jsonCraftingQueue = Type.Object({ })), }); const jsonTable = Type.Union([Type.Array(Type.Unknown()), Type.Object({})]); +// Lua's JSON loses precision past this, generations from here on are not counted up +export const generationLimit = 2 ** 50; const jsonImportedPlayerData = Type.Object({ generation: Type.Integer({ minimum: 0 }), epoch: Type.Optional(Type.Integer({ minimum: 0 })), name: Type.String(), - // Any physical controller, remote view is never stored - controller: Type.Optional(StringEnum(["character", "god", "editor", "cutscene", "spectator", "ghost"])), + // Any physical controller, remote view is never stored and a cutscene needs waypoints to restore + controller: Type.Optional(StringEnum(["character", "god", "editor", "spectator", "ghost"])), color: Type.Optional(jsonColor), chat_color: Type.Optional(jsonColor), tag: Type.Optional(Type.String()), diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index 8519127d..0195ce57 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -373,7 +373,12 @@ function player_sync.sync_player(acquire_response) player_record.sync_in_progress = false -- Deleted data keeps its generation and epoch so the next upload is accepted as the new baseline player_record.epoch = epoch - player_record.generation = math.max(player_record.generation, acquire_response.generation) + -- A copy from another epoch has nothing to do with the stored generation + if stale then + player_record.generation = acquire_response.generation + else + player_record.generation = math.max(player_record.generation, acquire_response.generation) + end end end diff --git a/plugins/player_sync/player_data.ts b/plugins/player_sync/player_data.ts index 1905b9c7..cbc1d661 100644 --- a/plugins/player_sync/player_data.ts +++ b/plugins/player_sync/player_data.ts @@ -28,15 +28,17 @@ export type InventorySummary = { export function summarizeInventory(inventory: SerializedInventory): ItemSummary[] { let items: ItemSummary[] = []; let stacks = new Map(); - for (let entry of Object.values(inventory.i ?? [])) { - let repeat = (entry.r ?? 0) + 1; - if (entry.e !== undefined) { - for (let i = 0; i < repeat; i++) { - items.push({ name: "exported item", count: 1, exportSize: entry.e.length }); - } + for (let entry of Object.values(inventory?.i ?? [])) { + // Uploads are not validated as strictly as imports + if (typeof entry !== "object" || entry === null) { continue; } - if (entry.n === undefined) { + let repeat = Number.isInteger(entry.r) ? entry.r! + 1 : 1; + if (typeof entry.e === "string") { + items.push({ name: "exported item", count: repeat, exportSize: entry.e.length }); + continue; + } + if (typeof entry.n !== "string") { continue; } let key = `${entry.n}\0${entry.q ?? ""}`; @@ -46,7 +48,7 @@ export function summarizeInventory(inventory: SerializedInventory): ItemSummary[ stacks.set(key, stack); items.push(stack); } - stack.count += (entry.c ?? 0) * repeat; + stack.count += (typeof entry.c === "number" ? entry.c : 0) * repeat; } return items; } diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index eb44d056..a7177d78 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -115,7 +115,7 @@ describe("player_sync controller", function() { return await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName)); } - beforeEach(async function() { + async function loadWithInstances() { controller = await mock.loadControllerPlugin(async (context) => { context.controller.mockConfigEntries.set("controller.database_directory", databaseDir); // Negative so locks expire as soon as their instance stops @@ -127,6 +127,10 @@ describe("player_sync controller", function() { new lib.InstanceConfig("controller", { "instance.id": id, "instance.name": name }), status, )); } + } + + beforeEach(async function() { + await loadWithInstances(); await handler(msg.UploadRequest)(new msg.UploadRequest(1, "stored", playerData("stored", 3))); assert(await acquire(1, "stored")); @@ -220,7 +224,19 @@ describe("player_sync controller", function() { await setJSON("stored", playerData("stored", 0)); const before = (await saved()).get("stored"); assert.equal(before.epoch, 1); - for (const extra of [{ crafting_queue: {} }, { deleted: false }, { deleted: true }]) { + const badFilter = { value: { name: "iron-plate" }, min: "many" }; + for (const extra of [ + { crafting_queue: {} }, + { deleted: false }, + { deleted: true }, + { controller: "cutscene" }, + { inventories: { main: { i: [{ e: null }] } } }, + { personal_logistic_slots: { + enabled: true, + trash_not_requested: false, + sections: [{ group: "", active: true, multiplier: 1, filters: [badFilter] }], + } }, + ]) { await assert.rejects( setJSON("stored", playerData("stored", 0, extra)), { message: "Request SetPlayerDataRequest failed validation" }, @@ -228,6 +244,34 @@ describe("player_sync controller", function() { } assert.deepEqual((await saved()).get("stored"), before); }); + it("should start over from imported generations past the limit", async function() { + await release("stored"); + assert.equal((await setJSON("stored", playerData("stored", 2 ** 53))).generation, 4); + await upload(1, playerData("stored", 5, { epoch: 1 })); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 5, { epoch: 1 })); + }); + for (const deleted of [false, true]) { + it(`should recover ${deleted ? "deleted" : "stored"} data past the generation limit`, async function() { + // Could be stored by uploads or by imports from before the limit + for (const generation of [msg.generationLimit, 2 ** 53]) { + const stuck = { ...playerData("stored", generation), ...deleted ? { deleted } : {} }; + const entries = [["stored", stuck]]; + await fs.writeFile(path.join(databaseDir, "player_sync.json"), JSON.stringify(entries)); + await loadWithInstances(); + // Importing the exported data as is + assert.equal((await setJSON("stored", playerData("stored", generation))).generation, 1); + await upload(1, playerData("stored", 2, { epoch: 1 })); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 2, { epoch: 1 })); + } + }); + } + it("should keep counting up below the generation limit", async function() { + const stored = playerData("stored", msg.generationLimit - 1); + await fs.writeFile(path.join(databaseDir, "player_sync.json"), JSON.stringify([["stored", stored]])); + await loadWithInstances(); + const response = await setJSON("stored", playerData("stored", 0)); + assert.equal(response.generation, msg.generationLimit); + }); it("should only load entries marked deleted: true as tombstones", async function() { // Imports from before the marker was reserved could store deleted: false const data = playerData("stored", 4, { epoch: 1 }); diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 52f8d0cf..ed521923 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -76,7 +76,15 @@ describe("player_sync plugin", function() { character_additional_mining_categories: {}, allow_dispatching_robots: true, inventories: { - main: { b: 80, i: [{ n: "iron-plate", c: 100, r: 2 }, { s: 10, f: { name: "coal" } }] }, + main: { b: 80, i: [ + { n: "iron-plate", c: 100, r: 2 }, + { s: 10, f: { name: "coal" } }, + { f: { quality: "rare", comparator: "=" } }, + { f: "stone" }, + { n: "firearm-magazine", c: 10, q: "rare", a: 5 }, + { n: "automation-science-pack", c: 3, d: 0.5, p: 0.25, h: 0.75 }, + { e: "0eNq" }, + ] }, armor: { i: {} }, }, }, @@ -87,7 +95,19 @@ describe("player_sync plugin", function() { group: "", active: true, multiplier: 1, - filters: [{ value: { name: "iron-plate", type: "item" }, min: 3 }], + // Empty slots export as {} + filters: [ + {}, + { value: { name: "iron-plate", type: "item" }, min: 3 }, + { value: { name: "stone", type: "item" }, min: -5, max: 10 }, + { + value: { name: "coal", type: "item", quality: "normal", comparator: "=" }, + min: 5, + max: 10, + minimum_delivery_count: 2, + import_from: null, + }, + ], }], }, quick_bar: { @@ -124,6 +144,7 @@ describe("player_sync plugin", function() { for (const [field, value] of [ ["controller", "not-a-controller"], ["controller", "remote"], + ["controller", "cutscene"], ["deleted", false], ["deleted", true], ["color", "red"], @@ -131,6 +152,10 @@ describe("player_sync plugin", function() { ["ticks_to_respawn", -1], ["inventories", { main: { i: "iron-plate" } }], ["inventories", { main: [] }], + ["inventories", { main: { i: [{ e: null }] } }], + ["inventories", { main: { i: [{ n: 5, c: 1 }] } }], + ["inventories", { main: { i: [{ n: "wood", c: "many" }] } }], + ["inventories", { main: { i: [{ n: "wood", c: 0 }] } }], ["character", { inventories: { main: { i: [{ s: 0 }] } } }], ["shortcuts", { "toggle-personal-roboport": "yes" }], ["character", { character_health_bonus: "100" }], @@ -157,6 +182,25 @@ describe("player_sync plugin", function() { ]) { assert(!accepts({ ...exported, [field]: value }), `accepted ${field} ${JSON.stringify(value)}`); } + for (const filter of [ + { value: { name: "iron-plate" }, min: "many" }, + { value: { name: "iron-plate" }, min: 2 ** 31 }, + { value: { name: "iron-plate" }, max: -1 }, + { value: { name: "iron-plate" }, minimum_delivery_count: -1 }, + { value: { type: "item" }, min: 1 }, + { value: { name: "iron-plate", type: "nope" }, min: 1 }, + { value: { name: "iron-plate", comparator: "~" }, min: 1 }, + { value: { name: "iron-plate", quality: 5 }, min: 1 }, + ]) { + const slots = { + enabled: true, + trash_not_requested: false, + sections: [{ group: "", active: true, multiplier: 1, filters: [filter] }], + }; + assert( + !accepts({ ...exported, personal_logistic_slots: slots }), `accepted ${JSON.stringify(filter)}`, + ); + } }); }); it("SetPlayerDataResponse should be round trip json serialisable", function() { @@ -184,10 +228,27 @@ describe("player_sync plugin", function() { ] }), [ { name: "iron-plate", quality: undefined, count: 307 }, { name: "iron-plate", quality: "rare", count: 50 }, - { name: "exported item", count: 1, exportSize: 4 }, - { name: "exported item", count: 1, exportSize: 4 }, + { name: "exported item", count: 2, exportSize: 4 }, ]); }); + it("should skip malformed entries", function() { + assert.deepEqual(summarizeInventory({ i: [ + { e: null }, + null, + "wood", + { n: 5, c: 1 }, + { n: "wood", c: "many", r: "x" }, + { n: "wood", c: 2, r: 1 }, + { e: "0eNq", r: 1e9 }, + ] }), [ + { name: "wood", quality: undefined, count: 4 }, + { name: "exported item", count: 1e9 + 1, exportSize: 4 }, + ]); + assert.deepEqual( + summarizePlayerInventories(playerData("player", 1, { inventories: { main: null } })), + [{ name: "main", items: [] }], + ); + }); it("should handle empty inventories", function() { assert.deepEqual(summarizeInventory({}), []); assert.deepEqual(summarizeInventory(JSON.parse('{"i":{}}')), []);