mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 14:01:37 +00:00
Refactor encoding logic in ChangeBatcher and enhance duplicate handling
- Updated `encodeInstance` and `encodeService` to utilize helper functions for encoding attributes and tags, improving code organization and readability. - Changed duplicate handling to allow encoding of instances with duplicate-named siblings, with the server managing name collisions through a dedup suffix system. - Added debug logging for duplicate detection to aid in diagnostics. - Streamlined property encoding by implementing a helper function to iterate over encodable properties, enhancing maintainability and consistency. These changes improve the clarity and efficiency of the encoding process while ensuring accurate handling of duplicate instances.
This commit is contained in:
Joe
parent
24dec4073d
commit
9d8762bbaf
5 changed files with 274 additions and 188 deletions
85
plugin/src/ChangeBatcher/encodeHelpers.lua
Normal file
85
plugin/src/ChangeBatcher/encodeHelpers.lua
Normal file
|
|
@ -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
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
142
plugin/src/ChangeBatcher/encodeService.spec.lua
Normal file
142
plugin/src/ChangeBatcher/encodeService.spec.lua
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue