Leave missing quick bar records alone once the quick bar is not synced

An instance whose quick bar sync was turned off while a record entry was
pending still set the slot when the library arrived.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JyNnVzRW4jPWaoWTxCyhM7

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. 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.
+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.
 
 ## Installation
 

plugins/player_sync/controller.ts

@@ -258,7 +258,6 @@ 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) => {

plugins/player_sync/instance.ts

@@ -200,12 +200,11 @@ 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 {
-			({ stored } = await instance.sendTo(
+			await instance.sendTo(
 				"controller",
 				new UploadRequest(instance.id, player_data.name, player_data, destinationInstanceId),
-			));
+			);
 
 		} catch (err: any) {
 			if (!(err instanceof lib.SessionLost)) {
@@ -214,10 +213,8 @@ 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.${reply}("${player_data.name}", ${player_data.generation})`, true, plugin.name
+			`/sc player_sync.confirm_upload("${player_data.name}", ${player_data.generation})`, true, plugin.name
 		);
 	}
 

plugins/player_sync/messages.ts

@@ -262,30 +262,12 @@ 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,
-	) {
-	}
-
-	// No type: a controller from before this response replies with nothing, which counts as stored
-	static jsonSchema = Type.Unsafe<{ stored: boolean } | undefined>({
-		properties: { "stored": { type: "boolean" } },
-	});
-
-	static fromJSON(json: Static<typeof UploadResponse.jsonSchema>) {
-		return new this(json?.stored ?? true);
-	}
-}
-
 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,

plugins/player_sync/module/player_sync.lua

@@ -384,14 +384,12 @@ 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, or the controller did not store its last upload
--- (reject_upload). A player without a record here has nothing to keep.
+-- that generation or newer with every component synced here now. 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)
@@ -545,8 +543,6 @@ 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
@@ -577,7 +573,6 @@ 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
@@ -639,6 +634,7 @@ player_sync.on_nth_tick[33] = function(event)
 	player_sync.check_players_in_cutscene()
 	player_sync.check_active_downloads()
 	player_sync.check_active_uploads()
+	player_sync.check_missing_quick_bar_records()
 end
 
 player_sync.events[defines.events.on_cutscene_waypoint_reached] = function(event)
@@ -729,32 +725,6 @@ 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 then
-		return
-	end
-	local script_data = get_script_data()
-	-- A player who is back already still waits for the acquire reply, which the controller sent after this one, and
-	-- that join then downloads. Past that reply the join has decided and this is not about the copy here any more.
-	if player.connected and not script_data.players_waiting_for_acquire[player_name] then
-		return
-	end
-	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 = {
@@ -812,6 +782,25 @@ function player_sync.check_active_downloads()
 	end
 end
 
+-- Quick bar slots pointing into the blueprint library are set once the save has the player's library
+function player_sync.check_missing_quick_bar_records()
+	local script_data = get_script_data()
+	-- Not once the quick bar stopped being synced here. The entries stay for the upload, which goes by the
+	-- components of the player's session.
+	if not serialize.syncs(script_data.components, "quick_bar") then
+		return
+	end
+	for player_name, failed in pairs(script_data.failed_deserialization) do
+		local player = failed.quick_bar and game.get_player(player_name)
+		if player and player.connected then
+			failed.quick_bar = serialize.restore_missing_quick_bar_records(player, failed.quick_bar)
+			if not next(failed) then
+				script_data.failed_deserialization[player_name] = nil
+			end
+		end
+	end
+end
+
 function player_sync.check_active_uploads()
 	local script_data = get_script_data()
 	for player_name, record in pairs(script_data.active_uploads) do

plugins/player_sync/module/serialize.lua

@@ -494,6 +494,27 @@ local function find_item_path(inventory, item_number)
 	return nil
 end
 
+--- The record a quick bar slot entry points at, if the player's library has it at that path
+--- @param player LuaPlayer
+--- @param entry QuickBarSlotEncoded.record
+--- @return LuaRecord?
+local function find_record(player, entry)
+	-- The record must still be what was stored, a shifted library must not set the wrong one
+	local records = player.blueprints
+	local record --[[@as LuaRecord?]]
+	for _, index in ipairs(entry.p) do
+		record = records[index]
+		if not (record and record.valid) then
+			return nil
+		end
+		records = record.type == "blueprint-book" and record.contents or {}
+	end
+	if record and record.type == entry.y and (record.label or "") == (entry.l or "") then
+		return record
+	end
+	return nil
+end
+
 --- @param filter ItemFilter
 --- @return QuickBarSlotEncoded
 local function serialize_filter(filter)
@@ -611,18 +632,8 @@ function serialize.deserialize_quick_bar_slot(entry, player)
 		end
 
 		if entry.t == "r" then
-			-- The record must still be what was stored, a shifted library must not set the wrong one
-			local records = player.blueprints
-			local record --[[@as LuaRecord?]]
-			for _, index in ipairs(entry.p) do
-				record = records[index]
-				if not (record and record.valid) then
-					record = nil
-					break
-				end
-				records = record.type == "blueprint-book" and record.contents or {}
-			end
-			if record and record.valid and record.type == entry.y and (record.label or "") == (entry.l or "") then
+			local record = find_record(player, entry)
+			if record then
 				return { type = "record", record = record } --[[@as QuickBarSlot]]
 			end
 
@@ -670,8 +681,9 @@ local function quick_bar_slot_count(player)
 end
 
 --- @param player LuaPlayer
+--- @param missing table<string, QuickBarSlotEncoded.record>? Entries deserialize_quick_bar could not find the record of
 --- @return table<string, QuickBarSlotEncoded>?
-function serialize.serialize_quick_bar(player)
+function serialize.serialize_quick_bar(player, missing)
 	local serialized
 	local width = v2_1_quick_bar_api and player.quick_bar_width
 
@@ -696,19 +708,63 @@ function serialize.serialize_quick_bar(player)
 		end
 	end
 
+	-- A record still missing here is stored again as it came, unless the player put something else in its slot
+	for index, entry in pairs(missing or {}) do
+		serialized = serialized or {}
+		serialized[index] = serialized[index] or entry
+	end
+
 	return serialized
 end
 
+--- Slot index to page and slot on it
+--- @param player LuaPlayer
+--- @param i integer
+local function quick_bar_location(player, i)
+	local width = player.quick_bar_width
+	return math.floor((i - 1) / width) + 1, ((i - 1) % width) + 1
+end
+
+--- Set the slots whose records were not in the player's library when the data was applied. A save gets the library
+--- of a player from the client some seconds after the join, later than the data of a player new to it is applied.
+--- @param player LuaPlayer
+--- @param missing table<string, QuickBarSlotEncoded.record>
+--- @return table<string, QuickBarSlotEncoded.record>? missing Entries whose record is still not there
+function serialize.restore_missing_quick_bar_records(player, missing)
+	for index, entry in pairs(missing) do
+		local page, page_slot = quick_bar_location(player, tonumber(index) --[[@as integer]])
+		if player.get_quick_bar_slot(page, page_slot) ~= nil then
+			-- The player put something else there
+			missing[index] = nil
+		else
+			local record = find_record(player, entry)
+			if record then
+				player.set_quick_bar_slot(page, page_slot, { type = "record", record = record })
+				missing[index] = nil
+			end
+		end
+	end
+	return next(missing) and missing or nil
+end
+
 --- @param player LuaPlayer
 --- @param serialized table<string, string | QuickBarSlotEncoded>?
+--- @return table<string, QuickBarSlotEncoded.record>? missing Entries whose record is not in the player's library
 function serialize.deserialize_quick_bar(player, serialized)
 	-- Every slot is set, and the ones not in the data are cleared, so a slot moved or cleared on the
 	-- previous server does not come back from this server's old copy
 	serialized = serialized or {}
 	local width = v2_1_quick_bar_api and player.quick_bar_width
+	local missing
 	for i = 1, quick_bar_slot_count(player) do
 		local entry = serialized[tostring(i)]
 		local slot = entry ~= nil and serialize.deserialize_quick_bar_slot(entry, player) or nil
+		if slot == nil and v2_1_quick_bar_api and type(entry) == "table" and entry.t == "r" then
+			-- ponytail: kept until the record shows up or the slot is used for something else, so the entry of a
+			-- record deleted from the library is carried along. Drop them by age if that ever matters.
+			missing = missing or {}
+			missing[tostring(i)] = entry
+		end
 		if v2_1_quick_bar_api then
 			local page = math.floor((i - 1) / width) + 1
 			local page_slot = ((i - 1) % width) + 1
@@ -718,6 +774,7 @@ function serialize.deserialize_quick_bar(player, serialized)
 			player.set_quick_bar_slot(i, slot)
 		end
 	end
+	return missing
 end
 
 --- Quick bar page shown on each quick bar row of the screen, kept by every save on its own
@@ -1212,7 +1269,7 @@ function serialize.serialize_player(player, failed_deserialization, components)
 
 	-- Serialize quick bar
 	if syncs(components, "quick_bar") then
-		serialized.quick_bar = serialize.serialize_quick_bar(player)
+		serialized.quick_bar = serialize.serialize_quick_bar(player, failed_deserialization.quick_bar)
 		serialized.quick_bar_pages = serialize.serialize_quick_bar_pages(player)
 	end
 
@@ -1333,6 +1390,7 @@ end
 
 --- @class FailedDeserializationPlayerData
 --- @field recipe_notifications string[]?
+--- @field quick_bar table<string, QuickBarSlotEncoded.record>? Slots whose record is not in the library (yet)
 
 --- @param player LuaPlayer
 --- @param serialized SerializedPlayerData
@@ -1495,7 +1553,9 @@ function serialize.deserialize_player(player, serialized, components, local_cont
 
 	-- Deserialize quick bar (named hotbar in old data)
 	if syncs(components, "quick_bar") then
-		serialize.deserialize_quick_bar(player, serialized.quick_bar or serialized.hotbar)
+		failed_deserialization.quick_bar = serialize.deserialize_quick_bar(
+			player, serialized.quick_bar or serialized.hotbar
+		)
 		serialize.deserialize_quick_bar_pages(player, serialized.quick_bar_pages)
 	end
 

plugins/player_sync/test/controller.js

@@ -77,18 +77,6 @@ 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);

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, UploadResponse } from "../dist/node/messages.js";
+import { AcquireResponse, DownloadResponse, PrefetchEvent } from "../dist/node/messages.js";
 import { components } from "../dist/node/components.js";
 import { plugin as info } from "../dist/node/index.js";
 
@@ -275,25 +275,6 @@ 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() {

plugins/player_sync/test/plugin.js

@@ -60,14 +60,6 @@ describe("player_sync plugin", function() {
 				assert(!validate({ instanceId: 1, playerName: "player", playerData: data }), `accepted ${deleted}`);
 			}
 		});
-		it("should take the empty reply of an older controller as a stored upload", function() {
-			const validate = lib.compile(msg.UploadResponse.jsonSchema);
-			assert(validate(undefined));
-			assert.equal(msg.UploadResponse.fromJSON(undefined).stored, true);
-			assert(validate({ stored: false }));
-			assert.equal(msg.UploadResponse.fromJSON({ stored: false }).stored, false);
-			assert(!validate({ stored: "no" }));
-		});
 		describe("SetPlayerDataRequest schema", function() {
 			const validate = lib.compile(msg.SetPlayerDataRequest.jsonSchema);
 			// Shaped like module/serialize.lua output