diff --git a/plugin/src/Reconciler/matching.lua b/plugin/src/Reconciler/matching.lua index a52178a4..b66c9c26 100644 --- a/plugin/src/Reconciler/matching.lua +++ b/plugin/src/Reconciler/matching.lua @@ -47,11 +47,17 @@ type MatchingSession = { costCache: { [string]: { [Instance]: number } }, } +type RefIdentity = { + name: string, + className: string, +} + type VCache = { props: { [string]: any }, extraProps: { string }, tags: { [string]: boolean }, attrs: { [string]: any }, + refs: { [string]: RefIdentity }, childCount: number, } @@ -60,6 +66,7 @@ type SCache = { props: { [string]: any }, tags: { [string]: boolean }, attrs: { [string]: any }, + refs: { [string]: RefIdentity }, children: { Instance }, childCount: number, } @@ -73,6 +80,12 @@ type VirtualInstance = { type VirtualInstances = { [string]: VirtualInstance } +type ClassComparisonKeys = { + propNames: { string }, + propNameSet: { [string]: boolean }, + defaults: { [string]: any }, +} + type ScoredPair = { vi: number, si: number, @@ -107,9 +120,10 @@ end -- Pre-computation helpers (called once per instance per group) -- ================================================================ -local function cacheVirtual(vInst: VirtualInstance, classKeys: any): VCache +local function cacheVirtual(vInst: VirtualInstance, classKeys: ClassComparisonKeys, virtualInstances: VirtualInstances): VCache local decoded: { [string]: any } = {} local extraProps: { string } = {} + local refs: { [string]: RefIdentity } = {} local vProps = vInst.Properties if vProps then @@ -120,6 +134,13 @@ local function cacheVirtual(vInst: VirtualInstance, classKeys: any): VCache local ty = next(encodedValue) if ty == "Ref" then + local targetId = encodedValue.Ref + if targetId then + local target = virtualInstances[targetId] + if target then + refs[propName] = { name = target.Name, className = target.ClassName } + end + end continue end @@ -156,11 +177,12 @@ local function cacheVirtual(vInst: VirtualInstance, classKeys: any): VCache extraProps = extraProps, tags = decodedTags, attrs = decodedAttrs, + refs = refs, childCount = if vInst.Children then #vInst.Children else 0, } end -local function cacheStudio(studioInstance: Instance, classKeys: any, extraPropNames: { string }): SCache +local function cacheStudio(studioInstance: Instance, classKeys: ClassComparisonKeys, extraPropNames: { string }, refPropNames: { string }): SCache local props: { [string]: any } = {} for _, propName in ipairs(classKeys.propNames) do @@ -180,6 +202,14 @@ local function cacheStudio(studioInstance: Instance, classKeys: any, extraPropNa end end + local refs: { [string]: RefIdentity } = {} + for _, propName in ipairs(refPropNames) do + local ok, value = safeGet(studioInstance, propName) + if ok and typeof(value) == "Instance" then + refs[propName] = { name = (value :: Instance).Name, className = (value :: Instance).ClassName } + end + end + local tags: { [string]: boolean } = {} for _, tag in ipairs(studioInstance:GetTags()) do tags[tag] = true @@ -192,6 +222,7 @@ local function cacheStudio(studioInstance: Instance, classKeys: any, extraPropNa props = props, tags = tags, attrs = studioInstance:GetAttributes(), + refs = refs, children = children, childCount = #children, } @@ -201,7 +232,7 @@ end -- Hot-path scoring (ZERO decode, ZERO reflection lookup) -- ================================================================ -local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: any): number +local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: ClassComparisonKeys): number local cost = 0 local vProps = vCache.props local sProps = sCache.props @@ -250,6 +281,24 @@ local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: any): nu end end + local vRefs = vCache.refs + local sRefs = sCache.refs + for propName, vRef in pairs(vRefs) do + local sRef = sRefs[propName] + if sRef then + if vRef.name ~= sRef.name or vRef.className ~= sRef.className then + cost += UNMATCHED_PENALTY + end + else + cost += UNMATCHED_PENALTY + end + end + for propName, _ in pairs(sRefs) do + if not vRefs[propName] then + cost += UNMATCHED_PENALTY + end + end + if vCache.childCount ~= sCache.childCount then cost += 1 end @@ -301,8 +350,12 @@ local function computePairCost( end local classKeys = RbxDom.getClassComparisonKeys(vInst.ClassName) - local vCache = cacheVirtual(vInst, classKeys) - local sCache = cacheStudio(studioInstance, classKeys, vCache.extraProps) + local vCache = cacheVirtual(vInst, classKeys, virtualInstances) + local refPropNames: { string } = {} + for propName, _ in pairs(vCache.refs) do + table.insert(refPropNames, propName) + end + local sCache = cacheStudio(studioInstance, classKeys, vCache.extraProps, refPropNames) local cost = countOwnDiffs(vCache, sCache, classKeys) if cost >= bestSoFar then @@ -312,29 +365,29 @@ local function computePairCost( local vChildren = vInst.Children local studioKids = sCache.children - if (not vChildren or #vChildren == 0) and #studioKids == 0 then - -- leaf - elseif not vChildren or #vChildren == 0 then - cost += #studioKids * UNMATCHED_PENALTY - elseif #studioKids == 0 then - for _, childId in ipairs(vChildren) do - if virtualInstances[childId] then - cost += UNMATCHED_PENALTY - end - end - else - local validVChildren: { string } = {} - for _, childId in ipairs(vChildren) do - if virtualInstances[childId] then - table.insert(validVChildren, childId) - end - end - if #validVChildren > 0 then - local childResult = - matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance) - cost += childResult.totalCost - else + if (vChildren and #vChildren > 0) or #studioKids > 0 then + if not vChildren or #vChildren == 0 then cost += #studioKids * UNMATCHED_PENALTY + elseif #studioKids == 0 then + for _, childId in ipairs(vChildren) do + if virtualInstances[childId] then + cost += UNMATCHED_PENALTY + end + end + else + local validVChildren: { string } = {} + for _, childId in ipairs(vChildren) do + if virtualInstances[childId] then + table.insert(validVChildren, childId) + end + end + if #validVChildren > 0 then + local childResult = + matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance) + cost += childResult.totalCost + else + cost += #studioKids * UNMATCHED_PENALTY + end end end @@ -469,6 +522,7 @@ matchChildren = function( local vCaches: { [number]: VCache } = {} local allExtraProps: { [string]: boolean } = {} + local allRefProps: { [string]: boolean } = {} for _, vi in ipairs(vIndices) do if matchedV2[vi] then continue @@ -477,11 +531,14 @@ matchChildren = function( if not vInst then continue end - local vCache = cacheVirtual(vInst, classKeys) + local vCache = cacheVirtual(vInst, classKeys, virtualInstances) vCaches[vi] = vCache for _, propName in ipairs(vCache.extraProps) do allExtraProps[propName] = true end + for propName, _ in pairs(vCache.refs) do + allRefProps[propName] = true + end end local extraPropNamesArray: { string } = {} @@ -489,10 +546,15 @@ matchChildren = function( table.insert(extraPropNamesArray, propName) end + local refPropNamesArray: { string } = {} + for propName, _ in pairs(allRefProps) do + table.insert(refPropNamesArray, propName) + end + local sCaches: { [number]: SCache } = {} for _, si in ipairs(sIndices) do if not matchedS2[si] then - sCaches[si] = cacheStudio(remainingStudio[si], classKeys, extraPropNamesArray) + sCaches[si] = cacheStudio(remainingStudio[si], classKeys, extraPropNamesArray, refPropNamesArray) end end @@ -529,35 +591,35 @@ matchChildren = function( local vChildren = vInst.Children local studioKids = sCache.children - if (not vChildren or #vChildren == 0) and #studioKids == 0 then - -- leaf - elseif not vChildren or #vChildren == 0 then - cost += #studioKids * UNMATCHED_PENALTY - elseif #studioKids == 0 then - for _, childId in ipairs(vChildren) do - if virtualInstances[childId] then - cost += UNMATCHED_PENALTY - end - end - else - local validVChildren: { string } = {} - for _, childId in ipairs(vChildren) do - if virtualInstances[childId] then - table.insert(validVChildren, childId) - end - end - if #validVChildren > 0 then - local childResult = matchChildren( - session, - validVChildren, - studioKids, - virtualInstances, - remainingVirtual[vi], - sCache.instance - ) - cost += childResult.totalCost - else + if (vChildren and #vChildren > 0) or #studioKids > 0 then + if not vChildren or #vChildren == 0 then cost += #studioKids * UNMATCHED_PENALTY + elseif #studioKids == 0 then + for _, childId in ipairs(vChildren) do + if virtualInstances[childId] then + cost += UNMATCHED_PENALTY + end + end + else + local validVChildren: { string } = {} + for _, childId in ipairs(vChildren) do + if virtualInstances[childId] then + table.insert(validVChildren, childId) + end + end + if #validVChildren > 0 then + local childResult = matchChildren( + session, + validVChildren, + studioKids, + virtualInstances, + remainingVirtual[vi], + sCache.instance + ) + cost += childResult.totalCost + else + cost += #studioKids * UNMATCHED_PENALTY + end end end end diff --git a/plugin/src/Reconciler/matching.spec.lua b/plugin/src/Reconciler/matching.spec.lua new file mode 100644 index 00000000..7f323ee0 --- /dev/null +++ b/plugin/src/Reconciler/matching.spec.lua @@ -0,0 +1,509 @@ +return function() + local Matching = require(script.Parent.matching) + + local function makeVirtualInstance(name, className, props, children) + return { + Name = name, + ClassName = className, + Properties = props or {}, + Children = children or {}, + } + end + + local function makeStudioFolder(name, parent) + local inst = Instance.new("Folder") + inst.Name = name + if parent then + inst.Parent = parent + end + return inst + end + + local function makeStudioPart(name, parent, transparency) + local inst = Instance.new("Part") + inst.Name = name + if transparency then + inst.Transparency = transparency + end + if parent then + inst.Parent = parent + end + return inst + end + + local function makeStudioModel(name, parent) + local inst = Instance.new("Model") + inst.Name = name + if parent then + inst.Parent = parent + end + return inst + end + + local container + beforeEach(function() + container = Instance.new("Folder") + container.Name = "MatchingTestContainer" + container.Parent = workspace + end) + + afterEach(function() + if container then + container:Destroy() + end + end) + + describe("session cache", function() + it("cache hit returns same reference", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Child", "Folder", nil, {}), + } + local studioChild = makeStudioFolder("Child", container) + + local parentInst = container + + local result1 = Matching.matchChildren( + session, { "A" }, { studioChild }, virtualInstances, "PARENT", parentInst + ) + local result2 = Matching.matchChildren( + session, { "A" }, { studioChild }, virtualInstances, "PARENT", parentInst + ) + expect(result1).to.equal(result2) + end) + + it("different parents get different cache entries", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("X", "Folder", nil, {}), + B = makeVirtualInstance("Y", "Folder", nil, {}), + } + local studioX = makeStudioFolder("X", container) + local studioY = makeStudioFolder("Y", container) + + local result1 = Matching.matchChildren( + session, { "A" }, { studioX }, virtualInstances, "PARENT_1", container + ) + local result2 = Matching.matchChildren( + session, { "B" }, { studioY }, virtualInstances, "PARENT_2", container + ) + expect(result1).never.to.equal(result2) + expect(result1.matched[1].virtualId).to.equal("A") + expect(result2.matched[1].virtualId).to.equal("B") + end) + + it("separate sessions are independent", function() + local session1 = Matching.newSession() + local session2 = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Child", "Folder", nil, {}), + } + local studioChild = makeStudioFolder("Child", container) + + local result1 = Matching.matchChildren( + session1, { "A" }, { studioChild }, virtualInstances, "P", container + ) + local result2 = Matching.matchChildren( + session2, { "A" }, { studioChild }, virtualInstances, "P", container + ) + expect(result1).never.to.equal(result2) + expect(result1.totalCost).to.equal(result2.totalCost) + end) + end) + + describe("totalCost", function() + it("zero for identical 1:1 match", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + } + local studioAlpha = makeStudioFolder("Alpha", container) + + local result = Matching.matchChildren( + session, { "A" }, { studioAlpha }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(1) + expect(result.totalCost).to.equal(0) + end) + + it("zero for multiple identical 1:1", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + B = makeVirtualInstance("Beta", "Folder", {}, {}), + } + local studioAlpha = makeStudioFolder("Alpha", container) + local studioBeta = makeStudioFolder("Beta", container) + + local result = Matching.matchChildren( + session, { "A", "B" }, { studioAlpha, studioBeta }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(2) + expect(result.totalCost).to.equal(0) + end) + + it("nonzero for property diff", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("P", "Part", { Transparency = { Float32 = 0.5 } }, {}), + } + local studioPart = makeStudioPart("P", container, 0) + + local result = Matching.matchChildren( + session, { "A" }, { studioPart }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(1) + expect(result.totalCost > 0).to.equal(true) + end) + + it("unmatched virtual adds penalty", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + B = makeVirtualInstance("Beta", "Folder", {}, {}), + } + local studioAlpha = makeStudioFolder("Alpha", container) + + local result = Matching.matchChildren( + session, { "A", "B" }, { studioAlpha }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(1) + expect(#result.unmatchedVirtual).to.equal(1) + expect(result.totalCost >= 10000).to.equal(true) + end) + + it("unmatched studio adds penalty", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + } + local studioAlpha = makeStudioFolder("Alpha", container) + local studioBeta = makeStudioFolder("Beta", container) + + local result = Matching.matchChildren( + session, { "A" }, { studioAlpha, studioBeta }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(1) + expect(#result.unmatchedStudio).to.equal(1) + expect(result.totalCost >= 10000).to.equal(true) + end) + + it("sums pair costs and unmatched", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + B = makeVirtualInstance("Beta", "Folder", {}, {}), + C = makeVirtualInstance("Gamma", "Folder", {}, {}), + } + local studioAlpha = makeStudioFolder("Alpha", container) + local studioBeta = makeStudioFolder("Beta", container) + + local result = Matching.matchChildren( + session, + { "A", "B", "C" }, + { studioAlpha, studioBeta }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + expect(#result.unmatchedVirtual).to.equal(1) + expect(result.totalCost).to.equal(10000) + end) + end) + + describe("Ref scoring", function() + it("Ref match pairs correctly in 2x2 ambiguous", function() + local session = Matching.newSession() + local virtualInstances = { + MODEL_A = makeVirtualInstance("M", "Model", { + PrimaryPart = { Ref = "HANDLE" }, + }, {}), + MODEL_B = makeVirtualInstance("M", "Model", { + PrimaryPart = { Ref = "GRIP" }, + }, {}), + HANDLE = makeVirtualInstance("Handle", "Part", {}, {}), + GRIP = makeVirtualInstance("Grip", "Part", {}, {}), + } + + local studioModelX = makeStudioModel("M", container) + local studioHandleX = makeStudioPart("Handle", studioModelX, nil) + studioModelX.PrimaryPart = studioHandleX + + local studioModelY = makeStudioModel("M", container) + local studioGripY = makeStudioPart("Grip", studioModelY, nil) + studioModelY.PrimaryPart = studioGripY + + local result = Matching.matchChildren( + session, + { "MODEL_A", "MODEL_B" }, + { studioModelX, studioModelY }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + + for _, pair in ipairs(result.matched) do + if pair.virtualId == "MODEL_A" then + expect(pair.studioInstance.PrimaryPart.Name).to.equal("Handle") + elseif pair.virtualId == "MODEL_B" then + expect(pair.studioInstance.PrimaryPart.Name).to.equal("Grip") + end + end + end) + + it("no Ref properties does not affect scoring", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Data", "Folder", {}, {}), + B = makeVirtualInstance("Data", "Folder", {}, {}), + } + local studioA = makeStudioFolder("Data", container) + local studioB = makeStudioFolder("Data", container) + + local result = Matching.matchChildren( + session, + { "A", "B" }, + { studioA, studioB }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + expect(result.totalCost).to.equal(0) + end) + end) + + describe("ambiguous groups", function() + it("2x2 same name different children", function() + local session = Matching.newSession() + local virtualInstances = { + FOLDER_A = makeVirtualInstance("Data", "Folder", {}, { "CHILD_A" }), + FOLDER_B = makeVirtualInstance("Data", "Folder", {}, { "CHILD_B" }), + CHILD_A = makeVirtualInstance("Alpha", "Folder", {}, {}), + CHILD_B = makeVirtualInstance("Beta", "Folder", {}, {}), + } + + local studioFolderX = makeStudioFolder("Data", container) + makeStudioFolder("Alpha", studioFolderX) + + local studioFolderY = makeStudioFolder("Data", container) + makeStudioFolder("Beta", studioFolderY) + + local result = Matching.matchChildren( + session, + { "FOLDER_A", "FOLDER_B" }, + { studioFolderX, studioFolderY }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + + for _, pair in ipairs(result.matched) do + local studioChildName = pair.studioInstance:GetChildren()[1].Name + if pair.virtualId == "FOLDER_A" then + expect(studioChildName).to.equal("Alpha") + elseif pair.virtualId == "FOLDER_B" then + expect(studioChildName).to.equal("Beta") + end + end + end) + + it("3x3 same name different children", function() + local session = Matching.newSession() + local virtualInstances = { + F1 = makeVirtualInstance("Data", "Folder", {}, { "C1" }), + F2 = makeVirtualInstance("Data", "Folder", {}, { "C2" }), + F3 = makeVirtualInstance("Data", "Folder", {}, { "C3" }), + C1 = makeVirtualInstance("One", "Folder", {}, {}), + C2 = makeVirtualInstance("Two", "Folder", {}, {}), + C3 = makeVirtualInstance("Three", "Folder", {}, {}), + } + + local s1 = makeStudioFolder("Data", container) + makeStudioFolder("One", s1) + local s2 = makeStudioFolder("Data", container) + makeStudioFolder("Two", s2) + local s3 = makeStudioFolder("Data", container) + makeStudioFolder("Three", s3) + + local result = Matching.matchChildren( + session, + { "F1", "F2", "F3" }, + { s1, s2, s3 }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(3) + + for _, pair in ipairs(result.matched) do + local studioChildName = pair.studioInstance:GetChildren()[1].Name + if pair.virtualId == "F1" then + expect(studioChildName).to.equal("One") + elseif pair.virtualId == "F2" then + expect(studioChildName).to.equal("Two") + elseif pair.virtualId == "F3" then + expect(studioChildName).to.equal("Three") + end + end + end) + + it("identical instances match all with zero cost", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Data", "Folder", {}, {}), + B = makeVirtualInstance("Data", "Folder", {}, {}), + } + local s1 = makeStudioFolder("Data", container) + local s2 = makeStudioFolder("Data", container) + + local result = Matching.matchChildren( + session, { "A", "B" }, { s1, s2 }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(2) + expect(result.totalCost).to.equal(0) + end) + + it("asymmetric 3v vs 2s", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Data", "Folder", {}, {}), + B = makeVirtualInstance("Data", "Folder", {}, {}), + C = makeVirtualInstance("Data", "Folder", {}, {}), + } + local s1 = makeStudioFolder("Data", container) + local s2 = makeStudioFolder("Data", container) + + local result = Matching.matchChildren( + session, { "A", "B", "C" }, { s1, s2 }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(2) + expect(#result.unmatchedVirtual).to.equal(1) + end) + end) + + describe("recursive matching", function() + it("nested ambiguous resolved by grandchildren", function() + local session = Matching.newSession() + local virtualInstances = { + OUTER_A = makeVirtualInstance("Data", "Folder", {}, { "INNER_A1", "INNER_A2" }), + OUTER_B = makeVirtualInstance("Data", "Folder", {}, { "INNER_B1", "INNER_B2" }), + INNER_A1 = makeVirtualInstance("P", "Part", { Transparency = { Float32 = 0.1 } }, {}), + INNER_A2 = makeVirtualInstance("P", "Part", { Transparency = { Float32 = 0.2 } }, {}), + INNER_B1 = makeVirtualInstance("P", "Part", { Transparency = { Float32 = 0.8 } }, {}), + INNER_B2 = makeVirtualInstance("P", "Part", { Transparency = { Float32 = 0.9 } }, {}), + } + + local studioOuterX = makeStudioFolder("Data", container) + makeStudioPart("P", studioOuterX, 0.1) + makeStudioPart("P", studioOuterX, 0.2) + + local studioOuterY = makeStudioFolder("Data", container) + makeStudioPart("P", studioOuterY, 0.8) + makeStudioPart("P", studioOuterY, 0.9) + + local result = Matching.matchChildren( + session, + { "OUTER_A", "OUTER_B" }, + { studioOuterY, studioOuterX }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + + for _, pair in ipairs(result.matched) do + local kids = pair.studioInstance:GetChildren() + if pair.virtualId == "OUTER_A" then + expect(kids[1].Transparency < 0.5).to.equal(true) + elseif pair.virtualId == "OUTER_B" then + expect(kids[1].Transparency > 0.5).to.equal(true) + end + end + end) + end) + + describe("edge cases", function() + it("empty both sides", function() + local session = Matching.newSession() + local result = Matching.matchChildren(session, {}, {}, {}, "ROOT", container) + expect(#result.matched).to.equal(0) + expect(#result.unmatchedVirtual).to.equal(0) + expect(#result.unmatchedStudio).to.equal(0) + expect(result.totalCost).to.equal(0) + end) + + it("all virtual unmatched", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Alpha", "Folder", {}, {}), + B = makeVirtualInstance("Beta", "Folder", {}, {}), + } + local result = Matching.matchChildren( + session, { "A", "B" }, {}, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(0) + expect(#result.unmatchedVirtual).to.equal(2) + expect(result.totalCost).to.equal(20000) + end) + + it("all studio unmatched", function() + local session = Matching.newSession() + local s1 = makeStudioFolder("Alpha", container) + local s2 = makeStudioFolder("Beta", container) + local result = Matching.matchChildren( + session, {}, { s1, s2 }, {}, "ROOT", container + ) + expect(#result.matched).to.equal(0) + expect(#result.unmatchedStudio).to.equal(2) + expect(result.totalCost).to.equal(20000) + end) + + it("class name discrimination", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Foo", "Folder", {}, {}), + B = makeVirtualInstance("Foo", "Part", {}, {}), + } + local studioFolder = makeStudioFolder("Foo", container) + local studioPart = makeStudioPart("Foo", container, nil) + + local result = Matching.matchChildren( + session, + { "A", "B" }, + { studioPart, studioFolder }, + virtualInstances, + "ROOT", + container + ) + expect(#result.matched).to.equal(2) + + for _, pair in ipairs(result.matched) do + if pair.virtualId == "A" then + expect(pair.studioInstance.ClassName).to.equal("Folder") + elseif pair.virtualId == "B" then + expect(pair.studioInstance.ClassName).to.equal("Part") + end + end + end) + + it("single child each side", function() + local session = Matching.newSession() + local virtualInstances = { + A = makeVirtualInstance("Only", "Folder", {}, {}), + } + local studioOnly = makeStudioFolder("Only", container) + + local result = Matching.matchChildren( + session, { "A" }, { studioOnly }, virtualInstances, "ROOT", container + ) + expect(#result.matched).to.equal(1) + expect(result.matched[1].virtualId).to.equal("A") + expect(result.matched[1].studioInstance).to.equal(studioOnly) + expect(result.totalCost).to.equal(0) + end) + end) +end diff --git a/plugin/src/Reconciler/trueEquals.spec.lua b/plugin/src/Reconciler/trueEquals.spec.lua new file mode 100644 index 00000000..8dccc3db --- /dev/null +++ b/plugin/src/Reconciler/trueEquals.spec.lua @@ -0,0 +1,396 @@ +return function() + local trueEquals = require(script.Parent.trueEquals) + + local NULL_REF = "00000000000000000000000000000000" + + describe("identity / rawequal", function() + it("same table reference", function() + local t = { a = 1 } + expect(trueEquals(t, t)).to.equal(true) + end) + + it("same string", function() + local s = "hello" + expect(trueEquals(s, s)).to.equal(true) + end) + + it("same number", function() + expect(trueEquals(42, 42)).to.equal(true) + end) + + it("same boolean", function() + expect(trueEquals(true, true)).to.equal(true) + end) + + it("same Vector3 reference", function() + local v = Vector3.new(1, 2, 3) + expect(trueEquals(v, v)).to.equal(true) + end) + + it("same Instance reference", function() + local inst = Instance.new("Folder") + expect(trueEquals(inst, inst)).to.equal(true) + inst:Destroy() + end) + end) + + describe("nil handling", function() + it("nil vs nil", function() + expect(trueEquals(nil, nil)).to.equal(true) + end) + + it("nil vs non-nil", function() + expect(trueEquals(nil, 1)).to.equal(false) + end) + + it("non-nil vs nil", function() + expect(trueEquals(1, nil)).to.equal(false) + end) + + it("nil vs null-ref table", function() + expect(trueEquals(nil, { Ref = NULL_REF })).to.equal(true) + end) + + it("null-ref table vs nil", function() + expect(trueEquals({ Ref = NULL_REF }, nil)).to.equal(true) + end) + + it("nil vs non-null ref", function() + expect(trueEquals(nil, { Ref = "abc123" })).to.equal(false) + end) + + it("nil vs null-ref with extra keys still matches", function() + expect(trueEquals(nil, { Ref = NULL_REF, extra = 1 })).to.equal(true) + end) + + it("nil vs empty table", function() + expect(trueEquals(nil, {})).to.equal(false) + end) + + it("nil vs table with wrong key", function() + expect(trueEquals(nil, { NotRef = NULL_REF })).to.equal(false) + end) + end) + + describe("numbers -- absolute epsilon", function() + it("0 vs 0", function() + expect(trueEquals(0, 0)).to.equal(true) + end) + + it("0 vs 0.00005 within epsilon", function() + expect(trueEquals(0, 0.00005)).to.equal(true) + end) + + it("0 vs 0.0001 at boundary (strict less-than)", function() + expect(trueEquals(0, 0.0001)).to.equal(false) + end) + + it("0 vs 0.00009999 just under", function() + expect(trueEquals(0, 0.00009999)).to.equal(true) + end) + + it("0 vs -0.00005 negative within", function() + expect(trueEquals(0, -0.00005)).to.equal(true) + end) + + it("0 vs 0.001 beyond", function() + expect(trueEquals(0, 0.001)).to.equal(false) + end) + end) + + describe("numbers -- relative epsilon", function() + it("1000 vs 1000.05 within relative", function() + expect(trueEquals(1000, 1000.05)).to.equal(true) + end) + + it("1000 vs 1000.5 beyond relative", function() + expect(trueEquals(1000, 1000.5)).to.equal(false) + end) + + it("1e6 vs 1e6+50 within relative", function() + expect(trueEquals(1000000, 1000050)).to.equal(true) + end) + + it("1e6 vs 1e6+200 beyond relative", function() + expect(trueEquals(1000000, 1000200)).to.equal(false) + end) + end) + + describe("numbers -- negative", function() + it("-5 vs -5.00005 within", function() + expect(trueEquals(-5, -5.00005)).to.equal(true) + end) + + it("-5 vs -5.001 beyond", function() + expect(trueEquals(-5, -5.001)).to.equal(false) + end) + + it("-1 vs 1 different", function() + expect(trueEquals(-1, 1)).to.equal(false) + end) + end) + + describe("numbers -- special values", function() + it("0 vs -0", function() + expect(trueEquals(0, -0)).to.equal(true) + end) + + it("huge vs huge", function() + expect(trueEquals(math.huge, math.huge)).to.equal(true) + end) + + it("-huge vs -huge", function() + expect(trueEquals(-math.huge, -math.huge)).to.equal(true) + end) + + it("huge vs -huge", function() + expect(trueEquals(math.huge, -math.huge)).to.equal(false) + end) + end) + + describe("NaN", function() + it("NaN vs NaN", function() + expect(trueEquals(0 / 0, 0 / 0)).to.equal(true) + end) + + it("NaN vs 0", function() + expect(trueEquals(0 / 0, 0)).to.equal(false) + end) + + it("NaN vs string", function() + expect(trueEquals(0 / 0, "nan")).to.equal(false) + end) + end) + + describe("Color3", function() + it("black vs black", function() + expect(trueEquals(Color3.new(0, 0, 0), Color3.new(0, 0, 0))).to.equal(true) + end) + + it("white vs white", function() + expect(trueEquals(Color3.new(1, 1, 1), Color3.new(1, 1, 1))).to.equal(true) + end) + + it("same mid-gray", function() + expect(trueEquals(Color3.new(0.5, 0.5, 0.5), Color3.new(0.5, 0.5, 0.5))).to.equal(true) + end) + + it("red vs green", function() + expect(trueEquals(Color3.new(1, 0, 0), Color3.new(0, 1, 0))).to.equal(false) + end) + + it("fromRGB same", function() + expect(trueEquals(Color3.fromRGB(128, 128, 128), Color3.fromRGB(128, 128, 128))).to.equal(true) + end) + + it("fromRGB one int off", function() + expect(trueEquals(Color3.fromRGB(128, 128, 128), Color3.fromRGB(129, 128, 128))).to.equal(false) + end) + + it("near-boundary same floor", function() + expect(trueEquals(Color3.new(128 / 255, 0, 0), Color3.new(128.4 / 255, 0, 0))).to.equal(true) + end) + + it("near-boundary different floor", function() + expect(trueEquals(Color3.new(128 / 255, 0, 0), Color3.new(129 / 255, 0, 0))).to.equal(false) + end) + end) + + describe("Vector3", function() + it("zero vs zero", function() + expect(trueEquals(Vector3.new(0, 0, 0), Vector3.new(0, 0, 0))).to.equal(true) + end) + + it("same components", function() + expect(trueEquals(Vector3.new(1, 2, 3), Vector3.new(1, 2, 3))).to.equal(true) + end) + + it("within epsilon on Z", function() + expect(trueEquals(Vector3.new(1, 2, 3), Vector3.new(1, 2, 3.00005))).to.equal(true) + end) + + it("beyond epsilon on Z", function() + expect(trueEquals(Vector3.new(1, 2, 3), Vector3.new(1, 2, 3.001))).to.equal(false) + end) + + it("Z differs by 1", function() + expect(trueEquals(Vector3.new(1, 2, 3), Vector3.new(1, 2, 4))).to.equal(false) + end) + + it("negative components", function() + expect(trueEquals(Vector3.new(-100, -200, -300), Vector3.new(-100, -200, -300))).to.equal(true) + end) + + it("large values within relative epsilon", function() + expect(trueEquals(Vector3.new(1e6, 1e6, 1e6), Vector3.new(1e6 + 50, 1e6, 1e6))).to.equal(true) + end) + end) + + describe("Vector2", function() + it("zero vs zero", function() + expect(trueEquals(Vector2.new(0, 0), Vector2.new(0, 0))).to.equal(true) + end) + + it("within epsilon on Y", function() + expect(trueEquals(Vector2.new(1, 2), Vector2.new(1, 2.00005))).to.equal(true) + end) + + it("Y differs by 1", function() + expect(trueEquals(Vector2.new(1, 2), Vector2.new(1, 3))).to.equal(false) + end) + end) + + describe("CFrame", function() + it("identity vs identity", function() + expect(trueEquals(CFrame.identity, CFrame.identity)).to.equal(true) + end) + + it("translation only same", function() + expect(trueEquals(CFrame.new(1, 2, 3), CFrame.new(1, 2, 3))).to.equal(true) + end) + + it("translation Z differs", function() + expect(trueEquals(CFrame.new(1, 2, 3), CFrame.new(1, 2, 4))).to.equal(false) + end) + + it("same rotation", function() + expect(trueEquals(CFrame.Angles(0, math.pi / 4, 0), CFrame.Angles(0, math.pi / 4, 0))).to.equal(true) + end) + + it("different rotation", function() + expect(trueEquals(CFrame.Angles(0, math.pi / 4, 0), CFrame.Angles(0, math.pi / 2, 0))).to.equal(false) + end) + + it("tiny translation perturbation within epsilon", function() + expect(trueEquals(CFrame.new(1, 2, 3), CFrame.new(1, 2, 3.00005))).to.equal(true) + end) + + it("constructed from components match", function() + local cf1 = CFrame.new(1, 2, 3) * CFrame.Angles(0.1, 0.2, 0.3) + local ax, ay, az, r00, r01, r02, r10, r11, r12, r20, r21, r22 = cf1:GetComponents() + local cf2 = CFrame.new(ax, ay, az, r00, r01, r02, r10, r11, r12, r20, r21, r22) + expect(trueEquals(cf1, cf2)).to.equal(true) + end) + + it("one rotation component perturbed beyond epsilon", function() + local cf1 = CFrame.new(1, 2, 3) * CFrame.Angles(0.1, 0.2, 0.3) + local ax, ay, az, r00, r01, r02, r10, r11, r12, r20, r21, r22 = cf1:GetComponents() + local cf2 = CFrame.new(ax, ay, az, r00, r01 + 0.001, r02, r10, r11, r12, r20, r21, r22) + expect(trueEquals(cf1, cf2)).to.equal(false) + end) + end) + + describe("tables -- deep equality", function() + it("empty vs empty", function() + expect(trueEquals({}, {})).to.equal(true) + end) + + it("same array", function() + expect(trueEquals({ 1, 2, 3 }, { 1, 2, 3 })).to.equal(true) + end) + + it("different rawlen", function() + expect(trueEquals({ 1, 2, 3 }, { 1, 2 })).to.equal(false) + end) + + it("same rawlen different element", function() + expect(trueEquals({ 1, 2, 3 }, { 1, 2, 4 })).to.equal(false) + end) + + it("same dict", function() + expect(trueEquals({ a = 1 }, { a = 1 })).to.equal(true) + end) + + it("different dict value", function() + expect(trueEquals({ a = 1 }, { a = 2 })).to.equal(false) + end) + + it("extra key in b", function() + expect(trueEquals({ a = 1 }, { a = 1, b = 2 })).to.equal(false) + end) + + it("extra key in a", function() + expect(trueEquals({ a = 1, b = 2 }, { a = 1 })).to.equal(false) + end) + + it("nested tables equal", function() + expect(trueEquals({ a = { b = 1 } }, { a = { b = 1 } })).to.equal(true) + end) + + it("nested tables different", function() + expect(trueEquals({ a = { b = 1 } }, { a = { b = 2 } })).to.equal(false) + end) + + it("mixed types in array", function() + expect(trueEquals({ 1, "two", true }, { 1, "two", true })).to.equal(true) + end) + + it("fuzzy number values in table", function() + expect(trueEquals({ x = 1.00005 }, { x = 1 })).to.equal(true) + end) + + it("encoded property table", function() + expect(trueEquals({ Vector3 = { 1, 2, 3 } }, { Vector3 = { 1, 2, 3 } })).to.equal(true) + end) + + it("null ref strings rawequal", function() + expect(trueEquals({ Ref = NULL_REF }, { Ref = NULL_REF })).to.equal(true) + end) + end) + + describe("EnumItem cross-type", function() + it("EnumItem vs matching number", function() + expect(trueEquals(Enum.SortOrder.LayoutOrder, Enum.SortOrder.LayoutOrder.Value)).to.equal(true) + end) + + it("number vs matching EnumItem", function() + expect(trueEquals(Enum.SortOrder.LayoutOrder.Value, Enum.SortOrder.LayoutOrder)).to.equal(true) + end) + + it("EnumItem vs wrong number", function() + expect(trueEquals(Enum.SortOrder.LayoutOrder, 999)).to.equal(false) + end) + + it("same EnumItem rawequal", function() + expect(trueEquals(Enum.SortOrder.LayoutOrder, Enum.SortOrder.LayoutOrder)).to.equal(true) + end) + + it("different EnumItem values", function() + expect(trueEquals(Enum.SortOrder.LayoutOrder, Enum.SortOrder.Name)).to.equal(false) + end) + end) + + describe("type mismatches", function() + it("string vs number", function() + expect(trueEquals("hello", 42)).to.equal(false) + end) + + it("boolean vs number", function() + expect(trueEquals(true, 1)).to.equal(false) + end) + + it("string '1' vs number 1", function() + expect(trueEquals("1", 1)).to.equal(false) + end) + + it("table vs string", function() + expect(trueEquals({}, "table")).to.equal(false) + end) + + it("Vector3 vs table", function() + expect(trueEquals(Vector3.new(1, 2, 3), { 1, 2, 3 })).to.equal(false) + end) + + it("Color3 vs Vector3", function() + expect(trueEquals(Color3.new(1, 0, 0), Vector3.new(1, 0, 0))).to.equal(false) + end) + + it("true vs false", function() + expect(trueEquals(true, false)).to.equal(false) + end) + + it("different strings", function() + expect(trueEquals("abc", "abd")).to.equal(false) + end) + end) +end diff --git a/rojo-test/serve-tests/non_default_project/named.project.json5 b/rojo-test/serve-tests/non_default_project/named.project.json5 new file mode 100644 index 00000000..b344dc28 --- /dev/null +++ b/rojo-test/serve-tests/non_default_project/named.project.json5 @@ -0,0 +1,10 @@ +{ + "name": "non_default_project", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "$className": "ReplicatedStorage", + "$path": "src" + } + } +} diff --git a/rojo-test/serve-tests/non_default_project/src/hello.luau b/rojo-test/serve-tests/non_default_project/src/hello.luau new file mode 100644 index 00000000..a5647075 --- /dev/null +++ b/rojo-test/serve-tests/non_default_project/src/hello.luau @@ -0,0 +1 @@ +return {} diff --git a/rojo-test/serve-tests/non_default_project/trap/bad.rbxm b/rojo-test/serve-tests/non_default_project/trap/bad.rbxm new file mode 100644 index 00000000..7b478fc2 --- /dev/null +++ b/rojo-test/serve-tests/non_default_project/trap/bad.rbxm @@ -0,0 +1,2 @@ +This is not a valid rbxm file. If the snapshot system walks into this +directory, it will error with "Malformed rbxm file: Invalid file header". diff --git a/src/change_processor.rs b/src/change_processor.rs index 6fe5543b..e0d4f180 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -83,6 +83,7 @@ impl ChangeProcessor { suppressed_paths: Arc>>, ref_path_index: Arc>, project_root: PathBuf, + project_file_path: PathBuf, critical_error_receiver: Option>, git_repo_root: Option, ) -> Self { @@ -95,6 +96,8 @@ impl ChangeProcessor { // Canonicalize project_root so path comparisons work with the // \\?\ prefix that std::fs::canonicalize adds on Windows. let project_root = std::fs::canonicalize(&project_root).unwrap_or(project_root); + let project_file_path = + std::fs::canonicalize(&project_file_path).unwrap_or(project_file_path); let task = JobThreadContext { tree, vfs, @@ -102,6 +105,7 @@ impl ChangeProcessor { pending_recovery: Mutex::new(Vec::new()), suppressed_paths, project_root, + project_file_path, ref_path_index, git_repo_root, }; @@ -240,6 +244,11 @@ struct JobThreadContext { /// Root directory of the project, used to display relative paths in logs. project_root: PathBuf, + /// Path to the project file (e.g. `plugin.project.json`). Used by + /// `reconcile_tree` so that re-snapshotting goes through the project + /// middleware instead of walking the directory as a generic Folder. + project_file_path: PathBuf, + /// Index of meta/model files that contain `Rojo_Ref_*` attributes. /// Shared with ApiService for efficient rename path updates. ref_path_index: Arc>, @@ -928,7 +937,8 @@ impl JobThreadContext { let start = Instant::now(); let instance_context = InstanceContext::new(); - let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_root) { + let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_file_path) + { Ok(s) => s, Err(e) => { log::error!("Tree reconciliation snapshot error: {:?}", e); diff --git a/src/serve_session.rs b/src/serve_session.rs index 3e5c5c2a..151484fc 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -193,6 +193,7 @@ impl ServeSession { Arc::clone(&suppressed_paths), Arc::clone(&ref_path_index), root_project.folder_location().to_path_buf(), + root_project.file_location.clone(), critical_error_receiver, git_repo_root.clone(), ); @@ -366,7 +367,7 @@ impl ServeSession { /// Does NOT apply corrections — the tree is left unchanged. pub fn check_tree_freshness(&self) -> TreeFreshnessReport { let start = Instant::now(); - let start_path = self.root_project.folder_location(); + let start_path: &Path = &self.root_project.file_location; let instance_context = InstanceContext::new(); let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, start_path) { @@ -429,7 +430,7 @@ impl ServeSession { } let start = Instant::now(); - let start_path = self.root_project.folder_location(); + let start_path: &Path = &self.root_project.file_location; let instance_context = InstanceContext::new(); let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, start_path) { diff --git a/src/snapshot/matching.rs b/src/snapshot/matching.rs index 12783f85..8f4e1b06 100644 --- a/src/snapshot/matching.rs +++ b/src/snapshot/matching.rs @@ -1308,4 +1308,136 @@ mod tests { elapsed ); } + + #[test] + fn total_cost_zero_for_identical() { + let snaps = vec![ + make_snapshot("Alpha", "Folder"), + make_snapshot("Beta", "Folder"), + ]; + let (tree, children) = make_tree_with_children(&[("Alpha", "Folder"), ("Beta", "Folder")]); + let result = match_forward(snaps, &children, &tree, &MatchingSession::new()); + assert_eq!(result.matched.len(), 2); + assert_eq!(result.total_cost, 0, "Identical instances should have zero cost"); + } + + #[test] + fn total_cost_nonzero_for_different() { + use rbx_dom_weak::types::Variant; + + let snaps = vec![make_snapshot_with_props( + "Part", + "Part", + vec![("Transparency", Variant::Float32(0.5))], + )]; + let (tree, children) = make_tree_from_snapshots(vec![make_snapshot_with_props( + "Part", + "Part", + vec![("Transparency", Variant::Float32(0.0))], + )]); + let result = match_forward(snaps, &children, &tree, &MatchingSession::new()); + assert_eq!(result.matched.len(), 1); + assert!( + result.total_cost > 0, + "Different properties should produce nonzero cost, got {}", + result.total_cost + ); + } + + #[test] + fn total_cost_includes_unmatched() { + let snaps = vec![ + make_snapshot("A", "Folder"), + make_snapshot("B", "Folder"), + make_snapshot("C", "Folder"), + ]; + let (tree, children) = make_tree_with_children(&[("A", "Folder"), ("B", "Folder")]); + let result = match_forward(snaps, &children, &tree, &MatchingSession::new()); + assert_eq!(result.matched.len(), 2); + assert_eq!(result.unmatched_snapshot.len(), 1); + assert!( + result.total_cost >= UNMATCHED_PENALTY, + "Should include penalty for unmatched snapshot, got {}", + result.total_cost + ); + } + + #[test] + fn total_cost_sums_correctly() { + use rbx_dom_weak::types::Variant; + + let snaps = vec![ + make_snapshot("A", "Folder"), + make_snapshot_with_props("B", "Part", vec![("Transparency", Variant::Float32(0.5))]), + make_snapshot("C", "Folder"), + ]; + let tree_children_snaps = vec![ + make_snapshot("A", "Folder"), + make_snapshot_with_props("B", "Part", vec![("Transparency", Variant::Float32(0.0))]), + ]; + let (tree, children) = make_tree_from_snapshots(tree_children_snaps); + let result = match_forward(snaps, &children, &tree, &MatchingSession::new()); + + assert_eq!(result.matched.len(), 2); + assert_eq!(result.unmatched_snapshot.len(), 1); + // total = matched costs (A=0 + B>=1) + unmatched (C=10000) + assert!( + result.total_cost >= UNMATCHED_PENALTY + 1, + "Should be at least penalty + 1 diff, got {}", + result.total_cost + ); + } + + #[test] + fn session_cache_consistent() { + let snaps1 = vec![ + make_snapshot("X", "Folder"), + make_snapshot("Y", "Folder"), + ]; + let snaps2 = vec![ + make_snapshot("X", "Folder"), + make_snapshot("Y", "Folder"), + ]; + let (tree, children) = make_tree_with_children(&[("X", "Folder"), ("Y", "Folder")]); + let session = MatchingSession::new(); + + let r1 = match_forward(snaps1, &children, &tree, &session); + let r2 = match_forward(snaps2, &children, &tree, &session); + + assert_eq!(r1.matched.len(), r2.matched.len()); + assert_eq!(r1.total_cost, r2.total_cost); + assert_eq!( + r1.unmatched_snapshot.len(), + r2.unmatched_snapshot.len() + ); + } + + #[test] + fn session_cache_does_not_corrupt() { + use rbx_dom_weak::types::Variant; + + let session = MatchingSession::new(); + + // Group A: identical folders + let snaps_a = vec![make_snapshot("A", "Folder")]; + let (tree_a, children_a) = make_tree_with_children(&[("A", "Folder")]); + let ra = match_forward(snaps_a, &children_a, &tree_a, &session); + assert_eq!(ra.total_cost, 0); + + // Group B: parts with property diff + let snaps_b = vec![make_snapshot_with_props( + "B", + "Part", + vec![("Transparency", Variant::Float32(1.0))], + )]; + let (tree_b, children_b) = make_tree_from_snapshots(vec![make_snapshot_with_props( + "B", + "Part", + vec![("Transparency", Variant::Float32(0.0))], + )]); + let rb = match_forward(snaps_b, &children_b, &tree_b, &session); + + assert_eq!(ra.total_cost, 0, "Group A should still be zero"); + assert!(rb.total_cost > 0, "Group B should have nonzero cost"); + } } diff --git a/src/syncback/matching.rs b/src/syncback/matching.rs index 47a49742..47ba37e4 100644 --- a/src/syncback/matching.rs +++ b/src/syncback/matching.rs @@ -758,4 +758,150 @@ mod tests { assert_eq!(a, b); } } + + #[test] + fn total_cost_zero_identical() { + let (new_dom, new_root, _, _) = build_test_dom(); + let (old_dom, old_root, _, _) = build_test_dom(); + + let new_children: Vec = new_dom.get_by_ref(new_root).unwrap().children().to_vec(); + let old_children: Vec = old_dom.get_by_ref(old_root).unwrap().children().to_vec(); + + let result = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &MatchingSession::new(), + ); + assert_eq!(result.matched.len(), 2); + assert_eq!( + result.total_cost, 0, + "Identical instances should have zero cost" + ); + } + + #[test] + fn total_cost_nonzero_different() { + use rbx_dom_weak::types::Variant; + + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("P") + .with_property("Transparency", Variant::Float32(0.5)), + ); + + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("P") + .with_property("Transparency", Variant::Float32(0.0)), + ); + + let new_children: Vec = new_dom.get_by_ref(new_root).unwrap().children().to_vec(); + let old_children: Vec = old_dom.get_by_ref(old_root).unwrap().children().to_vec(); + + let result = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &MatchingSession::new(), + ); + assert_eq!(result.matched.len(), 1); + assert!( + result.total_cost > 0, + "Different properties should produce nonzero cost, got {}", + result.total_cost + ); + } + + #[test] + fn total_cost_includes_unmatched() { + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + new_dom.insert( + new_root, + InstanceBuilder::new("Folder").with_name("A"), + ); + new_dom.insert( + new_root, + InstanceBuilder::new("Folder").with_name("B"), + ); + new_dom.insert( + new_root, + InstanceBuilder::new("Folder").with_name("C"), + ); + + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + old_dom.insert( + old_root, + InstanceBuilder::new("Folder").with_name("A"), + ); + old_dom.insert( + old_root, + InstanceBuilder::new("Folder").with_name("B"), + ); + + let new_children: Vec = new_dom.get_by_ref(new_root).unwrap().children().to_vec(); + let old_children: Vec = old_dom.get_by_ref(old_root).unwrap().children().to_vec(); + + let result = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &MatchingSession::new(), + ); + assert_eq!(result.matched.len(), 2); + assert_eq!(result.unmatched_new.len(), 1); + assert!( + result.total_cost >= UNMATCHED_PENALTY, + "Should include penalty for unmatched, got {}", + result.total_cost + ); + } + + #[test] + fn session_cache_consistent_syncback() { + let (new_dom, new_root, _, _) = build_test_dom(); + let (old_dom, old_root, _, _) = build_test_dom(); + + let new_children: Vec = new_dom.get_by_ref(new_root).unwrap().children().to_vec(); + let old_children: Vec = old_dom.get_by_ref(old_root).unwrap().children().to_vec(); + let session = MatchingSession::new(); + + let r1 = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &session, + ); + let r2 = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &session, + ); + assert_eq!(r1.matched.len(), r2.matched.len()); + assert_eq!(r1.total_cost, r2.total_cost); + } } diff --git a/tests/rojo_test/serve_util.rs b/tests/rojo_test/serve_util.rs index b993e8c3..8ac90eeb 100644 --- a/tests/rojo_test/serve_util.rs +++ b/tests/rojo_test/serve_util.rs @@ -130,6 +130,59 @@ impl TestServeSession { } } + /// Creates a test session using a specific (non-default) project file + /// inside the fixture directory. The fixture is copied as usual, but + /// `atlas serve` is pointed at `project_file` within the copied dir + /// instead of the directory itself. + pub fn new_with_project_file(name: &str, project_file: &str) -> Self { + let working_dir = get_working_dir_path(); + + let source_path = Path::new(SERVE_TESTS_PATH).join(name); + let dir = tempdir().expect("Couldn't create temporary directory"); + let fixture_dir = dir + .path() + .canonicalize() + .expect("Couldn't canonicalize temporary directory path") + .join(name); + + fs::create_dir(&fixture_dir).expect("Couldn't create temporary project subdirectory"); + copy_recursive(&source_path, &fixture_dir) + .expect("Couldn't copy project to temporary directory"); + + let project_path = fixture_dir.join(project_file); + assert!( + project_path.exists(), + "Project file {} does not exist in fixture {}", + project_file, + name + ); + + #[cfg(target_os = "macos")] + std::thread::sleep(Duration::from_millis(100)); + + let port = get_port_number(); + let port_string = port.to_string(); + + let rojo_process = Command::new(ROJO_PATH) + .args([ + "serve", + project_path.to_str().unwrap(), + "--port", + port_string.as_str(), + ]) + .current_dir(working_dir) + .stderr(Stdio::piped()) + .spawn() + .expect("Couldn't start Rojo"); + + TestServeSession { + rojo_process: KillOnDrop(rojo_process), + _dir: dir, + port, + project_path: fixture_dir, + } + } + /// Creates a test session with a git repo initialized in the project dir. /// The `setup` callback runs after the fixture is copied and git is initialized /// but BEFORE the serve process starts. Use it to commit initial files and diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index eb97c26a..d3957fd8 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -5,7 +5,7 @@ use tempfile::tempdir; use crate::rojo_test::{ internable::InternAndRedact, - serve_util::{run_serve_test, serialize_to_xml_model}, + serve_util::{run_serve_test, serialize_to_xml_model, TestServeSession}, }; use librojo::web_api::SocketPacketType; @@ -961,6 +961,24 @@ fn ref_path_nonexistent_target_no_crash() { }); } +/// When a non-default project file is used (e.g. `named.project.json5` +/// instead of `default.project.json5`), tree validation must snapshot +/// through the project middleware — not walk the parent directory as a +/// generic Folder. Regression test for a bug where `check_tree_freshness` +/// and `reconcile_tree` used `folder_location()` instead of the project +/// file path, causing the entire repo root to be walked (including +/// unrelated directories with potentially malformed files). +#[test] +fn non_default_project_file_tree_validation() { + let _ = env_logger::try_init(); + + let mut session = + TestServeSession::new_with_project_file("non_default_project", "named.project.json5"); + let _info = session.wait_to_come_online(); + + session.assert_tree_fresh(); +} + /// Multiple Rojo_Ref_* attributes on the same instance should all resolve. #[test] fn ref_path_multiple_attributes() {