feat(sync): add autoSerialize option (#42)

* feat(sync): add `autoSerialize` option

* docs: fix `autoSerialize` note

* docs: fix typo

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!
 
 ---
 

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

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

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)

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

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

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 },
 		}
 

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)