Enhance matching module with valid children tracking and code readability improvements

- Added a `validChildren` field to the `VCache` structure to track valid child instances during the matching process.
- Refactored the `cacheVirtual` function to improve readability by formatting parameters across multiple lines.
- Updated the `computePairCost` function to utilize the new `validChildren` field for more accurate cost calculations.
- Simplified the `matchChildren` function by removing redundant code and improving the handling of matched instances.
- Enhanced test cases for the `matchChildren` function to ensure consistent behavior and validate the new changes.

These updates improve the efficiency and clarity of the matching process, ensuring better performance and maintainability.
This commit is contained in:
Joe 2026-02-18 06:40:28 -06:00
parent b687c8862d
commit ddd47b9afa
9 changed files with 287 additions and 363 deletions

View file

@ -59,6 +59,7 @@ type VCache = {
attrs: { [string]: any }, attrs: { [string]: any },
refs: { [string]: RefIdentity }, refs: { [string]: RefIdentity },
childCount: number, childCount: number,
validChildren: { string },
} }
type SCache = { type SCache = {
@ -120,7 +121,11 @@ end
-- Pre-computation helpers (called once per instance per group) -- Pre-computation helpers (called once per instance per group)
-- ================================================================ -- ================================================================
local function cacheVirtual(vInst: VirtualInstance, classKeys: ClassComparisonKeys, virtualInstances: VirtualInstances): VCache local function cacheVirtual(
vInst: VirtualInstance,
classKeys: ClassComparisonKeys,
virtualInstances: VirtualInstances
): VCache
local decoded: { [string]: any } = {} local decoded: { [string]: any } = {}
local extraProps: { string } = {} local extraProps: { string } = {}
local refs: { [string]: RefIdentity } = {} local refs: { [string]: RefIdentity } = {}
@ -172,6 +177,15 @@ local function cacheVirtual(vInst: VirtualInstance, classKeys: ClassComparisonKe
end end
end end
local validChildren: { string } = {}
if vInst.Children then
for _, childId in ipairs(vInst.Children) do
if virtualInstances[childId] then
table.insert(validChildren, childId)
end
end
end
return { return {
props = decoded, props = decoded,
extraProps = extraProps, extraProps = extraProps,
@ -179,10 +193,16 @@ local function cacheVirtual(vInst: VirtualInstance, classKeys: ClassComparisonKe
attrs = decodedAttrs, attrs = decodedAttrs,
refs = refs, refs = refs,
childCount = if vInst.Children then #vInst.Children else 0, childCount = if vInst.Children then #vInst.Children else 0,
validChildren = validChildren,
} }
end end
local function cacheStudio(studioInstance: Instance, classKeys: ClassComparisonKeys, extraPropNames: { string }, refPropNames: { string }): SCache local function cacheStudio(
studioInstance: Instance,
classKeys: ClassComparisonKeys,
extraPropNames: { string },
refPropNames: { string }
): SCache
local props: { [string]: any } = {} local props: { [string]: any } = {}
for _, propName in ipairs(classKeys.propNames) do for _, propName in ipairs(classKeys.propNames) do
@ -244,13 +264,16 @@ local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: ClassCom
vVal = defaults[propName] vVal = defaults[propName]
end end
if not trueEquals(vVal, sProps[propName]) then local sVal = sProps[propName]
if vVal ~= sVal and not trueEquals(vVal, sVal) then
cost += 1 cost += 1
end end
end end
for _, propName in ipairs(vCache.extraProps) do for _, propName in ipairs(vCache.extraProps) do
if not trueEquals(vProps[propName], sProps[propName]) then local vVal = vProps[propName]
local sVal = sProps[propName]
if vVal ~= sVal and not trueEquals(vVal, sVal) then
cost += 1 cost += 1
end end
end end
@ -271,7 +294,8 @@ local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: ClassCom
local vAttrs = vCache.attrs local vAttrs = vCache.attrs
local sAttrs = sCache.attrs local sAttrs = sCache.attrs
for key, vVal in pairs(vAttrs) do for key, vVal in pairs(vAttrs) do
if not trueEquals(vVal, sAttrs[key]) then local sVal = sAttrs[key]
if vVal ~= sVal and not trueEquals(vVal, sVal) then
cost += 1 cost += 1
end end
end end
@ -306,23 +330,6 @@ local function countOwnDiffs(vCache: VCache, sCache: SCache, classKeys: ClassCom
return cost return cost
end end
-- ================================================================
-- Utilities
-- ================================================================
local function removeMatched(arr: { any }, matchedIndices: { [number]: boolean }): ()
local write = 1
for read = 1, #arr do
if not matchedIndices[read] then
arr[write] = arr[read]
write += 1
end
end
for i = write, #arr do
arr[i] = nil
end
end
-- ================================================================ -- ================================================================
-- Core: 2-function mutual recursion -- Core: 2-function mutual recursion
-- ================================================================ -- ================================================================
@ -362,32 +369,18 @@ local function computePairCost(
return cost return cost
end end
local vChildren = vInst.Children local validVChildren = vCache.validChildren
local studioKids = sCache.children local studioKids = sCache.children
if (vChildren and #vChildren > 0) or #studioKids > 0 then if #validVChildren > 0 or #studioKids > 0 then
if not vChildren or #vChildren == 0 then if #validVChildren == 0 then
cost += #studioKids * UNMATCHED_PENALTY cost += #studioKids * UNMATCHED_PENALTY
elseif #studioKids == 0 then elseif #studioKids == 0 then
for _, childId in ipairs(vChildren) do cost += #validVChildren * UNMATCHED_PENALTY
if virtualInstances[childId] then
cost += UNMATCHED_PENALTY
end
end
else 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 = local childResult =
matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance) matchChildren(session, validVChildren, studioKids, virtualInstances, virtualId, studioInstance)
cost += childResult.totalCost cost += childResult.totalCost
else
cost += #studioKids * UNMATCHED_PENALTY
end
end end
end end
@ -409,7 +402,6 @@ matchChildren = function(
parentVirtualId: string?, parentVirtualId: string?,
parentStudioInstance: Instance? parentStudioInstance: Instance?
): MatchResult ): MatchResult
-- Cache lookup
if parentVirtualId and parentStudioInstance then if parentVirtualId and parentStudioInstance then
local pc = session.matchCache[parentVirtualId] local pc = session.matchCache[parentVirtualId]
if pc then if pc then
@ -421,14 +413,14 @@ matchChildren = function(
end end
local matched: { MatchPair } = {} local matched: { MatchPair } = {}
local remainingVirtual: { string } = table.clone(virtualChildren) local matchedV: { [number]: boolean } = {}
local remainingStudio: { Instance } = table.clone(studioChildren) local matchedS: { [number]: boolean } = {}
-- ============================================================ -- ============================================================
-- Group by (Name, ClassName) -- direct property access, no pcall -- Group by (Name, ClassName) once
-- ============================================================ -- ============================================================
local vByKey: { [string]: { number } } = {} local vByKey: { [string]: { number } } = {}
for i, id in ipairs(remainingVirtual) do for i, id in ipairs(virtualChildren) do
local vInst = virtualInstances[id] local vInst = virtualInstances[id]
if vInst then if vInst then
local key = vInst.Name .. "\0" .. vInst.ClassName local key = vInst.Name .. "\0" .. vInst.ClassName
@ -442,7 +434,7 @@ matchChildren = function(
end end
local sByKey: { [string]: { number } } = {} local sByKey: { [string]: { number } } = {}
for i, inst in ipairs(remainingStudio) do for i, inst in ipairs(studioChildren) do
local key = inst.Name .. "\0" .. inst.ClassName local key = inst.Name .. "\0" .. inst.ClassName
local group = sByKey[key] local group = sByKey[key]
if not group then if not group then
@ -453,68 +445,26 @@ matchChildren = function(
end end
-- ============================================================ -- ============================================================
-- 1:1 instant match -- 1:1 instant match + ambiguous scoring in single pass
-- ============================================================ -- ============================================================
local matchedV: { [number]: boolean } = {}
local matchedS: { [number]: boolean } = {}
for key, vIndices in pairs(vByKey) do for key, vIndices in pairs(vByKey) do
local sIndices = sByKey[key] local sIndices = sByKey[key]
if sIndices and #vIndices == 1 and #sIndices == 1 then
local vi, si = vIndices[1], sIndices[1]
if not matchedV[vi] and not matchedS[si] then
table.insert(matched, {
virtualId = remainingVirtual[vi],
studioInstance = remainingStudio[si],
})
matchedV[vi] = true
matchedS[si] = true
end
end
end
removeMatched(remainingVirtual, matchedV)
removeMatched(remainingStudio, matchedS)
-- ============================================================
-- Ambiguous groups: change-count scoring + greedy assignment
-- ============================================================
if #remainingVirtual > 0 and #remainingStudio > 0 then
local vByKey2: { [string]: { number } } = {}
for i, id in ipairs(remainingVirtual) do
local vInst = virtualInstances[id]
if vInst then
local key = vInst.Name .. "\0" .. vInst.ClassName
local group = vByKey2[key]
if not group then
group = {}
vByKey2[key] = group
end
table.insert(group, i)
end
end
local sByKey2: { [string]: { number } } = {}
for i, inst in ipairs(remainingStudio) do
local key = inst.Name .. "\0" .. inst.ClassName
local group = sByKey2[key]
if not group then
group = {}
sByKey2[key] = group
end
table.insert(group, i)
end
local matchedV2: { [number]: boolean } = {}
local matchedS2: { [number]: boolean } = {}
for key, vIndices in pairs(vByKey2) do
local sIndices = sByKey2[key]
if not sIndices then if not sIndices then
continue continue
end end
local firstVInst = virtualInstances[remainingVirtual[vIndices[1]]] if #vIndices == 1 and #sIndices == 1 then
local vi, si = vIndices[1], sIndices[1]
table.insert(matched, {
virtualId = virtualChildren[vi],
studioInstance = studioChildren[si],
})
matchedV[vi] = true
matchedS[si] = true
continue
end
local firstVInst = virtualInstances[virtualChildren[vIndices[1]]]
if not firstVInst then if not firstVInst then
continue continue
end end
@ -524,10 +474,7 @@ matchChildren = function(
local allExtraProps: { [string]: boolean } = {} local allExtraProps: { [string]: boolean } = {}
local allRefProps: { [string]: boolean } = {} local allRefProps: { [string]: boolean } = {}
for _, vi in ipairs(vIndices) do for _, vi in ipairs(vIndices) do
if matchedV2[vi] then local vInst = virtualInstances[virtualChildren[vi]]
continue
end
local vInst = virtualInstances[remainingVirtual[vi]]
if not vInst then if not vInst then
continue continue
end end
@ -553,74 +500,47 @@ matchChildren = function(
local sCaches: { [number]: SCache } = {} local sCaches: { [number]: SCache } = {}
for _, si in ipairs(sIndices) do for _, si in ipairs(sIndices) do
if not matchedS2[si] then sCaches[si] = cacheStudio(studioChildren[si], classKeys, extraPropNamesArray, refPropNamesArray)
sCaches[si] = cacheStudio(remainingStudio[si], classKeys, extraPropNamesArray, refPropNamesArray)
end
end end
-- Score all (A, B) pairs
local scoredPairs: { ScoredPair } = {} local scoredPairs: { ScoredPair } = {}
local pairIdx = 0 local pairIdx = 0
local bestSoFar = math.huge local bestSoFar = math.huge
for _, vi in ipairs(vIndices) do for _, vi in ipairs(vIndices) do
if matchedV2[vi] then
continue
end
local vCache = vCaches[vi] local vCache = vCaches[vi]
if not vCache then if not vCache then
continue continue
end end
for _, si in ipairs(sIndices) do for _, si in ipairs(sIndices) do
if matchedS2[si] then
continue
end
local sCache = sCaches[si] local sCache = sCaches[si]
if not sCache then if not sCache then
continue continue
end end
pairIdx += 1 pairIdx += 1
local cost = countOwnDiffs(vCache, sCache, classKeys) local cost = countOwnDiffs(vCache, sCache, classKeys)
if cost < bestSoFar then if cost < bestSoFar then
local vInst = virtualInstances[remainingVirtual[vi]] local validVChildren = vCache.validChildren
if vInst then
local vChildren = vInst.Children
local studioKids = sCache.children local studioKids = sCache.children
if (vChildren and #vChildren > 0) or #studioKids > 0 then if #validVChildren > 0 or #studioKids > 0 then
if not vChildren or #vChildren == 0 then if #validVChildren == 0 then
cost += #studioKids * UNMATCHED_PENALTY cost += #studioKids * UNMATCHED_PENALTY
elseif #studioKids == 0 then elseif #studioKids == 0 then
for _, childId in ipairs(vChildren) do cost += #validVChildren * UNMATCHED_PENALTY
if virtualInstances[childId] then
cost += UNMATCHED_PENALTY
end
end
else 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( local childResult = matchChildren(
session, session,
validVChildren, validVChildren,
studioKids, studioKids,
virtualInstances, virtualInstances,
remainingVirtual[vi], virtualChildren[vi],
sCache.instance sCache.instance
) )
cost += childResult.totalCost cost += childResult.totalCost
else
cost += #studioKids * UNMATCHED_PENALTY
end
end
end end
end end
end end
@ -640,35 +560,45 @@ matchChildren = function(
end) end)
for _, pair in ipairs(scoredPairs) do for _, pair in ipairs(scoredPairs) do
if matchedV2[pair.vi] or matchedS2[pair.si] then if matchedV[pair.vi] or matchedS[pair.si] then
continue continue
end end
table.insert(matched, { table.insert(matched, {
virtualId = remainingVirtual[pair.vi], virtualId = virtualChildren[pair.vi],
studioInstance = remainingStudio[pair.si], studioInstance = studioChildren[pair.si],
}) })
matchedV2[pair.vi] = true matchedV[pair.vi] = true
matchedS2[pair.si] = true matchedS[pair.si] = true
end end
end end
removeMatched(remainingVirtual, matchedV2) -- ============================================================
removeMatched(remainingStudio, matchedS2) -- Build unmatched lists + compute totalCost
-- ============================================================
local unmatchedVirtual: { string } = {}
for i, id in ipairs(virtualChildren) do
if not matchedV[i] then
table.insert(unmatchedVirtual, id)
end
end
local unmatchedStudio: { Instance } = {}
for i, inst in ipairs(studioChildren) do
if not matchedS[i] then
table.insert(unmatchedStudio, inst)
end
end end
-- ============================================================
-- Compute totalCost for ALL matched pairs (session cache helps)
-- ============================================================
local totalCost = 0 local totalCost = 0
for _, pair in ipairs(matched) do for _, pair in ipairs(matched) do
totalCost += computePairCost(session, pair.virtualId, pair.studioInstance, virtualInstances, math.huge) totalCost += computePairCost(session, pair.virtualId, pair.studioInstance, virtualInstances, math.huge)
end end
totalCost += (#remainingVirtual + #remainingStudio) * UNMATCHED_PENALTY totalCost += (#unmatchedVirtual + #unmatchedStudio) * UNMATCHED_PENALTY
local result = { local result = {
matched = matched, matched = matched,
unmatchedVirtual = remainingVirtual, unmatchedVirtual = unmatchedVirtual,
unmatchedStudio = remainingStudio, unmatchedStudio = unmatchedStudio,
totalCost = totalCost, totalCost = totalCost,
} }

View file

@ -64,10 +64,20 @@ return function()
local parentInst = container local parentInst = container
local result1 = Matching.matchChildren( local result1 = Matching.matchChildren(
session, { "A" }, { studioChild }, virtualInstances, "PARENT", parentInst session,
{ "A" },
{ studioChild },
virtualInstances,
"PARENT",
parentInst
) )
local result2 = Matching.matchChildren( local result2 = Matching.matchChildren(
session, { "A" }, { studioChild }, virtualInstances, "PARENT", parentInst session,
{ "A" },
{ studioChild },
virtualInstances,
"PARENT",
parentInst
) )
expect(result1).to.equal(result2) expect(result1).to.equal(result2)
end) end)
@ -82,10 +92,20 @@ return function()
local studioY = makeStudioFolder("Y", container) local studioY = makeStudioFolder("Y", container)
local result1 = Matching.matchChildren( local result1 = Matching.matchChildren(
session, { "A" }, { studioX }, virtualInstances, "PARENT_1", container session,
{ "A" },
{ studioX },
virtualInstances,
"PARENT_1",
container
) )
local result2 = Matching.matchChildren( local result2 = Matching.matchChildren(
session, { "B" }, { studioY }, virtualInstances, "PARENT_2", container session,
{ "B" },
{ studioY },
virtualInstances,
"PARENT_2",
container
) )
expect(result1).never.to.equal(result2) expect(result1).never.to.equal(result2)
expect(result1.matched[1].virtualId).to.equal("A") expect(result1.matched[1].virtualId).to.equal("A")
@ -100,12 +120,8 @@ return function()
} }
local studioChild = makeStudioFolder("Child", container) local studioChild = makeStudioFolder("Child", container)
local result1 = Matching.matchChildren( local result1 = Matching.matchChildren(session1, { "A" }, { studioChild }, virtualInstances, "P", container)
session1, { "A" }, { studioChild }, virtualInstances, "P", container local result2 = Matching.matchChildren(session2, { "A" }, { studioChild }, virtualInstances, "P", container)
)
local result2 = Matching.matchChildren(
session2, { "A" }, { studioChild }, virtualInstances, "P", container
)
expect(result1).never.to.equal(result2) expect(result1).never.to.equal(result2)
expect(result1.totalCost).to.equal(result2.totalCost) expect(result1.totalCost).to.equal(result2.totalCost)
end) end)
@ -120,7 +136,12 @@ return function()
local studioAlpha = makeStudioFolder("Alpha", container) local studioAlpha = makeStudioFolder("Alpha", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A" }, { studioAlpha }, virtualInstances, "ROOT", container session,
{ "A" },
{ studioAlpha },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(1) expect(#result.matched).to.equal(1)
expect(result.totalCost).to.equal(0) expect(result.totalCost).to.equal(0)
@ -136,7 +157,12 @@ return function()
local studioBeta = makeStudioFolder("Beta", container) local studioBeta = makeStudioFolder("Beta", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A", "B" }, { studioAlpha, studioBeta }, virtualInstances, "ROOT", container session,
{ "A", "B" },
{ studioAlpha, studioBeta },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(2) expect(#result.matched).to.equal(2)
expect(result.totalCost).to.equal(0) expect(result.totalCost).to.equal(0)
@ -149,9 +175,7 @@ return function()
} }
local studioPart = makeStudioPart("P", container, 0) local studioPart = makeStudioPart("P", container, 0)
local result = Matching.matchChildren( local result = Matching.matchChildren(session, { "A" }, { studioPart }, virtualInstances, "ROOT", container)
session, { "A" }, { studioPart }, virtualInstances, "ROOT", container
)
expect(#result.matched).to.equal(1) expect(#result.matched).to.equal(1)
expect(result.totalCost > 0).to.equal(true) expect(result.totalCost > 0).to.equal(true)
end) end)
@ -165,7 +189,12 @@ return function()
local studioAlpha = makeStudioFolder("Alpha", container) local studioAlpha = makeStudioFolder("Alpha", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A", "B" }, { studioAlpha }, virtualInstances, "ROOT", container session,
{ "A", "B" },
{ studioAlpha },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(1) expect(#result.matched).to.equal(1)
expect(#result.unmatchedVirtual).to.equal(1) expect(#result.unmatchedVirtual).to.equal(1)
@ -181,7 +210,12 @@ return function()
local studioBeta = makeStudioFolder("Beta", container) local studioBeta = makeStudioFolder("Beta", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A" }, { studioAlpha, studioBeta }, virtualInstances, "ROOT", container session,
{ "A" },
{ studioAlpha, studioBeta },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(1) expect(#result.matched).to.equal(1)
expect(#result.unmatchedStudio).to.equal(1) expect(#result.unmatchedStudio).to.equal(1)
@ -361,7 +395,12 @@ return function()
local s2 = makeStudioFolder("Data", container) local s2 = makeStudioFolder("Data", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A", "B" }, { s1, s2 }, virtualInstances, "ROOT", container session,
{ "A", "B" },
{ s1, s2 },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(2) expect(#result.matched).to.equal(2)
expect(result.totalCost).to.equal(0) expect(result.totalCost).to.equal(0)
@ -378,7 +417,12 @@ return function()
local s2 = makeStudioFolder("Data", container) local s2 = makeStudioFolder("Data", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(
session, { "A", "B", "C" }, { s1, s2 }, virtualInstances, "ROOT", container session,
{ "A", "B", "C" },
{ s1, s2 },
virtualInstances,
"ROOT",
container
) )
expect(#result.matched).to.equal(2) expect(#result.matched).to.equal(2)
expect(#result.unmatchedVirtual).to.equal(1) expect(#result.unmatchedVirtual).to.equal(1)
@ -442,9 +486,7 @@ return function()
A = makeVirtualInstance("Alpha", "Folder", {}, {}), A = makeVirtualInstance("Alpha", "Folder", {}, {}),
B = makeVirtualInstance("Beta", "Folder", {}, {}), B = makeVirtualInstance("Beta", "Folder", {}, {}),
} }
local result = Matching.matchChildren( local result = Matching.matchChildren(session, { "A", "B" }, {}, virtualInstances, "ROOT", container)
session, { "A", "B" }, {}, virtualInstances, "ROOT", container
)
expect(#result.matched).to.equal(0) expect(#result.matched).to.equal(0)
expect(#result.unmatchedVirtual).to.equal(2) expect(#result.unmatchedVirtual).to.equal(2)
expect(result.totalCost).to.equal(20000) expect(result.totalCost).to.equal(20000)
@ -454,9 +496,7 @@ return function()
local session = Matching.newSession() local session = Matching.newSession()
local s1 = makeStudioFolder("Alpha", container) local s1 = makeStudioFolder("Alpha", container)
local s2 = makeStudioFolder("Beta", container) local s2 = makeStudioFolder("Beta", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(session, {}, { s1, s2 }, {}, "ROOT", container)
session, {}, { s1, s2 }, {}, "ROOT", container
)
expect(#result.matched).to.equal(0) expect(#result.matched).to.equal(0)
expect(#result.unmatchedStudio).to.equal(2) expect(#result.unmatchedStudio).to.equal(2)
expect(result.totalCost).to.equal(20000) expect(result.totalCost).to.equal(20000)
@ -497,9 +537,7 @@ return function()
} }
local studioOnly = makeStudioFolder("Only", container) local studioOnly = makeStudioFolder("Only", container)
local result = Matching.matchChildren( local result = Matching.matchChildren(session, { "A" }, { studioOnly }, virtualInstances, "ROOT", container)
session, { "A" }, { studioOnly }, virtualInstances, "ROOT", container
)
expect(#result.matched).to.equal(1) expect(#result.matched).to.equal(1)
expect(result.matched[1].virtualId).to.equal("A") expect(result.matched[1].virtualId).to.equal("A")
expect(result.matched[1].studioInstance).to.equal(studioOnly) expect(result.matched[1].studioInstance).to.equal(studioOnly)

View file

@ -937,8 +937,8 @@ impl JobThreadContext {
let start = Instant::now(); let start = Instant::now();
let instance_context = InstanceContext::new(); let instance_context = InstanceContext::new();
let snapshot = match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_file_path) let snapshot =
{ match snapshot_from_vfs(&instance_context, &self.vfs, &self.project_file_path) {
Ok(s) => s, Ok(s) => s,
Err(e) => { Err(e) => {
log::error!("Tree reconciliation snapshot error: {:?}", e); log::error!("Tree reconciliation snapshot error: {:?}", e);

View file

@ -4,7 +4,6 @@ use std::{
mem::forget, mem::forget,
net::{IpAddr, Ipv4Addr, SocketAddr}, net::{IpAddr, Ipv4Addr, SocketAddr},
path::{Path, PathBuf}, path::{Path, PathBuf},
process::{Command, Stdio},
sync::Arc, sync::Arc,
}; };
@ -131,7 +130,7 @@ fn run_live_syncback(project_path: &Path, payload: SyncbackPayload) -> anyhow::R
result.fs_snapshot.removed_paths().len() result.fs_snapshot.removed_paths().len()
); );
refresh_git_index(base_path); crate::git::refresh_git_index(base_path);
forget(session_old); forget(session_old);
@ -231,30 +230,3 @@ fn fixup_ref_properties(dom: &mut WeakDom, ref_map: &HashMap<Ref, Ref>) {
} }
} }
} }
fn refresh_git_index(project_dir: &Path) {
let mut check_dir = Some(project_dir);
let mut is_git_repo = false;
while let Some(dir) = check_dir {
if dir.join(".git").exists() {
is_git_repo = true;
break;
}
check_dir = dir.parent();
}
if is_git_repo {
log::info!("Refreshing git index...");
match Command::new("git")
.args(["update-index", "--refresh", "-q"])
.current_dir(project_dir)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
{
Ok(_) => log::info!("Git index refreshed."),
Err(e) => log::warn!("Failed to run git update-index --refresh: {}", e),
}
}
}

View file

@ -2,7 +2,6 @@ use std::{
io::{self, BufReader, Write as _}, io::{self, BufReader, Write as _},
mem::forget, mem::forget,
path::{Path, PathBuf}, path::{Path, PathBuf},
process::{Command, Stdio},
time::Instant, time::Instant,
}; };
@ -280,8 +279,7 @@ impl SyncbackCommand {
result.fs_snapshot.removed_paths().len() result.fs_snapshot.removed_paths().len()
); );
// Refresh git index if in a git repository crate::git::refresh_git_index(base_path);
refresh_git_index(base_path);
// Delete input file if using default Project.rbxl location // Delete input file if using default Project.rbxl location
if let Some(input_path) = &delete_input_after_syncback { if let Some(input_path) = &delete_input_after_syncback {
@ -386,42 +384,6 @@ fn download_place(place_id: u64) -> anyhow::Result<NamedTempFile> {
Ok(temp_file) Ok(temp_file)
} }
/// Refreshes the git index if the project is in a git repository.
///
/// This is useful because syncback may rewrite files with identical content,
/// which can cause git to report them as modified due to timestamp changes.
fn refresh_git_index(project_dir: &Path) {
// Check if .git exists in project dir or any parent
let mut check_dir = Some(project_dir);
let mut is_git_repo = false;
while let Some(dir) = check_dir {
if dir.join(".git").exists() {
is_git_repo = true;
break;
}
check_dir = dir.parent();
}
if is_git_repo {
log::info!("Refreshing git index...");
// Exit code 1 is normal - it just means files were refreshed.
// We only warn if git itself fails to run.
match Command::new("git")
.args(["update-index", "--refresh", "-q"])
.current_dir(project_dir)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
{
Ok(_) => log::info!("Git index refreshed."),
Err(e) => log::warn!("Failed to run git update-index --refresh: {}", e),
}
} else {
log::debug!("Not a git repository, skipping index refresh.");
}
}
fn read_dom(path: &Path, file_kind: FileKind) -> anyhow::Result<WeakDom> { fn read_dom(path: &Path, file_kind: FileKind) -> anyhow::Result<WeakDom> {
let content = BufReader::new(File::open(path)?); let content = BufReader::new(File::open(path)?);
match file_kind { match file_kind {

View file

@ -1,7 +1,7 @@
use std::{ use std::{
collections::{HashMap, HashSet}, collections::{HashMap, HashSet},
path::{Path, PathBuf}, path::{Path, PathBuf},
process::Command, process::{Command, Stdio},
sync::{Arc, Mutex}, sync::{Arc, Mutex},
}; };
@ -244,6 +244,39 @@ pub fn compute_git_metadata(tree_handle: &Arc<Mutex<RojoTree>>, repo_root: &Path
} }
} }
/// Refreshes the git index if the project is in a git repository.
///
/// This is useful because syncback may rewrite files with identical content,
/// which can cause git to report them as modified due to timestamp changes.
pub fn refresh_git_index(project_dir: &Path) {
let mut check_dir = Some(project_dir);
let mut is_git_repo = false;
while let Some(dir) = check_dir {
if dir.join(".git").exists() {
is_git_repo = true;
break;
}
check_dir = dir.parent();
}
if is_git_repo {
log::info!("Refreshing git index...");
match Command::new("git")
.args(["update-index", "--refresh", "-q"])
.current_dir(project_dir)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
{
Ok(_) => log::info!("Git index refreshed."),
Err(e) => log::warn!("Failed to run git update-index --refresh: {}", e),
}
} else {
log::debug!("Not a git repository, skipping index refresh.");
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;

View file

@ -1318,7 +1318,10 @@ mod tests {
let (tree, children) = make_tree_with_children(&[("Alpha", "Folder"), ("Beta", "Folder")]); let (tree, children) = make_tree_with_children(&[("Alpha", "Folder"), ("Beta", "Folder")]);
let result = match_forward(snaps, &children, &tree, &MatchingSession::new()); let result = match_forward(snaps, &children, &tree, &MatchingSession::new());
assert_eq!(result.matched.len(), 2); assert_eq!(result.matched.len(), 2);
assert_eq!(result.total_cost, 0, "Identical instances should have zero cost"); assert_eq!(
result.total_cost, 0,
"Identical instances should have zero cost"
);
} }
#[test] #[test]
@ -1390,14 +1393,8 @@ mod tests {
#[test] #[test]
fn session_cache_consistent() { fn session_cache_consistent() {
let snaps1 = vec![ let snaps1 = vec![make_snapshot("X", "Folder"), make_snapshot("Y", "Folder")];
make_snapshot("X", "Folder"), let snaps2 = vec![make_snapshot("X", "Folder"), make_snapshot("Y", "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 (tree, children) = make_tree_with_children(&[("X", "Folder"), ("Y", "Folder")]);
let session = MatchingSession::new(); let session = MatchingSession::new();
@ -1406,10 +1403,7 @@ mod tests {
assert_eq!(r1.matched.len(), r2.matched.len()); assert_eq!(r1.matched.len(), r2.matched.len());
assert_eq!(r1.total_cost, r2.total_cost); assert_eq!(r1.total_cost, r2.total_cost);
assert_eq!( assert_eq!(r1.unmatched_snapshot.len(), r2.unmatched_snapshot.len());
r1.unmatched_snapshot.len(),
r2.unmatched_snapshot.len()
);
} }
#[test] #[test]

View file

@ -829,29 +829,14 @@ mod tests {
fn total_cost_includes_unmatched() { fn total_cost_includes_unmatched() {
let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
let new_root = new_dom.root_ref(); let new_root = new_dom.root_ref();
new_dom.insert( new_dom.insert(new_root, InstanceBuilder::new("Folder").with_name("A"));
new_root, new_dom.insert(new_root, InstanceBuilder::new("Folder").with_name("B"));
InstanceBuilder::new("Folder").with_name("A"), new_dom.insert(new_root, InstanceBuilder::new("Folder").with_name("C"));
);
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 mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
let old_root = old_dom.root_ref(); let old_root = old_dom.root_ref();
old_dom.insert( old_dom.insert(old_root, InstanceBuilder::new("Folder").with_name("A"));
old_root, old_dom.insert(old_root, InstanceBuilder::new("Folder").with_name("B"));
InstanceBuilder::new("Folder").with_name("A"),
);
old_dom.insert(
old_root,
InstanceBuilder::new("Folder").with_name("B"),
);
let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec(); let new_children: Vec<Ref> = new_dom.get_by_ref(new_root).unwrap().children().to_vec();
let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec(); let old_children: Vec<Ref> = old_dom.get_by_ref(old_root).unwrap().children().to_vec();

View file

@ -61,12 +61,22 @@ pub fn filter_properties_with_stats<'inst>(
return true; return true;
} }
if !sync_unscriptable { if !sync_unscriptable {
if let Some(data) = class_data { let mut current = class_data;
loop {
let data = match current {
Some(d) => d,
None => break,
};
if let Some(prop_data) = data.properties.get(prop_name.as_str()) { if let Some(prop_data) = data.properties.get(prop_name.as_str()) {
if matches!(prop_data.scriptability, Scriptability::None) { if matches!(prop_data.scriptability, Scriptability::None) {
return true; return true;
} }
break;
} }
current = data
.superclass
.as_ref()
.and_then(|s| database.classes.get(&**s));
} }
} }
false false