From b8bb1233795c9380c46b4bfb9515cde73d14c504 Mon Sep 17 00:00:00 2001 From: richard <56808540+littensy@users.noreply.github.com> Date: Sat, 25 Jan 2025 14:07:14 -0800 Subject: [PATCH] feat(sync): add `autoSerialize` option (#42) * feat(sync): add `autoSerialize` option * docs: fix `autoSerialize` note * docs: fix typo --- README.md | 24 +++++++++++++++++------- packages/charm-sync/src/index.d.ts | 11 +++++------ packages/charm-sync/src/patch.luau | 20 ++++++++------------ packages/charm-sync/src/server.luau | 19 +++++++++---------- packages/charm-sync/src/validate.luau | 12 +++--------- tests/charm-sync/patch.spec.luau | 12 +++++++++++- tests/charm-sync/validate.spec.luau | 5 +---- tests/init.server.luau | 1 + 8 files changed, 55 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index ae188f8..7806828 100644 --- a/README.md +++ b/README.md @@ -438,10 +438,15 @@ Call `server` to create a server sync object. This synchronizes every client's a ```luau local syncer = CharmSync.server({ - atoms = atomsToSync, -- A dictionary of the atoms to sync, matching the client's - interval = 0, -- The minimum interval between state updates - preserveHistory = false, -- Whether to send a full history of changes made to the atoms (slower) - serializeArrays = true, -- Safety measures for arrays, should be false when using ByteNet or Zap + -- A dictionary of the atoms to sync, matching the client's + atoms = atomsToSync, + -- The minimum interval between state updates + interval = 0, + -- Whether to send a full history of changes made to the atoms (slower) + preserveHistory = false, + -- Whether to apply fixes for remote event limitations. Disable this option + -- when using a network library with custom ser/des, like ByteNep or Zap. + autoSerialize = true, }) -- Sends state updates to clients when a synced atom changes. @@ -467,7 +472,12 @@ end) - **optional** `preserveHistory`: Whether to sync an exhaustive history of changes made to the atoms since the last sync event. If `true`, the server sends multiple payloads instead of one. Defaults to `false` for performance. - - **optional** `serializeArrays`: Whether to convert problematic arrays to dictionaries before firing remote events. Because arrays in state patches can have holes, extra work is required to send them over a remote event intact. Defaults to `true`, but should be `false` if payloads are serialized (i.e. if you use [ByteNet](https://github.com/ffrostfall/ByteNet) or [Zap](https://github.com/red-blox/zap)). + - **optional** `autoSerialize`: Whether to apply validation and workarounds to certain [remote argument limitations](https://create.roblox.com/docs/scripting/events/remote#table-indexing). Defaults to `true`, but you should set it to `false` if you serialize remote arguments (i.e. if you use [ByteNet](https://github.com/ffrostfall/ByteNet) or [Zap](https://github.com/red-blox/zap)). + +> [!NOTE] +> Charm sends table updates in the form of partial tables, so arrays will contain `nil` values, which has undefined behavior in remotes without serialization. +> +> Charm's default `autoSerialize` behavior fixes this, but it can interfere with custom serialization. Disable this option if you use a network library that serializes remote event arguments. #### Returns @@ -479,11 +489,11 @@ end) #### Caveats -- **Do not use values that cannot be sent over remotes** in your shared atoms. This includes functions, threads, and non-string keys in dictionaries. +- **Do not use values that cannot be sent over remotes** in your shared atoms. This includes functions, threads, and non-string keys in dictionaries. [Read more about argument limitations in remotes.](https://create.roblox.com/docs/scripting/events/remote#argument-limitations) - **By default, Charm omits the individual changes made to atoms** between sync events (i.e. a `counterAtom` set to `1` and then `2` will only send the final state of `2`). If you need to preserve a history of changes, set `preserveHistory` to `true`. -- **Charm does not handle network communication.** Use remote events or a network library to send sync payloads - and remember to set `serializeArrays` accordingly! +- **Charm does not handle network communication.** Use remote events or a network library to send sync payloads - and remember to set `autoSerialize` accordingly! --- diff --git a/packages/charm-sync/src/index.d.ts b/packages/charm-sync/src/index.d.ts index 1c59675..41e88a5 100644 --- a/packages/charm-sync/src/index.d.ts +++ b/packages/charm-sync/src/index.d.ts @@ -132,14 +132,13 @@ declare namespace CharmSync { */ preserveHistory?: boolean; /** - * When `true`, convert problematic sparse arrays into dictionaries - * with string keys. Defaults to `true`. + * When `true`, Charm will apply validation and serialize unsafe arrays + * to address remote event argument limitations. Defaults to `true`. * - * While this is mandatory for safely syncing arrays over vanilla Roblox - * remotes, it can be disabled if your network library uses a custom - * serialization method (i.e. Zap, ByteNet). + * This option should be disabled if your network library uses a custom + * serialization method (i.e. Zap, ByteNet) to prevent interference. */ - serializeArrays?: boolean; + autoSerialize?: boolean; } interface ClientSyncer { diff --git a/packages/charm-sync/src/patch.luau b/packages/charm-sync/src/patch.luau index 7ad5554..3d021b5 100644 --- a/packages/charm-sync/src/patch.luau +++ b/packages/charm-sync/src/patch.luau @@ -45,10 +45,12 @@ end @param prevState The previous state. @param nextState The next state. - @param serializeArrays Whether to convert sparse arrays into dictionaries. Defaults to `true`. + @param serialize Whether to convert sparse arrays into dictionaries. Defaults to `true`. @returns A patch that can be applied to the previous state. ]=] -local function diff(prevState: { [any]: any }, nextState: { [any]: any }, serializeArrays: boolean?) +local function diff(prevState: { [any]: any }, nextState: { [any]: any }, serialize: boolean?) + serialize = serialize ~= false + local patches = table.clone(nextState) for key, previous in next, prevState do @@ -59,25 +61,19 @@ local function diff(prevState: { [any]: any }, nextState: { [any]: any }, serial elseif current == nil then patches[key] = None elseif type(previous) == "table" and type(current) == "table" then - patches[key] = diff(previous, current, serializeArrays) + patches[key] = diff(previous, current, serialize) end end -- Coerce sparse array patches into dictionaries - if serializeArrays ~= false and (prevState[1] ~= nil or nextState[1] ~= nil) then + if serialize and (prevState[1] ~= nil or nextState[1] ~= nil) then patches = stringifySparseArray(patches) end - if __DEV__ then - for key, value in next, prevState do + if serialize and __DEV__ then + for key, value in next, patches do validate(value, key) end - - for key, value in nextState do - if prevState[key] ~= value then - validate(value, key) - end - end end return patches diff --git a/packages/charm-sync/src/server.luau b/packages/charm-sync/src/server.luau index 3327b87..a2a8de7 100644 --- a/packages/charm-sync/src/server.luau +++ b/packages/charm-sync/src/server.luau @@ -24,14 +24,13 @@ type ServerOptions = { ]=] preserveHistory: boolean?, --[=[ - Whether to convert problematic sparse arrays into dictionaries with - string keys. Defaults to `true`. + When `true`, Charm will apply validation and serialize unsafe arrays + to address remote event argument limitations. Defaults to `true`. - While this is mandatory for safely syncing arrays over vanilla Roblox - remotes, it can be disabled if your network library uses a custom - serialization method (i.e. Zap, ByteNet). + This option should be disabled if your network library uses a custom + serialization method (i.e. Zap, ByteNet) to prevent interference. ]=] - serializeArrays: boolean?, + autoSerialize: boolean?, } type ServerSyncer = { @@ -144,7 +143,7 @@ local function server(options: ServerOptions): ServerSyncer table.insert(payloads, { type = "patch", - data = patch.diff(snapshots[index - 1], snapshot, options.serializeArrays), + data = patch.diff(snapshots[index - 1], snapshot, options.autoSerialize), }) end @@ -181,7 +180,7 @@ local function server(options: ServerOptions): ServerSyncer table.insert(payloads, { type = "patch", - data = patch.diff(snapshots[index - 1], snapshot, options.serializeArrays), + data = patch.diff(snapshots[index - 1], snapshot, options.autoSerialize), }) end @@ -218,7 +217,7 @@ local function server(options: ServerOptions): ServerSyncer local payload: SyncPayload = { type = "patch", - data = patch.diff(previousSnapshot, currentSnapshot, options.serializeArrays), + data = patch.diff(previousSnapshot, currentSnapshot, options.autoSerialize), } previousSnapshot = table.clone(currentSnapshot) @@ -247,7 +246,7 @@ local function server(options: ServerOptions): ServerSyncer local payload: SyncPayload = { type = "patch", - data = patch.diff(previousSnapshot, currentSnapshot, options.serializeArrays), + data = patch.diff(previousSnapshot, currentSnapshot, options.autoSerialize), } previousSnapshot = table.clone(currentSnapshot) diff --git a/packages/charm-sync/src/validate.luau b/packages/charm-sync/src/validate.luau index 833f4b6..c832542 100644 --- a/packages/charm-sync/src/validate.luau +++ b/packages/charm-sync/src/validate.luau @@ -1,5 +1,5 @@ local SAFE_KEYS = { string = true, number = true } -local SAFE_VALUES = { string = true, number = true, boolean = true, table = true, userdata = true, vector = true } +local UNSAFE_VALUES = { ["function"] = true, thread = true } local function isTableUnsafe(object: { [any]: any }) local keyType = nil @@ -40,7 +40,7 @@ local function validate(value: any, key: any) if not SAFE_KEYS[typeOfKey] then error(`Invalid key type '{typeOfKey}' at key '{key}'`) - elseif not SAFE_VALUES[typeOfValue] then + elseif UNSAFE_VALUES[typeOfValue] then error(`Invalid value type '{typeOfValue}' at key '{key}'`) elseif typeOfValue == "table" then if getmetatable(value) ~= nil then @@ -56,13 +56,7 @@ local function validate(value: any, key: any) end end - if typeOfValue == "number" then - if value ~= value then - error(`Cannot sync NaN at key '{key}'`) - elseif value == math.huge or value == -math.huge then - error(`Cannot sync infinity at key '{key}'`) - end - elseif typeOfKey == "number" then + if typeOfKey == "number" then if key == math.huge or key == -math.huge then error("Cannot sync infinity as key") elseif key ~= math.floor(key) then diff --git a/tests/charm-sync/patch.spec.luau b/tests/charm-sync/patch.spec.luau index e76becb..6e850b2 100644 --- a/tests/charm-sync/patch.spec.luau +++ b/tests/charm-sync/patch.spec.luau @@ -100,7 +100,7 @@ return function() end) it("optionally fixes sparse arrays", function() - for index in next, patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }) do + for index in next, patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }, true) do expect(index).to.be.a("string") end @@ -108,4 +108,14 @@ return function() expect(index).to.be.a("number") end end) + + it("optionally validates state", function() + expect(function() + patch.diff({}, { [0.1] = true }, true) + end).to.throw() + + expect(function() + patch.diff({}, { [0.1] = true }, false) + end).to.never.throw() + end) end diff --git a/tests/charm-sync/validate.spec.luau b/tests/charm-sync/validate.spec.luau index 8065b9f..0a378d8 100644 --- a/tests/charm-sync/validate.spec.luau +++ b/tests/charm-sync/validate.spec.luau @@ -12,12 +12,9 @@ return function() array_sparse_invalid = { value = { [1] = 0, [3] = 0 }, expected = false }, map_decimal_key = { value = { [1.5] = 0 }, expected = false }, + map_infinity_key = { value = { [math.huge] = true }, expected = false }, map_mixed_key = { value = { [1] = 0, a = 0 }, expected = false }, - number = { value = 1, expected = true }, - number_infinity = { value = math.huge, expected = false }, - number_nan = { value = 0 / 0, expected = false }, - nested_atom = { value = atom(0), expected = false }, } diff --git a/tests/init.server.luau b/tests/init.server.luau index 4b3505c..518d08b 100644 --- a/tests/init.server.luau +++ b/tests/init.server.luau @@ -7,6 +7,7 @@ local TestEZ = require(ReplicatedStorage.DevPackages.TestEZ) -- Start Charm in debug mode _G.__DEV__ = true require(ReplicatedStorage.DevPackages.Charm) +require(ReplicatedStorage.DevPackages.CharmSync) _G.__DEV__ = false TestEZ.TestBootstrap:run({ script.Parent }, TestEZ.Reporters.TextReporter)