Rework inventory_sync (#411)

Complete rework of the inventory synchronization to reduce the number of
times the inventory has to be downloaded and the possibilities for item
loss/duplication.

The basic concept is to acquire exclusive access to the player data
before downloading the data.  This prevents the player from logging in
to two servers at the same time and getting the synced inventory twice.
The acquisition will in most cases execute faster than the player can
provide input to the game on login, and if the player data on the master
isn't what was previously uploaded then the player inventory is deleted
and the player is turned into a spectator while the updated inventory is
being transferred into the game.  This model allows for the downloading
of the synced inventory to be skipped in the case the player joins the
same server they left from.

The synced data is expanded to include more than just the character and
inventory to properly handle sync of dead players.  If the player leaves
dead on one server this state will be saved and when the player joins
another server they continue off as dead.  The ghost, god, and spectator
controllers are supported, while the cutscene and editor controllers are
not.  For players in a cutscene when joining the acquisition will be
attempted immediately, but the scenario waits until the player is out of
the cutscene before acting upon the response to the acquisition.  For
players in the editor mode the scenario toggles them out of the mode
before syncing data.

Since uploads to the master can fail handling of this is also added.
The scenario will retry uploading the player data at exponentially
increasing intervals until it gets the message back that the upload
succeeded.  Since it's possible for servers to go down and stay down for
a long time the exclusive access held by these servers are released after
a timeout.  This means that a player could be playing and or have
uploaded newer data by the time the instance comes back online and tries
to upload the inventory, in this case the master will report the upload
succeeded but discard the data.  Instances may also upload old data if
they load back to an earlier save, which will also be discarded by the
master.

Other small improvements made by this rework:
- Add a retry button to the failed download dialog
- Update the existing progress dialog instead of destroying and
  recreating it every time.
- Automatically stuff the items from the temporary inventory into the
  synced inventory if they fit.
- Move player back to the position they started the download from if
  they were turned into a spectator for the download.
- Ensure the player has a character if the stored data is wiped.
- Add error checking of the RCON commands sent by the instance.

plugins/inventory_sync/README.md

@@ -19,31 +19,26 @@ Substitute clusteriomaster with clusterioslave or clusterioctl if this a dedicat
 
 ## Method of operation
 
-This plugin does event based synchronization of inventories. The data flow is as follows:
+This plugin does event based synchronization of inventories.
+The greatly simplified data flow is as follows:
 
-1. Player joins server, gets put in spectator mode
+1. Player joins server.
+2. Scenario script asks master for exclusive access to the player.
+3. Master grants exclusive access for the player to the instance.
+4. Scenario script checks the access response and then acts according to the situation:
 
-2. Lua script asks instance for inventory
+    1. If the player has no inventory on the master then the current inventory becomes the synced inventory and the process is done.
+    2. If the player inventory was previously uploaded and is the same as the one stored on the master then the current inventory becomes the synced inventory and the process is done.
+    3. If the player inventory was previously uploaded but is not the same then the player inventory is deleted and the player is turned into a spectator.
+    4. Otherwise the player inventory is kept.
 
-3. Instance asks master for inventory
+5. Scenario script asks for the player inventory from the master.
+6. Master sends it to the scenario in a stream of chunks.
+7. Scenario displays and updates a progress bar as each chunk is received.
+8. Once all chunks have loaded the player's synced inventory is recreated from the data and the player can start playing.
 
-4. Master does not find inventory, and responds to instance with undefined
-
-5. Instance sets player to survival
-
-6. Player keeps playing, and eventually leaves. Inventory is serialized and sent to instance through stdout/file
-
-7. Instance sends inventory to master
-
-8. Player joins a different server. Lua sends a request to instance, puts player into spectator mode
-
-9. Instance sends request to master
-
-10. Master responds with json data
-
-11. Instance calls command in scenario script using rcon
-
-12. Once the entire command is transfered, the scenario script clears the old inventory, applies the new inventory from the scenario script and sets the player to survival mode.
+When the player leaves the inventory is uploaded if it's a synced inventory and the exclusive access the scenario script holds is released.
+Should an error occur during this process the player is given the option to use a temporary inventory instead, which will be merged back into the synced inventory the next time the sync succeeds on that instance.
 
 Communication between the server and instance goes over stdout or rcon, depending on the size of the data.
 Communication between the instance and the master server goes over websockets.

plugins/inventory_sync/info.js

@@ -11,6 +11,14 @@ MasterConfigGroup.define({
 	type: "number",
 	initial_value: 600, // 10 minutes
 });
+MasterConfigGroup.define({
+	name: "player_lock_timeout",
+	title: "Player Lock Timeout",
+	description:
+		"Time in seconds before the lock on a player inventory expires after an instance stops or is disconnected",
+	type: "number",
+	initial_value: 60,
+});
 MasterConfigGroup.finalize();
 
 class InstanceConfigGroup extends libConfig.PluginConfigGroup { }
@@ -47,15 +55,39 @@ module.exports = {
 	routes: ["/inventory"],
 
 	messages: {
+		acquire: new libLink.Request({
+			type: "inventory_sync:acquire",
+			links: ["instance-slave", "slave-master"],
+			forwardTo: "master",
+			requestProperties: {
+				"instance_id": { type: "integer" },
+				"player_name": { type: "string" },
+			},
+			responseRequired: ["status"],
+			responseProperties: {
+				"status": { enum: ["acquired", "error", "busy"] },
+				"generation": { type: "integer" },
+				"has_data": { type: "boolean" },
+				"message": { type: "string" },
+			},
+		}),
+		release: new libLink.Request({
+			type: "inventory_sync:release",
+			links: ["instance-slave", "slave-master"],
+			forwardTo: "master",
+			requestProperties: {
+				"instance_id": { type: "integer" },
+				"player_name": { type: "string" },
+			},
+		}),
 		upload: new libLink.Request({
 			type: "inventory_sync:upload",
 			links: ["instance-slave", "slave-master"],
 			forwardTo: "master",
 			requestProperties: {
 				"instance_id": { type: "integer" },
-				"instance_name": { type: "string" },
 				"player_name": { type: "string" },
-				"inventory": { type: "object" },
+				"player_data": { type: "object" },
 			},
 		}),
 		download: new libLink.Request({
@@ -63,12 +95,11 @@ module.exports = {
 			links: ["instance-slave", "slave-master"],
 			forwardTo: "master",
 			requestProperties: {
+				"instance_id": { type: "integer" },
 				"player_name": { type: "string" },
 			},
 			responseProperties: {
-				"player_name": { type: "string" },
-				"inventory": { type: "object" },
-				"new_player": { type: "boolean" },
+				"player_data": { type: ["object", "null"] },
 			},
 		}),
 		databaseStats: new libLink.Request({

plugins/inventory_sync/instance.js

@@ -2,6 +2,7 @@
  * @module
  */
 "use strict";
+const libErrors = require("@clusterio/lib/errors");
 const libPlugin = require("@clusterio/lib/plugin");
 const libLuaTools = require("@clusterio/lib/lua_tools");
 
@@ -17,42 +18,141 @@ function chunkify(chunkSize, string) {
 
 class InstancePlugin extends libPlugin.BaseInstancePlugin {
 	async init() {
+		this.playersToRelease = new Set();
+		this.disconnecting = false;
+
 		// Handle IPC from scenario script
+		this.instance.server.on("ipc-inventory_sync_acquire", content => this.handleAcquire(content)
+			.catch(err => this.logger.error(`Error handling ipc-inventory_sync_acquire:\n${err.stack}`)));
+		this.instance.server.on("ipc-inventory_sync_release", content => this.handleRelease(content)
+			.catch(err => this.logger.error(`Error handling ipc-inventory_sync_release:\n${err.stack}`)));
 		this.instance.server.on("ipc-inventory_sync_upload", content => this.handleUpload(content)
 			.catch(err => this.logger.error(`Error handling ipc-inventory_sync_upload:\n${err.stack}`)));
 		this.instance.server.on("ipc-inventory_sync_download", content => this.handleDownload(content)
 			.catch(err => this.logger.error(`Error handling ipc-inventory_sync_download:\n${err.stack}`)));
 	}
 
-	async handleUpload(player) {
-		this.logger.verbose(`Uploading ${player.name} (${JSON.stringify(player).length / 1000}kB)`);
-		this.info.messages.upload.send(this.instance, {
-			instance_id: this.instance.id,
-			instance_name: this.instance.name,
-			player_name: player.name,
-			inventory: player,
-		});
+	onPrepareMasterDisconnect() {
+		this.disconnecting = true;
 	}
 
-	async handleDownload(player) {
-		this.logger.verbose(`Downloading ${player.player_name}`);
+	onMasterConnectionEvent(event) {
+		if (event === "connect") {
+			this.disconnecting = false;
+			(async () => {
+				for (let player_name of this.playersToRelease) {
+					if (!this.slave.connector.connected || this.disconnecting) {
+						return;
+					}
+					this.playersToRelease.delete(player);
+					await this.info.messages.release.send(this.instance, {
+						instance_id: this.instance.id,
+						player_name,
+					});
+				}
+			})().catch(
+				err => this.logger.error(`Unpexpected error releasing queued up players:\n${err.stack}`)
+			);
+		}
+	}
 
-		// Retrieves inventory data. If the master is offline it waits until it comes back online.
-		let response = await this.info.messages.download.send(this.instance, {
-			player_name: player.player_name,
-		});
-		if (response.new_player) {
-			await this.sendRcon(`/sc inventory_sync.welcome_new_player('${response.player_name}')`);
-		} else if (response.inventory) {
-			const chunkSize = this.instance.config.get("inventory_sync.rcon_chunk_size");
-			const chunks = chunkify(chunkSize, JSON.stringify(response.inventory));
-			this.logger.verbose(`Sending inventory for ${player.player_name} in ${chunks.length} chunks`);
-			for (let i = 0; i < chunks.length; i++) {
-				// this.logger.verbose(`Sending chunk ${i+1} of ${chunks.length}`)
-				await this.sendRcon(`/sc inventory_sync.download_inventory('${response.player_name}',` +
-					`'${libLuaTools.escapeString(chunks[i])}', ${i + 1}, ${chunks.length})`);
+	async handleAcquire(request) {
+		let response = {
+			player_name: request.player_name,
+			status: "error",
+			message: "Master is temporarily unavailable",
+		};
+
+		if (this.slave.connector.connected && !this.disconnecting) {
+			try {
+				response = {
+					player_name: request.player_name,
+					...await this.info.messages.acquire.send(this.instance, {
+						instance_id: this.instance.id,
+						player_name: request.player_name,
+					}),
+				};
+			} catch (err) {
+				if (!(err instanceof libErrors.SessionLost)) {
+					this.logger.error(`Unexpected error sending aquire request:\n${err.stack}`);
+					response.message = err.message;
+				}
 			}
 		}
+
+		let json = libLuaTools.escapeString(JSON.stringify(response));
+		await this.sendRcon(`/sc inventory_sync.acquire_response("${json}")`, true);
+	}
+
+	async handleRelease(request) {
+		if (!this.slave.connector.connected) {
+			this.playersToRelease.set(request.player_name);
+		}
+
+		try {
+			await this.info.messages.release.send(this.instance, {
+				instance_id: this.instance.id,
+				player_name: request.player_name,
+			});
+		} catch (err) {
+			if (err instanceof libErrors.SessionLost) {
+				this.playersToRelease.set(request.player_name);
+			} else {
+				this.logger.error(`Unexpected error releasing player ${request.player_name}:\n${err.stack}`);
+			}
+		}
+	}
+
+	async handleUpload(player_data) {
+		if (!this.slave.connector.connected || this.disconnecting) {
+			return;
+		}
+
+		this.logger.verbose(`Uploading ${player_data.name} (${JSON.stringify(player_data).length / 1000}kB)`);
+		try {
+			await this.info.messages.upload.send(this.instance, {
+				instance_id: this.instance.id,
+				player_name: player_data.name,
+				player_data: player_data,
+			});
+
+		} catch (err) {
+			if (!(err instanceof libErrors.SessionLost)) {
+				this.logger.error(`Unexpected error uploading inventory for ${player_data.name}:\n${err.stack}`);
+			}
+			return;
+		}
+
+		await this.sendRcon(
+			`/sc inventory_sync.confirm_upload("${player_data.name}", ${player_data.generation})`, true
+		);
+	}
+
+	async handleDownload(request) {
+		const player_name = request.player_name;
+		this.logger.verbose(`Downloading ${player_name}`);
+
+		let response = await this.info.messages.download.send(this.instance, {
+			instance_id: this.instance.id,
+			player_name,
+		});
+
+		if (!response.player_data) {
+			await this.sendRcon(`/sc inventory_sync.download_inventory('${player_name}',nil,0,0)`, true);
+			return;
+		}
+
+		const chunkSize = this.instance.config.get("inventory_sync.rcon_chunk_size");
+		const chunks = chunkify(chunkSize, JSON.stringify(response.player_data));
+		this.logger.verbose(`Sending inventory for ${player_name} in ${chunks.length} chunks`);
+		for (let i = 0; i < chunks.length; i++) {
+			// this.logger.verbose(`Sending chunk ${i+1} of ${chunks.length}`)
+			const chunk = libLuaTools.escapeString(chunks[i]);
+			await this.sendRcon(
+				`/sc inventory_sync.download_inventory('${player_name}','${chunk}',${i + 1},${chunks.length})`,
+				true
+			);
+		}
 	}
 }
 

plugins/inventory_sync/master.js

@@ -3,6 +3,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const libPlugin = require("@clusterio/lib/plugin");
+const libErrors = require("@clusterio/lib/errors");
 
 async function loadDatabase(config, logger) {
 	let itemsPath = path.resolve(config.get("master.database_directory"), "inventories.json");
@@ -13,65 +14,181 @@ async function loadDatabase(config, logger) {
 
 	} catch (err) {
 		if (err.code === "ENOENT") {
-			logger.verbose("Creating new inventory database");
+			logger.verbose("Creating new player data database");
 			return new Map();
 		}
 		throw err;
 	}
 }
 
-async function saveDatabase(masterConfig, inventories, logger) {
-	if (inventories) {
+async function saveDatabase(masterConfig, playerDatastore, logger) {
+	if (playerDatastore) {
 		let file = path.resolve(masterConfig.get("master.database_directory"), "inventories.json");
 		logger.verbose(`writing ${file}`);
-		let content = JSON.stringify(Array.from(inventories));
+		let content = JSON.stringify(Array.from(playerDatastore));
 		await fs.outputFile(file, content);
 	}
 }
 
 class MasterPlugin extends libPlugin.BaseMasterPlugin {
 	async init() {
-		this.inventories = await loadDatabase(this.master.config, this.logger);
+		this.acquiredPlayers = new Map();
+		this.playerDatastore = await loadDatabase(this.master.config, this.logger);
 		this.autosaveId = setInterval(() => {
-			saveDatabase(this.master.config, this.inventories, this.logger).catch(err => {
-				this.logger.error(`Unexpected error autosaving inventories:\n${err.stack}`);
+			saveDatabase(this.master.config, this.playerDatastore, this.logger).catch(err => {
+				this.logger.error(`Unexpected error autosaving player data:\n${err.stack}`);
 			});
 		}, this.master.config.get("inventory_sync.autosave_interval") * 1000);
 	}
 
+	async onInstanceStatusChanged(instance) {
+		let instanceId = instance.config.get("instance.id");
+		if (["unassigned", "deleted"].includes(instance.status)) {
+			for (let [playerName, acquisitionRecord] of this.acquiredPlayers) {
+				if (acquisitionRecord.instanceId === instanceId) {
+					this.acquiredPlayers.delete(playerName);
+				}
+			}
+		}
+
+		if (["unknown", "stopped"].includes(instance.status)) {
+			let timeout = this.master.config.get("inventory_sync.player_lock_timeout") * 1000;
+			for (let acquisitonRecord of this.acquiredPlayers.values()) {
+				if (acquisitonRecord.instanceId === instanceId && !acquisitonRecord.expires) {
+					acquisitonRecord.expires = Date.now() + timeout;
+				}
+			}
+		}
+
+		if (instance.status === "running") {
+			for (let acquisitonRecord of this.acquiredPlayers.values()) {
+				if (acquisitonRecord.instanceId === instanceId && acquisitonRecord.expires) {
+					delete acquisitonRecord.expires;
+				}
+			}
+		}
+	}
+
+	acquire(instanceId, playerName) {
+		let acquisitionRecord = this.acquiredPlayers.get(playerName);
+		if (
+			!acquisitionRecord
+			|| acquisitionRecord.instanceId === instanceId
+			|| !this.master.instances.has(acquisitionRecord.instanceId)
+			|| acquisitionRecord.expires && acquisitionRecord.expires < Date.now()
+		) {
+			this.acquiredPlayers.set(playerName, { instanceId });
+			return true;
+		}
+
+		return false;
+	}
+
+	async acquireRequestHandler(message) {
+		let { instance_id, player_name } = message.data;
+		if (!this.acquire(instance_id, player_name)) {
+			let acquisitionRecord = this.acquiredPlayers.get(player_name);
+			let instance = this.master.instances.get(acquisitionRecord.instanceId);
+			return {
+				status: "busy",
+				message: instance.config.get("instance.name"),
+			};
+		}
+
+		let playerData = this.playerDatastore.get(player_name);
+		return {
+			status: "acquired",
+			has_data: Boolean(playerData),
+			generation: playerData ? playerData.generation : 0,
+		};
+	}
+
+	async releaseRequestHandler(message) {
+		let { instance_id, player_name } = message.data;
+		let acquisitionRecord = this.acquiredPlayers.get(player_name);
+		if (!acquisitionRecord) {
+			return;
+		}
+
+		if (acquisitionRecord.instanceId === instance_id) {
+			this.acquiredPlayers.delete(player_name);
+		}
+	}
+
 	async uploadRequestHandler(message) {
-		this.logger.verbose(`Saving inventory for ${message.data.player_name}`);
-		this.inventories.set(message.data.player_name, message.data.inventory);
+		let { instance_id, player_name, player_data } = message.data;
+		let instanceName = this.master.instances.get(instance_id).config.get("instance.name");
+		let store = true;
+		let acquisitionRecord = this.acquiredPlayers.get(player_name);
+		if (!acquisitionRecord) {
+			this.logger.warn(`${instanceName} uploaded ${player_name} without an acquisition`);
+			// Allow upload in this case as it might come from a crashed instance that restarted and is now
+			// uploading the player data for all the players that were online during the last autosave.
+
+		} else if (acquisitionRecord.instanceId !== instance_id) {
+			this.logger.warn(`${instanceName} uploaded ${player_name} while another instance has acquired it`);
+			store = false;
+
+		} else {
+			this.acquiredPlayers.delete(player_name);
+		}
+
+		this.acquiredPlayers.delete(player_name);
+		let oldPlayerData = this.playerDatastore.get(player_name);
+		if (store && oldPlayerData && oldPlayerData.generation >= player_data.generation) {
+			this.logger.warn(
+				`${instanceName} uploaded generation ${player_data.generation} while the stored` +
+				`generation is ${oldPlayerData.generation} for ${player_name}`
+			);
+			store = false;
+		}
+
+		if (store) {
+			this.logger.verbose(`Received player data for ${player_name} from ${instanceName}`);
+			this.playerDatastore.set(player_name, player_data);
+		}
 	}
 
 	async downloadRequestHandler(message) {
-		this.logger.verbose(`Downloading inventory for ${message.data.player_name}`);
-		let inventory = this.inventories.get(message.data.player_name);
+		let { instance_id, player_name } = message.data;
+		let instanceName = this.master.instances.get(instance_id).config.get("instance.name");
+
+		let acquisitionRecord = this.acquiredPlayers.get(player_name);
+		if (!acquisitionRecord) {
+			this.logger.warn(`${instanceName} downloaded ${player_name} without an acquisition`);
+		} else if (acquisitionRecord.instanceId !== instance_id) {
+			this.logger.warn(`${instanceName} downloaded ${player_name} while another instance has acquired it`);
+		}
+
+		let playerData = this.playerDatastore.get(player_name);
+		if (!playerData) {
+			throw new libErrors.RequestError(`No player data exists for ${player_name}`);
+		}
+
+		this.logger.verbose(`Sending player data for ${player_name} to ${instanceName}`);
 		return {
-			player_name: message.data.player_name,
-			inventory: inventory || {},
-			new_player: !inventory,
+			player_data: playerData || null,
 		};
 	}
 
 	async onShutdown() {
 		clearInterval(this.autosaveId);
-		await saveDatabase(this.master.config, this.inventories, this.logger);
+		await saveDatabase(this.master.config, this.playerDatastore, this.logger);
 	}
 
 	async databaseStatsRequestHandler(message) {
-		let inventories = Array.from(this.inventories.keys())
+		let playerDatastore = Array.from(this.playerDatastore.keys())
 			.map(name => ({
 				name,
-				length: JSON.stringify(this.inventories.get(name)).length,
+				length: JSON.stringify(this.playerDatastore.get(name)).length,
 			}))
 			.sort((a, b) => a.length - b.length);
 		return {
-			database_size: inventories.map(x => x.length).reduce((a, b) => b - a, 0),
-			database_entries: inventories.length,
+			database_size: playerDatastore.map(x => x.length).reduce((a, b) => b - a, 0),
+			database_entries: playerDatastore.length,
 			largest_entry: {
-				name: inventories[0] && inventories[0].name || "-",
-				size: inventories[0] && inventories[0].length || 0,
+				name: playerDatastore[0] && playerDatastore[0].name || "-",
+				size: playerDatastore[0] && playerDatastore[0].length || 0,
 			},
 		};
 	}

plugins/inventory_sync/module/commands.lua

@@ -1,17 +0,0 @@
-local save_crafts = require("modules/inventory_sync/save_crafts")
-local restore_crafts = require("modules/inventory_sync/restore_crafts")
-
-function add_commands()
-	--[[
-		Command to serialize crafting queue for transport.
-		When serialized, the crafting queue is cleared and stored in global. The items received are cleared as well.
-		Upon leaving, the value in global is cleared and sent with the inventory to the master.
-		Upon joining a game the crafting queue is added to the end of the existing crafting queue.
-	]]
-	commands.add_command("save-crafts", "Save crafting queue for transport to a different server", save_crafts)
-	commands.add_command("csc", "Shorthand for /save-crafts", save_crafts)
-	commands.add_command("restore-crafts", "Load crafting queue from global", restore_crafts)
-	commands.add_command("crc", "Shorthand for /restore-crafts", restore_crafts)
-end
-
-return add_commands

plugins/inventory_sync/module/define_player_inventories.lua

@@ -1,10 +1,11 @@
 local inventories = {
-	defines.inventory.character_main,
-	defines.inventory.character_guns,
-	defines.inventory.character_ammo,
-	defines.inventory.character_armor,
-	defines.inventory.character_vehicle,
-	defines.inventory.character_trash,
+	-- Armor must be before main or the sync will fail when the inventory
+	-- bonus slots from the armor are in use.
+	armor = defines.inventory.character_armor,
+	main = defines.inventory.character_main,
+	guns = defines.inventory.character_guns,
+	ammo = defines.inventory.character_ammo,
+	trash = defines.inventory.character_trash,
 }
 
 return inventories

plugins/inventory_sync/module/define_player_stat_keys.lua

@@ -14,10 +14,4 @@ return {
 	"character_maximum_following_robot_count_bonus",
 	"character_health_bonus",
 	"character_personal_logistic_requests_enabled",
-	"build_distance",
-	"drop_item_distance",
-	"reach_distance",
-	"item_pickup_distance",
-	"loot_pickup_distance",
-	"resource_reach_distance",
 }

plugins/inventory_sync/module/download_inventory.lua

@@ -1,107 +1,139 @@
 local progress_dialog = require("modules/inventory_sync/gui/progress_dialog")
-local serialize = require("modules/clusterio/serialize")
-local load_crafting_queue = require("modules/inventory_sync/load_crafting_queue")
-local inventories = require("modules/inventory_sync/define_player_inventories")
 local ensure_character = require("modules/inventory_sync/ensure_character")
-local player_stat_keys = require("modules/inventory_sync/define_player_stat_keys")
+local serialize = require("modules/inventory_sync/serialize")
+local load_crafting_queue = require("modules/inventory_sync/load_crafting_queue")
 
-function download_inventory(player_name, data, number, total)
+local function download_inventory(player_name, data, number, total)
 	local player = game.get_player(player_name)
 	if player == nil then
-		log("Player not found! "..player_name)
+		rcon.print("Player " .. player_name .. " does not exist")
 		return
 	end
-	if number then
-		if global.inventory_sync.download_cache[player_name] == nil then
-			global.inventory_sync.download_cache[player_name] = ""
-		end
-		-- Append data to download cache
-		global.inventory_sync.download_cache[player_name] = global.inventory_sync.download_cache[player_name] .. data
+
+	local record = global.inventory_sync.active_downloads[player_name]
+	if record == nil then
+		rcon.print("No active download is in progress for " .. player_name)
+		return
 	end
 
-	if number ~= total then
-		-- Show progress in console
-		-- player.print("Downloaded "..(number + 1).."/"..total.." parts")
-		-- Show progress in GUI
-		progress_dialog(player, number+1, total)
+	local player_record = global.inventory_sync.players[player_name]
+	if record.restart then
+		rcon.print("Restarting outdated download")
+		inventory_sync.initiate_inventory_download(player, player_record, record.generation)
+		return
+	end
 
-		-- Request next segment
-		rcon.print("Segment downloaded")
+	if total == 0 then
+		-- Download was requested but the player had no data stored.
+		log("ERROR: Inventory sync failed, got empty player data for " .. player.name .. " from the master")
+		player.print("ERROR: Inventory sync failed, got empty player data from the master")
 
-		-- Reset sync_start_tick to indicate master connection
-		global.inventory_sync.players[player_name].sync_start_tick = game.tick
-	else
-		-- Stop tracking master connection status, sync is complete
-		global.inventory_sync.players[player_name].sync_start_tick = 0
-
-		-- Handle dirty inventory
-		if global.inventory_sync.players[player_name].dirty_inventory then
-			clean_dirty_inventory(player)
-		end
-
-		-- Recreate player character
+		-- Give the player a character and pretend that's the synced player data
 		ensure_character(player)
 
-		-- Remove freeplay skip cutscene label
-		if player.gui.screen.skip_cutscene_label then
-			player.gui.screen.skip_cutscene_label.destroy()
+		-- Restore player position in case they moved while being spectator
+		if record.surface and record.position then
+			player.teleport(record.position, record.surface)
 		end
 
-		local character = player.character or player.cutscene_character
-
-		-- Load downloaded inventory
-		local serialized_player = game.json_to_table(global.inventory_sync.download_cache[player_name])
-		global.inventory_sync.download_cache[player_name] = nil -- remove data to lower mapsize
-
-		-- Load inventories
-		for _, inv in pairs(inventories) do
-			local inventory = player.get_inventory(inv)
-			if inventory ~= nil and serialized_player.inventories[tostring(inv)] ~= nil then
-				serialize.deserialize_inventory(inventory, serialized_player.inventories[tostring(inv)])
-			end
-		end
-
-		-- Load personal logistics slots
-		for i = 1, 200 do
-			local slot = serialized_player.personal_logistic_slots[tostring(i+1)] -- 1 is empty to force array to be spare
-			if slot ~= nil then
-				player.set_personal_logistic_slot(i, slot)
-			end
-		end
-
-		-- Load hotbar, don't overwrite empty slots
-		for i = 1, 100 do
-			-- if serialized_player.hotbar[i] ~= nil then
-			player.set_quick_bar_slot(i, game.item_prototypes[serialized_player.hotbar[i]])
-			-- end
-		end
-
-		-- Load crafting queue
-		load_crafting_queue(serialized_player.crafting_queue, player)
-
-		-- Misc
-		player.cheat_mode = serialized_player.cheat_mode
-		player.force = serialized_player.force -- Force by name as string
-		player.tag = serialized_player.tag
-		player.color = serialized_player.color
-		player.chat_color = serialized_player.chat_color
-
-		-- Player stats
-		for _,v in pairs(player_stat_keys) do
-			character[v] = serialized_player[v]
-		end
-
-		if serialized_player.flashlight then
-			player.enable_flashlight()
-		else
-			player.disable_flashlight()
-		end
-
-		local startTick = global.inventory_sync.download_start_tick[player.name]
-		local log_line = "Imported inventory for "..player_name.." in "..game.tick - startTick.." ticks"
-		player.print(log_line)
-		log("[inventory_sync] "..log_line)
+		global.inventory_sync.active_downloads[player_name] = nil
+		player_record.dirty = player.connected
+		player_record.sync = true
+		player_record.generation = record.generation
+		return
 	end
+
+	record.data = record.data .. data
+
+	-- Show progress in console
+	-- player.print("Downloaded "..(number + 1).."/"..total.." parts")
+	if number ~= total then
+		-- Show progress in GUI
+		progress_dialog.display(player, number, total)
+
+		-- Update activity to indicate this session is still going
+		record.last_active = game.ticks_played
+		return
+	end
+
+	progress_dialog.remove(player)
+
+	-- Editor mode is not supported.
+	if player.controller_type == defines.controllers.editor then
+		player.toggle_map_editor()
+	end
+
+	if player.controller_type == defines.controllers.editor then
+		log("ERROR: Inventory sync failed, unable to switch " .. player.name .. " out of editor mode")
+		player.print("ERROR: Inventory sync failed, unable to switch out of editor mode")
+		return
+	end
+
+	-- Stash temporary inventory if it exists
+	local stashed_corpse
+	if player_record.dirty and player.character then
+		local character = player.character
+		local surface = character.surface
+		local position = character.position
+		local corpse_name = character.prototype.character_corpse.name
+		player.character = nil
+		character.die()
+		stashed_corpse = surface.find_entity(corpse_name, position)
+	end
+
+	-- Deserialize downloaded player data
+	local serialized_player = game.json_to_table(record.data)
+	serialize.deserialize_player(player, serialized_player)
+
+	-- Restore player position in case they moved while being spectator
+	if record.surface and record.position then
+		player.teleport(record.position, record.surface)
+	end
+
+	-- Load crafting queue
+	if player.character then
+		load_crafting_queue(serialized_player.crafting_queue, player)
+	end
+
+	-- Transfer items from stashed inventory
+	if stashed_corpse then
+		local main = player.get_main_inventory()
+		local stash = stashed_corpse.get_inventory(defines.inventory.character_corpse)
+		if main then
+			for i = 1, #stash do
+				-- Try transfering a stack
+				local source = stash[i]
+				if source and source.valid_for_read then
+					local target = main.find_empty_stack()
+					if not target then
+						break
+					end
+					target.transfer_stack(source)
+				end
+			end
+		end
+
+		if stash.is_empty() then
+			stashed_corpse.destroy()
+		else
+			player.print(
+				"Your temprorary inventory did not fit into your synced one and the remaining items have " ..
+				"been placed in a corpse below you."
+			)
+		end
+	end
+
+	-- Player may have left before the download completed
+	player_record.dirty = player.connected
+	player_record.sync = true
+	player_record.generation = serialized_player.generation
+
+	-- Remove download session and data
+	global.inventory_sync.active_downloads[player_name] = nil
+
+	local ticks = game.ticks_played - record.started
+	player.print("Imported player data in " .. ticks .. " ticks")
+	log("Imported player data for " .. player_name .. " in " .. ticks .. " ticks")
 end
 
 return download_inventory

plugins/inventory_sync/module/ensure_character.lua

@@ -1,9 +1,11 @@
+-- Ensures that the given player has a character if one doesn't exist
 local function ensure_character(player)
-	if player.character == nil then
-		player.set_controller {
-			type = defines.controllers.god,
-		}
-		player.create_character()
+	if not player.character and not player.ticks_to_respawn then
+		if player.controller_type ~= defines.controllers.ghost then
+			player.set_controller({ type = defines.controllers.ghost })
+		end
+		player.ticks_to_respawn = 0
+		player.ticks_to_respawn = nil
 	end
 end
 

plugins/inventory_sync/module/gui/dialog_failed_download.lua

@@ -1,5 +1,8 @@
--- Show inventory download progressbar
-function dialog_failed_download(player)
+local ensure_character = require("modules/inventory_sync/ensure_character")
+
+local dialog_failed_download = {}
+
+function dialog_failed_download.create(player, acquire_response)
 	if player.gui.screen.dialog_failed_download then
 		player.gui.screen.dialog_failed_download.destroy()
 	end
@@ -15,10 +18,15 @@ function dialog_failed_download(player)
 	}
 	titlebar.drag_target = frame
 
+	local title_caption = "Inventory sync download failed"
+	if acquire_response.status == "busy" then
+		title_caption = "Inventory is in use on " .. acquire_response.message
+	end
+
 	local title = titlebar.add {
 		type = "label",
 		style = "frame_title",
-		caption = "Inventory sync download failed",
+		caption = title_caption,
 	}
 	title.drag_target = frame
 
@@ -39,22 +47,101 @@ function dialog_failed_download(player)
 	}
 	content.style.width = 400
 
+	local reason
+	if acquire_response.status == "timeout" then
+		reason = "Timed out fetching inventory from the master server."
+
+	elseif acquire_response.status == "busy" then
+		reason =
+			"Your synced inventory is in in use on " .. acquire_response.message .. ". " ..
+			"If you are logged in there you will have to log out there before you can sync the inventory here. " ..
+			"If you just logged out from there it might take some time before the inventory is available."
+
+	elseif acquire_response.status == "error" then
+		reason = "Error fetching inventory from the master server: " .. acquire_response.message .. "."
+
+	else
+		reason = "Unknown status " .. acquire_response.status .. " fetching inventory from the master server."
+	end
+
 	local p1 = content.add {
 		type = "label",
-		caption = "The inventory download couldn't complete. You can choose to keep playing with an empty inventory until the connection is reestablished and the inventory download completes. At that point your temporary inventory will be put in a chest.",
+		caption = reason
 	}
 	p1.style.single_line = false
-	p1.style.bottom_margin = 8
 
-	local abort = content.add {
+	local p2 = content.add {
+		type = "label",
+		caption =
+			"You may choose to retry fetching the inventory or play with a temporary inventory which will be merged " ..
+			"with your synced inventory the next time you join this server."
+		,
+	}
+	p2.style.single_line = false
+	p2.style.top_margin = 8
+
+	local dialog_row = frame.add {
+		type = "flow",
+	}
+	dialog_row.style.top_margin = 8
+	dialog_row.drag_target = frame
+
+	local abort = dialog_row.add {
 		name = "inventory_sync_failed_download_abort",
 		type = "button",
-		caption = "Continue with new inventory",
-		tooltip = "Create a new empty temporary inventory. Items you pick up will be placed in a chest when the sync completes.",
+		style = "back_button",
+		caption = "Use temporary inventory",
+		tooltip = "Items you obtain here will be merged with your synced inventory the next time you join this server.",
+	}
+
+	local dialog_filler = dialog_row.add {
+		type = "empty-widget",
+		style = "draggable_space_header",
+	}
+	dialog_filler.style.horizontally_stretchable = "on"
+	dialog_filler.style.right_margin = 4
+	dialog_filler.style.height = 28
+	dialog_filler.drag_target = frame
+
+	local retry = dialog_row.add {
+		name = "inventory_sync_failed_download_retry",
+		type = "button",
+		style = "forward_button",
+		caption = "Retry",
 	}
 
 	frame.force_auto_center()
 	player.opened = frame
 end
 
+dialog_failed_download.events = {
+	[defines.events.on_gui_click] = function(event)
+		if not event.element.valid then
+			return
+		end
+
+		if event.element.name == "inventory_sync_failed_download_abort" then
+			-- Give the player a charatcer and let them play with that
+			local player = game.get_player(event.player_index)
+			ensure_character(player)
+			player.gui.screen.dialog_failed_download.destroy()
+			global.inventory_sync.players[player.name].dirty = true
+			global.inventory_sync.players[player.name].sync = false
+
+		elseif event.element.name == "inventory_sync_failed_download_retry" then
+			-- Retry acquiring the player from the master
+			local player = game.get_player(event.player_index)
+			inventory_sync.acquire(player)
+			player.gui.screen.dialog_failed_download.destroy()
+		end
+	end,
+
+	[defines.events.on_player_left_game] = function(event)
+		local player = game.get_player(event.player_index)
+		if player.gui.screen.dialog_failed_download then
+			player.gui.screen.dialog_failed_download.destroy()
+		end
+	end,
+}
+
 return dialog_failed_download

plugins/inventory_sync/module/gui/handle_gui_events.lua

@@ -1,25 +0,0 @@
-local ensure_character = require("modules/inventory_sync/ensure_character")
-
--- on_gui_click
-local handle_gui_events = function(event)
-	if not event.element.valid then
-		return
-	end
-	local player = game.get_player(event.player_index)
-	if event.element.name == "inventory_sync_failed_download_abort" then
-		-- Give the player a new inventory and mark it as dirty
-		-- Show dirty status and an explanation in a GUI
-
-		-- Remove GUI element
-		player.gui.screen.dialog_failed_download.destroy()
-		-- Handle inventory dirtying
-		if not global.inventory_sync.players[player.name].dirty_inventory then
-			global.inventory_sync.players[player.name].dirty_inventory = true
-
-			-- Restore player character
-			ensure_character(player)
-		end
-	end
-end
-
-return handle_gui_events

plugins/inventory_sync/module/gui/progress_dialog.lua

@@ -1,12 +1,7 @@
 -- Show inventory download progressbar
-function open_dialog(player, progress, total)
-	if player.gui.screen.inventory_sync_progress then
-		player.gui.screen.inventory_sync_progress.destroy()
-	end
-	if progress == total then
-		return -- Hide dialog on completed download
-	end
+local progress_dialog = {}
 
+function progress_dialog.create(player, progress, total)
 	local frame = player.gui.screen.add {
 		name = "inventory_sync_progress",
 		type = "frame",
@@ -34,15 +29,6 @@ function open_dialog(player, progress, total)
 	filler.style.height = 24
 	filler.drag_target = frame
 
-	titlebar.add {
-		name = "inventory_sync_progress_close_button",
-		type = "sprite-button",
-		sprite = "utility/close_white",
-		hovered_sprite = "utility/close_black",
-		clicked_sprite = "utility/close_black",
-		style = "frame_action_button",
-	}
-
 	local content = frame.add {
 		name = "inventory_sync_progress_dialog_content",
 		type = "frame",
@@ -53,7 +39,7 @@ function open_dialog(player, progress, total)
 
 	local p1 = content.add {
 		type = "label",
-		caption = "Downloading inventory chunk"..progress.."/"..total,
+		caption = "Downloading inventory chunk " .. progress .. " / " .. total,
 	}
 	p1.style.single_line = false
 	p1.style.bottom_margin = 8
@@ -68,4 +54,45 @@ function open_dialog(player, progress, total)
 	player.opened = frame
 end
 
-return open_dialog
+function progress_dialog.remove(player)
+	if player.gui.screen.inventory_sync_progress then
+		player.gui.screen.inventory_sync_progress.destroy()
+	end
+end
+
+function progress_dialog.update(player, progress, total)
+	-- Be defensive here in case an old GUI is present
+	local frame = player.gui.screen.inventory_sync_progress
+	if not frame then
+		return false
+	end
+
+	local content = frame.inventory_sync_progress_dialog_content
+	if not content then
+		return false
+	end
+
+	local p1 = content.children[1]
+	if not p1 or p1.type ~= "label" then
+		return false
+	end
+
+	local progressbar = content.children[2]
+	if not progressbar or progressbar.type ~= "progressbar" then
+		return false
+	end
+
+	p1.caption = "Downloading inventory chunk " .. progress .. " / " .. total
+	progressbar.value = progress / total
+
+	return true
+end
+
+function progress_dialog.display(player, progress, total)
+	if not progress_dialog.update(player, progress, total) then
+		progress_dialog.remove(player)
+		progress_dialog.create(player, progress, total)
+	end
+end
+
+return progress_dialog

plugins/inventory_sync/module/gui/welcome_new_player.lua

@@ -1,9 +0,0 @@
-local function welcome_new_player(player_name)
-	local player = game.get_player(player_name)
-	if player == nil then return end
-	player.print("Welcome to the server!")
-	player.print("Your inventory will be automatically synchronized between servers in this cluster.")
-	player.print("To improve sync performance, please avoid storing blueprints in your inventory.")
-end
-
-return welcome_new_player

plugins/inventory_sync/module/inventory_sync.lua

@@ -1,72 +1,74 @@
 --[[
 
-When a player leaves the game, serialize their inventory and upload it to the master.
+Inventory Sync
 
-When a player joins the game, wait until they have a character, then send a request to
-the master server for the players inventory.
-
-When the master sends an inventory, parse it and give it to the player.
-If the master responds that its a new player, show a welcome message.
+When a player leaves the game, serialize the player state and upload it to the master.
+When a player joins the game, request the player data from master and deserialize the player state from it.
 
 ]]
 
 local clusterio_api = require("modules/clusterio/api")
-local add_commands = require("modules/inventory_sync/commands")
 local upload_inventory = require("modules/inventory_sync/upload_inventory")
 local download_inventory = require("modules/inventory_sync/download_inventory")
-local welcome_new_player = require("modules/inventory_sync/gui/welcome_new_player")
-local handle_gui_events = require("modules/inventory_sync/gui/handle_gui_events")
+local progress_dialog = require("modules/inventory_sync/gui/progress_dialog")
 local dialog_failed_download = require("modules/inventory_sync/gui/dialog_failed_download")
-local clean_dirty_inventory = require("modules/inventory_sync/script/clean_dirty_inventory")
 
 
+-- Returns true if the player is currently in a cutscene
+local function is_in_cutscene(player)
+	return player.controller_type == defines.controllers.cutscene
+end
+
+-- Create player record for bookeeping
 local function create_player(player, dirty)
 	global.inventory_sync.players[player.name] = {
-		dirty_inventory = dirty, -- Player has a temporary non-synced inventory that should be persisted
-		sync_start_tick = 0, -- To track download failure timeout
+		dirty = dirty, -- Player inventory has changes that should be persisted
+		sync = false, -- Player inventory is synced from the master
+		generation = 0,
 	}
 end
 
+-- Remove all stored player data by this module
 local function remove_player(player)
-	-- Remove inventory download performance counter
-	global.inventory_sync.download_start_tick[player.name] = nil
-	-- Remove stored crafting queue
 	global.inventory_sync.saved_crafting_queue[player.name] = nil
-	-- Remove other player data
+	global.inventory_sync.players_waiting_for_acquire[player.name] = nil
+	global.inventory_sync.players_in_cutscene_to_sync[player.name] = nil
+	global.inventory_sync.active_downloads[player.name] = nil
+	global.inventory_sync.active_uploads[player.name] = nil
 	global.inventory_sync.players[player.name] = nil
 end
 
 
 inventory_sync = {}
-inventory_sync.welcome_new_player = welcome_new_player
 -- This function is called by instance.js as /sc inventory_sync.download_inventory("danielv", Escaped JSON string, package_number, total_packages_count) with data from the master
 inventory_sync.download_inventory = download_inventory
 -- This function is called internally when a player leaves the game for their inventory to upload to the master
 inventory_sync.upload_inventory = upload_inventory
 
-inventory_sync.add_commands = add_commands
 inventory_sync.events = {}
 inventory_sync.events[clusterio_api.events.on_server_startup] = function(event)
 	-- Set up global table
 	if not global.inventory_sync then
 		global.inventory_sync = {}
 	end
-	if not global.inventory_sync.download_start_tick then
-		global.inventory_sync.download_start_tick = {}
-	end
 	if not global.inventory_sync.saved_crafting_queue then
 		global.inventory_sync.saved_crafting_queue = {}
 	end
-	if not global.inventory_sync.download_cache then
-		global.inventory_sync.download_cache = {}
-	end
-	if not global.inventory_sync.players_waiting_for_download then
-		global.inventory_sync.players_waiting_for_download = {}
-	end
+
 	if not global.inventory_sync.players then
 		global.inventory_sync.players = {}
 	end
 
+	global.inventory_sync.players_waiting_for_acquire = {}
+	global.inventory_sync.players_in_cutscene_to_sync = {}
+
+	if not global.inventory_sync.active_downloads then
+		global.inventory_sync.active_downloads = {}
+	end
+	if not global.inventory_sync.active_uploads then
+		global.inventory_sync.active_uploads = {}
+	end
+
 	for _, player in pairs(game.players) do
 		if not global.inventory_sync.players[player.name] then
 			create_player(player, true)
@@ -83,23 +85,157 @@ inventory_sync.events[defines.events.on_player_created] = function(event)
 	create_player(game.get_player(event.player_index), false)
 end
 
-inventory_sync.events[defines.events.on_gui_click] = handle_gui_events
+function inventory_sync.acquire(player)
+	global.inventory_sync.players_waiting_for_acquire[player.name] = {
+		start_tick = game.ticks_played,
+	}
+	clusterio_api.send_json("inventory_sync_acquire", {
+		player_name = player.name,
+	})
+end
+
+function inventory_sync.check_players_waiting_for_acquire()
+	for player_name, record in pairs(global.inventory_sync.players_waiting_for_acquire) do
+		-- Check if the acquire player request timed out
+		if game.ticks_played > record.start_tick + 600 then
+			inventory_sync.acquire_response(game.table_to_json({
+				status = "timeout",
+				player_name = player_name,
+			}))
+		end
+	end
+end
+
+function inventory_sync.acquire_response(data)
+	local response = game.json_to_table(data)
+	if not global.inventory_sync.players_waiting_for_acquire[response.player_name] then
+		clusterio_api.send_json("inventory_sync_release", { player_name = response.player_name })
+		return
+	end
+	global.inventory_sync.players_waiting_for_acquire[response.player_name] = nil
+
+	local player = game.get_player(response.player_name)
+	if is_in_cutscene(player) then
+		response.player = player
+		global.inventory_sync.players_in_cutscene_to_sync[response.player_name] = response
+	else
+		inventory_sync.sync_player(response)
+	end
+end
+
+function inventory_sync.check_players_in_cutscene()
+	for player_name, response in pairs(global.inventory_sync.players_in_cutscene_to_sync) do
+		if not is_in_cutscene(response.player) then
+			global.inventory_sync.players_in_cutscene_to_sync[player_name] = nil
+			inventory_sync.sync_player(response)
+		end
+	end
+end
+
+function inventory_sync.sync_player(acquire_response)
+	local player = game.get_player(acquire_response.player_name)
+
+	-- Editor mode is not supported.
+	if player.controller_type == defines.controllers.editor then
+		player.toggle_map_editor()
+	end
+
+	if player.controller_type == defines.controllers.editor then
+		log("ERROR: Inventory sync failed, unable to switch " .. player.name .. " out of editor mode")
+		player.print("ERROR: Inventory sync failed, unable to switch out of editor mode")
+		return
+	end
+
+	local download_record = global.inventory_sync.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
+			game.print("Continuing active download")
+			return
+		end
+
+		-- This is the excaptional case where a long download started from
+		-- the previous time the player joined is still ongoing and we just
+		-- received a message saying that it's not valid any more.
+
+		if acquire_response.status == "error" or acquire_response.status == "timeout" then
+			-- Let the current download finish or fail on its own, we may end up
+			-- duplicating items or giving the player a stale inventory here
+			-- but it's better than throwing away all the downloaded data.
+			return
+		end
+
+		if acquire_response.status == "acquired" then
+			-- The stored data on the master server has changed since the
+			-- download was started. Restart it.
+			game.print("Restarting download")
+			download_record.restart = true
+			return
+		end
+
+		if acquire_response.status == "busy" then
+			-- Somehow another instance has claimed the player while we're
+			-- downloading it. Abort it and send failure to the player.
+			global.inventory_sync.active_downloads[acquire_response.player_name] = nil
+		end
+	end
+
+	local player_record = global.inventory_sync.players[acquire_response.player_name]
+	if acquire_response.status ~= "acquired" then
+		if player_record.sync and not player_record.dirty then
+			if player.controller_type == defines.controllers.character then
+				local character = player.character
+				player.set_controller({ type = defines.controllers.spectator })
+				character.destroy()
+			end
+			player_record.sync = false
+		end
+		dialog_failed_download.create(player, acquire_response)
+		return
+	end
+
+	if not player_record.sync or acquire_response.generation > player_record.generation then
+		inventory_sync.initiate_inventory_download(player, player_record, acquire_response.generation)
+
+	else
+		if acquire_response.generation == 0 then
+			player.print(
+				"Your inventory will be automatically synchronized between servers in this cluster. " ..
+				"To improve sync performance, please avoid storing large blueprints in your inventory.",
+				{0.75, 0.75, 1}
+			)
+		end
+
+		player_record.sync = true
+		player_record.dirty = true
+	end
+end
 
 -- Download inventory from master
 inventory_sync.events[defines.events.on_player_joined_game] = function(event)
-	-- Add player to download queue. Inventory won't be downloaded before the player has a character
-	global.inventory_sync.players_waiting_for_download[game.get_player(event.player_index).name] = true
-	-- Don't wait for on_nth_tick if not required
-	inventory_sync.check_player_character_before_download()
+	-- Send acquire request even if an active download is currently in plogress
+	local player = game.get_player(event.player_index)
+	inventory_sync.acquire(player, false)
+
+	-- Clear active upload if it existsi
+	global.inventory_sync.active_uploads[player.name] = nil
 end
 
 inventory_sync.on_nth_tick = {}
 inventory_sync.on_nth_tick[33] = function(event)
-	-- Periodically check if player has a character to download to. False in cutscenes.
-	inventory_sync.check_player_character_before_download()
+	-- Periodically check players currently in the process of syncing
+	inventory_sync.check_players_waiting_for_acquire()
+	inventory_sync.check_players_in_cutscene()
+	inventory_sync.check_active_downloads()
+	inventory_sync.check_active_uploads()
+end
 
-	-- Check if download has failed and we should create a dirty inventory
-	inventory_sync.check_inventory_download_failed()
+inventory_sync.events[defines.events.on_cutscene_waypoint_reached] = function(event)
+	inventory_sync.check_players_in_cutscene()
+end
+
+inventory_sync.events[defines.events.on_cutscene_cancelled] = function(event)
+	inventory_sync.check_players_in_cutscene()
 end
 
 -- Upload inventory when a player leaves the game. Triggers on restart after crash if player was online during crash.
@@ -110,63 +246,118 @@ inventory_sync.events[defines.events.on_pre_player_left_game] = function(event)
 		return
 	end
 
-	local player_index = event.player_index
-	local player = game.get_player(player_index)
-	-- Avoid uploading for unsynced players (due to not having a character, ex during freeplay cutscene)
-	if global.inventory_sync.players_waiting_for_download[player.name] == nil then
-		inventory_sync.upload_inventory(player_index)
-		global.inventory_sync.download_cache[player_index] = "" -- Clear cache to avoid leaking memory to people quitting during download
+	local player = game.get_player(event.player_index)
+	local player_record = global.inventory_sync.players[player.name]
+
+	-- If player has an active download release our acquisition but let the download continue
+	if global.inventory_sync.active_downloads[player.name] then
+		clusterio_api.send_json("inventory_sync_release", { player_name = player.name })
+		return
 	end
-	global.inventory_sync.players_waiting_for_download[player.name] = nil
+
+	if not player_record.dirty or not player_record.sync then
+		return
+	end
+
+	player_record.generation = player_record.generation + 1
+	global.inventory_sync.active_uploads[player.name] = {
+		last_attempt = game.ticks_played,
+		timeout = math.random(600, 1200), -- Start with 10-20 seconds for the timeout
+	}
+	inventory_sync.upload_inventory(player, player_record)
 end
 
-function inventory_sync.check_player_character_before_download()
-	-- Check if players waiting for download have a valid character
-	for name in pairs(global.inventory_sync.players_waiting_for_download) do
-		local player = game.get_player(name)
-		if player.character ~= nil then
-			global.inventory_sync.players_waiting_for_download[name] = nil
-			inventory_sync.initiate_inventory_download(player)
-		end
+-- Invoked by the instance code to confirm the master has received the uploaded data
+function inventory_sync.confirm_upload(player_name, generation)
+	local player = game.get_player(player_name)
+	if not player or player.connected then
+		log("no player, or player connected")
+		return
 	end
+	local player_record = global.inventory_sync.players[player.name]
+	if not player_record or player_record.generation ~= generation then
+		return
+	end
+
+	if not player_record.dirty then
+		log("ERROR: Received upload confirmation for " .. player_name .. " without the dirty flag")
+	end
+	if not player_record.sync then
+		log("ERROR: Received upload confirmation for " .. player_name .. " without the sync flag")
+	end
+	log("Confirmed upload of " .. player_name)
+	global.inventory_sync.active_uploads[player_name] = nil
+	player_record.dirty = false
 end
 
-function inventory_sync.initiate_inventory_download(player)
+function inventory_sync.initiate_inventory_download(player, player_record, generation)
 	player.print("Initiating inventory download...")
-	global.inventory_sync.download_start_tick[player.name] = game.tick
-	global.inventory_sync.download_cache[player.name] = ""
+	local record = {
+		started = game.ticks_played,
+		last_active = game.ticks_played,
+		generation = generation,
+		data = ""
+	}
+	global.inventory_sync.active_downloads[player.name] = record
+
 	clusterio_api.send_json("inventory_sync_download", {
 		player_name = player.name
 	})
 
-	-- Handle dirty inventories. A dirty inventory is a temporary inventory that has been used while waiting
-	-- for sync. It should be taken care of in some way, like putting it in a corpse or a chest.
-	if global.inventory_sync.players[player.name].dirty_inventory then
-		clean_dirty_inventory(player)
+	-- If this is a synced player turn them into a spectator while the
+	-- player data is downloading
+	if player_record.sync then
+		local character = player.character
+		player.set_controller({ type = defines.controllers.spectator })
+		if character ~= nil then character.destroy() end
+
+		-- Store original position to teleport back to
+		record.surface = player.surface
+		record.position = player.position
+
+		-- Indicate this state shouldn't be persisted
+		player_record.dirty = false
 	end
 
-	-- Set player into ghost mode for duration of download
-	local character = player.character
-	player.set_controller {
-		type = defines.controllers.ghost,
-	}
-	if character ~= nil then character.destroy() end
-
-	-- Start timeout for detecting master connection failure
-	global.inventory_sync.players[player.name].sync_start_tick = game.tick
+	progress_dialog.display(player, 0, 1)
 end
-function inventory_sync.check_inventory_download_failed()
-	-- Used for letting the master report the inventory download failing, like when the master is offline or an
-	-- error occurs somewhere in the server code. The player will be returned to a playable state.
-	for name, data in pairs(global.inventory_sync.players) do
-		if data.sync_start_tick ~= 0 and data.sync_start_tick <= game.tick - 600 then
-			-- Stop refreshing timer
-			data.sync_start_tick = 0
 
-			local player = game.get_player(name)
-			player.print("Inventory download failed due to master connection")
-			-- Show GUI with option to retry or abort download
-			dialog_failed_download(player)
+function inventory_sync.check_active_downloads()
+	-- Used for detecting the inventory download failing, like when the map is saved and
+	-- re-loaded in the middle of a download. The player will be returned to a playable state.
+	for player_name, record in pairs(global.inventory_sync.active_downloads) do
+		if record.last_active <= game.ticks_played - 600 then
+			global.inventory_sync.active_downloads[player_name] = nil
+			local player = game.get_player(player_name)
+			if player then
+				log("ERROR: Inventory download failed for " .. player_name)
+
+				if player.connected then
+					-- Show GUI with option to retry or abort download
+					dialog_failed_download.create(player, {
+						player_name = player_name,
+						status = "error",
+						message = "Inventory download failed",
+					})
+				end
+			end
+		end
+	end
+end
+
+function inventory_sync.check_active_uploads()
+	for player_name, record in pairs(global.inventory_sync.active_uploads) do
+		if record.last_attempt + record.timeout < game.ticks_played then
+			local player_record = global.inventory_sync.players[player_name]
+			local player = game.get_player(player_name)
+			if not player_record or not player then
+				global.inventory_sync.active_uploads[player_name] = nil
+			else
+				log("Retrying upload for " .. player_name)
+				record.last_attempt = game.ticks_played
+				record.timeout = math.random(record.timeout, record.timeout * 2)
+				inventory_sync.upload_inventory(player, player_record)
+			end
 		end
 	end
 end

plugins/inventory_sync/module/load_crafting_queue.lua

@@ -1,5 +1,5 @@
 -- Load crafting queue from a table
-function load_crafting_queue(crafting_queue, player)
+local function load_crafting_queue(crafting_queue, player)
 	local inventory = player.get_main_inventory()
 
 	-- Give player some more inventory space to avoid duplicating items

plugins/inventory_sync/module/module.json

@@ -1,4 +1,4 @@
 {
     "name": "inventory_sync",
-    "load": ["inventory_sync.lua"]
+    "load": ["inventory_sync.lua", "save_crafts.lua", "restore_crafts.lua", "gui/dialog_failed_download.lua"]
 }

plugins/inventory_sync/module/restore_crafts.lua

@@ -1,6 +1,8 @@
 local load_crafting_queue = require("modules/inventory_sync/load_crafting_queue")
 
-function restore_crafts(event)
+local restore_crafts = {}
+
+function restore_crafts.command(event)
 	if event.player_index then
 		local player = game.get_player(event.player_index)
 		local crafting_queue = global.inventory_sync.saved_crafting_queue[player.name]
@@ -16,4 +18,9 @@ function restore_crafts(event)
 	end
 end
 
+function restore_crafts.add_commands()
+	commands.add_command("restore-crafts", "Load crafting queue from global", restore_crafts.command)
+	commands.add_command("crc", "Shorthand for /restore-crafts", restore_crafts.command)
+end
+
 return restore_crafts

plugins/inventory_sync/module/save_crafts.lua

@@ -1,4 +1,6 @@
-function save_crafts(event)
+local save_crafts = {}
+
+function save_crafts.command(event)
 	if event.player_index then
 		local player = game.get_player(event.player_index)
 		local crafting_queue = {}
@@ -93,4 +95,9 @@ function save_crafts(event)
 	end
 end
 
+function save_crafts.add_commands()
+	commands.add_command("save-crafts", "Save crafting queue for transport to a different server", save_crafts.command)
+	commands.add_command("csc", "Shorthand for /save-crafts", save_crafts.command)
+end
+
 return save_crafts

plugins/inventory_sync/module/script/clean_dirty_inventory.lua

@@ -1,29 +0,0 @@
-local inventories = require("modules/inventory_sync/define_player_inventories")
-
---[[
-
-Take a player with a dirty inventory, create chests with all the items, clear the player,
-put them in ghost mode and mark the inventory as clean.
-
-]]
-
-function clean_dirty_inventory(player)
-	global.inventory_sync.players[player.name].dirty_inventory = false
-
-	local surface = player.surface
-	local position = player.position
-	local character = player.character
-	player.disassociate_character(character)
-	character.die()
-	player.ticks_to_respawn = nil
-	player.teleport(position, surface)
-
-	-- Remove player character
-	local character = player.character
-	player.set_controller {
-		type = defines.controllers.ghost,
-	}
-	if character ~= nil then character.destroy() end
-end
-
-return clean_dirty_inventory

plugins/inventory_sync/module/serialize.lua

@@ -0,0 +1,253 @@
+local clusterio_serialize = require("modules/clusterio/serialize")
+local character_inventories = require("modules/inventory_sync/define_player_inventories")
+local character_stat_keys = require("modules/inventory_sync/define_player_stat_keys")
+local serialize = {}
+
+function serialize.serialize_inventories(source, inventories)
+	local serialized = {}
+
+	for name, index in pairs(inventories) do
+		local inventory = source.get_inventory(index)
+		if inventory ~= nil then
+			serialized[name] = clusterio_serialize.serialize_inventory(inventory)
+		end
+	end
+
+	return serialized
+end
+
+function serialize.deserialize_inventories(destination, serialized, inventories)
+	for name, index in pairs(inventories) do
+		local inventory = destination.get_inventory(index)
+		if inventory ~= nil and serialized[name] ~= nil then
+			inventory.clear()
+			clusterio_serialize.deserialize_inventory(inventory, serialized[name])
+		end
+	end
+end
+
+-- Characters are serialized into a table with the following fields:
+--   character_crafting_speed_modifier
+--   character_mining_speed_modifier
+--   character_additional_mining_categories
+--   character_running_speed_modifier
+--   character_build_distance_bonus
+--   character_item_drop_distance_bonus
+--   character_reach_distance_bonus
+--   character_resource_reach_distance_bonus
+--   character_item_pickup_distance_bonus
+--   character_loot_pickup_distance_bonus
+--   character_inventory_slots_bonus
+--   character_trash_slot_count_bonus
+--   character_maximum_following_robot_count_bonus
+--   character_health_bonus
+--   character_personal_logistic_requests_enabled
+--   inventories: table of character inventory name to inventory content
+function serialize.serialize_character(character)
+	local serialized = { }
+
+	-- Serialize character stats
+	for _, key in pairs(character_stat_keys) do
+		serialized[key] = character[key]
+	end
+
+	-- Serialize character inventories
+	serialized.inventories = serialize.serialize_inventories(character, character_inventories)
+
+	return serialized
+end
+
+function serialize.deserialize_character(character, serialized)
+	-- Deserialize character stats
+	for _, key in pairs(character_stat_keys) do
+		character[key] = serialized[key]
+	end
+
+	-- Deserialize character inventories
+	serialize.deserialize_inventories(character, serialized.inventories, character_inventories)
+end
+
+-- Personal logistic slots is a table mapping string indexes to a table with the following fields:
+--   name
+--   min
+--   max
+function serialize.serialize_personal_logistic_slots(player)
+	local serialized = nil
+
+	-- Serialize personal logistic slots
+	for i = 1, 200 do -- Nobody will have more than 200 logistic slots, right?
+		local slot = player.get_personal_logistic_slot(i)
+		if slot.name ~= nil then
+			if not serialized then
+				serialized = {}
+			end
+			serialized[tostring(i)] = {
+				name = slot.name,
+				min = slot.min,
+				max = slot.max,
+			}
+		end
+	end
+
+	return serialized
+end
+
+function serialize.deserialize_personal_logistic_slots(player, serialized)
+	if not serialized then
+		return
+	end
+
+	-- Load personal logistic slots
+	for i = 1, 200 do
+		local slot = serialized[tostring(i)] -- 1 is empty to force array to be spare
+		if slot ~= nil then
+			player.set_personal_logistic_slot(i, slot)
+		end
+	end
+end
+
+local controller_to_name = {}
+for name, value in pairs(defines.controllers) do
+	controller_to_name[value] = name
+end
+
+-- Players are serialized into a table with the following fields:
+--   name
+--   controller: may not be cutscene or editor
+--   color
+--   chat_color
+--   tag
+--   force
+--   cheat_mod
+--   flashlight
+--   ticks_to_respawn (optional)
+--   character (optional)
+--   inventories: non-character inventories
+--   hotbar: table of string indexes to names (optional)
+--   personal_logistic_slots (optional)
+function serialize.serialize_player(player)
+	local serialized = {
+		controller = controller_to_name[player.controller_type],
+		name = player.name,
+		color = player.color,
+		chat_color = player.chat_color,
+		tag = player.tag,
+		force = player.force.name,
+		cheat_mode = player.cheat_mode,
+		flashlight = player.is_flashlight_enabled(),
+		ticks_to_respawn = player.ticks_to_respawn,
+	}
+
+	-- For the waiting to respawn state the inventory logistic requests and filters are hidden on the player
+	if player.controller_type == defines.controllers.ghost and player.ticks_to_respawn then
+		player.ticks_to_respawn = nil -- Respawn now
+
+		serialized.personal_logistic_slots = serialize.serialize_personal_logistic_slots(player)
+		serialized.inventories = serialize.serialize_inventories(player, character_inventories)
+
+		-- Go back to waiting for respawn
+		local character = player.character
+		player.ticks_to_respawn = serialized.ticks_to_respawn
+		character.destroy()
+	end
+
+	-- Serialize character
+	if player.character then
+		serialized.character = serialize.serialize_character(player.character)
+		serialized.personal_logistic_slots = serialize.serialize_personal_logistic_slots(player)
+	end
+
+	-- Serialize non-character inventories
+	if player.controller_type == defines.controllers.god then
+		serialized.inventories = serialize.serialize_inventories(player, { main = defines.inventory.god_main })
+	end
+
+	-- Serialize hotbar
+	for i = 1, 100 do
+		local slot = player.get_quick_bar_slot(i)
+		if slot ~= nil and slot.name ~= nil then
+			if not serialized.hotbar then
+				serialized.hotbar = {}
+			end
+			serialized.hotbar[tostring(i)] = slot.name
+		end
+	end
+
+	return serialized
+end
+
+function serialize.deserialize_player(player, serialized)
+	if player.controller_type ~= defines.controllers[serialized.controller] or serialized.controller == "ghost" then
+		-- If targeting the character or ghost controller then create a character
+		if serialized.controller == "character" or serialized.controller == "ghost" then
+			if player.controller_type == defines.controllers.ghost or player.controller_type == defines.controllers.spectator then
+				player.set_controller({ type = defines.controllers.god })
+			end
+			if player.controller_type == defines.controllers.god then
+				player.create_character()
+			end
+
+			-- The ghost state stores hidden logistic and filters which are only accessible in the character controller
+			if serialized.controller == "ghost" then
+				serialize.deserialize_personal_logistic_slots(player, serialized.personal_logistic_slots)
+				serialize.deserialize_inventories(player, serialized.inventories, character_inventories)
+				local character = player.character
+				if serialized.ticks_to_respawn then
+					player.ticks_to_respawn = serialized.ticks_to_respawn
+				else
+					-- We have to set ticks to respawn to save the hidden state into the player but we
+					-- can't unset tick_to_respawn by setting it back to nil as that triggers a respawn.
+					player.ticks_to_respawn = 0
+					player.set_controller({ type = defines.controller.god })
+					player.set_controller({ type = defines.controller.ghost })
+				end
+				character.destroy()
+			end
+
+		else
+			-- Targeting the god or spectator controller, if coming from the
+			-- character controller then destroy the character
+			if player.controller_type == defines.controllers.character then
+				player.character.destroy()
+			end
+
+			-- Switching to god or spectator is a matter of setting the controller
+			if player.controller_type ~= defines.controllers[serialized.controller] then
+				player.set_controller({ type = defines.controllers[serialized.controller] })
+			end
+		end
+	end
+
+	player.color = serialized.color
+	player.chat_color = serialized.chat_color
+	player.tag = serialized.tag
+	player.force = serialized.force
+	player.cheat_mode = serialized.cheat_mode
+	if serialized.flashlight then
+		player.enable_flashlight()
+	else
+		player.disable_flashlight()
+	end
+
+	-- Deserialize character
+	if player.character then
+		serialize.deserialize_character(player.character, serialized.character)
+		serialize.deserialize_personal_logistic_slots(player, serialized.personal_logistic_slots)
+	end
+
+	-- Deserialize non-character inventories
+	if player.controller_type == defines.controllers.god then
+		serialize.deserialize_inventories(player, serialized.inventories, { main = defines.inventory.god_main })
+	end
+
+	-- Deserialize hotbar
+	if serialized.hotbar then
+		for i = 1, 100 do
+			if serialized.hotbar[tostring(i)] ~= nil then
+				player.set_quick_bar_slot(i, serialized.hotbar[tostring(i)])
+			end
+		end
+	end
+end
+
+return serialize

plugins/inventory_sync/module/upload_inventory.lua

@@ -1,81 +1,19 @@
 local clusterio_api = require("modules/clusterio/api")
-local serialize = require("modules/clusterio/serialize")
-local inventories = require("modules/inventory_sync/define_player_inventories")
+local serialize = require("modules/inventory_sync/serialize")
 local save_crafts = require("modules/inventory_sync/save_crafts")
-local player_stat_keys = require("modules/inventory_sync/define_player_stat_keys")
 
-function upload_inventory(playerIndex)
-	local player = game.get_player(playerIndex)
-
-	if player.character == nil then
-		-- Panic!
-		log("ERROR: Player "..player.name.." quit without a character. Inventory not uploaded")
-		return false
-	end
-
-	local serialized_player = {
-		name = player.name,
-		inventories = {},
-		hotbar = {},
-		color = player.color,
-		chat_color = player.chat_color,
-		tag = player.tag,
-		-- admin = player.admin, -- This is handled by other parts of clusterio
-		personal_logistic_slots = {},
-		force = player.force.name,
-		cheat_mode = player.cheat_mode,
-		flashlight = player.is_flashlight_enabled(),
-	}
-
-	-- Transfer player stats
-	for _,v in pairs(player_stat_keys) do
-		serialized_player[v] = player[v]
-	end
-
-	-- Serialize hotbar
-	for i = 1, 100 do
-		serialized_player.hotbar[i] = {}
-		local slot = player.get_quick_bar_slot(i)
-		if slot ~= nil and slot.name ~= nil then
-			serialized_player.hotbar[i] = slot.name
-		end
-	end
-
-	-- Serialize personal logistics slots
-	for i = 1, 200 do -- Nobody will have more than 200 logistics slots, right?
-		local slot = player.get_personal_logistic_slot(i)
-		if slot.name ~= nil then
-			-- We leave [1] empty to force the JSON function to parse it as an object.
-			-- When its parsed as an array we get inconsistent key datatypes depending
-			-- on whether or not it is sparse.
-			serialized_player.personal_logistic_slots[i+1] = {
-				name = slot.name,
-				min = slot.min,
-				max = slot.max,
-			}
-		end
-	end
-
-	-- Serialize inventories
-	for _, inv in pairs(inventories) do
-		local inventory = player.get_inventory(inv)
-		if inventory ~= nil then
-			local serialized_inventory = serialize.serialize_inventory(inventory)
-			serialized_player.inventories[inv] = serialized_inventory
-		end
-	end
+local function upload_inventory(player, player_record)
+	local serialized_player = serialize.serialize_player(player)
+	serialized_player.generation = player_record.generation
 
 	-- Serialize crafting queue saved by /csc or /save-crafts OR automatically
-	save_crafts({player_index = player.name})
-	serialized_player.crafting_queue = global.inventory_sync.saved_crafting_queue[player.name]
-	if serialized_player.crafting_queue == nil then
-		serialized_player.crafting_queue = {}
+	if player.character then
+		save_crafts.command({player_index = player.name})
 	end
+	serialized_player.crafting_queue = global.inventory_sync.saved_crafting_queue[player.name]
+	global.inventory_sync.saved_crafting_queue[player.name] = nil
 
 	clusterio_api.send_json("inventory_sync_upload", serialized_player)
-
-	-- Clear saved crafting queue
-	global.inventory_sync.saved_crafting_queue[player.name] = nil
 end
 
 return upload_inventory