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));