diff --git a/plugin/src/ChangeBatcher/encodeHelpers.lua b/plugin/src/ChangeBatcher/encodeHelpers.lua new file mode 100644 index 00000000..83b198ce --- /dev/null +++ b/plugin/src/ChangeBatcher/encodeHelpers.lua @@ -0,0 +1,85 @@ +--[[ + Shared encoding helpers for ChangeBatcher encoders. Consolidates property + filter logic so encodeInstance.lua and encodeService.lua stay in sync. +]] + +local Packages = script.Parent.Parent.Parent.Packages +local RbxDom = require(Packages.RbxDom) + +local encodeProperty = require(script.Parent.encodeProperty) +local UNENCODABLE_DATA_TYPES = require(script.Parent.propertyFilter) + +local SKIP_PROPERTIES = { + Parent = true, + Name = true, + Archivable = true, +} + +local Helpers = {} + +function Helpers.encodeAttributes(instance, properties) + local success, attributes = pcall(function() + return instance:GetAttributes() + end) + if success and attributes and next(attributes) then + local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, "Attributes") + if descriptor then + local ok, result = encodeProperty(instance, "Attributes", descriptor) + if ok and result ~= nil then + properties.Attributes = result + end + end + end +end + +function Helpers.encodeTags(instance, properties) + local success, tags = pcall(function() + return instance:GetTags() + end) + if success and tags and #tags > 0 then + local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, "Tags") + if descriptor then + local ok, result = encodeProperty(instance, "Tags", descriptor) + if ok and result ~= nil then + properties.Tags = result + end + end + end +end + +-- Iterates all encodable properties for a class, applying common filters: +-- SKIP_PROPERTIES, per-class skip map, Attributes/Tags, scriptability, +-- serialization, and UNENCODABLE_DATA_TYPES. Calls callback(propertyName, +-- descriptor) for each property that passes all filters. +function Helpers.forEachEncodableProperty(className, classSkipProps, callback) + local classDescriptor = RbxDom.findClassDescriptor(className) + if not classDescriptor then + return + end + + for propertyName, propertyMeta in pairs(classDescriptor.properties) do + if SKIP_PROPERTIES[propertyName] then + continue + end + if classSkipProps and classSkipProps[propertyName] then + continue + end + if propertyName == "Attributes" or propertyName == "Tags" then + continue + end + + local isReadable = propertyMeta.scriptability == "ReadWrite" + or propertyMeta.scriptability == "Read" + or propertyMeta.scriptability == "Custom" + local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize" + + if isReadable and doesSerialize then + local descriptor = RbxDom.findCanonicalPropertyDescriptor(className, propertyName) + if descriptor and not UNENCODABLE_DATA_TYPES[descriptor.dataType] then + callback(propertyName, descriptor) + end + end + end +end + +return Helpers diff --git a/plugin/src/ChangeBatcher/encodeInstance.lua b/plugin/src/ChangeBatcher/encodeInstance.lua index a41f41c0..f621511d 100644 --- a/plugin/src/ChangeBatcher/encodeInstance.lua +++ b/plugin/src/ChangeBatcher/encodeInstance.lua @@ -10,10 +10,10 @@ Children are recursively encoded and included. DUPLICATE HANDLING: - - Instances with duplicate-named siblings at ANY level of the path are skipped - - This matches the behavior in ref_properties.rs::is_path_unique - - A path is ambiguous if duplicates exist anywhere from the instance to the root - - A warning is logged when duplicates are encountered + - Instances with duplicate-named siblings are still encoded and sent to the + server. The server handles filesystem name collisions via the dedup suffix + system (~2, ~3, etc.) in syncback/file_names.rs. + - A debug message is logged when duplicates are detected (for diagnostics only). ]] local Packages = script.Parent.Parent.Parent.Packages @@ -21,7 +21,7 @@ local Log = require(Packages.Log) local RbxDom = require(Packages.RbxDom) local encodeProperty = require(script.Parent.encodeProperty) -local UNENCODABLE_DATA_TYPES = require(script.Parent.propertyFilter) +local Helpers = require(script.Parent.encodeHelpers) -- Script class names that need Source property local SCRIPT_CLASS_NAMES = { @@ -30,13 +30,6 @@ local SCRIPT_CLASS_NAMES = { ModuleScript = true, } --- Properties to always skip when encoding (internal Roblox properties that never serialize) -local SKIP_PROPERTIES = { - Parent = true, - Name = true, -- We handle this separately - Archivable = true, -} - -- Properties to skip per class (matches filter_out_property in Rust syncback) -- These are properties whose values are encoded in the file itself, not metadata local SKIP_PROPERTIES_BY_CLASS = { @@ -117,45 +110,6 @@ end -- Forward declaration for recursion local encodeInstance --- Encode Attributes if present on any instance -local function encodeAttributes(instance, properties) - -- Try to get Attributes - this works for all instance types - local success, attributes = pcall(function() - return instance:GetAttributes() - end) - - if success and attributes and next(attributes) then - -- Attributes need to be encoded specially - -- The server expects them in the properties map under "Attributes" - local attributeDescriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, "Attributes") - if attributeDescriptor then - local encodeSuccess, encodeResult = encodeProperty(instance, "Attributes", attributeDescriptor) - if encodeSuccess and encodeResult ~= nil then - properties.Attributes = encodeResult - end - end - end -end - --- Encode Tags if present on any instance -local function encodeTags(instance, properties) - -- Try to get Tags - this works for all instance types - local success, tags = pcall(function() - return instance:GetTags() - end) - - if success and tags and #tags > 0 then - -- Tags need to be encoded specially - local tagDescriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, "Tags") - if tagDescriptor then - local encodeSuccess, encodeResult = encodeProperty(instance, "Tags", tagDescriptor) - if encodeSuccess and encodeResult ~= nil then - properties.Tags = encodeResult - end - end - end -end - encodeInstance = function(instance, parentId, _skipPathCheck) -- Log duplicate-named siblings as a debug message (server handles them via -- dedup suffixes) but do NOT skip encoding. @@ -174,9 +128,8 @@ encodeInstance = function(instance, parentId, _skipPathCheck) local properties = {} - -- Always try to encode Attributes and Tags for any instance type - encodeAttributes(instance, properties) - encodeTags(instance, properties) + Helpers.encodeAttributes(instance, properties) + Helpers.encodeTags(instance, properties) -- For scripts, encode the Source property first (required for script files) if SCRIPT_CLASS_NAMES[instance.ClassName] then @@ -195,61 +148,22 @@ encodeInstance = function(instance, parentId, _skipPathCheck) end end - -- Encode ALL properties for ANY instance type (matching Rust syncback behavior) - -- The server handles filtering defaults and serialization - local classDescriptor = RbxDom.findClassDescriptor(instance.ClassName) - if classDescriptor then - -- Get class-specific properties to skip - local classSkipProps = SKIP_PROPERTIES_BY_CLASS[instance.ClassName] or {} - - for propertyName, propertyMeta in pairs(classDescriptor.properties) do - -- Skip universally skipped properties - if SKIP_PROPERTIES[propertyName] then - continue - end - - -- Skip class-specific properties (Source, RunContext for scripts, etc.) - if classSkipProps[propertyName] then - continue - end - - -- Skip Attributes and Tags since we handle them separately above - if propertyName == "Attributes" or propertyName == "Tags" then - continue - end - - -- Only encode properties that: - -- 1. Are readable (ReadWrite or Read scriptability) - -- 2. Actually serialize (not "DoesNotSerialize") - local isReadable = propertyMeta.scriptability == "ReadWrite" - or propertyMeta.scriptability == "Read" - or propertyMeta.scriptability == "Custom" - local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize" - - if isReadable and doesSerialize then - -- Get the full PropertyDescriptor for encoding - local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName) - if descriptor then - if UNENCODABLE_DATA_TYPES[descriptor.dataType] then - continue - end - - -- Ref properties cannot be encoded during instance addition - -- because the target instance has no server ID yet. They are - -- handled separately by encodePatchUpdate.lua during property - -- update encoding via the InstanceMap. - if descriptor.dataType == "Ref" then - continue - end - - local encodeSuccess, encodeResult = encodeProperty(instance, propertyName, descriptor) - if encodeSuccess and encodeResult ~= nil then - properties[propertyName] = encodeResult - end - end - end + -- Encode ALL properties for ANY instance type (matching Rust syncback behavior). + -- The server handles filtering defaults and serialization. + local classSkipProps = SKIP_PROPERTIES_BY_CLASS[instance.ClassName] + Helpers.forEachEncodableProperty(instance.ClassName, classSkipProps, function(propertyName, descriptor) + -- Ref properties cannot be encoded during instance addition because the + -- target instance has no server ID yet. They are handled separately by + -- encodePatchUpdate.lua via the InstanceMap. + if descriptor.dataType == "Ref" then + return end - end + + local encodeSuccess, encodeResult = encodeProperty(instance, propertyName, descriptor) + if encodeSuccess and encodeResult ~= nil then + properties[propertyName] = encodeResult + end + end) -- Recursively encode children (including those with duplicate names; -- the server handles duplicates via dedup suffix system) diff --git a/plugin/src/ChangeBatcher/encodeService.lua b/plugin/src/ChangeBatcher/encodeService.lua index b7ab63f9..30841826 100644 --- a/plugin/src/ChangeBatcher/encodeService.lua +++ b/plugin/src/ChangeBatcher/encodeService.lua @@ -1,15 +1,9 @@ -local Packages = script.Parent.Parent.Parent.Packages -local RbxDom = require(Packages.RbxDom) - local encodeProperty = require(script.Parent.encodeProperty) -local UNENCODABLE_DATA_TYPES = require(script.Parent.propertyFilter) - -local SKIP_PROPERTIES = { - Parent = true, - Name = true, - Archivable = true, -} +local Helpers = require(script.Parent.encodeHelpers) +-- ClockTime and TimeOfDay on Lighting are linked properties representing +-- the same value (float vs string). Exclude ClockTime so only TimeOfDay +-- is sent, avoiding redundant/conflicting representations. local EXCLUDE_PROPERTIES = { Lighting = { ClockTime = true }, } @@ -20,78 +14,28 @@ return function(service: Instance) local children = service:GetChildren() local refTargets = {} - local classDescriptor = RbxDom.findClassDescriptor(service.ClassName) - if classDescriptor then - local classExcludes = EXCLUDE_PROPERTIES[service.ClassName] - for propertyName, propertyMeta in pairs(classDescriptor.properties) do - if SKIP_PROPERTIES[propertyName] then - continue - end - if classExcludes and classExcludes[propertyName] then - continue - end - if propertyName == "Attributes" or propertyName == "Tags" then - continue - end - - local isReadable = propertyMeta.scriptability == "ReadWrite" - or propertyMeta.scriptability == "Read" - or propertyMeta.scriptability == "Custom" - local doesSerialize = propertyMeta.serialization ~= "DoesNotSerialize" - - if isReadable and doesSerialize then - local descriptor = RbxDom.findCanonicalPropertyDescriptor(service.ClassName, propertyName) - if descriptor then - if UNENCODABLE_DATA_TYPES[descriptor.dataType] then - continue - end - - if descriptor.dataType == "Ref" then - local readOk, target = descriptor:read(service) - if readOk and target then - local carrier = Instance.new("ObjectValue") - carrier.Name = propertyName - carrier.Value = target - table.insert(refTargets, carrier) - refs[propertyName] = #refTargets - end - continue - end - - local encodeOk, encoded = encodeProperty(service, propertyName, descriptor) - if encodeOk and encoded ~= nil then - properties[propertyName] = encoded - end - end + local classExcludes = EXCLUDE_PROPERTIES[service.ClassName] + Helpers.forEachEncodableProperty(service.ClassName, classExcludes, function(propertyName, descriptor) + if descriptor.dataType == "Ref" then + local readOk, target = descriptor:read(service) + if readOk and target then + local carrier = Instance.new("ObjectValue") + carrier.Name = propertyName + carrier.Value = target + table.insert(refTargets, carrier) + refs[propertyName] = #refTargets end + return end - end - local attrOk, attrMap = pcall(function() - return service:GetAttributes() + local encodeOk, encoded = encodeProperty(service, propertyName, descriptor) + if encodeOk and encoded ~= nil then + properties[propertyName] = encoded + end end) - if attrOk and attrMap and next(attrMap) then - local descriptor = RbxDom.findCanonicalPropertyDescriptor(service.ClassName, "Attributes") - if descriptor then - local encodeOk, encoded = encodeProperty(service, "Attributes", descriptor) - if encodeOk and encoded ~= nil then - properties.Attributes = encoded - end - end - end - local tagOk, tagList = pcall(function() - return service:GetTags() - end) - if tagOk and tagList and #tagList > 0 then - local descriptor = RbxDom.findCanonicalPropertyDescriptor(service.ClassName, "Tags") - if descriptor then - local encodeOk, encoded = encodeProperty(service, "Tags", descriptor) - if encodeOk and encoded ~= nil then - properties.Tags = encoded - end - end - end + Helpers.encodeAttributes(service, properties) + Helpers.encodeTags(service, properties) local chunk = { className = service.ClassName, diff --git a/plugin/src/ChangeBatcher/encodeService.spec.lua b/plugin/src/ChangeBatcher/encodeService.spec.lua new file mode 100644 index 00000000..d0d79051 --- /dev/null +++ b/plugin/src/ChangeBatcher/encodeService.spec.lua @@ -0,0 +1,142 @@ +return function() + local encodeService = require(script.Parent.encodeService) + + describe("chunk structure", function() + it("should return className matching the service", function() + local lighting = game:GetService("Lighting") + local chunk = encodeService(lighting) + + expect(chunk.className).to.equal("Lighting") + end) + + it("should return childCount matching GetChildren", function() + local lighting = game:GetService("Lighting") + local chunk = encodeService(lighting) + + expect(chunk.childCount).to.equal(#lighting:GetChildren()) + end) + + it("should omit properties when none encode", function() + local folder = Instance.new("Folder") + folder.Name = "EmptyService" + + local chunk = encodeService(folder) + + expect(chunk.className).to.equal("Folder") + expect(chunk.childCount).to.equal(0) + expect(chunk.refTargetCount).to.equal(0) + end) + end) + + describe("property encoding", function() + it("should encode Lighting.TimeOfDay", function() + local lighting = game:GetService("Lighting") + local chunk = encodeService(lighting) + + expect(chunk.properties).to.be.ok() + expect(chunk.properties.TimeOfDay).to.be.ok() + end) + + it("should not include skipped properties (Parent, Name, Archivable)", function() + local lighting = game:GetService("Lighting") + local chunk = encodeService(lighting) + + if chunk.properties then + expect(chunk.properties.Parent).to.equal(nil) + expect(chunk.properties.Name).to.equal(nil) + expect(chunk.properties.Archivable).to.equal(nil) + end + end) + end) + + describe("EXCLUDE_PROPERTIES", function() + it("should not include Lighting.ClockTime", function() + local lighting = game:GetService("Lighting") + local chunk = encodeService(lighting) + + if chunk.properties then + expect(chunk.properties.ClockTime).to.equal(nil) + end + end) + end) + + describe("Attributes and Tags", function() + it("should encode Attributes when present", function() + local folder = Instance.new("Folder") + folder:SetAttribute("TestKey", 42) + + local chunk = encodeService(folder) + + expect(chunk.properties).to.be.ok() + expect(chunk.properties.Attributes).to.be.ok() + + folder:Destroy() + end) + + it("should encode Tags when present", function() + local folder = Instance.new("Folder") + folder:AddTag("SyncTag") + + local chunk = encodeService(folder) + + expect(chunk.properties).to.be.ok() + expect(chunk.properties.Tags).to.be.ok() + + folder:Destroy() + end) + + it("should omit Attributes when empty", function() + local folder = Instance.new("Folder") + local chunk = encodeService(folder) + + if chunk.properties then + expect(chunk.properties.Attributes).to.equal(nil) + end + + folder:Destroy() + end) + end) + + describe("Ref property carriers", function() + it("should create ObjectValue carriers for Ref properties", function() + local workspace = game:GetService("Workspace") + local _, refTargets = encodeService(workspace) + + for _, carrier in refTargets do + expect(carrier:IsA("ObjectValue")).to.equal(true) + expect(typeof(carrier.Name)).to.equal("string") + end + + for _, carrier in refTargets do + carrier:Destroy() + end + end) + + it("should report correct refTargetCount", function() + local workspace = game:GetService("Workspace") + local chunk, refTargets = encodeService(workspace) + + expect(chunk.refTargetCount).to.equal(#refTargets) + + for _, carrier in refTargets do + carrier:Destroy() + end + end) + + it("should map refs to 1-based carrier indices", function() + local workspace = game:GetService("Workspace") + local chunk, refTargets = encodeService(workspace) + + if chunk.refs then + for _, idx in chunk.refs do + expect(idx >= 1).to.equal(true) + expect(idx <= #refTargets).to.equal(true) + end + end + + for _, carrier in refTargets do + carrier:Destroy() + end + end) + end) +end diff --git a/src/web/interface.rs b/src/web/interface.rs index 393e348c..d1d1b640 100644 --- a/src/web/interface.rs +++ b/src/web/interface.rs @@ -37,9 +37,10 @@ pub const PROTOCOL_VERSION: u64 = 6; /// to the service). The next `ref_target_count` are ObjectValue carriers /// whose `Value` property holds a Ref to the actual target. /// -/// `refs` maps property names to 1-based indices into this combined range -/// (0 = nil). Indices <= child_count point directly to a child instance. -/// Indices > child_count point to an ObjectValue carrier. +/// `refs` maps property names to 1-based indices into the carrier slice +/// (the ObjectValue entries after the children). Index 1 is the first +/// carrier, 2 the second, etc. The server reads carriers via +/// `carriers[idx - 1]`. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ServiceChunk {