rojo/plugin/src/Reconciler/hydrate.spec.lua
boatbomber 85655ca84f
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.
2026-06-01 17:29:22 -07:00

265 lines
6.6 KiB
Lua

return function()
local hydrate = require(script.Parent.hydrate)
local InstanceMap = require(script.Parent.Parent.InstanceMap)
it("should match the root instance no matter what", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Model",
Name = "Foo",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(1)
expect(knownInstances.fromIds["ROOT"]).to.equal(rootInstance)
end)
it("should not match children with mismatched ClassName", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = {},
},
CHILD = {
ClassName = "Folder",
Name = "Child",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
-- ClassName of this instance is intentionally different
local child = Instance.new("Model")
child.Name = "Child"
child.Parent = rootInstance
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(1)
expect(knownInstances.fromIds["ROOT"]).to.equal(rootInstance)
end)
it("should not match children with mismatched Name", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = {},
},
CHILD = {
ClassName = "Folder",
Name = "Child",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
-- Name of this instance is intentionally different
local child = Instance.new("Folder")
child.Name = "Not Child"
child.Parent = rootInstance
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(1)
expect(knownInstances.fromIds["ROOT"]).to.equal(rootInstance)
end)
it("should pair instances with matching Name and ClassName", function()
local knownInstances = InstanceMap.new()
local virtualInstances = {
ROOT = {
ClassName = "Folder",
Name = "Root",
Properties = {},
Children = { "CHILD1", "CHILD2" },
},
CHILD1 = {
ClassName = "Folder",
Name = "Child 1",
Properties = {},
Children = {},
},
CHILD2 = {
ClassName = "Model",
Name = "Child 2",
Properties = {},
Children = {},
},
}
local rootInstance = Instance.new("Folder")
local child1 = Instance.new("Folder")
child1.Name = "Child 1"
child1.Parent = rootInstance
local child2 = Instance.new("Model")
child2.Name = "Child 2"
child2.Parent = rootInstance
hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
expect(knownInstances:size()).to.equal(3)
expect(knownInstances.fromIds["ROOT"]).to.equal(rootInstance)
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