diff --git a/plugins/player_sync/README.md b/plugins/player_sync/README.md index d0e7ff9e..0cdcd142 100644 --- a/plugins/player_sync/README.md +++ b/plugins/player_sync/README.md @@ -55,6 +55,8 @@ Should an error occur during this process the player is given the option to use Instances can leave parts of the player out of the sync with the `player_sync.sync_*` options below. A part which is not synced is left as it is when the player joins, and the instance's copy of it is not uploaded, so the stored copy from other instances stays as it was. +A player online when the options change keeps the old ones until they leave and join again, and a part that was turned on is downloaded when they join. +Turning off `player_sync.sync_inventories` leaves the items players have on the instance in place while the stored inventory still holds them too. Communication between the server and instance goes over stdout or rcon, depending on the size of the data. Communication between the instance and the controller goes over websockets. @@ -83,7 +85,7 @@ Defaults to `1000`. ### player_sync.sync_controller -Sync whether the player is a character, god or spectator, their respawn timer and cheat mode. +Sync whether the player is a character, god or spectator, their respawn timer, cheat mode, the god inventory and the inventory filters of a dead player. Switching controller can destroy the character holding the inventory, so this only takes effect while `player_sync.sync_inventories` is also enabled. Defaults to `true`. @@ -102,8 +104,8 @@ Defaults to `true`. ### player_sync.sync_inventories -Sync the character and god inventories, the character bonuses and the crafting queue. -While the controller is not synced only the inventory of a character is synced, a player without a character keeps the inventory they have on the instance and does not change the stored one. +Sync the character inventories, the character bonuses and the crafting queue. +While the controller is not synced a player without a character keeps what they have on the instance and does not change the stored character. A player waiting to respawn is respawned straight away if there is a synced character to give them. Defaults to `true`. diff --git a/plugins/player_sync/components.ts b/plugins/player_sync/components.ts index 7cef0493..ec86d93b 100644 --- a/plugins/player_sync/components.ts +++ b/plugins/player_sync/components.ts @@ -3,10 +3,11 @@ import type { IpcPlayerData } from "./messages.js"; /** Parts of the player data an instance can choose to sync, and the fields of the player data holding them. */ export const components = { - controller: ["controller", "ticks_to_respawn", "cheat_mode"], + // inventories holds the god inventory and the hidden ghost inventory, both belong to the controller + controller: ["controller", "ticks_to_respawn", "cheat_mode", "inventories"], force: ["force"], appearance: ["color", "chat_color", "tag"], - inventories: ["character", "inventories", "crafting_queue"], + inventories: ["character", "crafting_queue"], logistics: ["personal_logistic_slots"], quick_bar: ["quick_bar", "hotbar"], settings: ["flashlight", "shortcuts", "game_view_settings"], @@ -19,15 +20,15 @@ export type Component = keyof typeof components; const characterFields = ["character", "crafting_queue", "personal_logistic_slots"] as const; /** - * Components synced on an instance. + * Build the set of synced components. * * The controller is only synced together with the inventories, as * switching controller can destroy the character holding them. */ -export function enabledComponents(config: lib.InstanceConfig) { +function componentSet(isEnabled: (component: Component) => boolean) { const enabled = new Set(); for (const component of Object.keys(components) as Component[]) { - if (config.get(`player_sync.sync_${component}`)) { + if (isEnabled(component)) { enabled.add(component); } } @@ -37,6 +38,23 @@ export function enabledComponents(config: lib.InstanceConfig) { return enabled; } +/** Components synced on an instance. */ +export function enabledComponents(config: lib.InstanceConfig) { + return componentSet(component => Boolean(config.get(`player_sync.sync_${component}`))); +} + +/** + * Components an upload was serialized with, all of them when not given. + * + * Lua may encode an empty table as an array, which reads as no components. + */ +export function uploadedComponents(uploaded: Record | unknown[] | undefined) { + if (uploaded === undefined) { + return componentSet(() => true); + } + return componentSet(component => !Array.isArray(uploaded) && uploaded[component] === true); +} + /** Remove the fields of components which are not synced. */ export function stripPlayerData(playerData: IpcPlayerData, enabled: Set) { const fields = playerData as Record; @@ -53,13 +71,13 @@ export function stripPlayerData(playerData: IpcPlayerData, enabled: Set) { - const merged: Record = { ...uploaded }; +export function mergePlayerData(stored: IpcPlayerData | undefined, uploaded: IpcPlayerData) { + const { components: uploadComponents, ...merged } = uploaded as IpcPlayerData & Record; + const enabled = uploadedComponents(uploadComponents); function keep(keys: readonly string[]) { for (const key of keys) { const value = (stored as Record | undefined)?.[key]; @@ -76,11 +94,8 @@ export function mergePlayerData(stored: IpcPlayerData | undefined, uploaded: Ipc keep(keys); } } - if (!enabled.has("controller")) { - keep(["inventories"]); - if (!uploaded.character) { - keep(characterFields); - } + if (!enabled.has("controller") && !uploaded.character) { + keep(characterFields); } return merged as IpcPlayerData; } diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index bdc8bbab..887b4b06 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -1,6 +1,6 @@ import type { ControllerPluginContext, InstanceRecord } from "@clusterio/controller"; import type { IpcPlayerData } from "./messages.js"; -import { enabledComponents, mergePlayerData } from "./components.js"; +import { mergePlayerData } from "./components.js"; import fs from "node:fs/promises"; import path from "path"; @@ -138,8 +138,7 @@ export default async function(context: ControllerPluginContext) { controller.handle(msg.UploadRequest, async (request: msg.UploadRequest) => { let { instanceId, playerName, playerData } = request; - let instanceConfig = controller.instances.get(instanceId)!.config; - let instanceName = instanceConfig.get("instance.name"); + let instanceName = controller.instances.get(instanceId)!.config.get("instance.name"); let store = true; let acquisitionRecord = acquiredPlayers.get(playerName); if (!acquisitionRecord) { @@ -167,9 +166,7 @@ export default async function(context: ControllerPluginContext) { if (store) { logger.verbose(`Received player data for ${playerName} from ${instanceName}`); - playerDatastore.set( - playerName, mergePlayerData(oldPlayerData, playerData, enabledComponents(instanceConfig)) - ); + playerDatastore.set(playerName, mergePlayerData(oldPlayerData, playerData)); playerDatastoreDirty = true; } }); diff --git a/plugins/player_sync/index.ts b/plugins/player_sync/index.ts index 0b367f58..ef8df6ef 100644 --- a/plugins/player_sync/index.ts +++ b/plugins/player_sync/index.ts @@ -39,8 +39,8 @@ export const plugin: lib.PluginDeclaration = { "player_sync.sync_controller": { title: "Sync controller", description: - "Sync whether the player is a character, god or spectator, their respawn timer and cheat " + - "mode. Only synced while inventories are synced too.", + "Sync whether the player is a character, god or spectator, their respawn timer, cheat mode " + + "and god inventory. Only synced while inventories are synced too.", type: "boolean", initialValue: true, }, @@ -59,8 +59,8 @@ export const plugin: lib.PluginDeclaration = { "player_sync.sync_inventories": { title: "Sync inventories", description: - "Sync the character and god inventories, character bonuses and crafting queue. Without the " + - "controller synced only the inventory of a character is synced.", + "Sync the character inventories, character bonuses and crafting queue. Without the " + + "controller synced a player without a character keeps their local inventory.", type: "boolean", initialValue: true, }, diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 4b56ba95..993b56c4 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -22,6 +22,8 @@ export type IpcPlayerData = { personal_logistic_slots?: {name:string, min:number, max:number}[], crafting_queue?: any, recipe_notifications?: string, + // Components the upload was serialized with, all when missing + components?: Record | unknown[], } const jsonPlayerData = Type.Object({ diff --git a/plugins/player_sync/module/download_inventory.lua b/plugins/player_sync/module/download_inventory.lua index 120e346a..18f96828 100644 --- a/plugins/player_sync/module/download_inventory.lua +++ b/plugins/player_sync/module/download_inventory.lua @@ -34,7 +34,7 @@ local function download_inventory(player_name, data, number, total) progress_dialog.remove(player) -- Give the player a character and pretend that's the synced player data - if serialize.syncs(script_data.components, "controller") or player_record.local_controller then + if serialize.syncs(record.components, "controller") or player_record.local_controller then ensure_character(player) end player_record.local_controller = nil @@ -45,6 +45,7 @@ local function download_inventory(player_name, data, number, total) script_data.active_downloads[player_name] = nil player_record.dirty = player.connected player_record.sync = true + player_record.components = record.components player_record.generation = record.generation return end diff --git a/plugins/player_sync/module/player_sync.lua b/plugins/player_sync/module/player_sync.lua index 4f3b381b..5aefd033 100644 --- a/plugins/player_sync/module/player_sync.lua +++ b/plugins/player_sync/module/player_sync.lua @@ -24,9 +24,10 @@ local function is_in_cutscene(player) return player.controller_type == defines.controllers.cutscene end --- Returns true if the component is synced on this instance -local function syncs(name) - return serialize.syncs(get_script_data().components, name) +-- Remember the components a player's synced session uses, uploads are serialized with these +local function start_sync(player_record, components) + player_record.sync = true + player_record.components = components end -- Remember the controller the player had before sync code replaced it, restored when the controller is not synced @@ -95,7 +96,7 @@ function player_sync.serialize_player(player, player_record) end local failed_deserialization = get_script_data().failed_deserialization[player.name] or {} - local serialized_player = serialize.serialize_player(player, failed_deserialization, get_script_data().components) + local serialized_player = serialize.serialize_player(player, failed_deserialization, player_record.components) serialized_player.generation = player_record.generation return serialized_player @@ -118,7 +119,7 @@ function player_sync.deserialize_player(player, finished_record) -- Stash temporary inventory if it exists, without inventory sync the current inventory is the one to keep local stashed_corpse - if syncs("inventories") and player_record.dirty and player.character then + if serialize.syncs(finished_record.components, "inventories") and player_record.dirty and player.character then local character = player.character local surface = character.surface local position = character.position @@ -132,7 +133,7 @@ function player_sync.deserialize_player(player, finished_record) local serialized_player = compat.json_to_table(finished_record.data) assert(type(serialized_player) == "table", "wrong type for serialized_player") script_data.failed_deserialization[player.name] = - serialize.deserialize_player(player, serialized_player, script_data.components, local_controller) + serialize.deserialize_player(player, serialized_player, finished_record.components, local_controller) player_record.local_controller = nil -- Restore player position and driving state @@ -312,8 +313,10 @@ function player_sync.sync_player(acquire_response) local player_record = script_data.players[acquire_response.player_name] if acquire_response.status ~= "acquired" then - if player_record.sync and not player_record.dirty and syncs("inventories") then - if player.controller_type == defines.controllers.character then + if player_record.sync and not player_record.dirty then + if serialize.syncs(player_record.components, "inventories") + and player.controller_type == defines.controllers.character + then save_local_controller(player, player_record) local character = player.character player.set_controller({ type = defines.controllers.spectator }) @@ -327,7 +330,11 @@ function player_sync.sync_player(acquire_response) if acquire_response.has_data - and (not player_record.sync or acquire_response.generation > player_record.generation) + and ( + not player_record.sync + or acquire_response.generation > player_record.generation + or serialize.newly_synced(player_record.components, script_data.components) + ) then player_sync.initiate_inventory_download(player, player_record, acquire_response.generation) @@ -340,7 +347,7 @@ function player_sync.sync_player(acquire_response) ) end - player_record.sync = true + start_sync(player_record, script_data.components) player_record.dirty = true end end @@ -361,7 +368,7 @@ function player_sync.finish_download(player, finished_record) end player_record.dirty = true - player_record.sync = true + start_sync(player_record, finished_record.components) player_record.generation = finished_record.generation end @@ -469,14 +476,15 @@ function player_sync.initiate_inventory_download(player, player_record, generati started = game.ticks_played, last_active = game.ticks_played, generation = generation, - data = "" + data = "", } local script_data = get_script_data() + record.components = script_data.components script_data.active_downloads[player.name] = record -- The plugin only sends back what differs from the current notification state local recipe_notifications - if recipe_notifications_api and syncs("recipe_notifications") then + if recipe_notifications_api and serialize.syncs(record.components, "recipe_notifications") then recipe_notifications = serialize.serialize_crafting_notifications(player) end @@ -488,7 +496,10 @@ function player_sync.initiate_inventory_download(player, player_record, generati -- If this is a synced player turn them into a spectator while the -- player data is downloading -- Without the controller synced a player without a character has nothing synced to protect - if player_record.sync and syncs("inventories") and (syncs("controller") or player.character) then + local components = record.components + if player_record.sync and serialize.syncs(components, "inventories") + and (serialize.syncs(components, "controller") or player.character) + then -- Store original position to teleport back to if v2_remote_controller then record.surface = player.physical_surface diff --git a/plugins/player_sync/module/serialize.lua b/plugins/player_sync/module/serialize.lua index b37ce71f..ba8ad094 100644 --- a/plugins/player_sync/module/serialize.lua +++ b/plugins/player_sync/module/serialize.lua @@ -730,6 +730,7 @@ end --- @field shortcuts table? --- @field game_view_settings table? --- @field ticks_to_respawn number? +--- @field components SyncComponents Components this was serialized with --- @field character table? --- @field inventories table? --- @field hotbar table? @@ -795,6 +796,23 @@ local function syncs(components, name) end serialize.syncs = syncs +local component_names = { + "controller", "force", "appearance", "inventories", "logistics", "quick_bar", "settings", "recipe_notifications", +} + +--- True if a component synced in new_components was not synced in old_components +--- @param old_components SyncComponents +--- @param new_components SyncComponents +--- @return boolean +function serialize.newly_synced(old_components, new_components) + for _, name in ipairs(component_names) do + if syncs(new_components, name) and not syncs(old_components, name) then + return true + end + end + return false +end + --- @param player LuaPlayer --- @param failed_deserialization FailedDeserializationPlayerData --- @param components SyncComponents @@ -808,6 +826,7 @@ function serialize.serialize_player(player, failed_deserialization, components) local serialized = { generation = 0, -- Gets replaced later name = player.name, + components = components, } if sync_controller then diff --git a/plugins/player_sync/test/components.js b/plugins/player_sync/test/components.js index fc2db0c5..bb105cb2 100644 --- a/plugins/player_sync/test/components.js +++ b/plugins/player_sync/test/components.js @@ -1,6 +1,8 @@ import assert from "node:assert/strict"; -import { components, enabledComponents, mergePlayerData, stripPlayerData } from "../dist/node/components.js"; +import { + components, enabledComponents, mergePlayerData, stripPlayerData, uploadedComponents, +} from "../dist/node/components.js"; const allComponents = Object.keys(components); @@ -69,15 +71,45 @@ describe("player_sync components", function() { } assert.equal(playerData.force, "enemy"); }); + it("should remove the god inventory with the controller", function() { + const playerData = structuredClone(stored); + stripPlayerData(playerData, new Set(allComponents.filter(c => c !== "controller"))); + assert.equal("inventories" in playerData, false); + assert.equal("controller" in playerData, false); + assert.equal(playerData.tag, "stored"); + }); + }); + + describe("uploadedComponents()", function() { + it("should sync everything when the upload has no components", function() { + assert.deepEqual([...uploadedComponents(undefined)], allComponents); + }); + it("should sync the components marked true", function() { + const enabled = uploadedComponents({ force: true, quick_bar: true, settings: false }); + assert.deepEqual([...enabled], ["force", "quick_bar"]); + }); + it("should sync nothing for an empty table encoded as an array", function() { + assert.equal(uploadedComponents([]).size, 0); + }); + it("should not sync the controller without the inventories", function() { + assert.equal(uploadedComponents({ controller: true }).has("controller"), false); + }); }); describe("mergePlayerData()", function() { - it("should store the upload as is when all components are synced", function() { - assert.deepEqual(mergePlayerData(stored, uploaded, new Set(allComponents)), uploaded); + function withComponents(data, disabled) { + const enabled = allComponents.filter(c => !disabled.includes(c)); + return { ...data, components: Object.fromEntries(enabled.map(c => [c, true])) }; + } + + it("should store the upload as is when it has no components", function() { + assert.deepEqual(mergePlayerData(stored, uploaded), uploaded); + }); + it("should not store the components of the upload", function() { + assert.deepEqual(mergePlayerData(stored, withComponents(uploaded, [])), uploaded); }); it("should keep the stored fields of disabled components", function() { - const enabled = new Set(allComponents.filter(c => !["force", "appearance"].includes(c))); - const merged = mergePlayerData(stored, uploaded, enabled); + const merged = mergePlayerData(stored, withComponents(uploaded, ["force", "appearance"])); assert.equal(merged.force, "player"); assert.deepEqual(merged.color, [1, 0, 0]); assert.equal(merged.tag, "stored"); @@ -85,27 +117,29 @@ describe("player_sync components", function() { assert.equal(merged.generation, 2); }); it("should drop disabled components without stored data", function() { - const merged = mergePlayerData(undefined, uploaded, new Set(allComponents.filter(c => c !== "force"))); + const merged = mergePlayerData(undefined, withComponents(uploaded, ["force"])); assert.equal("force" in merged, false); assert.equal(merged.tag, "uploaded"); }); it("should take the character but not the controller when the controller is not synced", function() { - const merged = mergePlayerData(stored, uploaded, new Set(allComponents.filter(c => c !== "controller"))); + const merged = mergePlayerData(stored, withComponents(uploaded, ["controller"])); assert.equal(merged.controller, "god"); assert.equal(merged.cheat_mode, true); assert.equal(merged.character, character); assert.deepEqual(merged.inventories, { main: ["stored god item"] }, "god inventory is kept"); }); it("should keep the character data for a player without one when the controller is not synced", function() { - const storedCharacter = { ...stored, character, crafting_queue: ["stored"], personal_logistic_slots: [] }; + const storedCharacter = { + ...stored, character, crafting_queue: ["stored"], personal_logistic_slots: [{ name: "stored" }], + }; const god = { ...uploaded, controller: "god", inventories: { main: ["new god item"] } }; delete god.character; delete god.crafting_queue; - delete god.personal_logistic_slots; - const enabled = new Set(allComponents.filter(c => c !== "controller")); - const merged = mergePlayerData(storedCharacter, god, enabled); + god.personal_logistic_slots = [{ name: "uploaded" }]; + const merged = mergePlayerData(storedCharacter, withComponents(god, ["controller"])); assert.equal(merged.character, character); assert.deepEqual(merged.crafting_queue, ["stored"]); + assert.deepEqual(merged.personal_logistic_slots, [{ name: "stored" }], "logistics stay with the character"); assert.deepEqual(merged.inventories, { main: ["stored god item"] }); }); }); diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index ec2771d6..b1da5e64 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -5,6 +5,7 @@ import path from "node:path"; import * as lib from "@clusterio/lib"; import * as mock from "../../../test/mock.js"; import controllerEntrypoint from "../dist/node/controller.js"; +import { components as allComponents } from "../dist/node/components.js"; import { plugin as info } from "../dist/node/index.js"; import { DatabaseStatsRequest, DownloadRequest, UploadRequest } from "../dist/node/messages.js"; @@ -75,14 +76,15 @@ describe("player_sync controller", function() { .playerData; } - it("should store the upload as is when everything is synced", async function() { + 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); }); - it("should keep stored fields the uploading instance does not sync", async function() { - disabled = ["force"]; + it("should merge by the payload's components and ignore the instance config", async function() { + disabled = ["appearance"]; + const components = Object.fromEntries(Object.keys(allComponents).map(c => [c, c !== "force"])); assert.deepEqual( - await upload({ generation: 2, name: "test", tag: "uploaded" }), + await upload({ generation: 2, name: "test", tag: "uploaded", components }), { generation: 2, name: "test", force: "player", tag: "uploaded" }, ); }); diff --git a/plugins/player_sync/test/instance.js b/plugins/player_sync/test/instance.js index 3cc5d9d5..e869666d 100644 --- a/plugins/player_sync/test/instance.js +++ b/plugins/player_sync/test/instance.js @@ -169,5 +169,14 @@ describe("player_sync", function() { await instance.hooks.instanceConfigFieldChanged.invoke("player_sync.sync_force", false, true); assert.equal(instance.server.rconCommands.length, 1); }); + it("should not send the components when the instance is not running", async function() { + instance.status = "stopped"; + try { + await instance.hooks.instanceConfigFieldChanged.invoke("player_sync.sync_force", false, true); + } finally { + instance.status = "running"; + } + assert.equal(instance.server.rconCommands.length, 0); + }); }); });