diff --git a/README.md b/README.md index 01a90a8..a580e50 100644 --- a/README.md +++ b/README.md @@ -641,12 +641,12 @@ end) A configuration table that customizes the behavior of Charm Sync on the server. -| Option | Default | Description | -| --------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| interval | `0` | The frequency at which the server will send patches to the client, in seconds. A value of `0` sends updates on the next frame. Set to a negative value to disable the interval. | -| preserveHistory | `false` | Whether to preserve a full history of state changes since the last sync, at the cost of performance. This is useful if you need to replicate each individual change that occurs between sync events. | -| fixArrays | `true` | When `true`, Charm will attempt to work around Roblox remote event limitations regarding array patches. Disable this if your networking library serializes remote arguments (Zap, ByteNet, etc.). | -| validatePatches | `true` | When `true`, and both `fixArrays` and strict mode are enabled, patches will be validated to enforce remote event limitations. | +| Option | Default | Description | +| --------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| interval | `0` | The frequency at which the server will send patches to the client, in seconds. A value of `0` sends updates on the next frame. Set to a negative value to disable the interval. | +| preserveHistory | `false` | Whether to preserve a full history of state changes since the last sync, at the cost of performance. This is useful if you need to replicate each individual change that occurs between sync events. | +| fixArrays | `true` | When `true`, Charm will attempt to work around Roblox remote event limitations regarding array patches. Disable this if your networking library serializes remote arguments (Zap, ByteNet, etc.). | +| validatePatches | `true` | When `true`, and both `fixArrays` and strict mode are enabled, synced values containing unsafe sparse arrays or mixed tables will throw an error. See the [remote argument limitations](https://create.roblox.com/docs/scripting/events/remote#argument-limitations). | --- diff --git a/packages/charm-sync/src/__tests__/patch.spec.luau b/packages/charm-sync/src/__tests__/patch.spec.luau index fbf5607..a62b456 100644 --- a/packages/charm-sync/src/__tests__/patch.spec.luau +++ b/packages/charm-sync/src/__tests__/patch.spec.luau @@ -81,16 +81,18 @@ test("preserves array density after serdes", function() local patches = mockArraySerdes(patch.diff(prevState, nextState)) local applied = patch.apply(prevState, patches) - for index in next, prevState do + for index in prevState do assert(applied[index] == nextState[index], `expected {applied[index]} to be {nextState[index]}`) end - for index in next, applied do + for index in applied do assert(applied[index] == nextState[index], `expected {applied[index]} to be {nextState[index]}`) end end) test("cannot preserve sparse arrays", function() + config.validatePatches = false + local prevState = {} local nextState = { [2] = "foo" } @@ -106,13 +108,13 @@ end) test("optionally fixes sparse arrays", function() config.fixArrays = true - for index in next, patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }) do + for index in patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }) do assert(type(index) == "string", `expected {type(index)} {index} to be string`) end config.fixArrays = false - for index in next, patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }) do + for index in patch.diff({ 1, 1, 1, 1, 1 }, { 2, 1, 1, 2 }) do assert(type(index) == "number", `expected {type(index)} {index} to be number`) end end) @@ -120,14 +122,14 @@ end) test("optionally validates state", function() local success = pcall(function() config.validatePatches = true - patch.diff({}, { [0.1] = true }) + patch.diff({}, { [1] = 1, a = true }) end) - assert(not success, "should not allow non-integer keys") + assert(not success, "should not allow mixed tables") success = pcall(function() config.validatePatches = false - patch.diff({}, { [0.1] = true }) + patch.diff({}, { [1] = 1, a = true }) end) assert(success, "should skip validation if not requested") diff --git a/packages/charm-sync/src/__tests__/validate.spec.luau b/packages/charm-sync/src/__tests__/validate.spec.luau index 5d8b99a..1f107fd 100644 --- a/packages/charm-sync/src/__tests__/validate.spec.luau +++ b/packages/charm-sync/src/__tests__/validate.spec.luau @@ -1,28 +1,18 @@ -local Charm = require("../../Charm") local suite = require("../../../../test/suite") local validate = require("../validate") -local atom = Charm.atom local test = suite.test test("validates inputs", function() local cases: { [string]: { value: any, expected: any } } = { array = { value = { 1, 2, 3 }, expected = true }, - array_sparse = { value = { 1, nil :: any, 3 }, expected = true }, - 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 }, - - nested_atom = { value = atom(0), expected = false }, + arraySparse = { value = { 1, nil :: any, 3 }, expected = true }, + arraySparseInvalid = { value = { [1] = 0, [3] = 0 }, expected = false }, + mixedTable = { value = { [1] = "foo", a = true }, expected = false }, } - for key, case in next, cases do - local success, result = pcall(function() - validate(case.value, key) - end) - + for key, case in cases do + local success, result: any = pcall(validate, case.value) if case.expected and not success then error(`Expected {key} to be valid, threw {result}`) elseif not case.expected and success then diff --git a/packages/charm-sync/src/config.luau b/packages/charm-sync/src/config.luau index 6a71e12..9b528fc 100644 --- a/packages/charm-sync/src/config.luau +++ b/packages/charm-sync/src/config.luau @@ -13,8 +13,8 @@ return { --- This option should be disabled if your network library uses a custom --- serialization method (i.e. Zap, ByteNet) to prevent interference. fixArrays = true, - --- When `true`, patches will be validated before being sent to clients to - --- enforce remote event limitations. Only checked in strict mode and if + --- When `true`, synced state containing unsafe sparse arrays or mixed + --- tables will throw an error. Only checked in strict mode and if --- `fixArrays` is enabled. Defaults to `true`. validatePatches = true, } diff --git a/packages/charm-sync/src/index.d.ts b/packages/charm-sync/src/index.d.ts index 70a0f22..3fed7a8 100644 --- a/packages/charm-sync/src/index.d.ts +++ b/packages/charm-sync/src/index.d.ts @@ -90,8 +90,8 @@ export const config: { */ fixArrays: boolean; /** - * When `true`, patches will be validated before being sent to clients to - * enforce remote event limitations. Only checked in strict mode and if + * When `true`, synced state containing unsafe sparse arrays or mixed + * tables will throw an error. Only checked in strict mode and if * `fixArrays` is enabled. Defaults to `true`. * * @server diff --git a/packages/charm-sync/src/patch.luau b/packages/charm-sync/src/patch.luau index 88f414b..39723d5 100644 --- a/packages/charm-sync/src/patch.luau +++ b/packages/charm-sync/src/patch.luau @@ -74,15 +74,13 @@ local function diff(currentState: any, nextState: any): any end if config.fixArrays then - -- Coerce sparse array patches into dictionaries + -- Convert sparse array patches that could be misserialized if currentState[1] ~= nil or nextState[1] ~= nil then patches = stringifySparseArray(patches) end if globals.strict and config.validatePatches then - for key, value in patches do - validate(value, key) - end + validate(patches) end end diff --git a/packages/charm-sync/src/validate.luau b/packages/charm-sync/src/validate.luau index 64d7caf..f350bb7 100644 --- a/packages/charm-sync/src/validate.luau +++ b/packages/charm-sync/src/validate.luau @@ -1,66 +1,50 @@ -local SAFE_KEYS = { string = true, number = true } -local UNSAFE_VALUES = { ["function"] = true, thread = true } - local function isTableUnsafe(object: { [any]: any }) local key = next(object) if key == nil then - -- An empty table is always safe return false end local keyType = type(key) - local keyCount = 0 + local maxKey = 0 - -- Keys must either be all strings or all numbers + -- Keys must all be the same type for key in object do if type(key) ~= keyType then return true + elseif keyType == "number" and key > maxKey then + maxKey = key end - keyCount += 1 end -- Array keys must be sequential - if keyType == "number" and keyCount > #object then + if keyType == "number" and maxKey > #object then return true end return false end ---- Validates a value to ensure it can be synced over a remote event. +--- Throws an error if the value is a table that cannot be synced via +--- RemoteEvents or RemoteFunctions. This only aims to catch common mistakes +--- with remote argument limitations, such as sparse arrays or mixed tables. --- ---- @param value The value to validate. ---- @param key The key of the value in the table. +--- @param value The table value. +--- @param key The key of the value. --- @error Throws an error if the value cannot be synced. -local function validate(value: any, key: any) - local typeKey = type(key) - local typeValue = type(value) - - if not SAFE_KEYS[typeKey] then - error(`Invalid key type '{typeKey}' at key '{key}'`) - elseif UNSAFE_VALUES[typeValue] then - error(`Invalid value type '{typeValue}' at key '{key}'`) - elseif typeValue == "table" then - if getmetatable(value) ~= nil then - error(`Cannot sync tables with metatables! Got {value} at key '{key}'`) - elseif isTableUnsafe(value) then - error( - `Cannot sync tables unsupported by remote events! The table was found at the key '{key}'.\n\n` - .. "This can be for the following reasons:\n" - .. "1. The object is an array with non-sequential keys\n" - .. "2. The object is a dictionary with mixed key types (e.g. string and number)\n\n" - .. "Read more: https://create.roblox.com/docs/scripting/events/remote#argument-limitations" - ) - end - end - - if typeKey == "number" then - if key == math.huge or key == -math.huge then - error("Cannot sync infinity as key") - elseif key ~= math.floor(key) then - error("Cannot sync non-integer number as key") +local function validate(value: { [any]: any }) + if isTableUnsafe(value) then + -- Silence warnings during testing + if game then + warn("Cannot sync table:", value) end + error( + `Cannot sync tables unsupported by remote events! See the warning above for more info.\n\n` + .. "This can be for the following reasons:\n" + .. "1. The table is a sparse array with non-sequential indices\n" + .. "2. The table is a mixed table (not all keys have the same type)\n\n" + .. "Read more: https://create.roblox.com/docs/scripting/events/remote#argument-limitations" + ) end end