From bb36af08fe90ef4aca485745b974f00209e17957 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:26:38 +0000 Subject: [PATCH] Apply recipe notification deltas directly in Lua Review feedback: the download field now always carries the delta and the deserializer applies it against the current state instead of rebuilding the full list first. The previous failed recipe names are passed in so they survive the round trip. The version check moved to the call site. Co-Authored-By: Claude Fable 5.1 --- plugins/inventory_sync/instance.ts | 14 +-- plugins/inventory_sync/messages.ts | 1 - .../inventory_sync/module/inventory_sync.lua | 24 ++-- plugins/inventory_sync/module/serialize.lua | 109 ++++++------------ plugins/inventory_sync/test/instance.js | 30 ++--- 5 files changed, 69 insertions(+), 109 deletions(-) diff --git a/plugins/inventory_sync/instance.ts b/plugins/inventory_sync/instance.ts index 39ddea17..32324517 100644 --- a/plugins/inventory_sync/instance.ts +++ b/plugins/inventory_sync/instance.ts @@ -217,19 +217,19 @@ export class InstancePlugin extends BaseInstancePlugin { * Replace the stored recipe notifications with the difference from the * current state on the instance to reduce the amount of data sent to Lua. */ - async applyRecipeNotificationDelta(playerData: IpcPlayerData, current: string) { + async applyRecipeNotificationDelta(playerData: IpcPlayerData, current?: string) { if (!playerData.recipe_notifications) { return; } try { const delta = recipeNotificationDelta( JSON.parse(await decodeLuaString(playerData.recipe_notifications)), - JSON.parse(await decodeLuaString(current)), + current ? JSON.parse(await decodeLuaString(current)) : [], ); - playerData.recipe_notifications_delta = await encodeLuaString(JSON.stringify(delta)); - delete playerData.recipe_notifications; + playerData.recipe_notifications = await encodeLuaString(JSON.stringify(delta)); } catch (err: any) { - this.logger.warn(`Sending full recipe notifications for ${playerData.name}:\n${err.stack}`); + this.logger.warn(`Dropping invalid recipe notifications for ${playerData.name}:\n${err.stack}`); + delete playerData.recipe_notifications; } } @@ -247,9 +247,7 @@ export class InstancePlugin extends BaseInstancePlugin { return; } - if (request.recipe_notifications) { - await this.applyRecipeNotificationDelta(response.playerData, request.recipe_notifications); - } + await this.applyRecipeNotificationDelta(response.playerData, request.recipe_notifications); const chunkSize = this.instance.config.get("inventory_sync.rcon_chunk_size"); const chunks = chunkify(chunkSize, JSON.stringify(response.playerData)); diff --git a/plugins/inventory_sync/messages.ts b/plugins/inventory_sync/messages.ts index 5585fe55..028c39e0 100644 --- a/plugins/inventory_sync/messages.ts +++ b/plugins/inventory_sync/messages.ts @@ -21,7 +21,6 @@ export type IpcPlayerData = { personal_logistic_slots?: {name:string, min:number, max:number}[], crafting_queue?: any, recipe_notifications?: string, - recipe_notifications_delta?: string, } const jsonPlayerData = Type.Object({ diff --git a/plugins/inventory_sync/module/inventory_sync.lua b/plugins/inventory_sync/module/inventory_sync.lua index 6df45f77..89250148 100644 --- a/plugins/inventory_sync/module/inventory_sync.lua +++ b/plugins/inventory_sync/module/inventory_sync.lua @@ -17,6 +17,7 @@ local progress_dialog = require("modules/inventory_sync/gui/progress_dialog") local dialog_failed_download = require("modules/inventory_sync/gui/dialog_failed_download") local v2_remote_controller = compat.version_ge("2.0.0") +local recipe_notifications_api = compat.version_ge("2.0.67") -- Returns true if the player is currently in a cutscene local function is_in_cutscene(player) @@ -111,14 +112,9 @@ function inventory_sync.deserialize_player(player, finished_record) -- Deserialize downloaded player data local serialized_player = compat.json_to_table(finished_record.data) assert(type(serialized_player) == "table", "wrong type for serialized_player") - if serialized_player.recipe_notifications_delta then - serialized_player.recipe_notifications = serialize.apply_crafting_notification_delta( - assert(finished_record.recipe_notifications, "recipe notification delta without snapshot"), - serialized_player.recipe_notifications_delta - ) - serialized_player.recipe_notifications_delta = nil - end - script_data.failed_deserialization[player.name] = serialize.deserialize_player(player, serialized_player) + script_data.failed_deserialization[player.name] = serialize.deserialize_player( + player, serialized_player, script_data.failed_deserialization[player.name] + ) -- Restore player position and driving state restore_position(player, finished_record) @@ -449,13 +445,17 @@ function inventory_sync.initiate_inventory_download(player, player_record, gener script_data.active_downloads[player.name] = record -- The plugin only sends back what differs from the current notification state - record.recipe_notifications = serialize.crafting_notification_snapshot( - player, script_data.failed_deserialization[player.name] - ) + local recipe_notifications + if recipe_notifications_api then + local failed = script_data.failed_deserialization[player.name] + recipe_notifications = serialize.serialize_crafting_notifications( + player, failed and failed.recipe_notifications + ) + end clusterio_api.send_json("inventory_sync_download", { player_name = player.name, - recipe_notifications = record.recipe_notifications, + recipe_notifications = recipe_notifications, }) -- If this is a synced player turn them into a spectator while the diff --git a/plugins/inventory_sync/module/serialize.lua b/plugins/inventory_sync/module/serialize.lua index 78430de3..7fabb07c 100644 --- a/plugins/inventory_sync/module/serialize.lua +++ b/plugins/inventory_sync/module/serialize.lua @@ -545,95 +545,54 @@ function serialize.serialize_crafting_notifications(player, failed) return helpers.encode_string(helpers.table_to_json(recipe_names)) end ---- Current notification state, sent with download requests so the instance plugin only returns the difference ---- @param player LuaPlayer ---- @param failed FailedDeserializationPlayerData? ---- @return string? -function serialize.crafting_notification_snapshot(player, failed) - if not recipe_notifications_api then - return nil - end - return serialize.serialize_crafting_notifications(player, failed and failed.recipe_notifications) -end - --- @class CraftingNotificationDelta ---- @field add string[]? Recipe names to add to the cleared list ---- @field remove string[]? Recipe names to remove from the cleared list - ---- Rebuild the full serialized form from the snapshot the delta was computed against ---- @param snapshot string ---- @param serialized_delta string ---- @return string? -function serialize.apply_crafting_notification_delta(snapshot, serialized_delta) - local recipe_names = helpers.json_to_table(assert(helpers.decode_string(snapshot))) - assert(type(recipe_names) == "table", "wrong type decoded from json_to_table") - --- @type CraftingNotificationDelta - local delta = helpers.json_to_table(assert(helpers.decode_string(serialized_delta))) - assert(type(delta) == "table", "wrong type decoded from json_to_table") - local cleared = {} - for _, name in pairs(recipe_names) do - cleared[name] = true - end - for _, name in pairs(delta.remove or {}) do - cleared[name] = nil - end - for _, name in pairs(delta.add or {}) do - cleared[name] = true - end - - local merged = {} - for name in pairs(cleared) do - merged[#merged + 1] = name - end - return helpers.encode_string(helpers.table_to_json(merged)) -end +--- @field add string[]? Recipes cleared elsewhere that still have a notification here +--- @field remove string[]? Recipes with a notification elsewhere that are cleared here --- @param player LuaPlayer ---- @param serialized string +--- @param serialized string Encoded delta against the state sent with the download request +--- @param previous_failed string[]? Recipe names that did not exist at the last deserialization --- @return string[]? failed Recipe names that do not exist -function serialize.deserialize_crafting_notifications(player, serialized) - -- Decode the recipe name strings - local recipe_names = helpers.json_to_table(assert(helpers.decode_string(serialized))) - assert(type(recipe_names) == "table", "wrong type decoded from json_to_table") - local clear_notification = player.clear_recipe_notification - local add_notification = player.add_recipe_notification - local notifications = {} +function serialize.deserialize_crafting_notifications(player, serialized, previous_failed) + --- @type CraftingNotificationDelta + local delta = helpers.json_to_table(assert(helpers.decode_string(serialized))) + assert(type(delta) == "table", "wrong type decoded from json_to_table") local failed = {} + for _, recipe_name in pairs(previous_failed or {}) do + failed[recipe_name] = true + end - -- Assume all recipes need a notification + -- Recipes which can have a notification on this server + local enabled = {} for _, recipe in pairs(player.force.recipes) do if recipe.enabled and not recipe.hidden then - notifications[recipe.name] = true + enabled[recipe.name] = true end end - -- Remove recipes the player has cleared - for _, recipe_name in pairs(recipe_names) do - if notifications[recipe_name] then - notifications[recipe_name] = false + -- Clear notifications the player has cleared elsewhere + for _, recipe_name in pairs(delta.add or {}) do + if enabled[recipe_name] then + player.clear_recipe_notification(recipe_name) else - -- Recipe does not exist on this server - failed[#failed+1] = recipe_name + failed[recipe_name] = true -- Recipe does not exist on this server end end - -- Clear ones which are ones which are not required - for _, recipe in pairs(player.get_recipe_notifications()) do - if notifications[recipe.name] == false then - clear_notification(recipe.name) - end - notifications[recipe.name] = nil -- Prevents double add below - end - - -- Add any which the player did not have - for recipe_name, should_add in pairs(notifications) do - if should_add then - add_notification(recipe_name) + -- Add notifications the player has elsewhere + for _, recipe_name in pairs(delta.remove or {}) do + if enabled[recipe_name] then + player.add_recipe_notification(recipe_name) + else + failed[recipe_name] = nil end end - -- Return failed recipes if any - return next(failed) and failed or nil + local failed_names = {} + for recipe_name in pairs(failed) do + failed_names[#failed_names + 1] = recipe_name + end + return next(failed_names) and failed_names or nil end local controller_to_name = {} @@ -839,8 +798,9 @@ end --- @param player LuaPlayer --- @param serialized SerializedPlayerData +--- @param previous_failed FailedDeserializationPlayerData? --- @return FailedDeserializationPlayerData? -function serialize.deserialize_player(player, serialized) +function serialize.deserialize_player(player, serialized, previous_failed) local failed_deserialization = {} local target_controller = defines.controllers[serialized.controller] @@ -927,8 +887,9 @@ function serialize.deserialize_player(player, serialized) -- Deserialize recipe notifications if recipe_notifications_api and serialized.recipe_notifications then - failed_deserialization.recipe_notifications = - serialize.deserialize_crafting_notifications(player, serialized.recipe_notifications) + failed_deserialization.recipe_notifications = serialize.deserialize_crafting_notifications( + player, serialized.recipe_notifications, previous_failed and previous_failed.recipe_notifications + ) end return next(failed_deserialization) and failed_deserialization or nil diff --git a/plugins/inventory_sync/test/instance.js b/plugins/inventory_sync/test/instance.js index e292c19f..298a89ad 100644 --- a/plugins/inventory_sync/test/instance.js +++ b/plugins/inventory_sync/test/instance.js @@ -48,25 +48,27 @@ describe("inventory_sync", function() { it("should replace the stored list with an encoded delta", async function() { const playerData = { name: "test", recipe_notifications: await encode(["a", "b"]) }; await apply(playerData, await encode(["b", "c"])); - assert.equal(playerData.recipe_notifications, undefined); - assert.deepEqual(await decode(playerData.recipe_notifications_delta), { add: ["a"], remove: ["c"] }); + assert.deepEqual(await decode(playerData.recipe_notifications), { add: ["a"], remove: ["c"] }); }); it("should send an empty delta when nothing changed", async function() { const playerData = { name: "test", recipe_notifications: await encode(["a"]) }; await apply(playerData, await encode(["a"])); - assert.deepEqual(await decode(playerData.recipe_notifications_delta), {}); + assert.deepEqual(await decode(playerData.recipe_notifications), {}); + }); + it("should add everything without a current list", async function() { + const playerData = { name: "test", recipe_notifications: await encode(["a"]) }; + await apply(playerData, undefined); + assert.deepEqual(await decode(playerData.recipe_notifications), { add: ["a"] }); }); it("should do nothing without stored notifications", async function() { const playerData = { name: "test" }; await apply(playerData, await encode(["a"])); assert.deepEqual(playerData, { name: "test" }); }); - it("should fall back to the full list on invalid input", async function() { - const stored = await encode(["a"]); - const playerData = { name: "test", recipe_notifications: stored }; - await apply(playerData, "not base64 zlib"); - assert.equal(playerData.recipe_notifications, stored); - assert.equal(playerData.recipe_notifications_delta, undefined); + it("should drop the field on invalid input", async function() { + const playerData = { name: "test", recipe_notifications: "not base64 zlib" }; + await apply(playerData, await encode(["a"])); + assert.equal(playerData.recipe_notifications, undefined); assert.equal(warnings.length, 1); }); }); @@ -106,16 +108,16 @@ describe("inventory_sync", function() { ["/sc inventory_sync.download_inventory('test',nil,0,0)"] ); }); - it("should send the full list when no snapshot was given", async function() { - const playerData = { generation: 1, name: "test", recipe_notifications: await encode(["a"]) }; - respondWith({ ...playerData }); + it("should send everything as a delta when no snapshot was given", async function() { + respondWith({ generation: 1, name: "test", recipe_notifications: await encode(["a"]) }); await instancePlugin.handleDownload({ player_name: "test" }); - assert.deepEqual(instancePlugin.instance.server.rconCommands, [downloadCommand(playerData)]); + const expected = { generation: 1, name: "test", recipe_notifications: await encode({ add: ["a"] }) }; + assert.deepEqual(instancePlugin.instance.server.rconCommands, [downloadCommand(expected)]); }); it("should send a delta when a snapshot was given", async function() { respondWith({ generation: 1, name: "test", recipe_notifications: await encode(["a", "b"]) }); await instancePlugin.handleDownload({ player_name: "test", recipe_notifications: await encode(["b"]) }); - const expected = { generation: 1, name: "test", recipe_notifications_delta: await encode({ add: ["a"] }) }; + const expected = { generation: 1, name: "test", recipe_notifications: await encode({ add: ["a"] }) }; assert.deepEqual(instancePlugin.instance.server.rconCommands, [downloadCommand(expected)]); }); it("should be invoked by the download ipc", async function() {