diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index d148f834..4dd28f8a 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -6,7 +6,7 @@ As a player, it mostly just works. Most important to know is that factorio data A note on crashing servers: -When a server crashes while you are online there won't be time to immediately upload the inventory. Instead, the inventory is uploaded as soon as the server restarts. This means if you have been playing on a different server from the one that crashed and gathered/consumed items but are offline at the time of the first server coming online again your inventory will reset to the autosave. +When a server crashes while you are online there won't be time to immediately upload the inventory. Instead, the inventory is uploaded as soon as the server restarts. If you have been playing on a different server in the meantime the controller keeps what that server uploaded, and the crashed server downloads it the next time you join it. ## Installation diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index a94800cc..4d0bde24 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -258,6 +258,7 @@ export default async function(context: ControllerPluginContext) { prefetch(instanceId, request.destinationInstanceId, merged); } } + return new msg.UploadRequest.Response(store); }); controller.handle(msg.DownloadRequest, async (request: msg.DownloadRequest) => { diff --git a/plugins/player_sync/instance.ts b/plugins/player_sync/instance.ts index 4c185527..b461fcdf 100644 --- a/plugins/player_sync/instance.ts +++ b/plugins/player_sync/instance.ts @@ -200,11 +200,12 @@ export default async function(context: InstancePluginContext) { delete player_data.destination_instance_id; logger.verbose(`Uploading ${player_data.name} (${JSON.stringify(player_data).length / 1000}kB)`); + let stored: boolean; try { - await instance.sendTo( + ({ stored } = await instance.sendTo( "controller", new UploadRequest(instance.id, player_data.name, player_data, destinationInstanceId), - ); + )); } catch (err: any) { if (!(err instanceof lib.SessionLost)) { @@ -213,8 +214,10 @@ export default async function(context: InstancePluginContext) { return; } + // A copy the controller did not store must not pass for the stored one: the save downloads at the next join + const reply = stored ? "confirm_upload" : "reject_upload"; await instance.sendRcon( - `/sc player_sync.confirm_upload("${player_data.name}", ${player_data.generation})`, true, plugin.name + `/sc player_sync.${reply}("${player_data.name}", ${player_data.generation})`, true, plugin.name ); } diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index a3a1ce9b..506b46df 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -262,12 +262,29 @@ export class ReleaseRequest { } } +export class UploadResponse { + constructor( + /** False when the controller kept what it has: another copy is newer, locked or from another epoch */ + public stored: boolean, + ) { + } + + static jsonSchema = Type.Object({ + "stored": Type.Boolean(), + }); + + static fromJSON(json: Static) { + return new this(json.stored); + } +} + export class UploadRequest { declare ["constructor"]: typeof UploadRequest; static type = "request" as const; static src = "instance" as const; static dst = "controller" as const; static plugin = "player_sync" as const; + static Response = UploadResponse; constructor( public instanceId: number, diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index b2df31b6..8c27fd02 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -384,12 +384,14 @@ function player_sync.check_players_in_cutscene() end -- Whether a join should take the stored data: there is some, and this save hasn't synced the player in that epoch at --- that generation or newer with every component synced here now. A player without a record here has nothing to keep. +-- that generation or newer with every component synced here now, or the controller did not store its last upload +-- (reject_upload). A player without a record here has nothing to keep. local function download_needed(player_record, acquire_response, script_data) return acquire_response.has_data and ( not player_record or not player_record.sync + or player_record.outdated or (player_record.epoch or 0) ~= (acquire_response.epoch or 0) or acquire_response.generation > player_record.generation or serialize.newly_synced(player_record.components, script_data.components) @@ -543,6 +545,8 @@ function player_sync.sync_player(acquire_response) start_sync(player_record, script_data.components) player_record.dirty = true player_record.sync_in_progress = false + -- Nothing stored to download: the copy here is all there is + player_record.outdated = nil -- Deleted data keeps its generation and epoch so the next upload is accepted as the new baseline player_record.epoch = epoch -- A copy from another epoch has nothing to do with the stored generation @@ -573,6 +577,7 @@ function player_sync.finish_download(player, finished_record) player_record.dirty = true player_record.sync_in_progress = false + player_record.outdated = nil start_sync(player_record, finished_record.components) player_record.generation = finished_record.generation player_record.epoch = finished_record.epoch @@ -724,6 +729,27 @@ function player_sync.confirm_upload(player_name, generation) player_record.dirty = false end +-- Invoked by the instance code when the controller did not store the uploaded data: it holds another copy that is +-- newer, locked by another instance or from another epoch. This save's copy lost, even at the same generation, so it +-- is not uploaded again and the next join downloads. A retry of an upload the controller already stored ends up here +-- too, and then downloads what this save sent. +function player_sync.reject_upload(player_name, generation) + local player = game.get_player(player_name) + if not player or player.connected then + return + end + local script_data = get_script_data() + local player_record = script_data.players[player.name] + if not player_record or player_record.generation ~= generation then + return + end + + log("Upload of " .. player_name .. " was not stored, the next join downloads") + script_data.active_uploads[player_name] = nil + player_record.dirty = false + player_record.outdated = true +end + function player_sync.initiate_inventory_download(player, player_record, generation, epoch) player.print("Initiating inventory download...") local record = { diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index a249aa7c..7c619f57 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -77,6 +77,18 @@ describe("player_sync controller", function() { .playerData; } + it("should reply whether the upload was stored", async function() { + const handler = mock.getHandler(controller, msg.UploadRequest); + const send = (instanceId, generation) => handler( + { instanceId, playerName: "test", playerData: { generation, name: "test" } } + ); + assert.deepEqual(await send(1, 2), new msg.UploadRequest.Response(true)); + // A crashed instance restarting with the generation another instance already uploaded + assert.deepEqual(await send(1, 2), new msg.UploadRequest.Response(false)); + await mock.getHandler(controller, msg.AcquireRequest)(new msg.AcquireRequest(1, "test")); + controller.instances.set(2, controller.instances.get(1)); + assert.deepEqual(await send(2, 3), new msg.UploadRequest.Response(false)); + }); it("should store the upload as is when it has no components", async function() { const playerData = { generation: 2, name: "test", force: "enemy", tag: "uploaded" }; assert.deepEqual(await upload(playerData), playerData); diff --git a/plugins/player_sync/test/instance.js b/plugins/player_sync/test/instance.js index 67132ce6..9142521e 100644 --- a/plugins/player_sync/test/instance.js +++ b/plugins/player_sync/test/instance.js @@ -5,7 +5,7 @@ import * as lib from "@clusterio/lib"; import * as mock from "../../../test/mock.js"; import loadInstancePlugin, { recipeNotificationDelta, applyRecipeNotificationDelta } from "../dist/node/instance.js"; -import { AcquireResponse, DownloadResponse, PrefetchEvent } from "../dist/node/messages.js"; +import { AcquireResponse, DownloadResponse, PrefetchEvent, UploadResponse } from "../dist/node/messages.js"; import { components } from "../dist/node/components.js"; import { plugin as info } from "../dist/node/index.js"; @@ -275,6 +275,25 @@ describe("player_sync", function() { const request = await upload({ name: "test", generation: 2 }); assert.equal(request.destinationInstanceId, undefined); }); + async function rconAfterReply(stored) { + instance.server.reset(); + instance.connector.once("send", message => { + instance.connector.emit("message", new lib.MessageResponse( + 1, message.dst, message.src, new UploadResponse(stored) + )); + }); + instance.server.emit("ipc-player_sync_upload", { name: "test", generation: 2 }); + for (let i = 0; i < 100 && !instance.server.rconCommands.length; i++) { + await lib.wait(1); + } + return instance.server.rconCommands; + } + it("should confirm an upload the controller stored", async function() { + assert.deepEqual(await rconAfterReply(true), ['/sc player_sync.confirm_upload("test", 2)']); + }); + it("should reject an upload the controller did not store", async function() { + assert.deepEqual(await rconAfterReply(false), ['/sc player_sync.reject_upload("test", 2)']); + }); }); describe("PrefetchEvent", function() {