From 13e06dd44c03a3991bc4488acd56c617fcad4803 Mon Sep 17 00:00:00 2001 From: Richard Date: Wed, 30 Oct 2024 15:27:55 -0700 Subject: [PATCH] feat: add optional `convertArrays` --- .prettierrc | 8 ++-- README.md | 12 ++++- default.project.json | 2 +- src/charm-sync/src/index.d.ts | 9 ++++ src/charm-sync/src/patch.luau | 75 +++++++++++++------------------ src/charm-sync/src/server.luau | 17 +++++-- src/charm-sync/src/validate.luau | 4 +- tests/charm-sync/patch.spec.luau | 3 +- tests/charm-sync/server.spec.luau | 2 +- 9 files changed, 72 insertions(+), 60 deletions(-) diff --git a/.prettierrc b/.prettierrc index 701a273..537ac33 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,6 +1,6 @@ { - "printWidth": 120, - "tabWidth": 2, - "trailingComma": "all", - "useTabs": true + "printWidth": 120, + "tabWidth": 4, + "trailingComma": "all", + "useTabs": true } diff --git a/README.md b/README.md index ee9b816..e0e49a0 100644 --- a/README.md +++ b/README.md @@ -441,6 +441,7 @@ 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) + convertArrays = true, -- Safety measures for arrays, should be false when using ByteNet or Zap }) -- Sends state updates to clients when a synced atom changes. @@ -466,6 +467,8 @@ 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** `convertArrays`: Whether to convert sparse 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)). + #### Returns `server` returns an object with the following methods: @@ -476,9 +479,11 @@ end) #### Caveats -- 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`. +- **Do not use values that cannot be sent over remotes** in your shared atoms. This includes functions, threads, and non-string keys in dictionaries. -- Charm does not handle network communication. You must implement your own network layer to send and receive state updates. This is implemented via the `remotes` namespace in the example above. +- **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.** You must use remote events to send sync payloads and set `convertArrays` accordingly. This is implemented via the `remotes` namespace in the example above. --- @@ -489,6 +494,7 @@ Call `client` to create a client sync object. This synchronizes the client's ato ```luau local syncer = CharmSync.client({ atoms = atomsToSync, -- A dictionary of the atoms to sync, matching the server's + ignoreUnhydrated = true, -- Whether to ignore state updates before the initial update }) -- Applies state updates from the server to the client's atoms. @@ -508,6 +514,8 @@ remotes.requestState:fire() - `atoms`: A dictionary of the atoms to sync. The keys should match the keys on the server. + - **optional** `ignoreUnhydrated`: Whether to ignore state updates before setting the initial state. Defaults to `true`. + #### Returns `client` returns an object with the following methods: diff --git a/default.project.json b/default.project.json index 8817adb..880d017 100644 --- a/default.project.json +++ b/default.project.json @@ -23,7 +23,7 @@ "ServerScriptService": { "$className": "ServerScriptService", "Tests": { - "$path": "Tests" + "$path": "tests" } } } diff --git a/src/charm-sync/src/index.d.ts b/src/charm-sync/src/index.d.ts index 33168dd..842416e 100644 --- a/src/charm-sync/src/index.d.ts +++ b/src/charm-sync/src/index.d.ts @@ -132,6 +132,15 @@ declare namespace CharmSync { * to reconstruct the state's changes over time. */ preserveHistory?: boolean; + /** + * When this is enabled, arrays are coerced into dictionaries by adding a + * key of `0`, which prevents Roblox from incorrectly sending sparse arrays + * over a vanilla remote event. Defaults to `true`. + * + * Should be disabled when using network libraries like ByteNet and Zap, + * since they can represent numeric keys. + */ + convertArrays?: boolean; } interface ClientSyncer { diff --git a/src/charm-sync/src/patch.luau b/src/charm-sync/src/patch.luau index 6780d1f..8bec021 100644 --- a/src/charm-sync/src/patch.luau +++ b/src/charm-sync/src/patch.luau @@ -10,51 +10,32 @@ local function isNone(value: any): boolean return type(value) == "table" and value.__none == "__none" end ---[=[ - JSON serialization can drop the last values of a sparse array. For example, - `{ [1] = 1, [3] = 3 }` becomes `[1]` in some cases. Ambiguity in the - serialization of sparse arrays can be resolved by converting them to - dictionaries with string keys. - - The `apply` function will convert these dictionaries back into arrays as - long as the target array is not sparse. This function ensures that the diff - function produces a patch that can be correctly applied. -]=] -local function stringifySparseArray(object: { any }): { [string | number]: any } - local maxn = table.maxn(object) - - if maxn == 0 or maxn == #object then - -- The object is either not an array or a dense array - return object +local function diff(prevState: { [any]: any }, nextState: { [any]: any }, convertArrays: boolean?) + if convertArrays == nil then + convertArrays = true end - local dictionary = {} - - for index, value in next, object do - dictionary[tostring(index)] = value - end - - return dictionary -end - -local function diff(prevState: { [any]: any }, nextState: { [any]: any }) - local patches = table.clone(nextState) + local patch = table.clone(nextState) for key, previous in next, prevState do local current = nextState[key] if previous == current then - patches[key] = nil + patch[key] = nil elseif current == nil then - patches[key] = None + patch[key] = None elseif type(previous) == "table" and type(current) == "table" then - patches[key] = diff(previous, current) + patch[key] = diff(previous, current, convertArrays) end end - -- Coerce sparse array patches into dictionaries - if prevState[1] ~= nil or nextState[1] ~= nil then - patches = stringifySparseArray(patches) + -- JSON serialization can drop the last values of a sparse array. For example, + -- `{ [1] = 1, [3] = 3 }` becomes `[1]` in some cases. Ambiguity in the + -- serialization of sparse arrays can be resolved by coercing remote events + -- to convert these arrays into dictionaries. + if convertArrays and (prevState[1] ~= nil or nextState[1] ~= nil) then + -- Set to an existing value to satisfy serialization + patch[0] = prevState[1] or nextState[1] end if _G.__DEV__ then @@ -69,35 +50,39 @@ local function diff(prevState: { [any]: any }, nextState: { [any]: any }) end end - return patches + return patch end -local function apply(state: any, patches: any): any - if type(patches) == "table" and patches.__none == "__none" then +local function apply(state: any, patch: any): any + if type(patch) == "table" and patch.__none == "__none" then return nil - elseif type(state) ~= "table" or type(patches) ~= "table" then - return patches + elseif type(state) ~= "table" or type(patch) ~= "table" then + return patch end - local nextState = table.clone(state) - local stateIsArray = state[1] ~= nil + local result = table.clone(state) + local numeric = state[1] ~= nil - for key, patch in next, patches do + for key, value in next, patch do -- Diff-checking an array produces a sparse array, which will not be -- preserved when converted to JSON. To prevent this, we turn string -- keys back into numeric keys. - if stateIsArray and type(key) == "string" then + if numeric then + if key == "0" or key == 0 then + continue + end + key = tonumber(key) or key end - nextState[key] = apply(nextState[key], patch) + result[key] = apply(result[key], value) end - return nextState + return result end return { - NONE = None, + None = None, isNone = isNone, diff = diff, apply = apply, diff --git a/src/charm-sync/src/server.luau b/src/charm-sync/src/server.luau index 53fe26e..2b42efa 100644 --- a/src/charm-sync/src/server.luau +++ b/src/charm-sync/src/server.luau @@ -23,6 +23,15 @@ type ServerOptions = { per frame, where each individual change is important. Defaults to `false`. ]=] preserveHistory: boolean?, + --[=[ + When this is enabled, arrays are coerced into dictionaries by adding a + key of `0`, which prevents Roblox from incorrectly sending sparse arrays + over a vanilla remote event. Defaults to `true`. + + Should be disabled when using network libraries like ByteNet and Zap, + since they can represent numeric keys. + ]=] + convertArrays: boolean?, } type ServerSyncer = { @@ -127,7 +136,7 @@ local function server(options: ServerOptions): ServerSyncer table.insert(payloads, { type = "patch", - data = patch.diff(snapshots[index - 1], snapshot), + data = patch.diff(snapshots[index - 1], snapshot, options.convertArrays), }) end @@ -164,7 +173,7 @@ local function server(options: ServerOptions): ServerSyncer table.insert(payloads, { type = "patch", - data = patch.diff(snapshots[index - 1], snapshot), + data = patch.diff(snapshots[index - 1], snapshot, options.convertArrays), }) end @@ -199,7 +208,7 @@ local function server(options: ServerOptions): ServerSyncer local payload: SyncPayload = { type = "patch", - data = patch.diff(previousSnapshot, currentSnapshot), + data = patch.diff(previousSnapshot, currentSnapshot, options.convertArrays), } previousSnapshot = table.clone(currentSnapshot) @@ -228,7 +237,7 @@ local function server(options: ServerOptions): ServerSyncer local payload: SyncPayload = { type = "patch", - data = patch.diff(previousSnapshot, currentSnapshot), + data = patch.diff(previousSnapshot, currentSnapshot, options.convertArrays), } previousSnapshot = table.clone(currentSnapshot) diff --git a/src/charm-sync/src/validate.luau b/src/charm-sync/src/validate.luau index 911816d..b568013 100644 --- a/src/charm-sync/src/validate.luau +++ b/src/charm-sync/src/validate.luau @@ -3,7 +3,7 @@ local Charm = require(script.Parent.Parent.Charm) local SAFE_KEYS = { string = true, number = true } local SAFE_VALUES = { string = true, number = true, boolean = true, table = true, userdata = true } -local function isUnsafeTable(object: { [any]: any }) +local function isTableUnsafe(object: { [any]: any }) local keyType = nil local objectSize = 0 @@ -49,7 +49,7 @@ local function validate(value: any, key: any) error(`Cannot sync tables with metatables! Got {value} at key '{key}'`) elseif Charm.isAtom(value) then error(`Cannot sync nested atoms! Got atom at key '{key}'`) - elseif isUnsafeTable(value) then + elseif isTableUnsafe(value) then error( `Cannot sync tables unsupported by remote events! The value has the key '{key}'.\n\n` .. "This can be for the following reasons:\n" diff --git a/tests/charm-sync/patch.spec.luau b/tests/charm-sync/patch.spec.luau index 5128954..a00a19e 100644 --- a/tests/charm-sync/patch.spec.luau +++ b/tests/charm-sync/patch.spec.luau @@ -52,12 +52,13 @@ return function() expect(applied["4"]).to.never.be.ok() end) - it("preserves array density after serdes", function() + itFOCUS("preserves array density after serdes", function() -- This particular change results in a patch that cannot be serialized -- without losing information. Charm resolves this issue internally. local prevState = { "a", "b", "c", "d", "e" } local nextState = { "A", "b", "c", "D" } + -- FIXME: The [0] index used to fix serialization issues is not preserved local patches = serdes(patch.diff(prevState, nextState)) local applied = patch.apply(prevState, patches) diff --git a/tests/charm-sync/server.spec.luau b/tests/charm-sync/server.spec.luau index 7afd5dc..b63d91e 100644 --- a/tests/charm-sync/server.spec.luau +++ b/tests/charm-sync/server.spec.luau @@ -49,7 +49,7 @@ return function() server:_sendPatch(target) expect(payload.data.a.b).to.never.be.ok() - expect(payload.data.a.c).to.equal(patch.NONE) + expect(payload.data.a.c).to.equal(patch.None) expect(payload.data.a.d).to.equal(1) disconnect()