From a81d98e98167e83816088b555f9ded1f0960ad33 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:44:09 +0000 Subject: [PATCH 01/17] Add inspect and modify tools for player sync Admins had no way to look at or fix the player data the controller stores when a sync went wrong. The web page now lists every stored entry with its generation, size and the instance holding the lock, and opens a drawer per player with an inventory summary, JSON export and import, delete and lock release. The same operations are exposed as clusterioctl player-sync commands. Import bumps the generation past the stored one so instances holding an older copy download the new data on the next join. Import and delete are refused while an instance holds the lock, since its upload on leave would undo the change. A new player_sync.inventory.modify permission gates the write operations. Closes #1003 Co-Authored-By: Claude Fable 5.1 --- plugins/player_sync/README.md | 22 ++ plugins/player_sync/controller.ts | 84 ++++++- plugins/player_sync/ctl.ts | 125 ++++++++++ plugins/player_sync/index.ts | 12 + plugins/player_sync/messages.ts | 157 +++++++++++- plugins/player_sync/package.json | 2 + plugins/player_sync/player_data.ts | 56 +++++ plugins/player_sync/test/controller.js | 156 +++++++++++- plugins/player_sync/test/plugin.js | 82 +++++++ plugins/player_sync/tsconfig.browser.json | 2 +- plugins/player_sync/tsconfig.node.json | 1 + plugins/player_sync/web/index.tsx | 276 +++++++++++++++++++++- 12 files changed, 953 insertions(+), 22 deletions(-) create mode 100644 plugins/player_sync/ctl.ts create mode 100644 plugins/player_sync/player_data.ts create mode 100644 plugins/player_sync/test/plugin.js diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index 8df6829e..0c4a1a9c 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -30,6 +30,28 @@ Remove the old package and install the new one in its place: The `inventory_sync.*` config fields and the `inventory_sync.inventory.view` permission are renamed on load. The controller moves `inventories.json` in the database directory to `player_sync.json`, and saves keep their player data the next time they are started with the new module. +## Fixing broken inventories + +The controller stores one entry per player in `player_sync.json` and keeps a lock on the player while an instance has them online. +When sync goes wrong for a player, the Player sync page in the web interface lists every stored entry with its generation, size and the instance holding the lock. +Click an entry to see a summary of its inventories, export it as JSON, import an edited JSON file in its place, delete it, or release the lock. + +The same operations are available from clusterioctl: + + npx clusterioctl player-sync list + npx clusterioctl player-sync show + npx clusterioctl player-sync export [file] + npx clusterioctl player-sync import + npx clusterioctl player-sync delete + npx clusterioctl player-sync release + +Importing bumps the generation so instances holding an older copy download the new one the next time the player joins. +Deleting makes the next join turn whatever the player has on that instance into the new synced inventory. +Both are refused while an instance holds the lock on the player, because the instance uploads its own copy when the player leaves and that would undo the change. +Have the player leave first, or release the lock if the instance is stuck holding it. + +Viewing and exporting need the `player_sync.inventory.view` permission. Importing, deleting and releasing need `player_sync.inventory.modify`, which is not granted by default. + ## Method of operation This plugin does event based synchronization of inventories. diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 887b4b06..1355a18f 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -55,14 +55,22 @@ export default async function(context: ControllerPluginContext) { const playerDatastore: Map = await loadDatabase(controller.config, logger); let playerDatastoreDirty = false; - function acquire(instanceId: number, playerName: string): boolean { + // Returns the acquisition record for the player if it's still in effect + function getAcquisition(playerName: string) { let acquisitionRecord = acquiredPlayers.get(playerName); if ( !acquisitionRecord - || acquisitionRecord.instanceId === instanceId || !controller.instances.has(acquisitionRecord.instanceId) || acquisitionRecord.expiresMs && acquisitionRecord.expiresMs < Date.now() ) { + return undefined; + } + return acquisitionRecord; + } + + function acquire(instanceId: number, playerName: string): boolean { + let acquisitionRecord = getAcquisition(playerName); + if (!acquisitionRecord || acquisitionRecord.instanceId === instanceId) { acquiredPlayers.set(playerName, { instanceId }); return true; } @@ -70,6 +78,19 @@ export default async function(context: ControllerPluginContext) { return false; } + // Modifying stored data while an instance holds the player is pointless + // as the instance uploads its own copy when the player leaves. + function checkNotAcquired(playerName: string) { + let acquisitionRecord = getAcquisition(playerName); + if (acquisitionRecord) { + let instanceName = controller.instances.get(acquisitionRecord.instanceId)!.config.get("instance.name"); + throw new lib.RequestError( + `${playerName} is currently acquired by ${instanceName}, ` + + "have the player leave or release the lock first" + ); + } + } + controller.hooks.instanceStatusChanged.attach(plugin.name, async (instance: InstanceRecord) => { let instanceId = instance.id; if (["unassigned", "deleted"].includes(instance.status)) { @@ -202,4 +223,63 @@ export default async function(context: ControllerPluginContext) { }, ); }); + + controller.handle(msg.ListPlayersRequest, async () => { + let entries = new Map(); + for (let [name, playerData] of playerDatastore) { + entries.set(name, new msg.PlayerEntry(name, playerData.generation, JSON.stringify(playerData).length)); + } + for (let name of acquiredPlayers.keys()) { + let acquisitionRecord = getAcquisition(name); + if (!acquisitionRecord) { + continue; + } + let entry = entries.get(name) ?? new msg.PlayerEntry(name); + entry.instanceId = acquisitionRecord.instanceId; + entries.set(name, entry); + } + return [...entries.values()]; + }); + + controller.handle(msg.GetPlayerDataRequest, async (request: msg.GetPlayerDataRequest) => { + let { playerName } = request; + return new msg.GetPlayerDataRequest.Response( + playerDatastore.get(playerName) ?? null, + getAcquisition(playerName)?.instanceId, + ); + }); + + controller.handle(msg.SetPlayerDataRequest, async (request: msg.SetPlayerDataRequest) => { + let { playerName, playerData } = request; + checkNotAcquired(playerName); + + // Bump the generation so instances holding an older copy download this one + let oldPlayerData = playerDatastore.get(playerName); + playerData.name = playerName; + playerData.generation = Math.max(playerData.generation, (oldPlayerData?.generation ?? 0) + 1); + playerDatastore.set(playerName, playerData); + playerDatastoreDirty = true; + logger.info(`Replaced player data for ${playerName} with generation ${playerData.generation}`); + return new msg.SetPlayerDataRequest.Response(playerData.generation); + }); + + controller.handle(msg.DeletePlayerDataRequest, async (request: msg.DeletePlayerDataRequest) => { + let { playerName } = request; + if (!playerDatastore.has(playerName)) { + throw new lib.RequestError(`No player data stored for ${playerName}`); + } + checkNotAcquired(playerName); + + playerDatastore.delete(playerName); + playerDatastoreDirty = true; + logger.info(`Deleted player data for ${playerName}`); + }); + + controller.handle(msg.ForceReleaseRequest, async (request: msg.ForceReleaseRequest) => { + let { playerName } = request; + if (!acquiredPlayers.delete(playerName)) { + throw new lib.RequestError(`${playerName} is not acquired by any instance`); + } + logger.info(`Released lock on ${playerName}`); + }); } diff --git a/plugins/player_sync/ctl.ts b/plugins/player_sync/ctl.ts new file mode 100644 index 00000000..d92b6550 --- /dev/null +++ b/plugins/player_sync/ctl.ts @@ -0,0 +1,125 @@ +import fs from "node:fs/promises"; +import * as lib from "@clusterio/lib"; +import type { Ctl, CtlPluginContext } from "@clusterio/ctl"; +import * as msg from "./messages.js"; +import { summarizePlayerInventories } from "./player_data.js"; + +function print(...content: any) { + // eslint-disable-next-line no-console + console.log(...content); +} + +async function instanceName(ctl: Ctl, instanceId: number | undefined) { + if (instanceId === undefined) { + return "-"; + } + let instance = await ctl.send(new lib.InstanceDetailsGetRequest(instanceId)); + return `${instance.name} (${instanceId})`; +} + +const playerSyncCommands = new lib.CommandTree({ + name: "player-sync", description: "Player sync plugin commands", +}); +playerSyncCommands.add(new lib.Command({ + definition: [["list", "l"], "List player data stored on the controller"], + handler: async function(args: object, ctl: Ctl) { + let players = await ctl.send(new msg.ListPlayersRequest()); + players.sort((a, b) => a.name.localeCompare(b.name)); + print("name | generation | size | acquired by"); + for (let player of players) { + print( + `${player.name} | ${player.generation ?? "-"} | ` + + `${player.size === undefined ? "-" : lib.formatBytes(player.size)} | ` + + `${await instanceName(ctl, player.instanceId)}` + ); + } + }, +})); + +playerSyncCommands.add(new lib.Command({ + definition: ["show ", "Show a summary of the stored player data", (yargs) => { + yargs.positional("player", { describe: "Name of the player", type: "string" }); + }], + handler: async function(args: { player: string }, ctl: Ctl) { + let response = await ctl.send(new msg.GetPlayerDataRequest(args.player)); + print(`acquired by: ${await instanceName(ctl, response.instanceId)}`); + let playerData = response.playerData; + if (!playerData) { + print(`No player data stored for ${args.player}`); + return; + } + print(`generation: ${playerData.generation}`); + print(`size: ${lib.formatBytes(JSON.stringify(playerData).length)}`); + print(`controller: ${playerData.controller}`); + print(`force: ${playerData.force}`); + for (let inventory of summarizePlayerInventories(playerData)) { + print(`${inventory.name}:`); + for (let item of inventory.items) { + let quality = item.quality ? ` (${item.quality})` : ""; + let size = item.exportSize ? ` ${lib.formatBytes(item.exportSize)}` : ""; + print(` ${item.count} ${item.name}${quality}${size}`); + } + } + }, +})); + +playerSyncCommands.add(new lib.Command({ + definition: ["export [file]", "Export stored player data as JSON", (yargs) => { + yargs.positional("player", { describe: "Name of the player", type: "string" }); + yargs.positional("file", { describe: "File to write to, prints to stdout if omitted", type: "string" }); + }], + handler: async function(args: { player: string, file?: string }, ctl: Ctl) { + let response = await ctl.send(new msg.GetPlayerDataRequest(args.player)); + if (!response.playerData) { + throw new lib.CommandError(`No player data stored for ${args.player}`); + } + let content = JSON.stringify(response.playerData, null, "\t"); + if (args.file) { + await fs.writeFile(args.file, content); + print(`Wrote player data for ${args.player} to ${args.file}`); + } else { + print(content); + } + }, +})); + +playerSyncCommands.add(new lib.Command({ + definition: ["import ", "Replace stored player data with JSON from a file", (yargs) => { + yargs.positional("player", { describe: "Name of the player", type: "string" }); + yargs.positional("file", { describe: "File to read player data from", type: "string" }); + }], + handler: async function(args: { player: string, file: string }, ctl: Ctl) { + let playerData = JSON.parse(await fs.readFile(args.file, "utf8")); + if (typeof playerData !== "object" || playerData === null || Array.isArray(playerData)) { + throw new lib.CommandError("Player data must be a JSON object"); + } + playerData.name = args.player; + playerData.generation ??= 0; + let response = await ctl.send(new msg.SetPlayerDataRequest(args.player, playerData)); + print(`Stored player data for ${args.player} as generation ${response.generation}`); + }, +})); + +playerSyncCommands.add(new lib.Command({ + definition: ["delete ", "Delete stored player data", (yargs) => { + yargs.positional("player", { describe: "Name of the player", type: "string" }); + }], + handler: async function(args: { player: string }, ctl: Ctl) { + await ctl.send(new msg.DeletePlayerDataRequest(args.player)); + }, +})); + +playerSyncCommands.add(new lib.Command({ + definition: ["release ", "Release the lock an instance holds on a player", (yargs) => { + yargs.positional("player", { describe: "Name of the player", type: "string" }); + }], + handler: async function(args: { player: string }, ctl: Ctl) { + await ctl.send(new msg.ForceReleaseRequest(args.player)); + }, +})); + +export default async function(context: CtlPluginContext) { + context.hooks.addCommands.attach(context.plugin.name, async (rootCommand) => { + rootCommand.add(playerSyncCommands); + }); +} diff --git a/plugins/player_sync/index.ts b/plugins/player_sync/index.ts index fe76dfba..11d0e0b1 100644 --- a/plugins/player_sync/index.ts +++ b/plugins/player_sync/index.ts @@ -18,6 +18,7 @@ declare module "@clusterio/lib" { } export interface Permissions { "player_sync.inventory.view": never; + "player_sync.inventory.modify": never; } } @@ -91,6 +92,7 @@ export const plugin: lib.PluginDeclaration = { }, controllerEntrypoint: "dist/node/controller.js", + ctlEntrypoint: "dist/node/ctl.js", controllerConfigFields: { "player_sync.player_lock_timeout": { title: "Player Lock Timeout", @@ -113,6 +115,11 @@ export const plugin: lib.PluginDeclaration = { messages.UploadRequest, messages.DownloadRequest, messages.DatabaseStatsRequest, + messages.ListPlayersRequest, + messages.GetPlayerDataRequest, + messages.SetPlayerDataRequest, + messages.DeletePlayerDataRequest, + messages.ForceReleaseRequest, ], permissions: [ { @@ -121,6 +128,11 @@ export const plugin: lib.PluginDeclaration = { description: "View player inventories", grantByDefault: true, }, + { + name: "player_sync.inventory.modify", + title: "Modify player inventories", + description: "Replace, delete and release the lock on player inventories stored on the controller", + }, ], webEntrypoint: "./web", routes: ["/inventory"], diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 993b56c4..6a5433b2 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -1,5 +1,5 @@ import { Type, Static } from "@sinclair/typebox"; -import { StringEnum } from "@clusterio/lib"; +import { StringEnum, jsonArray } from "@clusterio/lib"; // .\module\serialize.lua:serialize.serialize_player() export type IpcPlayerData = { @@ -193,3 +193,158 @@ export class DatabaseStatsRequest { static permission = "player_sync.inventory.view" as const; static Response = DatabaseStatsResponse; } + +export class PlayerEntry { + constructor( + public name: string, + public generation?: number, + public size?: number, + public instanceId?: number, + ) { + } + + static jsonSchema = Type.Object({ + "name": Type.String(), + "generation": Type.Optional(Type.Integer()), + "size": Type.Optional(Type.Integer()), + "instanceId": Type.Optional(Type.Integer()), + }); + + static fromJSON(json: Static): PlayerEntry { + return new this(json.name, json.generation, json.size, json.instanceId); + } +} + +export class ListPlayersRequest { + declare ["constructor"]: typeof ListPlayersRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static plugin = "player_sync" as const; + static permission = "player_sync.inventory.view" as const; + static Response = jsonArray(PlayerEntry); +} + +export class GetPlayerDataResponse { + constructor( + public playerData: IpcPlayerData | null, + public instanceId?: number, + ) { + } + + static jsonSchema = Type.Object({ + "playerData": Type.Union([jsonPlayerData, Type.Null()]), + "instanceId": Type.Optional(Type.Integer()), + }); + + static fromJSON(json: Static): GetPlayerDataResponse { + return new this(json.playerData as IpcPlayerData | null, json.instanceId); + } +} + +export class GetPlayerDataRequest { + declare ["constructor"]: typeof GetPlayerDataRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static plugin = "player_sync" as const; + static permission = "player_sync.inventory.view" as const; + static Response = GetPlayerDataResponse; + + constructor( + public playerName: string, + ) { + } + + static jsonSchema = Type.Object({ + "playerName": Type.String(), + }); + + static fromJSON(json: Static) { + return new this(json.playerName); + } +} + +export class SetPlayerDataResponse { + constructor( + public generation: number, + ) { + } + + static jsonSchema = Type.Object({ + "generation": Type.Integer(), + }); + + static fromJSON(json: Static): SetPlayerDataResponse { + return new this(json.generation); + } +} + +export class SetPlayerDataRequest { + declare ["constructor"]: typeof SetPlayerDataRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static plugin = "player_sync" as const; + static permission = "player_sync.inventory.modify" as const; + static Response = SetPlayerDataResponse; + + constructor( + public playerName: string, + public playerData: IpcPlayerData, + ) { + } + + static jsonSchema = Type.Object({ + "playerName": Type.String(), + "playerData": jsonPlayerData, + }); + + static fromJSON(json: Static) { + return new this(json.playerName, json.playerData as IpcPlayerData); + } +} + +export class DeletePlayerDataRequest { + declare ["constructor"]: typeof DeletePlayerDataRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static plugin = "player_sync" as const; + static permission = "player_sync.inventory.modify" as const; + + constructor( + public playerName: string, + ) { + } + + static jsonSchema = Type.Object({ + "playerName": Type.String(), + }); + + static fromJSON(json: Static) { + return new this(json.playerName); + } +} + +export class ForceReleaseRequest { + declare ["constructor"]: typeof ForceReleaseRequest; + static type = "request" as const; + static src = "control" as const; + static dst = "controller" as const; + static plugin = "player_sync" as const; + static permission = "player_sync.inventory.modify" as const; + + constructor( + public playerName: string, + ) { + } + + static jsonSchema = Type.Object({ + "playerName": Type.String(), + }); + + static fromJSON(json: Static) { + return new this(json.playerName); + } +} diff --git a/plugins/player_sync/package.json b/plugins/player_sync/package.json index 2f4db121..ff06cf62 100644 --- a/plugins/player_sync/package.json +++ b/plugins/player_sync/package.json @@ -35,6 +35,7 @@ "peerDependencies": { "@clusterio/controller": "workspace:^", + "@clusterio/ctl": "workspace:^", "@clusterio/host": "workspace:^", "@clusterio/lib": "workspace:^" }, @@ -51,6 +52,7 @@ }, "devDependencies": { "@clusterio/controller": "workspace:^", + "@clusterio/ctl": "workspace:^", "@clusterio/host": "workspace:^", "@clusterio/lib": "workspace:^", "@clusterio/web_ui": "workspace:^", diff --git a/plugins/player_sync/player_data.ts b/plugins/player_sync/player_data.ts new file mode 100644 index 00000000..b4857ee5 --- /dev/null +++ b/plugins/player_sync/player_data.ts @@ -0,0 +1,56 @@ +import type { IpcPlayerData } from "./messages.js"; + +// See packages/host/modules/clusterio/serialize.lua for the item stack format +type SerializedItem = { + n?: string, + c?: number, + q?: string, + r?: number, + e?: string, +}; + +export type ItemSummary = { + name: string, + quality?: string, + count: number, + /** Length of the export string for blueprints and other exportable items */ + exportSize?: number, +}; + +export type InventorySummary = { + name: string, + items: ItemSummary[], +}; + +export function summarizeInventory(inventory: { i?: SerializedItem[] }): ItemSummary[] { + let items: ItemSummary[] = []; + let stacks = new Map(); + for (let entry of 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 }); + } + continue; + } + if (entry.n === undefined) { + continue; + } + let key = `${entry.n}\0${entry.q ?? ""}`; + let stack = stacks.get(key); + if (!stack) { + stack = { name: entry.n, quality: entry.q, count: 0 }; + stacks.set(key, stack); + items.push(stack); + } + stack.count += (entry.c ?? 0) * repeat; + } + return items; +} + +export function summarizePlayerInventories(playerData: IpcPlayerData): InventorySummary[] { + let inventories = playerData.character?.inventories ?? playerData.inventories ?? {}; + return Object.entries(inventories).map( + ([name, inventory]) => ({ name, items: summarizeInventory(inventory as { i?: SerializedItem[] }) }) + ); +} diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index b1da5e64..67b9c4c7 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -3,11 +3,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import * as lib from "@clusterio/lib"; +import { InstanceRecord } from "@clusterio/controller"; import * as mock from "../../../test/mock.js"; import controllerEntrypoint from "../dist/node/controller.js"; import { components as allComponents } from "../dist/node/components.js"; import { plugin as info } from "../dist/node/index.js"; -import { DatabaseStatsRequest, DownloadRequest, UploadRequest } from "../dist/node/messages.js"; +import * as msg from "../dist/node/messages.js"; describe("player_sync controller", function() { const databaseDir = path.join("temp", "test", "player_sync"); @@ -36,7 +37,7 @@ describe("player_sync controller", function() { await fs.writeFile(path.join(databaseDir, "inventories.json"), JSON.stringify([["test", playerData]])); const controller = await load(); - const stats = await mock.getHandler(controller, DatabaseStatsRequest)(); + const stats = await mock.getHandler(controller, msg.DatabaseStatsRequest)(); assert.equal(stats.databaseEntries, 1); assert.equal(stats.largestEntry.name, "test"); await assert.rejects(fs.access(path.join(databaseDir, "inventories.json")), { code: "ENOENT" }); @@ -47,7 +48,7 @@ describe("player_sync controller", function() { await fs.writeFile(path.join(databaseDir, "player_sync.json"), JSON.stringify([["new", { name: "new" }]])); const controller = await load(); - const stats = await mock.getHandler(controller, DatabaseStatsRequest)(); + const stats = await mock.getHandler(controller, msg.DatabaseStatsRequest)(); assert.equal(stats.databaseEntries, 1); assert.equal(stats.largestEntry.name, "new"); }); @@ -71,8 +72,8 @@ describe("player_sync controller", function() { }); async function upload(playerData) { - await mock.getHandler(controller, UploadRequest)({ instanceId: 1, playerName: "test", playerData }); - return (await mock.getHandler(controller, DownloadRequest)({ instanceId: 1, playerName: "test" })) + await mock.getHandler(controller, msg.UploadRequest)({ instanceId: 1, playerName: "test", playerData }); + return (await mock.getHandler(controller, msg.DownloadRequest)({ instanceId: 1, playerName: "test" })) .playerData; } @@ -89,4 +90,149 @@ describe("player_sync controller", function() { ); }); }); + + describe("player data tools", function() { + function playerData(name, generation, extra = {}) { + return { generation, name, controller: "character", force: "player", ...extra }; + } + + let controller; + function handler(Class) { + return mock.getHandler(controller, Class); + } + async function acquire(instanceId, playerName) { + const response = await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName)); + return response.status === "acquired"; + } + async function saved() { + await controller.hooks.save.invoke(); + return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, "player_sync.json"), "utf8"))); + } + + beforeEach(async function() { + controller = await mock.loadControllerPlugin(async (context) => { + context.controller.mockConfigEntries.set("controller.database_directory", databaseDir); + // Negative so locks expire as soon as their instance stops + context.controller.mockConfigEntries.set("player_sync.player_lock_timeout", -1); + await controllerEntrypoint(context); + }, info); + for (const [id, name, status] of [[1, "Running", "running"], [2, "Stopped", "stopped"]]) { + controller.instances.records.set(id, new InstanceRecord( + new lib.InstanceConfig("controller", { "instance.id": id, "instance.name": name }), status, + )); + } + + await handler(msg.UploadRequest)(new msg.UploadRequest(1, "stored", playerData("stored", 3))); + assert(await acquire(1, "stored")); + assert(await acquire(1, "online")); + assert(await acquire(2, "expired")); + await controller.hooks.instanceStatusChanged.invoke(controller.instances.get(2), "running"); + assert(await acquire(99, "gone")); + await saved(); + }); + + describe("ListPlayersRequest", function() { + it("should list stored data and live acquisitions", async function() { + const entries = await handler(msg.ListPlayersRequest)(new msg.ListPlayersRequest()); + entries.sort((a, b) => a.name.localeCompare(b.name)); + assert.deepEqual(entries, [ + new msg.PlayerEntry("online", undefined, undefined, 1), + new msg.PlayerEntry("stored", 3, JSON.stringify(playerData("stored", 3)).length, 1), + ]); + }); + }); + + describe("GetPlayerDataRequest", function() { + async function get(name) { + return await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest(name)); + } + it("should return stored data and acquisition", async function() { + assert.deepEqual(await get("stored"), new msg.GetPlayerDataResponse(playerData("stored", 3), 1)); + }); + it("should return null for unknown players", async function() { + assert.deepEqual(await get("unknown"), new msg.GetPlayerDataResponse(null, undefined)); + }); + it("should ignore expired and dangling acquisitions", async function() { + for (const name of ["expired", "gone"]) { + assert.equal((await get(name)).instanceId, undefined); + } + }); + }); + + const acquiredError = new lib.RequestError( + "stored is currently acquired by Running, have the player leave or release the lock first" + ); + + describe("SetPlayerDataRequest", function() { + async function set(name, data) { + return await handler(msg.SetPlayerDataRequest)(new msg.SetPlayerDataRequest(name, data)); + } + async function release(name) { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest(name)); + } + it("should store data with a bumped generation", async function() { + await release("stored"); + assert.deepEqual(await set("stored", playerData("other", 1)), new msg.SetPlayerDataResponse(4)); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 4)); + }); + it("should keep a higher provided generation", async function() { + await release("stored"); + assert.equal((await set("stored", playerData("stored", 10))).generation, 10); + }); + it("should store data for new players", async function() { + assert.equal((await set("new", playerData("new", 0))).generation, 1); + assert.equal((await saved()).get("new").generation, 1); + }); + it("should reject when the player is acquired", async function() { + await assert.rejects(set("stored", playerData("stored", 3)), acquiredError); + assert.equal((await saved()).get("stored").generation, 3); + }); + }); + + describe("DeletePlayerDataRequest", function() { + async function del(name) { + await handler(msg.DeletePlayerDataRequest)(new msg.DeletePlayerDataRequest(name)); + } + it("should delete stored data", async function() { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); + await del("stored"); + assert(!(await saved()).has("stored")); + }); + it("should reject unknown players", async function() { + await assert.rejects(del("unknown"), new lib.RequestError("No player data stored for unknown")); + }); + it("should reject when the player is acquired", async function() { + await assert.rejects(del("stored"), acquiredError); + assert((await saved()).has("stored")); + }); + }); + + describe("ForceReleaseRequest", function() { + async function release(name) { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest(name)); + } + it("should remove the acquisition", async function() { + await release("online"); + assert(await acquire(2, "online")); + }); + it("should reject players that are not acquired", async function() { + await assert.rejects( + release("unknown"), new lib.RequestError("unknown is not acquired by any instance"), + ); + }); + }); + + describe("AcquireRequest", function() { + it("should refuse players held by another instance", async function() { + assert(!await acquire(2, "online")); + assert(await acquire(1, "online")); + }); + it("should take over expired and dangling acquisitions", async function() { + assert(await acquire(1, "expired")); + assert(await acquire(1, "gone")); + const response = await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest("gone")); + assert.equal(response.instanceId, 1); + }); + }); + }); }); diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js new file mode 100644 index 00000000..bfa82933 --- /dev/null +++ b/plugins/player_sync/test/plugin.js @@ -0,0 +1,82 @@ +import assert from "node:assert/strict"; + +import { testRoundTripJsonSerialisable, testMatrix } from "../../../test/common.js"; +import * as msg from "../dist/node/messages.js"; +import { summarizeInventory, summarizePlayerInventories } from "../dist/node/player_data.js"; + + +function playerData(name, generation, extra = {}) { + return { generation, name, controller: "character", force: "player", ...extra }; +} + +describe("player_sync plugin", function() { + describe("messages", function() { + it("PlayerEntry should be round trip json serialisable", function() { + testRoundTripJsonSerialisable(msg.PlayerEntry, testMatrix( + ["player"], // name + [undefined, 0, 4], // generation + [undefined, 0, 1234], // size + [undefined, 1], // instanceId + )); + }); + it("GetPlayerDataResponse should be round trip json serialisable", function() { + testRoundTripJsonSerialisable(msg.GetPlayerDataResponse, testMatrix( + [null, playerData("player", 1)], // playerData + [undefined, 1], // instanceId + )); + }); + it("SetPlayerDataResponse should be round trip json serialisable", function() { + testRoundTripJsonSerialisable(msg.SetPlayerDataResponse, testMatrix([1, 5])); + }); + it("requests should be round trip json serialisable", function() { + testRoundTripJsonSerialisable(msg.GetPlayerDataRequest, testMatrix(["player"])); + testRoundTripJsonSerialisable(msg.DeletePlayerDataRequest, testMatrix(["player"])); + testRoundTripJsonSerialisable(msg.ForceReleaseRequest, testMatrix(["player"])); + testRoundTripJsonSerialisable(msg.SetPlayerDataRequest, testMatrix( + ["player"], // playerName + [playerData("player", 3)], // playerData + )); + }); + }); + + describe("summarizeInventory()", function() { + it("should merge stacks and expand repeats", function() { + assert.deepEqual(summarizeInventory({ i: [ + { n: "iron-plate", c: 100, r: 2 }, + { n: "iron-plate", c: 50, q: "rare" }, + { n: "iron-plate", c: 7, s: 10 }, + { e: "0eNq", r: 1 }, + { f: "copper-plate" }, + ] }), [ + { 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 }, + ]); + }); + it("should handle empty inventories", function() { + assert.deepEqual(summarizeInventory({}), []); + }); + }); + + describe("summarizePlayerInventories()", function() { + it("should prefer character inventories", function() { + const data = playerData("player", 1, { + character: { inventories: { main: { i: [{ n: "wood", c: 1 }] } } }, + inventories: { main: { i: [{ n: "stone", c: 1 }] } }, + }); + assert.deepEqual(summarizePlayerInventories(data), [ + { name: "main", items: [{ name: "wood", quality: undefined, count: 1 }] }, + ]); + }); + it("should fall back to player inventories", function() { + const data = playerData("player", 1, { inventories: { main: { i: [{ n: "stone", c: 1 }] } } }); + assert.deepEqual(summarizePlayerInventories(data), [ + { name: "main", items: [{ name: "stone", quality: undefined, count: 1 }] }, + ]); + }); + it("should handle missing inventories", function() { + assert.deepEqual(summarizePlayerInventories(playerData("player", 1)), []); + }); + }); +}); diff --git a/plugins/player_sync/tsconfig.browser.json b/plugins/player_sync/tsconfig.browser.json index ba2d8c39..a41ff3bb 100644 --- a/plugins/player_sync/tsconfig.browser.json +++ b/plugins/player_sync/tsconfig.browser.json @@ -4,5 +4,5 @@ { "path": "../../packages/lib/tsconfig.browser.json" }, { "path": "../../packages/web_ui/tsconfig.browser.json" }, ], - "include": [ "web/**/*.tsx", "web/**/*.ts", "index.ts", "messages.ts", "package.json" ], + "include": [ "web/**/*.tsx", "web/**/*.ts", "index.ts", "messages.ts", "player_data.ts", "package.json" ], } diff --git a/plugins/player_sync/tsconfig.node.json b/plugins/player_sync/tsconfig.node.json index 55d31e46..a6b81e60 100644 --- a/plugins/player_sync/tsconfig.node.json +++ b/plugins/player_sync/tsconfig.node.json @@ -4,6 +4,7 @@ { "path": "../../packages/lib/tsconfig.node.json" }, { "path": "../../packages/host/tsconfig.node.json" }, { "path": "../../packages/controller/tsconfig.node.json" }, + { "path": "../../packages/ctl/tsconfig.node.json" }, ], "include": ["./**/*.ts"], "exclude": ["test/*", "./dist/*"], diff --git a/plugins/player_sync/web/index.tsx b/plugins/player_sync/web/index.tsx index dea3d6bf..1d14c190 100644 --- a/plugins/player_sync/web/index.tsx +++ b/plugins/player_sync/web/index.tsx @@ -1,29 +1,279 @@ -import React, { useContext, useEffect, useState } from "react"; +import React, { useCallback, useContext, useEffect, useState } from "react"; +import { Alert, Button, Descriptions, Drawer, Popconfirm, Space, Table, Typography, Upload } from "antd"; -import { PageLayout, PageHeader, ControlContext, type WebPluginContext } from "@clusterio/web_ui"; -import { DatabaseStatsRequest, DatabaseStatsResponse } from "../messages.js"; +import * as lib from "@clusterio/lib"; +import { + PageLayout, PageHeader, ControlContext, FactorioIcon, notify, notifyErrorHandler, + useAccount, useInstances, useDefaultModPack, useExportLocale, useExportPrototypeMetadata, + useTableQueryState, useColumnSearch, type WebPluginContext, +} from "@clusterio/web_ui"; +import { + DatabaseStatsRequest, DatabaseStatsResponse, DeletePlayerDataRequest, ForceReleaseRequest, + GetPlayerDataRequest, GetPlayerDataResponse, IpcPlayerData, ListPlayersRequest, PlayerEntry, + SetPlayerDataRequest, +} from "../messages.js"; +import { type ItemSummary, summarizePlayerInventories } from "../player_data.js"; import "./style.css"; +const strcmp = new Intl.Collator(undefined, { numeric: true, sensitivity: "base" }).compare; + +function useInstanceName() { + const [instances] = useInstances(); + return (instanceId?: number) => { + if (instanceId === undefined) { + return ""; + } + return instances.get(instanceId)?.name ?? `Instance ${instanceId}`; + }; +} + +function InventoryTable(props: { name: string, items: ItemSummary[] }) { + const modPack = useDefaultModPack(); + const locale = useExportLocale(modPack); + const prototypes = useExportPrototypeMetadata(modPack); + const itemMetadata = prototypes?.get("item"); + + function getLocaleName(itemName: string) { + let meta = itemMetadata?.get(itemName); + if (typeof meta?.localised_name === "string") { + return meta.localised_name; + } + if (meta?.localised_name) { + return locale.get(meta.localised_name[0]) ?? itemName; + } + return locale.get(`item-name.${itemName}`) ?? locale.get(`entity-name.${itemName}`) ?? itemName; + } + + return props.name} + columns={[ + { + title: "Item", + key: "item", + render: (_, item) => (item.exportSize !== undefined + ? {item.name} + : <> + + {getLocaleName(item.name)} + + ), + }, + { + title: "Quality", + key: "quality", + render: (_, item) => item.quality, + }, + { + title: "Count", + key: "count", + align: "right", + render: (_, item) => item.count, + }, + { + title: "Size", + key: "size", + align: "right", + render: (_, item) => (item.exportSize === undefined ? "" : lib.formatBytes(item.exportSize)), + }, + ]} + dataSource={props.items.map((item, key) => ({ ...item, key }))} + pagination={false} + />; +} + +function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChange: () => void }) { + const control = useContext(ControlContext); + const account = useAccount(); + const instanceName = useInstanceName(); + const [response, setResponse] = useState(); + const { playerName } = props; + + const reload = useCallback(() => { + if (playerName === undefined) { + setResponse(undefined); + return; + } + control.send(new GetPlayerDataRequest(playerName)).then(setResponse).catch( + notifyErrorHandler("Error loading player data") + ); + }, [control, playerName]); + useEffect(reload, [reload]); + + function changed() { + reload(); + props.onChange(); + } + + async function upload(file: File) { + let playerData: IpcPlayerData; + try { + playerData = JSON.parse(await file.text()); + } catch (err: any) { + notify("Invalid JSON", "error", err.message); + return; + } + playerData.name = playerName!; + playerData.generation ??= 0; + let result = await control.send(new SetPlayerDataRequest(playerName!, playerData)); + notify(`Stored player data for ${playerName} as generation ${result.generation}`); + changed(); + } + + function download() { + let json = JSON.stringify(response!.playerData, null, "\t"); + let url = URL.createObjectURL(new Blob([json], { type: "application/json" })); + let link = document.createElement("a"); + link.href = url; + link.download = `${playerName}.json`; + link.click(); + URL.revokeObjectURL(url); + } + + const canModify = account.hasPermission("player_sync.inventory.modify"); + const playerData = response?.playerData; + const acquiredBy = response?.instanceId !== undefined ? instanceName(response.instanceId) : undefined; + return + {response && + {acquiredBy !== undefined && control.send(new ForceReleaseRequest(playerName!)).then(changed).catch( + notifyErrorHandler("Error releasing lock") + )} + > + + } + />} + + {playerData?.generation ?? "-"} + + {playerData ? lib.formatBytes(JSON.stringify(playerData).length) : "-"} + + {playerData?.controller ?? "-"} + {playerData?.force ?? "-"} + + + + {canModify && { + upload(file).catch(notifyErrorHandler("Error storing player data")); + return false; + }} + > + + } + {canModify && control.send(new DeletePlayerDataRequest(playerName!)).then(changed).catch( + notifyErrorHandler("Error deleting player data") + )} + > + + } + + {!playerData && No player data stored} + {playerData && summarizePlayerInventories(playerData).map( + inventory => + )} + } + ; +} + function InventoryPage() { - let control = useContext(ControlContext); + const control = useContext(ControlContext); + const instanceName = useInstanceName(); + const [statsData, setStatsData] = useState(); + const [players, setPlayers] = useState([]); + const [selected, setSelected] = useState(); - let [statsData, updateStatsData] = useState(); + const tableState = useTableQueryState({ + namespace: "inventory", defaultSortKey: "name", pagination: { defaultPageSize: 50 }, + }); + const nameSearch = useColumnSearch(tableState, "name", player => player.name, "Search"); - useEffect(() => { - (async () => { - // Get statistics - updateStatsData(await control.send(new DatabaseStatsRequest())); - })(); - }, []); + const reload = useCallback(() => { + control.send(new DatabaseStatsRequest()).then(setStatsData).catch( + notifyErrorHandler("Error loading player sync statistics") + ); + control.send(new ListPlayersRequest()).then(setPlayers).catch( + notifyErrorHandler("Error loading player list") + ); + }, [control]); + useEffect(reload, [reload]); return {statsData && <> -

Database size: {Math.round(statsData.databaseSize / 1000)}kB

+

Database size: {lib.formatBytes(statsData.databaseSize)}

Database entries: {statsData.databaseEntries}

-

Largest entry is {statsData.largestEntry.name} with {(statsData.largestEntry.size / 1000)}kB

+

Largest entry is {statsData.largestEntry.name} with {lib.formatBytes(statsData.largestEntry.size)}

} +
strcmp(a.name, b.name), + sortOrder: tableState.sortOrder("name"), + }, + { + title: "Generation", + key: "generation", + align: "right", + sorter: (a, b) => (a.generation ?? -1) - (b.generation ?? -1), + sortOrder: tableState.sortOrder("generation"), + render: (_, player) => player.generation ?? "-", + }, + { + title: "Size", + key: "size", + align: "right", + sorter: (a, b) => (a.size ?? -1) - (b.size ?? -1), + sortOrder: tableState.sortOrder("size"), + render: (_, player) => (player.size === undefined ? "-" : lib.formatBytes(player.size)), + }, + { + title: "Acquired by", + key: "instance", + sorter: (a, b) => strcmp(instanceName(a.instanceId), instanceName(b.instanceId)), + sortOrder: tableState.sortOrder("instance"), + render: (_, player) => instanceName(player.instanceId), + }, + ]} + dataSource={players} + rowKey="name" + pagination={tableState.pagination} + onChange={tableState.onChange} + onRow={player => ({ + style: { cursor: "pointer" }, + onClick: () => setSelected(player.name), + })} + /> + setSelected(undefined)} onChange={reload} /> ; } From 9cc5c64852d891165a8bfaa59a82140ca7ce699c Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:58:48 +0000 Subject: [PATCH 02/17] Cover the player-sync ctl commands in tests Codecov flagged ctl.ts at 21% patch coverage. Run each command through a fake control link with canned responses and check what it prints and sends. Co-Authored-By: Claude Fable 5.1 --- plugins/player_sync/test/plugin.js | 135 +++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index bfa82933..56968025 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -1,8 +1,17 @@ import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import * as lib from "@clusterio/lib"; +import { CtlHooks } from "@clusterio/ctl"; + +import * as mock from "../../../test/mock.js"; import { testRoundTripJsonSerialisable, testMatrix } from "../../../test/common.js"; import * as msg from "../dist/node/messages.js"; import { summarizeInventory, summarizePlayerInventories } from "../dist/node/player_data.js"; +import ctlEntrypoint from "../dist/node/ctl.js"; +import { plugin as info } from "../dist/node/index.js"; function playerData(name, generation, extra = {}) { @@ -56,6 +65,10 @@ describe("player_sync plugin", function() { }); it("should handle empty inventories", function() { assert.deepEqual(summarizeInventory({}), []); + assert.deepEqual( + summarizeInventory({ i: [{ n: "wood" }] }), + [{ name: "wood", quality: undefined, count: 0 }], + ); }); }); @@ -77,6 +90,128 @@ describe("player_sync plugin", function() { }); it("should handle missing inventories", function() { assert.deepEqual(summarizePlayerInventories(playerData("player", 1)), []); + assert.deepEqual(summarizePlayerInventories(playerData("player", 1, { character: {} })), []); + }); + }); + + describe("ctl commands", function() { + let commands; + let sent; + let printed; + let responses; + const ctl = { + async send(message) { + sent.push(message); + const handler = responses.get(message.constructor); + if (!handler) { + throw new Error(`Unexpected ${message.constructor.name}`); + } + return handler(message); + }, + }; + const storedData = playerData("alice", 2, { + character: { inventories: { main: { i: [{ n: "wood", c: 5, q: "rare" }, { e: "0eNq" }] } } }, + }); + let tempDir; + + before(async function() { + const root = new lib.CommandTree({ name: "clusterioctl", description: "root" }); + const hooks = new CtlHooks(new mock.MockLogger()); + await ctlEntrypoint({ hooks, logger: new mock.MockLogger(), plugin: info }); + await hooks.addCommands.invoke(root); + commands = root.subCommands.get("player-sync").subCommands; + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "player_sync-ctl-")); + }); + after(async function() { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + beforeEach(function() { + sent = []; + printed = []; + responses = new Map([ + [lib.InstanceDetailsGetRequest, request => ({ id: request.instanceId, name: "Test" })], + [msg.ListPlayersRequest, () => [ + new msg.PlayerEntry("bob", undefined, undefined, 1), + new msg.PlayerEntry("alice", 2, 100), + ]], + [msg.GetPlayerDataRequest, request => new msg.GetPlayerDataResponse( + request.playerName === "alice" ? storedData : null, request.playerName === "alice" ? 1 : undefined + )], + [msg.SetPlayerDataRequest, () => new msg.SetPlayerDataResponse(3)], + [msg.DeletePlayerDataRequest, () => undefined], + [msg.ForceReleaseRequest, () => undefined], + ]); + }); + + async function run(name, args = {}) { + /* eslint-disable no-console */ + const original = console.log; + console.log = (...values) => printed.push(values.join(" ")); + try { + await commands.get(name).run(args, ctl); + } finally { + console.log = original; + } + /* eslint-enable no-console */ + } + + it("list should print sorted entries", async function() { + await run("list"); + assert.deepEqual(printed, [ + "name | generation | size | acquired by", + `alice | 2 | ${lib.formatBytes(100)} | -`, + "bob | - | - | Test (1)", + ]); + }); + it("show should print a summary", async function() { + await run("show", { player: "alice" }); + assert.deepEqual(printed, [ + "acquired by: Test (1)", + "generation: 2", + `size: ${lib.formatBytes(JSON.stringify(storedData).length)}`, + "controller: character", + "force: player", + "main:", + " 5 wood (rare)", + ` 1 exported item ${lib.formatBytes(4)}`, + ]); + }); + it("show should handle players without data", async function() { + await run("show", { player: "carol" }); + assert.deepEqual(printed, ["acquired by: -", "No player data stored for carol"]); + }); + it("export should print or write JSON", async function() { + await run("export", { player: "alice" }); + assert.deepEqual(JSON.parse(printed.join("\n")), storedData); + const file = path.join(tempDir, "alice.json"); + await run("export", { player: "alice", file }); + assert.deepEqual(JSON.parse(await fs.readFile(file, "utf8")), storedData); + await assert.rejects( + run("export", { player: "carol" }), + new lib.CommandError("No player data stored for carol"), + ); + }); + it("import should send the file contents", async function() { + const file = path.join(tempDir, "import.json"); + await fs.writeFile(file, JSON.stringify({ name: "other", controller: "god" })); + await run("import", { player: "alice", file }); + assert.deepEqual(sent, [ + new msg.SetPlayerDataRequest("alice", { name: "alice", controller: "god", generation: 0 }), + ]); + assert.deepEqual(printed, ["Stored player data for alice as generation 3"]); + }); + it("import should reject non-object JSON", async function() { + const file = path.join(tempDir, "bad.json"); + await fs.writeFile(file, "[1]"); + await assert.rejects( + run("import", { player: "alice", file }), + new lib.CommandError("Player data must be a JSON object"), + ); + }); + it("delete and release should send requests", async function() { + await run("delete", { player: "alice" }); + await run("release", { player: "alice" }); + assert.deepEqual(sent, [new msg.DeletePlayerDataRequest("alice"), new msg.ForceReleaseRequest("alice")]); }); }); }); From f67097257b4328c04e553b4b651d6d637359e88e Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:32:20 +0000 Subject: [PATCH 03/17] Ignore player data responses for an earlier selection A request for one player could resolve after the drawer switched to another and show its inventory, and export it, under the wrong name. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/web/index.tsx | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/plugins/player_sync/web/index.tsx b/plugins/player_sync/web/index.tsx index 1d14c190..49535e6a 100644 --- a/plugins/player_sync/web/index.tsx +++ b/plugins/player_sync/web/index.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useContext, useEffect, useState } from "react"; +import React, { useCallback, useContext, useEffect, useRef, useState } from "react"; import { Alert, Button, Descriptions, Drawer, Popconfirm, Space, Table, Typography, Upload } from "antd"; import * as lib from "@clusterio/lib"; @@ -89,17 +89,27 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang const instanceName = useInstanceName(); const [response, setResponse] = useState(); const { playerName } = props; + // Only the latest request may set the response, older ones may be for another player + const requestId = useRef(0); const reload = useCallback(() => { + requestId.current += 1; + let id = requestId.current; if (playerName === undefined) { - setResponse(undefined); return; } - control.send(new GetPlayerDataRequest(playerName)).then(setResponse).catch( + control.send(new GetPlayerDataRequest(playerName)).then(result => { + if (id === requestId.current) { + setResponse(result); + } + }).catch( notifyErrorHandler("Error loading player data") ); }, [control, playerName]); - useEffect(reload, [reload]); + useEffect(() => { + setResponse(undefined); + reload(); + }, [reload]); function changed() { reload(); From 004fb13f1cb3105d640a929ef0845fa45da7e31d Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:32:20 +0000 Subject: [PATCH 04/17] Handle empty inventories serialized as objects Factorio's table_to_json writes an empty inventory as {"i":{}}, which made the summary throw for show and the web drawer. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/player_data.ts | 9 ++++++--- plugins/player_sync/test/plugin.js | 5 +++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/player_sync/player_data.ts b/plugins/player_sync/player_data.ts index b4857ee5..1905b9c7 100644 --- a/plugins/player_sync/player_data.ts +++ b/plugins/player_sync/player_data.ts @@ -9,6 +9,9 @@ type SerializedItem = { e?: string, }; +// Factorio serializes an empty table as {} +type SerializedInventory = { i?: SerializedItem[] | Record }; + export type ItemSummary = { name: string, quality?: string, @@ -22,10 +25,10 @@ export type InventorySummary = { items: ItemSummary[], }; -export function summarizeInventory(inventory: { i?: SerializedItem[] }): ItemSummary[] { +export function summarizeInventory(inventory: SerializedInventory): ItemSummary[] { let items: ItemSummary[] = []; let stacks = new Map(); - for (let entry of inventory.i ?? []) { + 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++) { @@ -51,6 +54,6 @@ export function summarizeInventory(inventory: { i?: SerializedItem[] }): ItemSum export function summarizePlayerInventories(playerData: IpcPlayerData): InventorySummary[] { let inventories = playerData.character?.inventories ?? playerData.inventories ?? {}; return Object.entries(inventories).map( - ([name, inventory]) => ({ name, items: summarizeInventory(inventory as { i?: SerializedItem[] }) }) + ([name, inventory]) => ({ name, items: summarizeInventory(inventory as SerializedInventory) }) ); } diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 56968025..be8442be 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -65,6 +65,7 @@ describe("player_sync plugin", function() { }); it("should handle empty inventories", function() { assert.deepEqual(summarizeInventory({}), []); + assert.deepEqual(summarizeInventory(JSON.parse('{"i":{}}')), []); assert.deepEqual( summarizeInventory({ i: [{ n: "wood" }] }), [{ name: "wood", quality: undefined, count: 0 }], @@ -88,6 +89,10 @@ describe("player_sync plugin", function() { { name: "main", items: [{ name: "stone", quality: undefined, count: 1 }] }, ]); }); + it("should handle empty serialized inventories", function() { + const data = JSON.parse('{"generation":1,"name":"player","character":{"inventories":{"armor":{"i":{}}}}}'); + assert.deepEqual(summarizePlayerInventories(data), [{ name: "armor", items: [] }]); + }); it("should handle missing inventories", function() { assert.deepEqual(summarizePlayerInventories(playerData("player", 1)), []); assert.deepEqual(summarizePlayerInventories(playerData("player", 1, { character: {} })), []); From 1d14148606c41b9560b01600e015b2e711bab9bb Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:32:20 +0000 Subject: [PATCH 05/17] Show the instance ID when its details can't be fetched InstanceDetailsGetRequest needs core.instance.get, which the player-sync list and show commands are not documented to require. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/ctl.ts | 9 +++++++-- plugins/player_sync/test/plugin.js | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/plugins/player_sync/ctl.ts b/plugins/player_sync/ctl.ts index d92b6550..c0028faf 100644 --- a/plugins/player_sync/ctl.ts +++ b/plugins/player_sync/ctl.ts @@ -13,8 +13,13 @@ async function instanceName(ctl: Ctl, instanceId: number | undefined) { if (instanceId === undefined) { return "-"; } - let instance = await ctl.send(new lib.InstanceDetailsGetRequest(instanceId)); - return `${instance.name} (${instanceId})`; + try { + let instance = await ctl.send(new lib.InstanceDetailsGetRequest(instanceId)); + return `${instance.name} (${instanceId})`; + } catch (err) { + // Viewing player data does not require core.instance.get + return String(instanceId); + } } const playerSyncCommands = new lib.CommandTree({ diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index be8442be..3c0e5f7a 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -181,6 +181,16 @@ describe("player_sync plugin", function() { ` 1 exported item ${lib.formatBytes(4)}`, ]); }); + it("should show the instance ID when details can't be fetched", async function() { + responses.set(lib.InstanceDetailsGetRequest, () => { + throw new lib.PermissionError("Permission denied"); + }); + await run("list"); + assert.equal(printed[2], "bob | - | - | 1"); + printed = []; + await run("show", { player: "alice" }); + assert.equal(printed[0], "acquired by: 1"); + }); it("show should handle players without data", async function() { await run("show", { player: "carol" }); assert.deepEqual(printed, ["acquired by: -", "No player data stored for carol"]); From f12b016cec4d9fc8c2b61c2e784e11d8f040b0dc Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 15:32:20 +0000 Subject: [PATCH 06/17] Invalidate instance copies when player data is imported or deleted Generations alone could not order a repair against copies held by instances. Deleting dropped the watermark, so a stale instance could skip downloading the new baseline. An import could land on the same generation as an upload that never reached the controller, and the instance would then keep its own copy. Imports and deletes now bump a per-player epoch, which the controller returns on acquire and checks on upload. Instances throw away a copy from an earlier epoch, and deleted players keep their generation and epoch in player_sync_deleted.json. A restarted download also takes the new generation instead of the outdated one. Generations in player data must now be non-negative integers, so an invalid import is refused before it reaches the datastore. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/README.md | 7 +- plugins/player_sync/controller.ts | 75 ++++++++++++------- plugins/player_sync/instance.ts | 3 + plugins/player_sync/messages.ts | 9 ++- .../player_sync/module/download_inventory.lua | 3 +- plugins/player_sync/module/player_sync.lua | 28 ++++++- plugins/player_sync/module/serialize.lua | 1 + plugins/player_sync/test/controller.js | 55 +++++++++++++- plugins/player_sync/test/plugin.js | 17 +++++ 9 files changed, 159 insertions(+), 39 deletions(-) diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index 0c4a1a9c..e3e28153 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -45,8 +45,11 @@ The same operations are available from clusterioctl: npx clusterioctl player-sync delete npx clusterioctl player-sync release -Importing bumps the generation so instances holding an older copy download the new one the next time the player joins. -Deleting makes the next join turn whatever the player has on that instance into the new synced inventory. +Importing and deleting both start a new epoch for the player. +An instance holding a copy from an earlier epoch throws it away the next time the player joins, even if it is newer than what the controller has, and an upload from an earlier epoch is discarded. +After an import the instance downloads the new data. +After a delete the next join turns whatever the player has on that instance into the new synced inventory. +Deleted players stay listed in `player_sync_deleted.json` in the database directory so their epoch survives a controller restart. Both are refused while an instance holds the lock on the player, because the instance uploads its own copy when the player leaves and that would undo the change. Have the player leave first, or release the lock if the instance is stuck holding it. diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 1355a18f..902dc179 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -22,39 +22,47 @@ async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) { } } } - logger.verbose(`Loading ${itemsPath}`); + return await loadMap(itemsPath, logger); +} + +async function loadMap(file: string, logger: lib.Logger): Promise> { + logger.verbose(`Loading ${file}`); try { - let content = await fs.readFile(itemsPath, { encoding: "utf8" }); + let content = await fs.readFile(file, { encoding: "utf8" }); return new Map(JSON.parse(content)); } catch (err: any) { if (err.code === "ENOENT") { - logger.verbose("Creating new player data database"); + logger.verbose(`Creating new ${path.basename(file)}`); return new Map(); } throw err; } } -async function saveDatabase( - controllerConfig: lib.ControllerConfig, - playerDatastore: Map | undefined, - logger: lib.Logger, -) { - if (playerDatastore) { - let file = path.resolve(controllerConfig.get("controller.database_directory"), "player_sync.json"); - logger.verbose(`writing ${file}`); - let content = JSON.stringify(Array.from(playerDatastore)); - await lib.safeOutputFile(file, content); - } +async function saveMap(file: string, map: Map, logger: lib.Logger) { + logger.verbose(`writing ${file}`); + await lib.safeOutputFile(file, JSON.stringify(Array.from(map))); } +// Left behind by deleted player data so instances can tell their copy is stale +type Tombstone = { generation: number, epoch: number }; + export default async function(context: ControllerPluginContext) { const { controller, logger, plugin } = context; const acquiredPlayers = new Map(); + const databaseDirectory = controller.config.get("controller.database_directory"); const playerDatastore: Map = await loadDatabase(controller.config, logger); + const tombstonesPath = path.resolve(databaseDirectory, "player_sync_deleted.json"); + const tombstones = await loadMap(tombstonesPath, logger); let playerDatastoreDirty = false; + // Generation and epoch instances should compare their copy against + function storedVersion(playerName: string) { + let stored = playerDatastore.get(playerName) ?? tombstones.get(playerName); + return { generation: stored?.generation ?? 0, epoch: stored?.epoch ?? 0 }; + } + // Returns the acquisition record for the player if it's still in effect function getAcquisition(playerName: string) { let acquisitionRecord = acquiredPlayers.get(playerName); @@ -122,7 +130,8 @@ export default async function(context: ControllerPluginContext) { controller.hooks.save.attach(plugin.name, async () => { if (playerDatastoreDirty) { playerDatastoreDirty = false; - await saveDatabase(controller.config, playerDatastore, logger); + await saveMap(path.resolve(databaseDirectory, "player_sync.json"), playerDatastore, logger); + await saveMap(tombstonesPath, tombstones, logger); } }); @@ -137,11 +146,13 @@ export default async function(context: ControllerPluginContext) { }; } - let playerData = playerDatastore.get(playerName); + let { generation, epoch } = storedVersion(playerName); return new msg.AcquireRequest.Response( "acquired", - playerData ? playerData.generation : 0, - Boolean(playerData), + generation, + playerDatastore.has(playerName), + undefined, + epoch || undefined, ); }); @@ -176,18 +187,26 @@ export default async function(context: ControllerPluginContext) { } acquiredPlayers.delete(playerName); - let oldPlayerData = playerDatastore.get(playerName); - if (store && oldPlayerData && oldPlayerData.generation >= playerData.generation) { + let stored = storedVersion(playerName); + if (store && (playerData.epoch ?? 0) !== stored.epoch) { logger.warn( - `${instanceName} uploaded generation ${playerData.generation} while the stored` + - `generation is ${oldPlayerData.generation} for ${playerName}` + `${instanceName} uploaded ${playerName} from epoch ${playerData.epoch ?? 0} while the stored ` + + `epoch is ${stored.epoch}, discarding the outdated copy` + ); + store = false; + + } else if (store && stored.generation >= playerData.generation) { + logger.warn( + `${instanceName} uploaded generation ${playerData.generation} while the stored ` + + `generation is ${stored.generation} for ${playerName}` ); store = false; } if (store) { logger.verbose(`Received player data for ${playerName} from ${instanceName}`); - playerDatastore.set(playerName, mergePlayerData(oldPlayerData, playerData)); + playerDatastore.set(playerName, mergePlayerData(playerDatastore.get(playerName), playerData)); + tombstones.delete(playerName); playerDatastoreDirty = true; } }); @@ -253,11 +272,13 @@ export default async function(context: ControllerPluginContext) { let { playerName, playerData } = request; checkNotAcquired(playerName); - // Bump the generation so instances holding an older copy download this one - let oldPlayerData = playerDatastore.get(playerName); + // 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, (oldPlayerData?.generation ?? 0) + 1); + playerData.generation = Math.max(playerData.generation, stored.generation + 1); + playerData.epoch = stored.epoch + 1; playerDatastore.set(playerName, playerData); + tombstones.delete(playerName); playerDatastoreDirty = true; logger.info(`Replaced player data for ${playerName} with generation ${playerData.generation}`); return new msg.SetPlayerDataRequest.Response(playerData.generation); @@ -270,6 +291,8 @@ export default async function(context: ControllerPluginContext) { } checkNotAcquired(playerName); + let { generation, epoch } = storedVersion(playerName); + tombstones.set(playerName, { generation, epoch: epoch + 1 }); playerDatastore.delete(playerName); playerDatastoreDirty = true; logger.info(`Deleted player data for ${playerName}`); diff --git a/plugins/player_sync/instance.ts b/plugins/player_sync/instance.ts index b4e6b978..d7fcec40 100644 --- a/plugins/player_sync/instance.ts +++ b/plugins/player_sync/instance.ts @@ -27,6 +27,7 @@ type IpcAcquireResponse = { generation?: number, has_data?: boolean, message?: string, + epoch?: number, } /** @@ -118,6 +119,7 @@ export default async function(context: InstancePluginContext) { message: "Controller is temporarily unavailable", has_data: undefined, generation: undefined, + epoch: undefined, }; if (host.connector.connected && !disconnecting) { @@ -132,6 +134,7 @@ export default async function(context: InstancePluginContext) { generation: acquireResponse.generation, has_data: acquireResponse.hasData, message: acquireResponse.message, + epoch: acquireResponse.epoch, }; } catch (err: any) { if (!(err instanceof lib.SessionLost)) { diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 6a5433b2..07019686 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -4,6 +4,8 @@ import { StringEnum, jsonArray } from "@clusterio/lib"; // .\module\serialize.lua:serialize.serialize_player() export type IpcPlayerData = { generation: number, + /** Bumped when the stored data is imported or deleted, invalidating copies held by instances */ + epoch?: number, name: string, controller?: string, color?: number[], @@ -27,7 +29,8 @@ export type IpcPlayerData = { } const jsonPlayerData = Type.Object({ - generation: Type.Number(), + generation: Type.Integer({ minimum: 0 }), + epoch: Type.Optional(Type.Integer({ minimum: 0 })), name: Type.String(), }); @@ -37,6 +40,7 @@ export class AcquireResponse { public generation?: number, public hasData?: boolean, public message?: string, + public epoch?: number, ) { } @@ -45,10 +49,11 @@ export class AcquireResponse { "generation": Type.Optional(Type.Integer()), "hasData": Type.Optional(Type.Boolean()), "message": Type.Optional(Type.String()), + "epoch": Type.Optional(Type.Integer()), }); static fromJSON(json: Static): AcquireResponse { - return new this(json.status, json.generation, json.hasData, json.message); + return new this(json.status, json.generation, json.hasData, json.message, json.epoch); } }; export class AcquireRequest { diff --git a/plugins/player_sync/module/download_inventory.lua b/plugins/player_sync/module/download_inventory.lua index d6f6e54d..2d5a07b4 100644 --- a/plugins/player_sync/module/download_inventory.lua +++ b/plugins/player_sync/module/download_inventory.lua @@ -29,7 +29,7 @@ local function download_inventory(player_name, data, number, total, components) end if record.restart then rcon.print("Restarting outdated download") - player_sync.initiate_inventory_download(player, player_record, record.generation) + player_sync.initiate_inventory_download(player, player_record, record.generation, record.epoch) return end @@ -55,6 +55,7 @@ local function download_inventory(player_name, data, number, total, components) player_record.sync = true player_record.components = record.components player_record.generation = record.generation + player_record.epoch = record.epoch return end diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index 6285c8fe..1a01600d 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -99,6 +99,7 @@ function player_sync.serialize_player(player, player_record) local failed_deserialization = get_script_data().failed_deserialization[player.name] or {} local serialized_player = serialize.serialize_player(player, failed_deserialization, player_record.components) serialized_player.generation = player_record.generation + serialized_player.epoch = player_record.epoch return serialized_player end @@ -269,9 +270,15 @@ function player_sync.sync_player(acquire_response) end local script_data = get_script_data() + -- Epoch is bumped when an admin replaces or deletes the stored data + local epoch = acquire_response.epoch or 0 local finished_record = script_data.finished_downloads[acquire_response.player_name] if finished_record then - if acquire_response.status == "acquired" and finished_record.generation == acquire_response.generation then + if + acquire_response.status == "acquired" + and finished_record.generation == acquire_response.generation + and (finished_record.epoch or 0) == epoch + then player_sync.finish_download(player, finished_record) return end @@ -284,7 +291,11 @@ function player_sync.sync_player(acquire_response) local download_record = script_data.active_downloads[acquire_response.player_name] if download_record then -- Ignore the response if it affirms the current active download - if acquire_response.status == "acquired" and download_record.generation == acquire_response.generation then + if + acquire_response.status == "acquired" + and download_record.generation == acquire_response.generation + and (download_record.epoch or 0) == epoch + then game.print("Continuing active download") return end @@ -305,6 +316,8 @@ function player_sync.sync_player(acquire_response) -- download was started. Restart it. game.print("Restarting download") download_record.restart = true + download_record.generation = acquire_response.generation + download_record.epoch = epoch return end @@ -333,15 +346,17 @@ function player_sync.sync_player(acquire_response) return end + local stale = (player_record.epoch or 0) ~= epoch if acquire_response.has_data and ( not player_record.sync + or stale or acquire_response.generation > player_record.generation or serialize.newly_synced(player_record.components, script_data.components) ) then - player_sync.initiate_inventory_download(player, player_record, acquire_response.generation) + player_sync.initiate_inventory_download(player, player_record, acquire_response.generation, epoch) else if acquire_response.generation == 0 then @@ -355,6 +370,9 @@ function player_sync.sync_player(acquire_response) start_sync(player_record, script_data.components) player_record.dirty = true 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) end end @@ -378,6 +396,7 @@ function player_sync.finish_download(player, finished_record) player_record.sync_in_progress = false start_sync(player_record, finished_record.components) player_record.generation = finished_record.generation + player_record.epoch = finished_record.epoch end -- Download inventory from controller @@ -478,12 +497,13 @@ function player_sync.confirm_upload(player_name, generation) player_record.dirty = false end -function player_sync.initiate_inventory_download(player, player_record, generation) +function player_sync.initiate_inventory_download(player, player_record, generation, epoch) player.print("Initiating inventory download...") local record = { started = game.ticks_played, last_active = game.ticks_played, generation = generation, + epoch = epoch, data = "", } local script_data = get_script_data() diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index 7f9b3b2a..a2176ebf 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -897,6 +897,7 @@ end --- @class SerializedPlayerData --- @field generation number +--- @field epoch number? --- @field name string --- @field controller string? --- @field color Color? diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 67b9c4c7..b555031d 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -104,9 +104,15 @@ describe("player_sync controller", function() { const response = await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName)); return response.status === "acquired"; } - async function saved() { + async function saved(file = "player_sync.json") { await controller.hooks.save.invoke(); - return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, "player_sync.json"), "utf8"))); + return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, file), "utf8"))); + } + async function upload(instanceId, data) { + await handler(msg.UploadRequest)(new msg.UploadRequest(instanceId, data.name, data)); + } + async function acquireResponse(instanceId, playerName) { + return await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName)); } beforeEach(async function() { @@ -170,10 +176,25 @@ describe("player_sync controller", function() { async function release(name) { await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest(name)); } - it("should store data with a bumped generation", async function() { + it("should store data with a bumped generation and epoch", async function() { await release("stored"); assert.deepEqual(await set("stored", playerData("other", 1)), new msg.SetPlayerDataResponse(4)); - assert.deepEqual((await saved()).get("stored"), playerData("stored", 4)); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 4, { epoch: 1 })); + }); + it("should invalidate unconfirmed uploads from instances", async function() { + // Instance 1 left with generation 4 but the upload never reached the controller + await release("stored"); + await set("stored", playerData("stored", 0)); + await upload(1, playerData("stored", 4)); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 4, { epoch: 1 })); + assert.deepEqual( + await acquireResponse(1, "stored"), new msg.AcquireResponse("acquired", 4, true, undefined, 1), + ); + }); + it("should ignore the epoch in imported data", async function() { + await release("stored"); + await set("stored", playerData("stored", 0, { epoch: 7 })); + assert.equal((await saved()).get("stored").epoch, 1); }); it("should keep a higher provided generation", async function() { await release("stored"); @@ -198,6 +219,32 @@ describe("player_sync controller", function() { await del("stored"); assert(!(await saved()).has("stored")); }); + it("should keep the generation and bump the epoch", async function() { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); + await del("stored"); + assert.deepEqual( + await acquireResponse(2, "stored"), new msg.AcquireResponse("acquired", 3, false, undefined, 1), + ); + assert.deepEqual((await saved("player_sync_deleted.json")).get("stored"), { generation: 3, epoch: 1 }); + + controller = await load(); + assert.deepEqual( + await acquireResponse(2, "stored"), new msg.AcquireResponse("acquired", 3, false, undefined, 1), + ); + }); + it("should accept the next upload from the new epoch only", async function() { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); + await del("stored"); + await upload(1, playerData("stored", 11)); + assert(!(await saved()).has("stored")); + await upload(2, playerData("stored", 4, { epoch: 1 })); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 4, { epoch: 1 })); + assert(!(await saved("player_sync_deleted.json")).has("stored")); + + // The other instance downloads the baseline instead of uploading its stale copy + await upload(1, playerData("stored", 11)); + assert.equal((await saved()).get("stored").generation, 4); + }); it("should reject unknown players", async function() { await assert.rejects(del("unknown"), new lib.RequestError("No player data stored for unknown")); }); diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 3c0e5f7a..3c30f6be 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -34,6 +34,23 @@ describe("player_sync plugin", function() { [undefined, 1], // instanceId )); }); + it("AcquireResponse should be round trip json serialisable", function() { + testRoundTripJsonSerialisable(msg.AcquireResponse, testMatrix( + ["acquired", "busy"], // status + [undefined, 3], // generation + [undefined, true], // hasData + [undefined, "Instance"], // message + [undefined, 2], // epoch + )); + }); + it("should reject non-integer generations", function() { + const validate = lib.compile(msg.SetPlayerDataRequest.jsonSchema); + assert(validate({ playerName: "player", playerData: playerData("player", 5) })); + for (const generation of [5.5, -1, "5"]) { + const request = { playerName: "player", playerData: playerData("player", generation) }; + assert(!validate(request), `accepted ${generation}`); + } + }); it("SetPlayerDataResponse should be round trip json serialisable", function() { testRoundTripJsonSerialisable(msg.SetPlayerDataResponse, testMatrix([1, 5])); }); From 1ad5be0ff666821c9d57b4f57b84ba6ced6a0fdf Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:12:12 +0000 Subject: [PATCH 07/17] Keep each player's drawer state separate An import, delete or release for one player that finished after another player was selected reloaded the drawer with the first player's data under the second player's name. The drawer body is now keyed by player, so a late completion can only update the view it was started from. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/web/index.tsx | 40 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/plugins/player_sync/web/index.tsx b/plugins/player_sync/web/index.tsx index 49535e6a..38f101fc 100644 --- a/plugins/player_sync/web/index.tsx +++ b/plugins/player_sync/web/index.tsx @@ -84,20 +84,30 @@ function InventoryTable(props: { name: string, items: ItemSummary[] }) { } function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChange: () => void }) { + return + {/* Keyed so a request that finishes after switching players can only update the old player's view */} + {props.playerName !== undefined + && } + ; +} + +function PlayerDetails(props: { playerName: string, onChange: () => void }) { const control = useContext(ControlContext); const account = useAccount(); const instanceName = useInstanceName(); const [response, setResponse] = useState(); const { playerName } = props; - // Only the latest request may set the response, older ones may be for another player + // Only the latest request may set the response, an import or delete can finish after a reload const requestId = useRef(0); const reload = useCallback(() => { requestId.current += 1; let id = requestId.current; - if (playerName === undefined) { - return; - } control.send(new GetPlayerDataRequest(playerName)).then(result => { if (id === requestId.current) { setResponse(result); @@ -106,10 +116,7 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang notifyErrorHandler("Error loading player data") ); }, [control, playerName]); - useEffect(() => { - setResponse(undefined); - reload(); - }, [reload]); + useEffect(reload, [reload]); function changed() { reload(); @@ -124,9 +131,9 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang notify("Invalid JSON", "error", err.message); return; } - playerData.name = playerName!; + playerData.name = playerName; playerData.generation ??= 0; - let result = await control.send(new SetPlayerDataRequest(playerName!, playerData)); + let result = await control.send(new SetPlayerDataRequest(playerName, playerData)); notify(`Stored player data for ${playerName} as generation ${result.generation}`); changed(); } @@ -144,12 +151,7 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang const canModify = account.hasPermission("player_sync.inventory.modify"); const playerData = response?.playerData; const acquiredBy = response?.instanceId !== undefined ? instanceName(response.instanceId) : undefined; - return + return <> {response && {acquiredBy !== undefined && void, onChang } action={canModify && control.send(new ForceReleaseRequest(playerName!)).then(changed).catch( + onConfirm={() => control.send(new ForceReleaseRequest(playerName)).then(changed).catch( notifyErrorHandler("Error releasing lock") )} > @@ -195,7 +197,7 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang } okText="Delete" okButtonProps={{ danger: true }} - onConfirm={() => control.send(new DeletePlayerDataRequest(playerName!)).then(changed).catch( + onConfirm={() => control.send(new DeletePlayerDataRequest(playerName)).then(changed).catch( notifyErrorHandler("Error deleting player data") )} > @@ -207,7 +209,7 @@ function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChang inventory => )} } - ; + ; } function InventoryPage() { From c260baaf033dcef3277ed0921a479841b9d42ba1 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:12:12 +0000 Subject: [PATCH 08/17] Validate imported player data before storing it An import with an unknown controller or malformed inventories replaced the stored snapshot and bumped its epoch, then broke the player's next join. SetPlayerDataRequest now checks the fields the scenario restores, leaving item stacks to the Lua deserializer and allowing the partial snapshots instances with disabled components upload. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/messages.ts | 45 +++++++++++++++++++++++++- plugins/player_sync/test/plugin.js | 51 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 07019686..3557ac62 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -34,6 +34,49 @@ const jsonPlayerData = Type.Object({ name: Type.String(), }); +// The parts of an import that module/serialize.lua relies on, item stacks are checked by the Lua deserializer +const jsonInventory = Type.Object({ + b: Type.Optional(Type.Integer({ minimum: 0 })), + // An empty Lua table is encoded as an object + i: Type.Union([ + Type.Array(Type.Object({ + s: Type.Optional(Type.Integer({ minimum: 1 })), + r: Type.Optional(Type.Integer({ minimum: 0 })), + })), + Type.Object({}, { maxProperties: 0 }), + ]), +}); +const jsonInventories = Type.Record(Type.String(), jsonInventory); +const jsonColor = Type.Union([ + Type.Object({ r: Type.Number(), g: Type.Number(), b: Type.Number(), a: Type.Optional(Type.Number()) }), + Type.Array(Type.Number(), { minItems: 3, maxItems: 4 }), +]); +const jsonTable = Type.Union([Type.Array(Type.Unknown()), Type.Object({})]); +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"])), + color: Type.Optional(jsonColor), + chat_color: Type.Optional(jsonColor), + tag: Type.Optional(Type.String()), + force: Type.Optional(Type.String()), + cheat_mode: Type.Optional(Type.Boolean()), + flashlight: Type.Optional(Type.Boolean()), + shortcuts: Type.Optional(Type.Record(Type.String(), Type.Boolean())), + game_view_settings: Type.Optional(Type.Record(Type.String(), Type.Boolean())), + ticks_to_respawn: Type.Optional(Type.Integer({ minimum: 0 })), + character: Type.Optional(Type.Object({ inventories: Type.Optional(jsonInventories) })), + inventories: Type.Optional(jsonInventories), + quick_bar: Type.Optional(jsonTable), + hotbar: Type.Optional(jsonTable), + personal_logistic_slots: Type.Optional(jsonTable), + crafting_queue: Type.Optional(jsonTable), + recipe_notifications: Type.Optional(Type.String()), + components: Type.Optional(jsonTable), +}); + export class AcquireResponse { constructor( public status: string, @@ -302,7 +345,7 @@ export class SetPlayerDataRequest { static jsonSchema = Type.Object({ "playerName": Type.String(), - "playerData": jsonPlayerData, + "playerData": jsonImportedPlayerData, }); static fromJSON(json: Static) { diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 3c30f6be..aabfbe57 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -51,6 +51,57 @@ describe("player_sync plugin", function() { assert(!validate(request), `accepted ${generation}`); } }); + describe("SetPlayerDataRequest schema", function() { + const validate = lib.compile(msg.SetPlayerDataRequest.jsonSchema); + // Shaped like module/serialize.lua output + const exported = playerData("player", 7, { + epoch: 2, + color: { r: 1, g: 0.5, b: 0, a: 0.5 }, + chat_color: { r: 1, g: 0.5, b: 0, a: 1 }, + tag: "", + cheat_mode: false, + flashlight: true, + shortcuts: { "toggle-personal-roboport": true }, + game_view_settings: { show_minimap: true }, + character: { + character_running_speed_modifier: 0, + inventories: { + main: { b: 80, i: [{ n: "iron-plate", c: 100, r: 2 }, { s: 10, f: { name: "coal" } }] }, + armor: { i: {} }, + }, + }, + personal_logistic_slots: [], + quick_bar: { 1: { n: "iron-plate" } }, + crafting_queue: { crafting_queue: [], ingredients: [] }, + recipe_notifications: "", + components: { controller: true, inventories: true }, + }); + function accepts(data) { + return validate({ playerName: "player", playerData: data }); + } + + it("should accept exported and partial player data", function() { + assert(accepts(exported), JSON.stringify(validate.errors)); + assert(accepts({ name: "player", generation: 0 })); + assert(accepts(playerData("player", 0, { controller: "ghost", ticks_to_respawn: 300 }))); + assert(accepts(playerData("player", 0, { color: [1, 0, 0] }))); + }); + it("should reject data the scenario can't restore", function() { + for (const [field, value] of [ + ["controller", "not-a-controller"], + ["controller", "remote"], + ["color", "red"], + ["force", 1], + ["ticks_to_respawn", -1], + ["inventories", { main: { i: "iron-plate" } }], + ["inventories", { main: [] }], + ["character", { inventories: { main: { i: [{ s: 0 }] } } }], + ["shortcuts", { "toggle-personal-roboport": "yes" }], + ]) { + assert(!accepts({ ...exported, [field]: value }), `accepted ${field} ${JSON.stringify(value)}`); + } + }); + }); it("SetPlayerDataResponse should be round trip json serialisable", function() { testRoundTripJsonSerialisable(msg.SetPlayerDataResponse, testMatrix([1, 5])); }); From 2529683c2bef43ac43c3bf4a6991adbf89557adc Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:12:19 +0000 Subject: [PATCH 09/17] Save deleted players in the same file as player data The tombstones lived in player_sync_deleted.json, written after player_sync.json. A stop between the two writes lost the tombstone of a fresh delete, and the next upload from an old copy brought the deleted data back. Deleted players are now entries with deleted: true in player_sync.json, so both are written in one safeOutputFile call. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/README.md | 2 +- plugins/player_sync/controller.ts | 57 +++++++++++++++----------- plugins/player_sync/test/controller.js | 13 +++--- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index e3e28153..718211a1 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -49,7 +49,7 @@ Importing and deleting both start a new epoch for the player. An instance holding a copy from an earlier epoch throws it away the next time the player joins, even if it is newer than what the controller has, and an upload from an earlier epoch is discarded. After an import the instance downloads the new data. After a delete the next join turns whatever the player has on that instance into the new synced inventory. -Deleted players stay listed in `player_sync_deleted.json` in the database directory so their epoch survives a controller restart. +Deleted players stay in `player_sync.json` as an entry with `deleted: true` so their epoch survives a controller restart. Both are refused while an instance holds the lock on the player, because the instance uploads its own copy when the player leaves and that would undo the change. Have the player leave first, or release the lock if the instance is stuck holding it. diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 902dc179..b98570d4 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -7,6 +7,9 @@ import path from "path"; import * as lib from "@clusterio/lib"; import * as msg from "./messages.js"; +// Left behind by deleted player data so instances can tell their copy is stale +type Tombstone = { generation: number, epoch: number, deleted: true }; + async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) { let databaseDirectory = config.get("controller.database_directory"); let itemsPath = path.resolve(databaseDirectory, "player_sync.json"); @@ -22,39 +25,48 @@ async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) { } } } - return await loadMap(itemsPath, logger); -} - -async function loadMap(file: string, logger: lib.Logger): Promise> { - logger.verbose(`Loading ${file}`); + let entries: [string, IpcPlayerData | Tombstone][] = []; + logger.verbose(`Loading ${itemsPath}`); try { - let content = await fs.readFile(file, { encoding: "utf8" }); - return new Map(JSON.parse(content)); + let content = await fs.readFile(itemsPath, { encoding: "utf8" }); + entries = JSON.parse(content); } catch (err: any) { - if (err.code === "ENOENT") { - logger.verbose(`Creating new ${path.basename(file)}`); - return new Map(); + if (err.code !== "ENOENT") { + throw err; } - throw err; + logger.verbose("Creating new player data database"); } + + let playerDatastore = new Map(); + let tombstones = new Map(); + for (let [name, entry] of entries) { + if ("deleted" in entry) { + tombstones.set(name, entry); + } else { + playerDatastore.set(name, entry); + } + } + return { playerDatastore, tombstones }; } -async function saveMap(file: string, map: Map, logger: lib.Logger) { +// Tombstones share the file with the player data so a delete is written in one go +async function saveDatabase( + controllerConfig: lib.ControllerConfig, + playerDatastore: Map, + tombstones: Map, + logger: lib.Logger, +) { + let file = path.resolve(controllerConfig.get("controller.database_directory"), "player_sync.json"); logger.verbose(`writing ${file}`); - await lib.safeOutputFile(file, JSON.stringify(Array.from(map))); + let content = JSON.stringify([...playerDatastore, ...tombstones]); + await lib.safeOutputFile(file, content); } -// Left behind by deleted player data so instances can tell their copy is stale -type Tombstone = { generation: number, epoch: number }; - export default async function(context: ControllerPluginContext) { const { controller, logger, plugin } = context; const acquiredPlayers = new Map(); - const databaseDirectory = controller.config.get("controller.database_directory"); - const playerDatastore: Map = await loadDatabase(controller.config, logger); - const tombstonesPath = path.resolve(databaseDirectory, "player_sync_deleted.json"); - const tombstones = await loadMap(tombstonesPath, logger); + const { playerDatastore, tombstones } = await loadDatabase(controller.config, logger); let playerDatastoreDirty = false; // Generation and epoch instances should compare their copy against @@ -130,8 +142,7 @@ export default async function(context: ControllerPluginContext) { controller.hooks.save.attach(plugin.name, async () => { if (playerDatastoreDirty) { playerDatastoreDirty = false; - await saveMap(path.resolve(databaseDirectory, "player_sync.json"), playerDatastore, logger); - await saveMap(tombstonesPath, tombstones, logger); + await saveDatabase(controller.config, playerDatastore, tombstones, logger); } }); @@ -292,7 +303,7 @@ export default async function(context: ControllerPluginContext) { checkNotAcquired(playerName); let { generation, epoch } = storedVersion(playerName); - tombstones.set(playerName, { generation, epoch: epoch + 1 }); + tombstones.set(playerName, { generation, epoch: epoch + 1, deleted: true }); playerDatastore.delete(playerName); playerDatastoreDirty = true; logger.info(`Deleted player data for ${playerName}`); diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index b555031d..114ed49f 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -104,9 +104,9 @@ describe("player_sync controller", function() { const response = await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName)); return response.status === "acquired"; } - async function saved(file = "player_sync.json") { + async function saved() { await controller.hooks.save.invoke(); - return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, file), "utf8"))); + return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, "player_sync.json"), "utf8"))); } async function upload(instanceId, data) { await handler(msg.UploadRequest)(new msg.UploadRequest(instanceId, data.name, data)); @@ -217,7 +217,8 @@ describe("player_sync controller", function() { it("should delete stored data", async function() { await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); await del("stored"); - assert(!(await saved()).has("stored")); + const response = await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest("stored")); + assert.equal(response.playerData, null); }); it("should keep the generation and bump the epoch", async function() { await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); @@ -225,7 +226,8 @@ describe("player_sync controller", function() { assert.deepEqual( await acquireResponse(2, "stored"), new msg.AcquireResponse("acquired", 3, false, undefined, 1), ); - assert.deepEqual((await saved("player_sync_deleted.json")).get("stored"), { generation: 3, epoch: 1 }); + // In the same file as the player data so the delete and its tombstone are written together + assert.deepEqual((await saved()).get("stored"), { generation: 3, epoch: 1, deleted: true }); controller = await load(); assert.deepEqual( @@ -236,10 +238,9 @@ describe("player_sync controller", function() { await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); await del("stored"); await upload(1, playerData("stored", 11)); - assert(!(await saved()).has("stored")); + assert((await saved()).get("stored").deleted); await upload(2, playerData("stored", 4, { epoch: 1 })); assert.deepEqual((await saved()).get("stored"), playerData("stored", 4, { epoch: 1 })); - assert(!(await saved("player_sync_deleted.json")).has("stored")); // The other instance downloads the baseline instead of uploading its stale copy await upload(1, playerData("stored", 11)); From 5634651de1d4f361ceaffa1be24f0bb0e3f91e54 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:12:46 +0000 Subject: [PATCH 10/17] Keep accepting uploads from instances without epochs A host running the plugin from before epochs never sends one, so after an import or delete every upload it made was discarded while the instance believed it succeeded. The Lua now always sends its epoch, and an upload without one is compared by generation against the stored data the way it was before, taking over the stored epoch. Those hosts can still overwrite an import with a copy they failed to upload, which the README now says. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/README.md | 2 ++ plugins/player_sync/controller.ts | 9 +++++- plugins/player_sync/messages.ts | 5 +++- plugins/player_sync/module/player_sync.lua | 3 +- plugins/player_sync/test/controller.js | 34 ++++++++++++++++++++-- 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index 718211a1..850f1724 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -50,6 +50,8 @@ An instance holding a copy from an earlier epoch throws it away the next time th After an import the instance downloads the new data. After a delete the next join turns whatever the player has on that instance into the new synced inventory. Deleted players stay in `player_sync.json` as an entry with `deleted: true` so their epoch survives a controller restart. +Hosts running a version of this plugin from before epochs keep comparing generations only, so they can still overwrite an import with a copy they failed to upload. +Update the plugin on every host to get the full guarantee. Both are refused while an instance holds the lock on the player, because the instance uploads its own copy when the player leaves and that would undo the change. Have the player leave first, or release the lock if the instance is stuck holding it. diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index b98570d4..2938ffa9 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -199,9 +199,16 @@ export default async function(context: ControllerPluginContext) { acquiredPlayers.delete(playerName); let stored = storedVersion(playerName); + if (playerData.epoch === undefined) { + // Instances from before epochs were added never send one, they keep the rules they had before + stored.generation = playerDatastore.get(playerName)?.generation ?? 0; + if (stored.epoch) { + playerData.epoch = stored.epoch; + } + } if (store && (playerData.epoch ?? 0) !== stored.epoch) { logger.warn( - `${instanceName} uploaded ${playerName} from epoch ${playerData.epoch ?? 0} while the stored ` + + `${instanceName} uploaded ${playerName} from epoch ${playerData.epoch} while the stored ` + `epoch is ${stored.epoch}, discarding the outdated copy` ); store = false; diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 3557ac62..478bdafd 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -4,7 +4,10 @@ import { StringEnum, jsonArray } from "@clusterio/lib"; // .\module\serialize.lua:serialize.serialize_player() export type IpcPlayerData = { generation: number, - /** Bumped when the stored data is imported or deleted, invalidating copies held by instances */ + /** + * Bumped when the stored data is imported or deleted, invalidating copies held by instances. + * Instances from before epochs were added leave it out of uploads. + */ epoch?: number, name: string, controller?: string, diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index 1a01600d..8519127d 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -99,7 +99,8 @@ function player_sync.serialize_player(player, player_record) local failed_deserialization = get_script_data().failed_deserialization[player.name] or {} local serialized_player = serialize.serialize_player(player, failed_deserialization, player_record.components) serialized_player.generation = player_record.generation - serialized_player.epoch = player_record.epoch + -- Always sent so the controller can tell this instance supports epochs + serialized_player.epoch = player_record.epoch or 0 return serialized_player end diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 114ed49f..6328dbac 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -93,7 +93,7 @@ describe("player_sync controller", function() { describe("player data tools", function() { function playerData(name, generation, extra = {}) { - return { generation, name, controller: "character", force: "player", ...extra }; + return { generation, epoch: 0, name, controller: "character", force: "player", ...extra }; } let controller; @@ -251,7 +251,37 @@ describe("player_sync controller", function() { }); it("should reject when the player is acquired", async function() { await assert.rejects(del("stored"), acquiredError); - assert((await saved()).has("stored")); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 3)); + }); + }); + + describe("uploads without an epoch", function() { + // What instances running a version from before epochs send + function legacyData(generation) { + const data = playerData("stored", generation); + delete data.epoch; + return data; + } + beforeEach(async function() { + await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored")); + }); + it("should store them by generation", async function() { + await upload(1, legacyData(3)); + await upload(1, legacyData(4)); + assert.deepEqual((await saved()).get("stored"), legacyData(4)); + }); + it("should keep accepting them after an import", async function() { + const request = new msg.SetPlayerDataRequest("stored", playerData("stored", 0)); + await handler(msg.SetPlayerDataRequest)(request); + await upload(1, legacyData(4)); + assert.equal((await saved()).get("stored").epoch, 1); + await upload(1, legacyData(5)); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 5, { epoch: 1 })); + }); + it("should accept any generation after a delete", async function() { + await handler(msg.DeletePlayerDataRequest)(new msg.DeletePlayerDataRequest("stored")); + await upload(1, legacyData(1)); + assert.deepEqual((await saved()).get("stored"), playerData("stored", 1, { epoch: 1 })); }); }); From c06555c5da8ab7ce54a498c67c30f9138d37f2df Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:18:51 +0000 Subject: [PATCH 11/17] Validate supplied components in imported player data An import with `crafting_queue: {}` passed the schema and broke the next restore in deserialize_crafting_queue. The import schema now checks the members module/serialize.lua reads from the crafting queue, quick bar, personal logistic slots and character stats, while still allowing a component to be left out. Character stats missing from the data are left as they are instead of being set to nil. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/messages.ts | 95 ++++++++++++++++++++---- plugins/player_sync/module/serialize.lua | 5 +- plugins/player_sync/test/controller.js | 17 +++++ plugins/player_sync/test/plugin.js | 38 +++++++++- 4 files changed, 137 insertions(+), 18 deletions(-) diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 478bdafd..4b246f01 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -1,4 +1,4 @@ -import { Type, Static } from "@sinclair/typebox"; +import { Type, Static, TSchema } from "@sinclair/typebox"; import { StringEnum, jsonArray } from "@clusterio/lib"; // .\module\serialize.lua:serialize.serialize_player() @@ -38,22 +38,89 @@ const jsonPlayerData = Type.Object({ }); // The parts of an import that module/serialize.lua relies on, item stacks are checked by the Lua deserializer +// An empty Lua table is encoded as an object +function jsonList(item: T) { + return Type.Union([Type.Array(item), Type.Object({}, { maxProperties: 0 })]); +} const jsonInventory = Type.Object({ b: Type.Optional(Type.Integer({ minimum: 0 })), - // An empty Lua table is encoded as an object - i: Type.Union([ - Type.Array(Type.Object({ - s: Type.Optional(Type.Integer({ minimum: 1 })), - r: Type.Optional(Type.Integer({ minimum: 0 })), - })), - Type.Object({}, { maxProperties: 0 }), - ]), + i: jsonList(Type.Object({ + s: Type.Optional(Type.Integer({ minimum: 1 })), + r: Type.Optional(Type.Integer({ minimum: 0 })), + })), }); const jsonInventories = Type.Record(Type.String(), jsonInventory); const jsonColor = Type.Union([ Type.Object({ r: Type.Number(), g: Type.Number(), b: Type.Number(), a: Type.Optional(Type.Number()) }), Type.Array(Type.Number(), { minItems: 3, maxItems: 4 }), ]); +// module/define_player_stat_keys.lua, missing stats are left as they are +const jsonCharacter = Type.Object({ + character_crafting_speed_modifier: Type.Optional(Type.Number()), + character_mining_speed_modifier: Type.Optional(Type.Number()), + character_additional_mining_categories: Type.Optional(jsonList(Type.String())), + character_running_speed_modifier: Type.Optional(Type.Number()), + character_build_distance_bonus: Type.Optional(Type.Number()), + character_item_drop_distance_bonus: Type.Optional(Type.Number()), + character_reach_distance_bonus: Type.Optional(Type.Number()), + character_resource_reach_distance_bonus: Type.Optional(Type.Number()), + character_item_pickup_distance_bonus: Type.Optional(Type.Number()), + character_loot_pickup_distance_bonus: Type.Optional(Type.Number()), + character_inventory_slots_bonus: Type.Optional(Type.Number()), + character_trash_slot_count_bonus: Type.Optional(Type.Number()), + character_maximum_following_robot_count_bonus: Type.Optional(Type.Number()), + character_health_bonus: Type.Optional(Type.Number()), + character_personal_logistic_requests_enabled: Type.Optional(Type.Boolean()), + allow_dispatching_robots: Type.Optional(Type.Boolean()), + inhibit_movement_bonus: Type.Optional(Type.Boolean()), + inventories: Type.Optional(jsonInventories), +}); +const jsonLogisticSlot = Type.Object({ + name: Type.String(), + min: Type.Optional(Type.Integer({ minimum: 0 })), + max: Type.Optional(Type.Integer({ minimum: 0 })), +}); +const jsonPersonalLogisticSlots = Type.Union([ + Type.Object({ + enabled: Type.Boolean(), + trash_not_requested: Type.Boolean(), + sections: jsonList(Type.Object({ + group: Type.String(), + active: Type.Boolean(), + multiplier: Type.Number({ minimum: 0 }), + filters: jsonList(Type.Object({})), + })), + }), + // From before 2.0, slot index to request + Type.Record(Type.String({ pattern: "^[1-9][0-9]*$" }), jsonLogisticSlot, { additionalProperties: false }), + Type.Array(jsonLogisticSlot), +]); +// QuickBarSlotEncoded in module/serialize.lua +const jsonQuickBarSlot = Type.Union([ + Type.String(), + Type.Object({ t: Type.Literal("f"), n: Type.String(), q: Type.String(), c: Type.String() }), + Type.Object({ + t: Type.Literal("i"), + n: Type.String(), + s: Type.Integer({ minimum: 1 }), + b: Type.Optional(jsonList(Type.Integer({ minimum: 1 }))), + }), + Type.Object({ + t: Type.Literal("r"), + p: jsonList(Type.Integer({ minimum: 1 })), + y: Type.String(), + l: Type.Optional(Type.String()), + }), +]); +const jsonCraftingQueue = Type.Object({ + crafting_queue: jsonList(Type.Object({ recipe: Type.String(), count: Type.Integer({ minimum: 1 }) })), + crafting_queue_progress: Type.Number({ minimum: 0, maximum: 1 }), + ingredients: jsonList(Type.Object({ + name: Type.String(), + count: Type.Integer({ minimum: 1 }), + quality: Type.Optional(Type.String()), + })), +}); const jsonTable = Type.Union([Type.Array(Type.Unknown()), Type.Object({})]); const jsonImportedPlayerData = Type.Object({ generation: Type.Integer({ minimum: 0 }), @@ -70,12 +137,12 @@ const jsonImportedPlayerData = Type.Object({ shortcuts: Type.Optional(Type.Record(Type.String(), Type.Boolean())), game_view_settings: Type.Optional(Type.Record(Type.String(), Type.Boolean())), ticks_to_respawn: Type.Optional(Type.Integer({ minimum: 0 })), - character: Type.Optional(Type.Object({ inventories: Type.Optional(jsonInventories) })), + character: Type.Optional(jsonCharacter), inventories: Type.Optional(jsonInventories), - quick_bar: Type.Optional(jsonTable), - hotbar: Type.Optional(jsonTable), - personal_logistic_slots: Type.Optional(jsonTable), - crafting_queue: Type.Optional(jsonTable), + quick_bar: Type.Optional(Type.Record(Type.String(), jsonQuickBarSlot)), + hotbar: Type.Optional(Type.Record(Type.String(), Type.String())), + personal_logistic_slots: Type.Optional(jsonPersonalLogisticSlots), + crafting_queue: Type.Optional(jsonCraftingQueue), recipe_notifications: Type.Optional(Type.String()), components: Type.Optional(jsonTable), }); diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index a2176ebf..40b351e0 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -69,7 +69,10 @@ end --- Restore the stats of a character, the inventories are restored with place_items function serialize.deserialize_character(character, serialized) for _, key in pairs(character_stat_keys) do - character[key] = serialized[key] + -- Missing from imports and from data saved by another Factorio version + if serialized[key] ~= nil then + character[key] = serialized[key] + end end end diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 6328dbac..4177bdf1 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -208,6 +208,23 @@ describe("player_sync controller", function() { await assert.rejects(set("stored", playerData("stored", 3)), acquiredError); assert.equal((await saved()).get("stored").generation, 3); }); + it("should keep the stored data when an import fails validation", async function() { + // The link validates requests before the handler runs + const fromJSON = lib.Link.requestFromJSON(msg.SetPlayerDataRequest, "SetPlayerDataRequest"); + async function setJSON(name, data) { + const request = fromJSON({ playerName: name, playerData: data }); + return await handler(msg.SetPlayerDataRequest)(request); + } + await release("stored"); + await setJSON("stored", playerData("stored", 0)); + const before = (await saved()).get("stored"); + assert.equal(before.epoch, 1); + await assert.rejects( + setJSON("stored", playerData("stored", 0, { crafting_queue: {} })), + { message: "Request SetPlayerDataRequest failed validation" }, + ); + assert.deepEqual((await saved()).get("stored"), before); + }); }); describe("DeletePlayerDataRequest", function() { diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index aabfbe57..e04d69bb 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -65,14 +65,29 @@ describe("player_sync plugin", function() { game_view_settings: { show_minimap: true }, character: { character_running_speed_modifier: 0, + 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" } }] }, armor: { i: {} }, }, }, - personal_logistic_slots: [], - quick_bar: { 1: { n: "iron-plate" } }, - crafting_queue: { crafting_queue: [], ingredients: [] }, + personal_logistic_slots: { + enabled: true, + trash_not_requested: false, + sections: [{ group: "", active: true, multiplier: 1, filters: [{ index: 1, min: 50 }] }], + }, + quick_bar: { + 1: "iron-plate", + 2: { t: "f", n: "iron-gear-wheel", q: "normal", c: "=" }, + 3: { t: "i", n: "main", s: 4, b: [2] }, + 4: { t: "r", p: [1], y: "blueprint" }, + }, + crafting_queue: { + crafting_queue: [{ recipe: "iron-gear-wheel", count: 5 }], + crafting_queue_progress: 0.25, + ingredients: [{ name: "iron-plate", count: 10 }], + }, recipe_notifications: "", components: { controller: true, inventories: true }, }); @@ -85,6 +100,11 @@ describe("player_sync plugin", function() { assert(accepts({ name: "player", generation: 0 })); assert(accepts(playerData("player", 0, { controller: "ghost", ticks_to_respawn: 300 }))); assert(accepts(playerData("player", 0, { color: [1, 0, 0] }))); + assert(accepts(playerData("player", 0, { + crafting_queue: { crafting_queue: {}, crafting_queue_progress: 0, ingredients: {} }, + personal_logistic_slots: { 1: { name: "iron-plate", min: 50 } }, + hotbar: { 1: "iron-plate" }, + }))); }); it("should reject data the scenario can't restore", function() { for (const [field, value] of [ @@ -97,6 +117,18 @@ describe("player_sync plugin", function() { ["inventories", { main: [] }], ["character", { inventories: { main: { i: [{ s: 0 }] } } }], ["shortcuts", { "toggle-personal-roboport": "yes" }], + ["character", { character_health_bonus: "100" }], + ["crafting_queue", {}], + ["crafting_queue", { crafting_queue: [], crafting_queue_progress: 0 }], + ["crafting_queue", { + crafting_queue: [{ recipe: "iron-gear-wheel" }], crafting_queue_progress: 0, ingredients: [], + }], + ["quick_bar", { 1: { t: "i", n: "main" } }], + ["quick_bar", { 1: { t: "r", y: "blueprint" } }], + ["quick_bar", { 1: 5 }], + ["personal_logistic_slots", { enabled: true, sections: [] }], + ["personal_logistic_slots", { enabled: true, trash_not_requested: false, sections: [{}] }], + ["personal_logistic_slots", { 1: { min: 50 } }], ]) { assert(!accepts({ ...exported, [field]: value }), `accepted ${field} ${JSON.stringify(value)}`); } From 5ca10d579d2bc58c0c51a21d51f7cff8c37f4b73 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:25:29 +0000 Subject: [PATCH 12/17] Restore pre-2.0 logistic slots and bound imported stats Logistic slots saved before 2.0 are keyed by index strings, which json_to_table keeps as strings, so the migration branch never ran and the 2.0 branch failed on the missing sections. Detect the old format by the missing sections and convert the keys to numbers. The import schema now uses the minimums Factorio enforces for character stats, checks the filter value type and the quick bar comparator, and allows filters without quality or comparator. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/messages.ts | 29 ++++++++++++++---------- plugins/player_sync/module/serialize.lua | 6 ++--- plugins/player_sync/test/plugin.js | 15 +++++++++++- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 4b246f01..53251537 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -23,8 +23,8 @@ export type IpcPlayerData = { character?: any, inventories?: any, quick_bar?: any, - hotbar?: string[], - personal_logistic_slots?: {name:string, min:number, max:number}[], + hotbar?: Record, + personal_logistic_slots?: any, crafting_queue?: any, recipe_notifications?: string, // Components the upload was serialized with, all when missing @@ -54,22 +54,22 @@ const jsonColor = Type.Union([ Type.Object({ r: Type.Number(), g: Type.Number(), b: Type.Number(), a: Type.Optional(Type.Number()) }), Type.Array(Type.Number(), { minItems: 3, maxItems: 4 }), ]); -// module/define_player_stat_keys.lua, missing stats are left as they are +// Missing stats are left as they are, the minimums are the ones Factorio enforces const jsonCharacter = Type.Object({ - character_crafting_speed_modifier: Type.Optional(Type.Number()), - character_mining_speed_modifier: Type.Optional(Type.Number()), + character_crafting_speed_modifier: Type.Optional(Type.Number({ minimum: -1 })), + character_mining_speed_modifier: Type.Optional(Type.Number({ minimum: -1 })), character_additional_mining_categories: Type.Optional(jsonList(Type.String())), - character_running_speed_modifier: Type.Optional(Type.Number()), + character_running_speed_modifier: Type.Optional(Type.Number({ minimum: -1 })), character_build_distance_bonus: Type.Optional(Type.Number()), character_item_drop_distance_bonus: Type.Optional(Type.Number()), character_reach_distance_bonus: Type.Optional(Type.Number()), character_resource_reach_distance_bonus: Type.Optional(Type.Number()), character_item_pickup_distance_bonus: Type.Optional(Type.Number()), character_loot_pickup_distance_bonus: Type.Optional(Type.Number()), - character_inventory_slots_bonus: Type.Optional(Type.Number()), - character_trash_slot_count_bonus: Type.Optional(Type.Number()), - character_maximum_following_robot_count_bonus: Type.Optional(Type.Number()), - character_health_bonus: Type.Optional(Type.Number()), + character_inventory_slots_bonus: Type.Optional(Type.Integer({ minimum: 0 })), + character_trash_slot_count_bonus: Type.Optional(Type.Integer({ minimum: 0 })), + character_maximum_following_robot_count_bonus: Type.Optional(Type.Integer({ minimum: 0 })), + character_health_bonus: Type.Optional(Type.Number({ minimum: 0 })), character_personal_logistic_requests_enabled: Type.Optional(Type.Boolean()), allow_dispatching_robots: Type.Optional(Type.Boolean()), inhibit_movement_bonus: Type.Optional(Type.Boolean()), @@ -88,7 +88,7 @@ const jsonPersonalLogisticSlots = Type.Union([ group: Type.String(), active: Type.Boolean(), multiplier: Type.Number({ minimum: 0 }), - filters: jsonList(Type.Object({})), + filters: jsonList(Type.Object({ value: Type.Optional(Type.Object({})) })), })), }), // From before 2.0, slot index to request @@ -98,7 +98,12 @@ const jsonPersonalLogisticSlots = Type.Union([ // QuickBarSlotEncoded in module/serialize.lua const jsonQuickBarSlot = Type.Union([ Type.String(), - Type.Object({ t: Type.Literal("f"), n: Type.String(), q: Type.String(), c: Type.String() }), + Type.Object({ + t: Type.Literal("f"), + n: Type.String(), + q: Type.Optional(Type.String()), + c: Type.Optional(StringEnum(["=", ">", "<", "≥", ">=", "≤", "<=", "≠", "!="])), + }), Type.Object({ t: Type.Literal("i"), n: Type.String(), diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index 40b351e0..867f82cd 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -346,13 +346,13 @@ function serialize.deserialize_personal_logistic_slots(player, serialized) logistic_point.remove_section(i) end - -- If this is an array instead of a table, migrate to v2 format + -- Data from before 2.0 maps slot indices to requests, migrate it to a section if next(serialized) == nil then return - elseif serialized[1] ~= nil then + elseif serialized.sections == nil then local section = logistic_point.add_section() for i, slot in pairs(serialized) do - section.set_slot(i, { + section.set_slot(tonumber(i), { value = { name = slot.name, quality = "normal", diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index e04d69bb..57d7b4a4 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -75,7 +75,12 @@ describe("player_sync plugin", function() { personal_logistic_slots: { enabled: true, trash_not_requested: false, - sections: [{ group: "", active: true, multiplier: 1, filters: [{ index: 1, min: 50 }] }], + sections: [{ + group: "", + active: true, + multiplier: 1, + filters: [{ value: { name: "iron-plate", type: "item" }, min: 3 }], + }], }, quick_bar: { 1: "iron-plate", @@ -118,6 +123,9 @@ describe("player_sync plugin", function() { ["character", { inventories: { main: { i: [{ s: 0 }] } } }], ["shortcuts", { "toggle-personal-roboport": "yes" }], ["character", { character_health_bonus: "100" }], + ["character", { character_inventory_slots_bonus: -1 }], + ["character", { character_crafting_speed_modifier: -5 }], + ["quick_bar", { 1: { t: "f", n: "iron-plate", q: "normal", c: "~" } }], ["crafting_queue", {}], ["crafting_queue", { crafting_queue: [], crafting_queue_progress: 0 }], ["crafting_queue", { @@ -129,6 +137,11 @@ describe("player_sync plugin", function() { ["personal_logistic_slots", { enabled: true, sections: [] }], ["personal_logistic_slots", { enabled: true, trash_not_requested: false, sections: [{}] }], ["personal_logistic_slots", { 1: { min: 50 } }], + ["personal_logistic_slots", { + enabled: true, + trash_not_requested: false, + sections: [{ group: "", active: true, multiplier: 1, filters: [{ value: 5 }] }], + }], ]) { assert(!accepts({ ...exported, [field]: value }), `accepted ${field} ${JSON.stringify(value)}`); } From 41dc29454278881c793e6069daaebcb0d8145da8 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:26:50 +0000 Subject: [PATCH 13/17] Reject negative distance bonuses in imported player data Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/messages.ts | 12 ++++++------ plugins/player_sync/test/plugin.js | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 53251537..6799e41c 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -60,12 +60,12 @@ const jsonCharacter = Type.Object({ character_mining_speed_modifier: Type.Optional(Type.Number({ minimum: -1 })), character_additional_mining_categories: Type.Optional(jsonList(Type.String())), character_running_speed_modifier: Type.Optional(Type.Number({ minimum: -1 })), - character_build_distance_bonus: Type.Optional(Type.Number()), - character_item_drop_distance_bonus: Type.Optional(Type.Number()), - character_reach_distance_bonus: Type.Optional(Type.Number()), - character_resource_reach_distance_bonus: Type.Optional(Type.Number()), - character_item_pickup_distance_bonus: Type.Optional(Type.Number()), - character_loot_pickup_distance_bonus: Type.Optional(Type.Number()), + character_build_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), + character_item_drop_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), + character_reach_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), + character_resource_reach_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), + character_item_pickup_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), + character_loot_pickup_distance_bonus: Type.Optional(Type.Number({ minimum: 0 })), character_inventory_slots_bonus: Type.Optional(Type.Integer({ minimum: 0 })), character_trash_slot_count_bonus: Type.Optional(Type.Integer({ minimum: 0 })), character_maximum_following_robot_count_bonus: Type.Optional(Type.Integer({ minimum: 0 })), diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 57d7b4a4..123ab9e6 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -124,6 +124,7 @@ describe("player_sync plugin", function() { ["shortcuts", { "toggle-personal-roboport": "yes" }], ["character", { character_health_bonus: "100" }], ["character", { character_inventory_slots_bonus: -1 }], + ["character", { character_reach_distance_bonus: -1 }], ["character", { character_crafting_speed_modifier: -5 }], ["quick_bar", { 1: { t: "f", n: "iron-plate", q: "normal", c: "~" } }], ["crafting_queue", {}], From feb3253bec0f68844583072fb0b751dd5a4c6a6a Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:53:36 +0000 Subject: [PATCH 14/17] Restore each imported appearance field on its own An import with color but no chat_color or tag assigned nil to the missing LuaPlayer properties and failed restoration. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/module/serialize.lua | 9 +++++---- plugins/player_sync/test/plugin.js | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index 867f82cd..d2bf0666 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -1324,10 +1324,11 @@ function serialize.deserialize_player(player, serialized, components, local_cont end end - if syncs(components, "appearance") and serialized.color then - player.color = serialized.color - player.chat_color = serialized.chat_color - player.tag = serialized.tag + if syncs(components, "appearance") then + -- Imports may supply any subset of these + if serialized.color then player.color = serialized.color end + if serialized.chat_color then player.chat_color = serialized.chat_color end + if serialized.tag then player.tag = serialized.tag end end if syncs(components, "force") and serialized.force then player.force = serialized.force diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 123ab9e6..4752f356 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -105,6 +105,7 @@ describe("player_sync plugin", function() { assert(accepts({ name: "player", generation: 0 })); assert(accepts(playerData("player", 0, { controller: "ghost", ticks_to_respawn: 300 }))); assert(accepts(playerData("player", 0, { color: [1, 0, 0] }))); + assert(accepts(playerData("player", 0, { chat_color: [1, 0, 0], tag: "tag" }))); assert(accepts(playerData("player", 0, { crafting_queue: { crafting_queue: {}, crafting_queue_progress: 0, ingredients: {} }, personal_logistic_slots: { 1: { name: "iron-plate", min: 50 } }, From 9ecd497f1e61653d03b803406c6dfff022271792 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:53:36 +0000 Subject: [PATCH 15/17] Reserve the deleted marker for tombstones Imports and uploads could carry deleted: false, which the loader took for a tombstone on the next restart. The schemas now reject the property, and the loader only treats deleted: true as a tombstone and strips the key from entries already saved with it. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/controller.ts | 6 +++-- plugins/player_sync/messages.ts | 4 ++++ plugins/player_sync/test/controller.js | 32 ++++++++++++++++++-------- plugins/player_sync/test/plugin.js | 10 ++++++++ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 2938ffa9..419c1e0e 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -41,10 +41,12 @@ async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) { let playerDatastore = new Map(); let tombstones = new Map(); for (let [name, entry] of entries) { - if ("deleted" in entry) { + if ("deleted" in entry && entry.deleted === true) { tombstones.set(name, entry); } else { - playerDatastore.set(name, entry); + // Imports used to keep the marker + delete (entry as any).deleted; + playerDatastore.set(name, entry as IpcPlayerData); } } return { playerDatastore, tombstones }; diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 6799e41c..ea780235 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -31,10 +31,13 @@ export type IpcPlayerData = { components?: Record | unknown[], } +// Marks tombstones in player_sync.json +const jsonReserved = { deleted: Type.Optional(Type.Never()) }; const jsonPlayerData = Type.Object({ generation: Type.Integer({ minimum: 0 }), epoch: Type.Optional(Type.Integer({ minimum: 0 })), name: Type.String(), + ...jsonReserved, }); // The parts of an import that module/serialize.lua relies on, item stacks are checked by the Lua deserializer @@ -150,6 +153,7 @@ const jsonImportedPlayerData = Type.Object({ crafting_queue: Type.Optional(jsonCraftingQueue), recipe_notifications: Type.Optional(Type.String()), components: Type.Optional(jsonTable), + ...jsonReserved, }); export class AcquireResponse { diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 4177bdf1..eb44d056 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -208,23 +208,35 @@ describe("player_sync controller", function() { await assert.rejects(set("stored", playerData("stored", 3)), acquiredError); assert.equal((await saved()).get("stored").generation, 3); }); + + // The link validates requests before the handler runs + const fromJSON = lib.Link.requestFromJSON(msg.SetPlayerDataRequest, "SetPlayerDataRequest"); + async function setJSON(name, data) { + const request = fromJSON({ playerName: name, playerData: data }); + return await handler(msg.SetPlayerDataRequest)(request); + } it("should keep the stored data when an import fails validation", async function() { - // The link validates requests before the handler runs - const fromJSON = lib.Link.requestFromJSON(msg.SetPlayerDataRequest, "SetPlayerDataRequest"); - async function setJSON(name, data) { - const request = fromJSON({ playerName: name, playerData: data }); - return await handler(msg.SetPlayerDataRequest)(request); - } await release("stored"); await setJSON("stored", playerData("stored", 0)); const before = (await saved()).get("stored"); assert.equal(before.epoch, 1); - await assert.rejects( - setJSON("stored", playerData("stored", 0, { crafting_queue: {} })), - { message: "Request SetPlayerDataRequest failed validation" }, - ); + for (const extra of [{ crafting_queue: {} }, { deleted: false }, { deleted: true }]) { + await assert.rejects( + setJSON("stored", playerData("stored", 0, extra)), + { message: "Request SetPlayerDataRequest failed validation" }, + ); + } assert.deepEqual((await saved()).get("stored"), before); }); + 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 }); + const entries = [["stored", { ...data, deleted: false }]]; + await fs.writeFile(path.join(databaseDir, "player_sync.json"), JSON.stringify(entries)); + controller = await load(); + const response = await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest("stored")); + assert.deepEqual(response.playerData, data); + }); }); describe("DeletePlayerDataRequest", function() { diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 4752f356..52f8d0cf 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -51,6 +51,14 @@ describe("player_sync plugin", function() { assert(!validate(request), `accepted ${generation}`); } }); + it("should reject uploads carrying the tombstone marker", function() { + const validate = lib.compile(msg.UploadRequest.jsonSchema); + assert(validate({ instanceId: 1, playerName: "player", playerData: playerData("player", 5) })); + for (const deleted of [false, true]) { + const data = playerData("player", 5, { deleted }); + assert(!validate({ instanceId: 1, playerName: "player", playerData: data }), `accepted ${deleted}`); + } + }); describe("SetPlayerDataRequest schema", function() { const validate = lib.compile(msg.SetPlayerDataRequest.jsonSchema); // Shaped like module/serialize.lua output @@ -116,6 +124,8 @@ describe("player_sync plugin", function() { for (const [field, value] of [ ["controller", "not-a-controller"], ["controller", "remote"], + ["deleted", false], + ["deleted", true], ["color", "red"], ["force", 1], ["ticks_to_respawn", -1], 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 16/17] 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":{}}')), []); From ec7b5f2db8ceace01a25b7a9263a9fa9ae57cd7a Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:24:02 +0000 Subject: [PATCH 17/17] Rework the player sync web page after UX review The drawer actions move to its header as Export, Import and a trash icon for Delete. Import and Delete are disabled while an instance holds the player, since the stored data is out of date until it uploads. The drawer also shows color and cheat mode. Inventory tables get title case names, pagination at 10 rows, item search, sorting and quality icons. The player list shows force with a filter instead of generation, and the database stats use Descriptions without the largest entry. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/controller.ts | 3 +- plugins/player_sync/messages.ts | 4 +- plugins/player_sync/package.json | 1 + plugins/player_sync/test/controller.js | 2 +- plugins/player_sync/test/plugin.js | 1 + plugins/player_sync/web/index.tsx | 308 ++++++++++++++++--------- plugins/player_sync/web/style.css | 8 + 7 files changed, 219 insertions(+), 108 deletions(-) diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 93ba3aa5..16589fdd 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -266,7 +266,8 @@ export default async function(context: ControllerPluginContext) { controller.handle(msg.ListPlayersRequest, async () => { let entries = new Map(); for (let [name, playerData] of playerDatastore) { - entries.set(name, new msg.PlayerEntry(name, playerData.generation, JSON.stringify(playerData).length)); + let size = JSON.stringify(playerData).length; + entries.set(name, new msg.PlayerEntry(name, playerData.generation, size, undefined, playerData.force)); } for (let name of acquiredPlayers.keys()) { let acquisitionRecord = getAcquisition(name); diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 7663a794..146d2727 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -359,6 +359,7 @@ export class PlayerEntry { public generation?: number, public size?: number, public instanceId?: number, + public force?: string, ) { } @@ -367,10 +368,11 @@ export class PlayerEntry { "generation": Type.Optional(Type.Integer()), "size": Type.Optional(Type.Integer()), "instanceId": Type.Optional(Type.Integer()), + "force": Type.Optional(Type.String()), }); static fromJSON(json: Static): PlayerEntry { - return new this(json.name, json.generation, json.size, json.instanceId); + return new this(json.name, json.generation, json.size, json.instanceId, json.force); } } diff --git a/plugins/player_sync/package.json b/plugins/player_sync/package.json index ff06cf62..c95e068f 100644 --- a/plugins/player_sync/package.json +++ b/plugins/player_sync/package.json @@ -51,6 +51,7 @@ "@sinclair/typebox": "catalog:" }, "devDependencies": { + "@ant-design/icons": "^5.6.1", "@clusterio/controller": "workspace:^", "@clusterio/ctl": "workspace:^", "@clusterio/host": "workspace:^", diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index a7177d78..2a5328df 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -147,7 +147,7 @@ describe("player_sync controller", function() { entries.sort((a, b) => a.name.localeCompare(b.name)); assert.deepEqual(entries, [ new msg.PlayerEntry("online", undefined, undefined, 1), - new msg.PlayerEntry("stored", 3, JSON.stringify(playerData("stored", 3)).length, 1), + new msg.PlayerEntry("stored", 3, JSON.stringify(playerData("stored", 3)).length, 1, "player"), ]); }); }); diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index ed521923..77516d93 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -26,6 +26,7 @@ describe("player_sync plugin", function() { [undefined, 0, 4], // generation [undefined, 0, 1234], // size [undefined, 1], // instanceId + [undefined, "player"], // force )); }); it("GetPlayerDataResponse should be round trip json serialisable", function() { diff --git a/plugins/player_sync/web/index.tsx b/plugins/player_sync/web/index.tsx index 38f101fc..dcfd2c9d 100644 --- a/plugins/player_sync/web/index.tsx +++ b/plugins/player_sync/web/index.tsx @@ -1,5 +1,7 @@ import React, { useCallback, useContext, useEffect, useRef, useState } from "react"; -import { Alert, Button, Descriptions, Drawer, Popconfirm, Space, Table, Typography, Upload } from "antd"; +import { Alert, Button, Descriptions, Drawer, Input, Popconfirm, Space, Table, Typography, Upload } from "antd"; +import type { ColumnType } from "antd/es/table"; +import { DeleteOutlined, SearchOutlined } from "@ant-design/icons"; import * as lib from "@clusterio/lib"; import { @@ -28,89 +30,134 @@ function useInstanceName() { }; } +function titleCase(text: string) { + return text.charAt(0).toUpperCase() + text.slice(1); +} + +// Factorio reads colors back as 0-1 floats but accepts 0-255 as well +function formatColor(color: any) { + let [r, g, b] = Array.isArray(color) ? color : [color.r, color.g, color.b]; + let scale = Math.max(r, g, b) > 1 ? 1 : 255; + let hex = (c: number) => Math.min(255, Math.max(0, Math.round(c * scale))).toString(16).padStart(2, "0"); + return `#${hex(r)}${hex(g)}${hex(b)}`; +} + +// Filters the table locally, the inventory tables are not kept in the URL +function itemSearch(getText: (record: T) => string): ColumnType { + return { + filterIcon: (filtered: boolean) => , + filterDropdown: ({ selectedKeys, setSelectedKeys, confirm }) => ( +
e.stopPropagation()}> + { + setSelectedKeys(e.target.value ? [e.target.value] : []); + confirm({ closeDropdown: false }); + }} + onPressEnter={() => confirm()} + /> +
+ ), + onFilter: (value, record) => getText(record).toLowerCase().includes(String(value).toLowerCase()), + }; +} + function InventoryTable(props: { name: string, items: ItemSummary[] }) { const modPack = useDefaultModPack(); const locale = useExportLocale(modPack); const prototypes = useExportPrototypeMetadata(modPack); const itemMetadata = prototypes?.get("item"); + const qualityMetadata = prototypes?.get("quality"); + // The export lists qualities in the order mods define them, normal is left out of the stored data + const qualityOrder = [...qualityMetadata?.keys() ?? []]; + const qualityRank = (quality?: string) => (quality === undefined ? -1 : qualityOrder.indexOf(quality)); - function getLocaleName(itemName: string) { - let meta = itemMetadata?.get(itemName); + function getLocaleName(type: "item" | "quality", name: string) { + let meta = (type === "item" ? itemMetadata : qualityMetadata)?.get(name); if (typeof meta?.localised_name === "string") { return meta.localised_name; } if (meta?.localised_name) { - return locale.get(meta.localised_name[0]) ?? itemName; + return locale.get(meta.localised_name[0]) ?? name; } - return locale.get(`item-name.${itemName}`) ?? locale.get(`entity-name.${itemName}`) ?? itemName; + if (type === "quality") { + return locale.get(`quality-name.${name}`) ?? name; + } + return locale.get(`item-name.${name}`) ?? locale.get(`entity-name.${name}`) ?? name; } + const itemText = (item: ItemSummary) => ( + item.exportSize !== undefined ? item.name : `${getLocaleName("item", item.name)} ${item.name}` + ); - return
size="small" - title={() => props.name} + title={() => titleCase(props.name)} columns={[ { title: "Item", key: "item", + ...itemSearch(itemText), + sorter: (a, b) => strcmp(itemText(a), itemText(b)), render: (_, item) => (item.exportSize !== undefined ? {item.name} - : <> + : - {getLocaleName(item.name)} - + {getLocaleName("item", item.name)} + ), }, { title: "Quality", key: "quality", - render: (_, item) => item.quality, + sorter: (a, b) => qualityRank(a.quality) - qualityRank(b.quality), + render: (_, item) => item.quality !== undefined && + + {getLocaleName("quality", item.quality)} + , }, { title: "Count", key: "count", align: "right", + sorter: (a, b) => a.count - b.count, render: (_, item) => item.count, }, { title: "Size", key: "size", align: "right", + sorter: (a, b) => (a.exportSize ?? -1) - (b.exportSize ?? -1), render: (_, item) => (item.exportSize === undefined ? "" : lib.formatBytes(item.exportSize)), }, ]} dataSource={props.items.map((item, key) => ({ ...item, key }))} - pagination={false} + pagination={{ pageSize: 10, hideOnSinglePage: true, showSizeChanger: false }} />; } -function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChange: () => void }) { - return - {/* Keyed so a request that finishes after switching players can only update the old player's view */} - {props.playerName !== undefined - && } - ; -} - -function PlayerDetails(props: { playerName: string, onChange: () => void }) { +function usePlayerData(playerName?: string) { const control = useContext(ControlContext); - const account = useAccount(); - const instanceName = useInstanceName(); - const [response, setResponse] = useState(); - const { playerName } = props; + const [loaded, setLoaded] = useState<{ playerName: string, response: GetPlayerDataResponse }>(); // Only the latest request may set the response, an import or delete can finish after a reload const requestId = useRef(0); + const currentPlayer = useRef(playerName); + currentPlayer.current = playerName; const reload = useCallback(() => { + // Ignore reloads from actions that finish after switching to another player + if (playerName !== currentPlayer.current) { + return; + } requestId.current += 1; let id = requestId.current; - control.send(new GetPlayerDataRequest(playerName)).then(result => { + if (playerName === undefined) { + return; + } + control.send(new GetPlayerDataRequest(playerName)).then(response => { if (id === requestId.current) { - setResponse(result); + setLoaded({ playerName, response }); } }).catch( notifyErrorHandler("Error loading player data") @@ -118,11 +165,37 @@ function PlayerDetails(props: { playerName: string, onChange: () => void }) { }, [control, playerName]); useEffect(reload, [reload]); + return [loaded?.playerName === playerName ? loaded?.response : undefined, reload] as const; +} + +type PlayerProps = { playerName: string, response: GetPlayerDataResponse, onChange: () => void }; + +function PlayerDrawer(props: { playerName?: string, onClose: () => void, onChange: () => void }) { + const { playerName } = props; + const [response, reload] = usePlayerData(playerName); + function changed() { reload(); props.onChange(); } + const loaded = response && playerName !== undefined; + return } + > + {loaded && } + ; +} + +function PlayerActions(props: PlayerProps) { + const control = useContext(ControlContext); + const account = useAccount(); + const { playerName, response } = props; + async function upload(file: File) { let playerData: IpcPlayerData; try { @@ -135,11 +208,11 @@ function PlayerDetails(props: { playerName: string, onChange: () => void }) { playerData.generation ??= 0; let result = await control.send(new SetPlayerDataRequest(playerName, playerData)); notify(`Stored player data for ${playerName} as generation ${result.generation}`); - changed(); + props.onChange(); } function download() { - let json = JSON.stringify(response!.playerData, null, "\t"); + let json = JSON.stringify(response.playerData, null, "\t"); let url = URL.createObjectURL(new Blob([json], { type: "application/json" })); let link = document.createElement("a"); link.href = url; @@ -149,67 +222,89 @@ function PlayerDetails(props: { playerName: string, onChange: () => void }) { } const canModify = account.hasPermission("player_sync.inventory.modify"); - const playerData = response?.playerData; - const acquiredBy = response?.instanceId !== undefined ? instanceName(response.instanceId) : undefined; - return <> - {response && - {acquiredBy !== undefined && control.send(new ForceReleaseRequest(playerName)).then(changed).catch( - notifyErrorHandler("Error releasing lock") - )} - > - - } - />} - - {playerData?.generation ?? "-"} - - {playerData ? lib.formatBytes(JSON.stringify(playerData).length) : "-"} - - {playerData?.controller ?? "-"} - {playerData?.force ?? "-"} - - - - {canModify && { - upload(file).catch(notifyErrorHandler("Error storing player data")); - return false; - }} - > - - } - {canModify && control.send(new DeletePlayerDataRequest(playerName)).then(changed).catch( - notifyErrorHandler("Error deleting player data") - )} - > - - } - - {!playerData && No player data stored} - {playerData && summarizePlayerInventories(playerData).map( - inventory => + const acquired = response.instanceId !== undefined; + return + + {canModify && { + upload(file).catch(notifyErrorHandler("Error storing player data")); + return false; + }} + > + + } + {canModify && control.send(new DeletePlayerDataRequest(playerName)).then(props.onChange).catch( + notifyErrorHandler("Error deleting player data") )} - } - ; + > + + } + />} + + {playerData?.generation ?? "-"} + + {playerData ? lib.formatBytes(JSON.stringify(playerData).length) : "-"} + + {playerData?.controller ?? "-"} + {playerData?.force ?? "-"} + + {color ? + + {color} + : "-"} + + {cheatMode} + + {!playerData && No player data stored} + {playerData && summarizePlayerInventories(playerData).map( + inventory => + )} + ; } function InventoryPage() { @@ -223,6 +318,7 @@ function InventoryPage() { namespace: "inventory", defaultSortKey: "name", pagination: { defaultPageSize: 50 }, }); const nameSearch = useColumnSearch(tableState, "name", player => player.name, "Search"); + const forces = [...new Set(players.flatMap(player => (player.force === undefined ? [] : [player.force])))]; const reload = useCallback(() => { control.send(new DatabaseStatsRequest()).then(setStatsData).catch( @@ -236,11 +332,10 @@ function InventoryPage() { return - {statsData && <> -

Database size: {lib.formatBytes(statsData.databaseSize)}

-

Database entries: {statsData.databaseEntries}

-

Largest entry is {statsData.largestEntry.name} with {lib.formatBytes(statsData.largestEntry.size)}

- } + {statsData && + {lib.formatBytes(statsData.databaseSize)} + {statsData.databaseEntries} + }
(a.generation ?? -1) - (b.generation ?? -1), - sortOrder: tableState.sortOrder("generation"), - render: (_, player) => player.generation ?? "-", + title: "Force", + key: "force", + filters: forces.sort(strcmp).map(force => ({ text: force, value: force })), + filterMultiple: true, + filteredValue: tableState.filteredValue("force"), + onFilter: (value, player) => player.force === value, + sorter: (a, b) => strcmp(a.force ?? "", b.force ?? ""), + sortOrder: tableState.sortOrder("force"), + render: (_, player) => player.force ?? "-", }, { title: "Size", diff --git a/plugins/player_sync/web/style.css b/plugins/player_sync/web/style.css index e69de29b..48549508 100644 --- a/plugins/player_sync/web/style.css +++ b/plugins/player_sync/web/style.css @@ -0,0 +1,8 @@ +.player-sync-color { + display: inline-block; + width: 16px; + height: 16px; + border: 1px solid rgba(0, 0, 0, 0.25); + border-radius: 2px; + vertical-align: middle; +}