mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 22:11:38 +00:00
Refactor Reconciler equality checks and enhance path handling
- Introduced a new `trueEquals` function in `trueEquals.lua` for shared value equality checks, improving consistency between `diff.lua` and `matching.lua`. - Removed redundant equality logic from `diff.lua` and `matching.lua`, streamlining the codebase. - Updated path handling in various test snapshots to ensure correct reference values, enhancing the accuracy of reconciliation tests. - Implemented a post-processing step in `syncback` to fix `Rojo_Ref_*` attribute paths with correct dedup suffixes, improving path resolution. These changes enhance the clarity and efficiency of the Reconciler's equality checks and path handling, paving the way for improved project management and user experience.
This commit is contained in:
parent
aabbfb23b2
commit
e207a398b1
14 changed files with 832 additions and 218 deletions
|
|
@ -20,6 +20,11 @@ use super::{InstanceSnapshot, InstanceWithMeta, RojoTree};
|
|||
|
||||
const UNMATCHED_PENALTY: u32 = 10_000;
|
||||
|
||||
/// Maximum recursion depth for `compute_change_count`. Beyond this depth,
|
||||
/// only flat property comparison is used (no subtree recursion). Prevents
|
||||
/// O(n^k) explosion on deeply nested trees with many same-named instances.
|
||||
const MAX_SCORING_DEPTH: u32 = 3;
|
||||
|
||||
/// Result of the forward sync matching algorithm.
|
||||
pub struct ForwardMatchResult {
|
||||
/// Pairs of (snapshot_child, tree_ref) that were matched.
|
||||
|
|
@ -128,7 +133,7 @@ pub fn match_forward(
|
|||
continue;
|
||||
}
|
||||
|
||||
// Score all (A, B) pairs in child order
|
||||
// Score all (A, B) pairs using recursive change-count scoring
|
||||
let mut pairs: Vec<(u32, usize, usize)> = Vec::new();
|
||||
let mut best_so_far = u32::MAX;
|
||||
for &si in &avail_snap {
|
||||
|
|
@ -137,12 +142,7 @@ pub fn match_forward(
|
|||
None => continue,
|
||||
};
|
||||
for &ti in &avail_tree {
|
||||
let inst = match tree.get_instance(tree_children[ti]) {
|
||||
Some(i) => i,
|
||||
None => continue,
|
||||
};
|
||||
let cost = count_own_diffs(snap, &inst);
|
||||
// Children count diff included in count_own_diffs
|
||||
let cost = compute_change_count(snap, tree_children[ti], tree, best_so_far, 0);
|
||||
pairs.push((cost, si, ti));
|
||||
if cost < best_so_far {
|
||||
best_so_far = cost;
|
||||
|
|
@ -197,6 +197,191 @@ fn build_result(
|
|||
}
|
||||
}
|
||||
|
||||
/// Lightweight matching result used during recursive scoring.
|
||||
/// Returns index pairs (snap_index, tree_child_index) without consuming
|
||||
/// the snapshots.
|
||||
struct ScoringMatchResult {
|
||||
matched: Vec<(usize, usize)>,
|
||||
unmatched_snap: usize,
|
||||
unmatched_tree: usize,
|
||||
}
|
||||
|
||||
/// Match children by reference for scoring purposes (non-consuming).
|
||||
/// Groups by (Name, ClassName), instant-matches 1:1 groups, and scores
|
||||
/// ambiguous groups using `compute_change_count` (mutually recursive).
|
||||
fn match_children_for_scoring(
|
||||
snap_children: &[InstanceSnapshot],
|
||||
tree_children: &[Ref],
|
||||
tree: &RojoTree,
|
||||
depth: u32,
|
||||
) -> ScoringMatchResult {
|
||||
let mut snap_matched = vec![false; snap_children.len()];
|
||||
let mut tree_matched = vec![false; tree_children.len()];
|
||||
let mut matched = Vec::new();
|
||||
|
||||
if snap_children.is_empty() && tree_children.is_empty() {
|
||||
return ScoringMatchResult {
|
||||
matched,
|
||||
unmatched_snap: 0,
|
||||
unmatched_tree: 0,
|
||||
};
|
||||
}
|
||||
|
||||
// Group by (Name, ClassName)
|
||||
let mut snap_by_key: HashMap<(String, String), Vec<usize>> = HashMap::new();
|
||||
for (i, snap) in snap_children.iter().enumerate() {
|
||||
snap_by_key
|
||||
.entry((snap.name.to_string(), snap.class_name.to_string()))
|
||||
.or_default()
|
||||
.push(i);
|
||||
}
|
||||
|
||||
let mut tree_by_key: HashMap<(String, String), Vec<usize>> = HashMap::new();
|
||||
for (i, &child_ref) in tree_children.iter().enumerate() {
|
||||
if let Some(inst) = tree.get_instance(child_ref) {
|
||||
tree_by_key
|
||||
.entry((inst.name().to_string(), inst.class_name().to_string()))
|
||||
.or_default()
|
||||
.push(i);
|
||||
}
|
||||
}
|
||||
|
||||
// 1:1 groups: instant match
|
||||
for (key, snap_indices) in &snap_by_key {
|
||||
if let Some(tree_indices) = tree_by_key.get(key) {
|
||||
if snap_indices.len() == 1 && tree_indices.len() == 1 {
|
||||
let si = snap_indices[0];
|
||||
let ti = tree_indices[0];
|
||||
matched.push((si, ti));
|
||||
snap_matched[si] = true;
|
||||
tree_matched[ti] = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Ambiguous groups: score + greedy assign
|
||||
for (key, snap_indices) in &snap_by_key {
|
||||
let Some(tree_indices) = tree_by_key.get(key) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let avail_snap: Vec<usize> = snap_indices
|
||||
.iter()
|
||||
.filter(|&&si| !snap_matched[si])
|
||||
.copied()
|
||||
.collect();
|
||||
let avail_tree: Vec<usize> = tree_indices
|
||||
.iter()
|
||||
.filter(|&&ti| !tree_matched[ti])
|
||||
.copied()
|
||||
.collect();
|
||||
|
||||
if avail_snap.is_empty() || avail_tree.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if avail_snap.len() == 1 && avail_tree.len() == 1 {
|
||||
let si = avail_snap[0];
|
||||
let ti = avail_tree[0];
|
||||
matched.push((si, ti));
|
||||
snap_matched[si] = true;
|
||||
tree_matched[ti] = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
let mut pairs: Vec<(u32, usize, usize)> = Vec::new();
|
||||
let mut best_so_far = u32::MAX;
|
||||
for &si in &avail_snap {
|
||||
for &ti in &avail_tree {
|
||||
let cost = compute_change_count(
|
||||
&snap_children[si],
|
||||
tree_children[ti],
|
||||
tree,
|
||||
best_so_far,
|
||||
depth,
|
||||
);
|
||||
pairs.push((cost, si, ti));
|
||||
if cost < best_so_far {
|
||||
best_so_far = cost;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pairs.sort_by_key(|&(cost, _, _)| cost);
|
||||
|
||||
for &(_, si, ti) in &pairs {
|
||||
if snap_matched[si] || tree_matched[ti] {
|
||||
continue;
|
||||
}
|
||||
matched.push((si, ti));
|
||||
snap_matched[si] = true;
|
||||
tree_matched[ti] = true;
|
||||
}
|
||||
}
|
||||
|
||||
let unmatched_snap = snap_matched.iter().filter(|&&m| !m).count();
|
||||
let unmatched_tree = tree_matched.iter().filter(|&&m| !m).count();
|
||||
|
||||
ScoringMatchResult {
|
||||
matched,
|
||||
unmatched_snap,
|
||||
unmatched_tree,
|
||||
}
|
||||
}
|
||||
|
||||
/// Compute total change count between a snapshot and a tree instance,
|
||||
/// including recursive subtree scoring. Returns the number of reconciler
|
||||
/// operations needed to turn the tree instance into the snapshot.
|
||||
///
|
||||
/// Mutually recursive with `match_children_for_scoring`: this function
|
||||
/// scores a single pair, while `match_children_for_scoring` groups and
|
||||
/// assigns children pairs (calling this function for ambiguous scoring).
|
||||
fn compute_change_count(
|
||||
snap: &InstanceSnapshot,
|
||||
tree_ref: Ref,
|
||||
tree: &RojoTree,
|
||||
best_so_far: u32,
|
||||
depth: u32,
|
||||
) -> u32 {
|
||||
let inst = match tree.get_instance(tree_ref) {
|
||||
Some(i) => i,
|
||||
None => return UNMATCHED_PENALTY,
|
||||
};
|
||||
|
||||
let mut cost = count_own_diffs(snap, &inst);
|
||||
if cost >= best_so_far || depth >= MAX_SCORING_DEPTH {
|
||||
return cost;
|
||||
}
|
||||
|
||||
let snap_children = &snap.children;
|
||||
let tree_children = inst.children();
|
||||
|
||||
if snap_children.is_empty() && tree_children.is_empty() {
|
||||
return cost;
|
||||
}
|
||||
|
||||
let scoring = match_children_for_scoring(snap_children, tree_children, tree, depth + 1);
|
||||
|
||||
for &(si, ti) in &scoring.matched {
|
||||
let remaining = best_so_far.saturating_sub(cost);
|
||||
cost += compute_change_count(
|
||||
&snap_children[si],
|
||||
tree_children[ti],
|
||||
tree,
|
||||
remaining,
|
||||
depth + 1,
|
||||
);
|
||||
if cost >= best_so_far {
|
||||
return cost;
|
||||
}
|
||||
}
|
||||
|
||||
cost = cost.saturating_add(
|
||||
(scoring.unmatched_snap + scoring.unmatched_tree) as u32 * UNMATCHED_PENALTY,
|
||||
);
|
||||
|
||||
cost
|
||||
}
|
||||
|
||||
/// Count own property diffs between a snapshot and a tree instance.
|
||||
/// Each differing property = +1. Tags and Attributes counted granularly.
|
||||
/// Children count diff = +1.
|
||||
|
|
@ -435,4 +620,144 @@ mod tests {
|
|||
assert!(result.unmatched_snapshot.is_empty());
|
||||
assert!(result.unmatched_tree.is_empty());
|
||||
}
|
||||
|
||||
fn make_snapshot_with_children(
|
||||
name: &str,
|
||||
class: &str,
|
||||
children: Vec<InstanceSnapshot>,
|
||||
) -> InstanceSnapshot {
|
||||
InstanceSnapshot {
|
||||
snapshot_id: Ref::none(),
|
||||
metadata: InstanceMetadata::default(),
|
||||
name: Cow::Owned(name.to_string()),
|
||||
class_name: ustr(class),
|
||||
properties: Default::default(),
|
||||
children,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recursive_matching_distinguishes_by_children() {
|
||||
// Two Folders both named "Data" with identical top-level properties
|
||||
// but different children. The recursive scoring should pair each
|
||||
// Folder with the tree Folder that has matching children.
|
||||
use rbx_dom_weak::types::Variant;
|
||||
|
||||
let snap_a = make_snapshot_with_children(
|
||||
"Data",
|
||||
"Folder",
|
||||
vec![make_snapshot_with_props(
|
||||
"Child",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(0.0))],
|
||||
)],
|
||||
);
|
||||
let snap_b = make_snapshot_with_children(
|
||||
"Data",
|
||||
"Folder",
|
||||
vec![make_snapshot_with_props(
|
||||
"Child",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(1.0))],
|
||||
)],
|
||||
);
|
||||
let snaps = vec![snap_a, snap_b];
|
||||
|
||||
// Build tree with the two Folders in REVERSED order
|
||||
let tree_child_a = make_snapshot_with_children(
|
||||
"Data",
|
||||
"Folder",
|
||||
vec![make_snapshot_with_props(
|
||||
"Child",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(1.0))],
|
||||
)],
|
||||
);
|
||||
let tree_child_b = make_snapshot_with_children(
|
||||
"Data",
|
||||
"Folder",
|
||||
vec![make_snapshot_with_props(
|
||||
"Child",
|
||||
"Part",
|
||||
vec![("Transparency", Variant::Float32(0.0))],
|
||||
)],
|
||||
);
|
||||
let root_snap = InstanceSnapshot {
|
||||
snapshot_id: Ref::none(),
|
||||
metadata: InstanceMetadata::default(),
|
||||
name: Cow::Borrowed("DataModel"),
|
||||
class_name: ustr("DataModel"),
|
||||
properties: Default::default(),
|
||||
children: vec![tree_child_a, tree_child_b],
|
||||
};
|
||||
let tree = RojoTree::new(root_snap);
|
||||
let root_id = tree.get_root_id();
|
||||
let tree_children: Vec<Ref> = tree.get_instance(root_id).unwrap().children().to_vec();
|
||||
|
||||
let result = match_forward(snaps, &tree_children, &tree);
|
||||
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert!(result.unmatched_snapshot.is_empty());
|
||||
assert!(result.unmatched_tree.is_empty());
|
||||
|
||||
// Verify recursive matching paired by children content, not position.
|
||||
// snap[0] has child Transparency=0.0, should match tree[1] (also 0.0).
|
||||
// snap[1] has child Transparency=1.0, should match tree[0] (also 1.0).
|
||||
for (snap, tree_ref) in &result.matched {
|
||||
let tree_inst = tree.get_instance(*tree_ref).unwrap();
|
||||
let tree_child_ref = tree_inst.children()[0];
|
||||
let tree_child = tree.get_instance(tree_child_ref).unwrap();
|
||||
|
||||
let snap_child = &snap.children[0];
|
||||
let snap_val = snap_child.properties.get(&ustr("Transparency"));
|
||||
let tree_val = tree_child.properties().get(&ustr("Transparency"));
|
||||
assert_eq!(
|
||||
snap_val, tree_val,
|
||||
"Recursive matching failed: snap child Transparency={:?}, tree child Transparency={:?}",
|
||||
snap_val, tree_val
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn depth_limit_completes_quickly() {
|
||||
// Deeply nested tree (depth=6) with same-named instances at each level.
|
||||
// Should complete without hanging thanks to the depth limit.
|
||||
fn make_deep_snap(depth: u32) -> InstanceSnapshot {
|
||||
if depth == 0 {
|
||||
return make_snapshot("Leaf", "Part");
|
||||
}
|
||||
make_snapshot_with_children(
|
||||
"Node",
|
||||
"Folder",
|
||||
vec![make_deep_snap(depth - 1), make_deep_snap(depth - 1)],
|
||||
)
|
||||
}
|
||||
|
||||
let snaps = vec![make_deep_snap(6), make_deep_snap(6)];
|
||||
|
||||
let tree_children_snaps = vec![make_deep_snap(6), make_deep_snap(6)];
|
||||
let root_snap = InstanceSnapshot {
|
||||
snapshot_id: Ref::none(),
|
||||
metadata: InstanceMetadata::default(),
|
||||
name: Cow::Borrowed("DataModel"),
|
||||
class_name: ustr("DataModel"),
|
||||
properties: Default::default(),
|
||||
children: tree_children_snaps,
|
||||
};
|
||||
let tree = RojoTree::new(root_snap);
|
||||
let root_id = tree.get_root_id();
|
||||
let tree_children: Vec<Ref> = tree.get_instance(root_id).unwrap().children().to_vec();
|
||||
|
||||
let start = std::time::Instant::now();
|
||||
let result = match_forward(snaps, &tree_children, &tree);
|
||||
let elapsed = start.elapsed();
|
||||
|
||||
assert_eq!(result.matched.len(), 2);
|
||||
assert!(
|
||||
elapsed.as_secs() < 5,
|
||||
"Depth-limited matching took too long: {:?}",
|
||||
elapsed
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue