feat: add optional convertArrays

.prettierrc

@@ -1,6 +1,6 @@
 {
-  "printWidth": 120,
-  "tabWidth": 2,
-  "trailingComma": "all",
-  "useTabs": true
+	"printWidth": 120,
+	"tabWidth": 4,
+	"trailingComma": "all",
+	"useTabs": true
 }

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:

default.project.json

@@ -23,7 +23,7 @@
 		"ServerScriptService": {
 			"$className": "ServerScriptService",
 			"Tests": {
-				"$path": "Tests"
+				"$path": "tests"
 			}
 		}
 	}

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<Atoms extends AtomMap> {

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,

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)

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"

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)
 

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()