mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-06-12 23:00:26 +00:00
Heuristic hydration matching instead of hard dependency on GetChildren ordering (#1266)
### Summary When two or more sibling instances share the same `Name` and `ClassName`, Rojo's reconciler previously paired them with their server-side counterparts purely by child order (first-unvisited match in `GetChildren()` order). If the workspace child order ever diverged from the server's, the wrong instance got paired so each duplicate could inherit a sibling's properties. This is the root of the #1257 bug: the welded parts would oscillate between positions on each connect/disconnect because hydration kept mis-pairing them. (#1265 stopped the sync fallback from scrambling child order in the first place but this PR makes hydration robust even when order *does* diverge.) This PR makes `hydrate` break ties by comparing properties: when several existing children match on `Name`+`ClassName`, it scores each candidate by how many of the virtual instance's properties match the candidate's live values, and picks the best. Order remains the tiebreak when scores are equal, so behavior is unchanged for uniquely-named instances and for indistinguishable siblings. ### Changes - `trueEquals.lua`: extracted verbatim from `diff.lua` (the fuzzy value-equality helper) so it can be shared. No behavior change; `diff.lua` now requires it. - `countMatchingProperties.lua`: added `countMatchingProperties(instance, virtualInstance, instanceMap) -> number`. Skips `Ref` properties (the instanceMap isn't fully built mid-hydrate, so refs can't be decoded reliably, and they're a poor disambiguator anyway) and any property that can't be read or decoded. - `hydrate.lua`: See details below. ### Hydrate Changes This touches `hydrate`, which runs over the whole tree on every connect/resync, so I want state clearly that **the common path is faster than before, not slower** even for parents with thousands of children! The old algorithm was a nested scan: for each of `V` virtual children, scan existing children until the first unvisited `Name`+`ClassName` match. Two costs stand out: - A `pcall` (to guard DataModel permission errors) ran on every comparison (up to `V*E` `pcall`s per parent). - Even for in-order trees the re-scanning of the visited prefix made it `O(V^2)`. The new algorithm does a single bucketing pass, then `O(1)` lookups: 1. One `O(E)` pass groups existing children into nested `buckets[name][className]` tables. This runs exactly `E` `pcall`s total (one per child), down from the `V*E` worst case. 2. Each virtual child does an `O(1)` bucket lookup to find its candidates. 3. A per-bucket cursor skips already-paired children, so order-based matching is amortized `O(1)` per child instead of rescanning. | Scenario | Old | New | | ------------------------------------------------ | --------------------------------------- | --------------------------------------------- | | Unique-named children (typical, incl. thousands) | `O(V^2)`, plus up to `V*E` pcalls | `O(V + E)`, plus exactly `E` pcalls | | `C <= 32` candidates | `O(C^2)` | `O(P * C^2)` scoring | | `C > 32` candidates | `O(C^2)` | `O(C)` | Property scoring (`getProperty`/`decodeValue`/`trueEquals`) is the only new expense, and it's gated two ways: - It runs only when a `Name`+`ClassName` group has >=2 candidates (i.e. never for uniquely-named instances). - A cap, `MAX_CANDIDATES_TO_SCORE = 32`, means scoring only kicks in once a group has <=32 unvisited candidates. A folder of thousands of identically-named parts therefore does not trigger scoring; it falls back to the original order-based pairing. The worst-case added scoring work is bounded to roughly `32^2` property comparisons per group, independent of group size. So overall this is faster when you have unique names or many children. It is slower but more robust when you have small groups of duplicate names. Memory usage is increased as it creates the candidate buckets.
This commit is contained in:
@@ -40,6 +40,7 @@ Making a new release? Simply add the new header with the version and date undern
|
||||
* Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215])
|
||||
* Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217])
|
||||
* Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265])
|
||||
* Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266])
|
||||
|
||||
[#1176]: https://github.com/rojo-rbx/rojo/pull/1176
|
||||
[#1179]: https://github.com/rojo-rbx/rojo/pull/1179
|
||||
@@ -50,6 +51,7 @@ Making a new release? Simply add the new header with the version and date undern
|
||||
[#1215]: https://github.com/rojo-rbx/rojo/pull/1215
|
||||
[#1217]: https://github.com/rojo-rbx/rojo/pull/1217
|
||||
[#1265]: https://github.com/rojo-rbx/rojo/pull/1265
|
||||
[#1266]: https://github.com/rojo-rbx/rojo/pull/1266
|
||||
|
||||
## [7.7.0-rc.1] (November 27th, 2025)
|
||||
|
||||
|
||||
44
plugin/src/Reconciler/countMatchingProperties.lua
Normal file
44
plugin/src/Reconciler/countMatchingProperties.lua
Normal file
@@ -0,0 +1,44 @@
|
||||
--[[
|
||||
Counts how many of a virtual instance's properties match the live values on a
|
||||
candidate Roblox instance. `hydrate` uses this to break ties when several
|
||||
existing children share the same Name and ClassName.
|
||||
|
||||
This mirrors the read -> decode -> compare flow that `diff` uses, reusing the
|
||||
same `getProperty`, `decodeValue`, and `trueEquals` helpers.
|
||||
]]
|
||||
|
||||
local getProperty = require(script.Parent.getProperty)
|
||||
local decodeValue = require(script.Parent.decodeValue)
|
||||
local trueEquals = require(script.Parent.trueEquals)
|
||||
|
||||
local function countMatchingProperties(instance, virtualInstance, instanceMap)
|
||||
local score = 0
|
||||
|
||||
for propertyName, virtualValue in virtualInstance.Properties do
|
||||
-- Skip refs. During hydration the instanceMap is still being built
|
||||
-- top-down, so a ref may point at an instance we haven't hydrated yet
|
||||
-- and therefore can't decode reliably. Refs are also a poor
|
||||
-- disambiguator between same-named siblings.
|
||||
if next(virtualValue) == "Ref" then
|
||||
continue
|
||||
end
|
||||
|
||||
local getSuccess, existingValue = getProperty(instance, propertyName)
|
||||
if not getSuccess then
|
||||
continue
|
||||
end
|
||||
|
||||
local decodeSuccess, decodedValue = decodeValue(virtualValue, instanceMap)
|
||||
if not decodeSuccess then
|
||||
continue
|
||||
end
|
||||
|
||||
if trueEquals(existingValue, decodedValue) then
|
||||
score += 1
|
||||
end
|
||||
end
|
||||
|
||||
return score
|
||||
end
|
||||
|
||||
return countMatchingProperties
|
||||
91
plugin/src/Reconciler/countMatchingProperties.spec.lua
Normal file
91
plugin/src/Reconciler/countMatchingProperties.spec.lua
Normal file
@@ -0,0 +1,91 @@
|
||||
return function()
|
||||
local countMatchingProperties = require(script.Parent.countMatchingProperties)
|
||||
|
||||
local InstanceMap = require(script.Parent.Parent.InstanceMap)
|
||||
|
||||
it("counts properties whose values match the instance", function()
|
||||
local instance = Instance.new("StringValue")
|
||||
instance.Value = "hello"
|
||||
|
||||
local virtualInstance = {
|
||||
ClassName = "StringValue",
|
||||
Name = "Value",
|
||||
Properties = {
|
||||
Value = { String = "hello" },
|
||||
},
|
||||
Children = {},
|
||||
}
|
||||
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
|
||||
end)
|
||||
|
||||
it("does not count properties whose values differ", function()
|
||||
local instance = Instance.new("StringValue")
|
||||
instance.Value = "hello"
|
||||
|
||||
local virtualInstance = {
|
||||
ClassName = "StringValue",
|
||||
Name = "Value",
|
||||
Properties = {
|
||||
Value = { String = "different" },
|
||||
},
|
||||
Children = {},
|
||||
}
|
||||
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
|
||||
end)
|
||||
|
||||
it("counts multiple matching properties independently", function()
|
||||
local instance = Instance.new("Part")
|
||||
instance.Anchored = true
|
||||
instance.CanCollide = false
|
||||
|
||||
local virtualInstance = {
|
||||
ClassName = "Part",
|
||||
Name = "Part",
|
||||
Properties = {
|
||||
Anchored = { Bool = true },
|
||||
CanCollide = { Bool = false },
|
||||
},
|
||||
Children = {},
|
||||
}
|
||||
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(2)
|
||||
|
||||
-- Flip one so only a single property matches.
|
||||
instance.CanCollide = true
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
|
||||
end)
|
||||
|
||||
it("skips unknown properties without counting or erroring", function()
|
||||
local instance = Instance.new("Folder")
|
||||
|
||||
local virtualInstance = {
|
||||
ClassName = "Folder",
|
||||
Name = "Folder",
|
||||
Properties = {
|
||||
FAKE_PROPERTY = { String = "nope" },
|
||||
},
|
||||
Children = {},
|
||||
}
|
||||
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
|
||||
end)
|
||||
|
||||
it("skips Ref properties without counting or erroring", function()
|
||||
local instance = Instance.new("ObjectValue")
|
||||
|
||||
local virtualInstance = {
|
||||
ClassName = "ObjectValue",
|
||||
Name = "ObjectValue",
|
||||
Properties = {
|
||||
-- A ref must be skipped rather than decoded: during hydration
|
||||
-- the target may not be in the map yet.
|
||||
Value = { Ref = "00000000000000000000000000000000" },
|
||||
},
|
||||
Children = {},
|
||||
}
|
||||
|
||||
expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
|
||||
end)
|
||||
end
|
||||
@@ -10,104 +10,12 @@ local invariant = require(script.Parent.Parent.invariant)
|
||||
local getProperty = require(script.Parent.getProperty)
|
||||
local Error = require(script.Parent.Error)
|
||||
local decodeValue = require(script.Parent.decodeValue)
|
||||
local trueEquals = require(script.Parent.trueEquals)
|
||||
|
||||
local function isEmpty(table)
|
||||
return next(table) == nil
|
||||
end
|
||||
|
||||
local function fuzzyEq(a: number, b: number, epsilon: number): boolean
|
||||
return math.abs(a - b) < epsilon
|
||||
end
|
||||
|
||||
local function trueEquals(a, b): boolean
|
||||
-- Exit early for simple equality values
|
||||
if a == b then
|
||||
return true
|
||||
end
|
||||
|
||||
-- Treat nil and { Ref = "000...0" } as equal
|
||||
if
|
||||
(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
|
||||
or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
|
||||
then
|
||||
return true
|
||||
end
|
||||
|
||||
local typeA, typeB = typeof(a), typeof(b)
|
||||
|
||||
-- For tables, try recursive deep equality
|
||||
if typeA == "table" and typeB == "table" then
|
||||
local checkedKeys = {}
|
||||
for key, value in a do
|
||||
checkedKeys[key] = true
|
||||
if not trueEquals(value, b[key]) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
for key, value in b do
|
||||
if checkedKeys[key] then
|
||||
continue
|
||||
end
|
||||
if not trueEquals(value, a[key]) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For NaN, check if both values are not equal to themselves
|
||||
elseif a ~= a and b ~= b then
|
||||
return true
|
||||
|
||||
-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "number" and typeB == "number" then
|
||||
return fuzzyEq(a, b, 0.0001)
|
||||
|
||||
-- For EnumItem->number, compare the EnumItem's value
|
||||
elseif typeA == "number" and typeB == "EnumItem" then
|
||||
return a == b.Value
|
||||
elseif typeA == "EnumItem" and typeB == "number" then
|
||||
return a.Value == b
|
||||
|
||||
-- For Color3s, compare to RGB ints to avoid floating point inequality
|
||||
elseif typeA == "Color3" and typeB == "Color3" then
|
||||
local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
|
||||
local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
|
||||
return aR == bR and aG == bG and aB == bB
|
||||
|
||||
-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "CFrame" and typeB == "CFrame" then
|
||||
local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "Vector3" and typeB == "Vector3" then
|
||||
local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "Vector2" and typeB == "Vector2" then
|
||||
local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
return false
|
||||
end
|
||||
|
||||
local function shouldDeleteUnknownInstances(virtualInstance)
|
||||
if virtualInstance.Metadata ~= nil then
|
||||
return not virtualInstance.Metadata.ignoreUnknownInstances
|
||||
|
||||
@@ -3,9 +3,22 @@
|
||||
concrete instances and assigning them IDs.
|
||||
]]
|
||||
|
||||
local invariant = require(script.Parent.Parent.invariant)
|
||||
local Packages = script.Parent.Parent.Parent.Packages
|
||||
local Log = require(Packages.Log)
|
||||
|
||||
local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
|
||||
local invariant = require(script.Parent.Parent.invariant)
|
||||
local countMatchingProperties = require(script.Parent.countMatchingProperties)
|
||||
|
||||
-- When several existing children share a Name and ClassName we disambiguate
|
||||
-- them by scoring how well each one's properties match the virtual instance.
|
||||
-- That scoring is far more expensive than a Name/ClassName check, so we only do
|
||||
-- it for reasonably-sized groups. Larger groups (e.g. a folder with thousands of
|
||||
-- identically-named parts) fall back to the original order-based matching, which
|
||||
-- bounds the added work to roughly MAX_CANDIDATES_TO_SCORE^2 property reads per
|
||||
-- group regardless of how large the group is.
|
||||
local MAX_CANDIDATES_TO_SCORE = 32
|
||||
|
||||
local function hydrateInner(stats, instanceMap, virtualInstances, rootId, rootInstance)
|
||||
local virtualInstance = virtualInstances[rootId]
|
||||
|
||||
if virtualInstance == nil then
|
||||
@@ -13,38 +26,163 @@ local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
|
||||
end
|
||||
|
||||
instanceMap:insert(rootId, rootInstance)
|
||||
stats.hydrated += 1
|
||||
|
||||
local existingChildren = rootInstance:GetChildren()
|
||||
|
||||
-- For each existing child, we'll track whether it's been paired with an
|
||||
-- instance that the Rojo server knows about.
|
||||
local isExistingChildVisited = {}
|
||||
for i = 1, #existingChildren do
|
||||
isExistingChildVisited[i] = false
|
||||
-- Group existing children by Name then ClassName so each virtual child can
|
||||
-- find its candidate matches without scanning every sibling. This is what
|
||||
-- keeps hydration fast for parents with thousands of children. Nesting the
|
||||
-- two tables (rather than a combined key) keeps the Name and ClassName checks
|
||||
-- exact, with no way for one to bleed into the other.
|
||||
local buckets = {}
|
||||
for _, childInstance in existingChildren do
|
||||
-- We guard accessing Name and ClassName in order to avoid tripping over
|
||||
-- children of DataModel that Rojo won't have permissions to access at all.
|
||||
local accessSuccess, name, className = pcall(function()
|
||||
return childInstance.Name, childInstance.ClassName
|
||||
end)
|
||||
if not accessSuccess then
|
||||
continue
|
||||
end
|
||||
|
||||
local bucketsByClassName = buckets[name]
|
||||
if bucketsByClassName == nil then
|
||||
bucketsByClassName = {}
|
||||
buckets[name] = bucketsByClassName
|
||||
end
|
||||
|
||||
local bucket = bucketsByClassName[className]
|
||||
if bucket == nil then
|
||||
bucket = { cursor = 1, instances = {} }
|
||||
bucketsByClassName[className] = bucket
|
||||
end
|
||||
|
||||
table.insert(bucket.instances, childInstance)
|
||||
end
|
||||
|
||||
-- Tracks which existing children have already been paired, so one instance
|
||||
-- isn't matched to two different virtual instances.
|
||||
local visited = {}
|
||||
|
||||
for _, childId in ipairs(virtualInstance.Children) do
|
||||
local virtualChild = virtualInstances[childId]
|
||||
|
||||
for childIndex, childInstance in existingChildren do
|
||||
if not isExistingChildVisited[childIndex] then
|
||||
-- We guard accessing Name and ClassName in order to avoid
|
||||
-- tripping over children of DataModel that Rojo won't have
|
||||
-- permissions to access at all.
|
||||
local accessSuccess, name, className = pcall(function()
|
||||
return childInstance.Name, childInstance.ClassName
|
||||
end)
|
||||
local bucketsByClassName = buckets[virtualChild.Name]
|
||||
local bucket = bucketsByClassName and bucketsByClassName[virtualChild.ClassName]
|
||||
if bucket == nil then
|
||||
-- No existing instance matches; diff will mark this id for creation.
|
||||
Log.trace(
|
||||
"hydrate: no existing instance matches {} ({}) for id {}",
|
||||
virtualChild.Name,
|
||||
virtualChild.ClassName,
|
||||
childId
|
||||
)
|
||||
continue
|
||||
end
|
||||
|
||||
-- This rule is very conservative and could be loosened in the
|
||||
-- future, or more heuristics could be introduced.
|
||||
if accessSuccess and name == virtualChild.Name and className == virtualChild.ClassName then
|
||||
isExistingChildVisited[childIndex] = true
|
||||
hydrate(instanceMap, virtualInstances, childId, childInstance)
|
||||
break
|
||||
local instances = bucket.instances
|
||||
|
||||
-- Advance past any leading children that have already been paired. The
|
||||
-- cursor makes order-based matching amortized O(1) per child even for
|
||||
-- very large groups, rather than rescanning the visited prefix.
|
||||
while bucket.cursor <= #instances and visited[instances[bucket.cursor]] do
|
||||
bucket.cursor += 1
|
||||
end
|
||||
if bucket.cursor > #instances then
|
||||
-- Every matching instance has already been paired with an earlier id.
|
||||
Log.trace(
|
||||
"hydrate: no unpaired instance left for {} ({}) for id {}",
|
||||
virtualChild.Name,
|
||||
virtualChild.ClassName,
|
||||
childId
|
||||
)
|
||||
continue
|
||||
end
|
||||
|
||||
-- The cursor points at the earliest unvisited child, so the slots from
|
||||
-- here to the end bound how many candidates remain. Visited children
|
||||
-- after the cursor (gaps) only appear once a group is small enough to be
|
||||
-- scored -- the order-based path below always takes the earliest, which
|
||||
-- keeps the visited region a contiguous prefix. So whenever this count
|
||||
-- exceeds the cap it is exact, and we can pick the earliest match without
|
||||
-- collecting anything.
|
||||
local remaining = #instances - bucket.cursor + 1
|
||||
|
||||
local match
|
||||
if remaining > MAX_CANDIDATES_TO_SCORE then
|
||||
-- Too many to score affordably; take the earliest in child order,
|
||||
-- reproducing the original Name + ClassName behavior.
|
||||
match = instances[bucket.cursor]
|
||||
Log.trace(
|
||||
"hydrate: {} candidates named {} ({}) exceeds the scoring cap of {}; matching id {} by child order",
|
||||
remaining,
|
||||
virtualChild.Name,
|
||||
virtualChild.ClassName,
|
||||
MAX_CANDIDATES_TO_SCORE,
|
||||
childId
|
||||
)
|
||||
else
|
||||
-- Collect the (at most `remaining`) unvisited candidates.
|
||||
local candidates = {}
|
||||
for index = bucket.cursor, #instances do
|
||||
local childInstance = instances[index]
|
||||
if not visited[childInstance] then
|
||||
table.insert(candidates, childInstance)
|
||||
end
|
||||
end
|
||||
|
||||
if #candidates == 1 then
|
||||
-- Only one candidate, so there's nothing to disambiguate.
|
||||
match = candidates[1]
|
||||
else
|
||||
-- Break the tie by choosing the candidate whose properties best
|
||||
-- match the virtual instance, falling back to the earliest in
|
||||
-- child order when scores are equal.
|
||||
local bestScore = -1
|
||||
for _, childInstance in candidates do
|
||||
local score = countMatchingProperties(childInstance, virtualChild, instanceMap)
|
||||
if score > bestScore then
|
||||
bestScore = score
|
||||
match = childInstance
|
||||
end
|
||||
end
|
||||
|
||||
stats.ambiguousGroups += 1
|
||||
stats.candidatesScored += #candidates
|
||||
Log.trace(
|
||||
"hydrate: disambiguated {} candidates named {} ({}) for id {} by property match (best score {})",
|
||||
#candidates,
|
||||
virtualChild.Name,
|
||||
virtualChild.ClassName,
|
||||
childId,
|
||||
bestScore
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
visited[match] = true
|
||||
hydrateInner(stats, instanceMap, virtualInstances, childId, match)
|
||||
end
|
||||
end
|
||||
|
||||
local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
|
||||
-- Tallies of the work hydration did, surfaced in a single debug log below so
|
||||
-- the cost of property-based disambiguation is visible without per-node spam.
|
||||
local stats = {
|
||||
hydrated = 0,
|
||||
ambiguousGroups = 0,
|
||||
candidatesScored = 0,
|
||||
}
|
||||
|
||||
hydrateInner(stats, instanceMap, virtualInstances, rootId, rootInstance)
|
||||
|
||||
Log.debug(
|
||||
"Hydrated {} instances ({} ambiguous name+class groups, {} candidates scored)",
|
||||
stats.hydrated,
|
||||
stats.ambiguousGroups,
|
||||
stats.candidatesScored
|
||||
)
|
||||
end
|
||||
|
||||
return hydrate
|
||||
|
||||
@@ -126,4 +126,140 @@ return function()
|
||||
expect(knownInstances.fromIds["CHILD1"]).to.equal(child1)
|
||||
expect(knownInstances.fromIds["CHILD2"]).to.equal(child2)
|
||||
end)
|
||||
|
||||
it("should disambiguate duplicate-named siblings by matching properties", function()
|
||||
local knownInstances = InstanceMap.new()
|
||||
local virtualInstances = {
|
||||
ROOT = {
|
||||
ClassName = "Folder",
|
||||
Name = "Root",
|
||||
Properties = {},
|
||||
Children = { "CHILD_A", "CHILD_B" },
|
||||
},
|
||||
|
||||
CHILD_A = {
|
||||
ClassName = "StringValue",
|
||||
Name = "a",
|
||||
Properties = { Value = { String = "first" } },
|
||||
Children = {},
|
||||
},
|
||||
|
||||
CHILD_B = {
|
||||
ClassName = "StringValue",
|
||||
Name = "a",
|
||||
Properties = { Value = { String = "second" } },
|
||||
Children = {},
|
||||
},
|
||||
}
|
||||
|
||||
local rootInstance = Instance.new("Folder")
|
||||
|
||||
-- Created in the reverse order of the virtual children, so a purely
|
||||
-- order-based tiebreak would mis-pair them.
|
||||
local child1 = Instance.new("StringValue")
|
||||
child1.Name = "a"
|
||||
child1.Value = "second"
|
||||
child1.Parent = rootInstance
|
||||
|
||||
local child2 = Instance.new("StringValue")
|
||||
child2.Name = "a"
|
||||
child2.Value = "first"
|
||||
child2.Parent = rootInstance
|
||||
|
||||
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
|
||||
|
||||
expect(knownInstances:size()).to.equal(3)
|
||||
expect(knownInstances.fromIds["CHILD_A"]).to.equal(child2)
|
||||
expect(knownInstances.fromIds["CHILD_B"]).to.equal(child1)
|
||||
end)
|
||||
|
||||
it("should fall back to child order for duplicate-named siblings with no distinguishing properties", function()
|
||||
local knownInstances = InstanceMap.new()
|
||||
local virtualInstances = {
|
||||
ROOT = {
|
||||
ClassName = "Folder",
|
||||
Name = "Root",
|
||||
Properties = {},
|
||||
Children = { "CHILD_A", "CHILD_B" },
|
||||
},
|
||||
|
||||
CHILD_A = {
|
||||
ClassName = "Folder",
|
||||
Name = "a",
|
||||
Properties = {},
|
||||
Children = {},
|
||||
},
|
||||
|
||||
CHILD_B = {
|
||||
ClassName = "Folder",
|
||||
Name = "a",
|
||||
Properties = {},
|
||||
Children = {},
|
||||
},
|
||||
}
|
||||
|
||||
local rootInstance = Instance.new("Folder")
|
||||
|
||||
local child1 = Instance.new("Folder")
|
||||
child1.Name = "a"
|
||||
child1.Parent = rootInstance
|
||||
|
||||
local child2 = Instance.new("Folder")
|
||||
child2.Name = "a"
|
||||
child2.Parent = rootInstance
|
||||
|
||||
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
|
||||
|
||||
expect(knownInstances:size()).to.equal(3)
|
||||
-- With equal scores the earliest unvisited child wins, preserving the
|
||||
-- original order-based behavior.
|
||||
expect(knownInstances.fromIds["CHILD_A"]).to.equal(child1)
|
||||
expect(knownInstances.fromIds["CHILD_B"]).to.equal(child2)
|
||||
end)
|
||||
|
||||
it("should fall back to child order for very large duplicate-named groups", function()
|
||||
-- More candidates than hydrate is willing to score at once. The group
|
||||
-- must fall back to order-based matching, so virtual child N pairs with
|
||||
-- existing child N regardless of properties.
|
||||
local count = 64
|
||||
|
||||
local knownInstances = InstanceMap.new()
|
||||
local virtualInstances = {
|
||||
ROOT = {
|
||||
ClassName = "Folder",
|
||||
Name = "Root",
|
||||
Properties = {},
|
||||
Children = {},
|
||||
},
|
||||
}
|
||||
|
||||
local rootInstance = Instance.new("Folder")
|
||||
|
||||
local expectedInstances = {}
|
||||
for i = 1, count do
|
||||
local id = "CHILD_" .. i
|
||||
table.insert(virtualInstances.ROOT.Children, id)
|
||||
virtualInstances[id] = {
|
||||
ClassName = "StringValue",
|
||||
Name = "a",
|
||||
-- Distinct values that, if scored, would pair by value rather
|
||||
-- than by order.
|
||||
Properties = { Value = { String = "value " .. i } },
|
||||
Children = {},
|
||||
}
|
||||
|
||||
local child = Instance.new("StringValue")
|
||||
child.Name = "a"
|
||||
child.Value = "value " .. (count - i + 1)
|
||||
child.Parent = rootInstance
|
||||
expectedInstances[id] = child
|
||||
end
|
||||
|
||||
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
|
||||
|
||||
expect(knownInstances:size()).to.equal(count + 1)
|
||||
for id, expectedInstance in expectedInstances do
|
||||
expect(knownInstances.fromIds[id]).to.equal(expectedInstance)
|
||||
end
|
||||
end)
|
||||
end
|
||||
|
||||
100
plugin/src/Reconciler/trueEquals.lua
Normal file
100
plugin/src/Reconciler/trueEquals.lua
Normal file
@@ -0,0 +1,100 @@
|
||||
--[[
|
||||
Fuzzy value-equality used to compare a decoded virtual property value against
|
||||
the live value read from a real instance. Shared by `diff` (to decide whether
|
||||
a property changed) and `hydrate` (to score candidate instances).
|
||||
]]
|
||||
|
||||
local function fuzzyEq(a: number, b: number, epsilon: number): boolean
|
||||
return math.abs(a - b) < epsilon
|
||||
end
|
||||
|
||||
local function trueEquals(a, b): boolean
|
||||
-- Exit early for simple equality values
|
||||
if a == b then
|
||||
return true
|
||||
end
|
||||
|
||||
-- Treat nil and { Ref = "000...0" } as equal
|
||||
if
|
||||
(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
|
||||
or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
|
||||
then
|
||||
return true
|
||||
end
|
||||
|
||||
local typeA, typeB = typeof(a), typeof(b)
|
||||
|
||||
-- For tables, try recursive deep equality
|
||||
if typeA == "table" and typeB == "table" then
|
||||
local checkedKeys = {}
|
||||
for key, value in a do
|
||||
checkedKeys[key] = true
|
||||
if not trueEquals(value, b[key]) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
for key, value in b do
|
||||
if checkedKeys[key] then
|
||||
continue
|
||||
end
|
||||
if not trueEquals(value, a[key]) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For NaN, check if both values are not equal to themselves
|
||||
elseif a ~= a and b ~= b then
|
||||
return true
|
||||
|
||||
-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "number" and typeB == "number" then
|
||||
return fuzzyEq(a, b, 0.0001)
|
||||
|
||||
-- For EnumItem->number, compare the EnumItem's value
|
||||
elseif typeA == "number" and typeB == "EnumItem" then
|
||||
return a == b.Value
|
||||
elseif typeA == "EnumItem" and typeB == "number" then
|
||||
return a.Value == b
|
||||
|
||||
-- For Color3s, compare to RGB ints to avoid floating point inequality
|
||||
elseif typeA == "Color3" and typeB == "Color3" then
|
||||
local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
|
||||
local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
|
||||
return aR == bR and aG == bG and aB == bB
|
||||
|
||||
-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "CFrame" and typeB == "CFrame" then
|
||||
local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "Vector3" and typeB == "Vector3" then
|
||||
local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
|
||||
-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
|
||||
elseif typeA == "Vector2" and typeB == "Vector2" then
|
||||
local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
|
||||
for i, aComponent in aComponents do
|
||||
if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
|
||||
return false
|
||||
end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
return false
|
||||
end
|
||||
|
||||
return trueEquals
|
||||
Reference in New Issue
Block a user