diff --git a/.cursor/plans/matching_cost_semantics_376e45e2.plan.md b/.cursor/plans/matching_cost_semantics_376e45e2.plan.md new file mode 100644 index 00000000..ce8ad41c --- /dev/null +++ b/.cursor/plans/matching_cost_semantics_376e45e2.plan.md @@ -0,0 +1,324 @@ +--- +name: matching cost semantics +overview: "Split `variant_eq` into two variants: keep fuzzy comparison for Studio-facing operations (patch_compute, api, property_filter), and add a new disk-representation comparison for both Rust matching files where cost should reflect actual file changes." +todos: + - id: expose-fns + content: Make format_f32/format_f64 pub(crate) in json.rs and cleanup_f32 pub(crate) in resolution.rs + status: completed + - id: disk-eq-helpers + content: Add disk_eq_f32, disk_eq_f32_cleaned, disk_eq_f64 zero-allocation helpers to variant_eq.rs + status: completed + - id: variant-eq-disk + content: Implement variant_eq_disk in variant_eq.rs dispatching per Variant type (Float32 standalone vs composite cleanup) + status: completed + - id: update-fwd-matching + content: Replace variant_eq with variant_eq_disk in src/snapshot/matching.rs (count_own_diffs, diff_variant_pair, count_attributes_diff) + status: completed + - id: update-sb-matching + content: Replace variant_eq with variant_eq_disk in src/syncback/matching.rs (count_own_diffs, diff_variant_pair, attribute diff) + status: completed + - id: tests-veq + content: "Add variant_eq_disk unit tests: fuzzy-vs-disk divergence, cleanup normalization, NaN/Inf/-0, all composite types" + status: completed + - id: tests-fwd-large + content: Add 50+ same-named instance matching tests to src/snapshot/matching.rs with 5 variation groups + status: completed + - id: tests-sb-large + content: Add 50+ same-named instance matching tests to src/syncback/matching.rs with 5 variation groups + status: completed + - id: tests-disk-boundary + content: Add tests with values at format_f32 boundaries (close-but-different-on-disk, different-but-same-on-disk) + status: completed + - id: tests-update-existing + content: Update existing matching test assertions to match variant_eq_disk semantics + status: completed +isProject: false +--- + +# Matching Cost Semantics Fix + +## Problem + +`variant_eq` currently uses fuzzy epsilon comparison (absolute + relative, `EPSILON = 0.0001`) in both Rust matching files. This is wrong because: + +- **Rust cost = "will this value result in a file change?"** Two values that format identically on disk should score 0 cost, even if their raw f32 bits differ. Conversely, two values within epsilon but with different disk representations should score 1. +- **Lua cost = "will this change be applied in Studio?"** Fuzzy epsilon is correct here -- it prevents oscillation from Studio float precision rounding. + +The fuzzy epsilon can diverge from disk reality in both directions: + +- `format_f32(1.00005)` = `"1.00005"`, `format_f32(1.00015)` = `"1.00015"` -- different on disk, but `fuzzy_eq_f32` says "equal" (diff = 0.0001) +- `format_f32(0.123456)` could round both `0.1234561` and `0.1234559` to the same string -- same on disk, but `fuzzy_eq_f32` might say "different" + +## Data Flow Analysis + +### Disk serialization pipeline (how floats reach files) + +``` +Variant value + | + v +resolution.rs::from_variant() + |-- Standalone Float32 → AmbiguousValue::Number32(v) [NO cleanup] + |-- Standalone Float64 → AmbiguousValue::Number(v as f64) [NO cleanup] + |-- Composite types (Vector3, Color3, CFrame, UDim, etc.) + | → cleanup_f32(component) then into Array/FullyQualified + v +json.rs::Json5ValueSerializer + |-- serialize_f32 → format_f32(v) [6 significant digits, lexical-write-float] + |-- serialize_f64 → format_f64(v) [full precision] + v +.meta.json5 / .model.json5 on disk +``` + +`**cleanup_f32` matters for composites.** It rounds to 6 decimal places (via `format!("{:.6}")` round-trip), which zeros out very small values. Example: `cleanup_f32(0.0000001)` = `0.0`, so `format_f32(0.0)` = `"0"` vs `format_f32(0.0000001)` = `"1e-7"` -- completely different disk output. + +### Where values come from in each matching file + +`**[src/syncback/matching.rs](src/syncback/matching.rs)`:** + +- `new_dom` = from rbxl/rbxm binary parse. Raw Studio values, NOT through resolution.rs +- `old_dom` = from filesystem build. Values already in disk-normalized form + +`**[src/snapshot/matching.rs](src/snapshot/matching.rs)`:** + +- `snap` = from filesystem re-snapshot. Values read from disk, already in disk form +- `tree` = from RojoTree. Could be from initial forward sync (disk form) or two-way sync push (raw Studio values) + +In both cases, at least one side may not have been through `cleanup_f32 + format_f32`. The comparison must normalize both sides through the full disk pipeline. + +### Other callers of `variant_eq` (KEEP fuzzy -- different semantics) + + +| File | Purpose | Why fuzzy is correct | +| ------------------------------------------------------- | ----------------------------------- | -------------------------------------------------------------- | +| `[patch_compute.rs](src/snapshot/patch_compute.rs)` | Compute patches to send to plugin | Must agree with Lua `diff.lua` (which uses fuzzy `trueEquals`) | +| `[api.rs](src/web/api.rs)` | Filter defaults during two-way sync | "Is this at default?" -- not a disk question | +| `[property_filter.rs](src/syncback/property_filter.rs)` | Filter defaults during syncback | Same rationale | +| `[hash/mod.rs](src/syncback/hash/mod.rs)` | Exclude defaults from content hash | Same rationale | +| `[project.rs](src/snapshot_middleware/project.rs)` | Check project node value drift | Same rationale | + + +## Implementation + +### 1. Expose formatting functions + +- `**[src/json.rs](src/json.rs)`:** Make `format_f32`, `format_f64` and the `Options` constants `pub(crate)` +- `**[src/resolution.rs](src/resolution.rs)`:** Make `cleanup_f32` `pub(crate)` + +### 2. Add disk-representation comparison to `[src/variant_eq.rs](src/variant_eq.rs)` + +Add zero-allocation comparison helpers that write to stack buffers: + +```rust +fn disk_eq_f32(a: f32, b: f32) -> bool { + if a.to_bits() == b.to_bits() { return true; } + // format both into stack buffers, compare byte slices +} + +fn disk_eq_f32_cleaned(a: f32, b: f32) -> bool { + disk_eq_f32(cleanup_f32(a), cleanup_f32(b)) +} + +fn disk_eq_f64(a: f64, b: f64) -> bool { + if a.to_bits() == b.to_bits() { return true; } + // format both into stack buffers, compare byte slices +} +``` + +Then `pub fn variant_eq_disk(a: &Variant, b: &Variant) -> bool` that dispatches: + + +| Variant type | Comparison | +| ----------------------------------------------- | -------------------------------------------------- | +| Float32 | `disk_eq_f32` (no cleanup -- standalone) | +| Float64 | `disk_eq_f64` (no cleanup) | +| Vector3, Color3, CFrame components | `disk_eq_f32_cleaned` (cleanup -- composite) | +| UDim scale / UDim2 scales | `disk_eq_f32_cleaned` (cleanup) | +| NumberRange, Rect min/max | `disk_eq_f32_cleaned` (cleanup) | +| NumberSequence, ColorSequence keypoint floats | `disk_eq_f32_cleaned` (cleanup) | +| PhysicalProperties floats | `disk_eq_f32_cleaned` (cleanup) | +| Ray, Region3 components | `disk_eq_f32_cleaned` (cleanup) | +| Non-float types (Bool, Int, String, Enum, etc.) | Exact equality (no epsilon) | +| Tags | Sorted set comparison (same as now) | +| Attributes | Recursive with `variant_eq_disk` for nested values | + + +### 3. Update both matching files + +`**[src/snapshot/matching.rs](src/snapshot/matching.rs)`:** Replace all `variant_eq` calls in `count_own_diffs`, `diff_variant_pair`, and `count_attributes_diff` with `variant_eq_disk`. + +`**[src/syncback/matching.rs](src/syncback/matching.rs)`:** Same replacements in `count_own_diffs`, `diff_variant_pair`, and attribute comparison. + +### 4. Lua side -- no changes needed (verified) + +The plugin's cost model is correct as-is: + +- `trueEquals` is shared by both `matching.lua` (scoring) and `diff.lua` (patch generation), so they cannot disagree -- if matching says "equal", diff will also say "equal", and the change won't appear in the patch visualizer +- The fuzzy epsilon prevents Studio float-precision oscillation (set 0.5 -> Studio stores 0.49999999 -> reads back as ~0.5 -> trueEquals says "equal" -> no phantom re-diff) +- `matching.lua`'s `countOwnDiffs` fills defaults from `classKeys.defaults` for missing virtual properties, matching what diff/hydrate will do + +The Lua side's cost is correctly "will this change be flagged in the patch visualizer / applied to Studio." + +### 5. Testing Suite + +Existing `variant_eq` tests stay unchanged (they test the fuzzy function which is still used by patch_compute, api, etc.). All new tests below are additive. + +#### 5a. `variant_eq_disk` unit tests (in `src/variant_eq.rs`) + +**Fuzzy-vs-disk divergence tests** -- these are the cases that prove fuzzy is wrong for file-change detection: + +```rust +// Fuzzy says "equal" but disk would differ +assert!(fuzzy_eq_f32(1.00005, 1.00015)); // within epsilon +assert!(!disk_eq_f32(1.00005, 1.00015)); // format_f32 gives different strings + +// Fuzzy says "different" but disk would be identical +assert!(!fuzzy_eq_f32(0.1234561, 0.1234559)); // outside relative epsilon +assert!(disk_eq_f32(0.1234561, 0.1234559)); // format_f32 rounds both the same +``` + +**cleanup_f32 normalization tests** -- composite types get cleaned before formatting: + +```rust +// Very small values zeroed by cleanup +assert!(disk_eq_f32_cleaned(0.0000001, 0.0)); // cleanup makes both 0.0 +assert!(!disk_eq_f32(0.0000001, 0.0)); // without cleanup, different + +// Full Vector3 comparison +assert!(variant_eq_disk( + &Variant::Vector3(Vector3::new(1.0, 2.0, 0.0000001)), + &Variant::Vector3(Vector3::new(1.0, 2.0, 0.0)), +)); // cleanup zeros the tiny z component +``` + +**Edge cases:** + +- NaN == NaN (both format to "NaN") +- Inf == Inf (both format to "Infinity") +- -0.0 == 0.0 (both format to "0") +- All composite types: Vector2, Vector3, Color3, CFrame, UDim, UDim2, Rect, Ray, Region3, NumberRange, NumberSequence, ColorSequence, PhysicalProperties +- Non-float types: exact equality (Bool, Int32, Int64, String, Enum, BrickColor) +- Tags: sorted set comparison +- Attributes: recursive disk comparison of nested Variants + +#### 5b. Large ambiguous group matching -- forward sync (`src/snapshot/matching.rs`) + +**Test: `fifty_same_name_parts_five_variation_groups`** + +50 Parts all named `"Line"` with 5 variation groups of 10 exact duplicates each: + + +| Group | Position | Color | Count | +| ----- | ------------ | ------------------- | ----- | +| A | `[0, 0, 0]` | `[1, 0, 0]` (red) | 10 | +| B | `[0, 5, 0]` | `[1, 0, 0]` (red) | 10 | +| C | `[0, 10, 0]` | `[0, 1, 0]` (green) | 10 | +| D | `[0, 0, 0]` | `[0, 1, 0]` (green) | 10 | +| E | `[0, 5, 0]` | `[0, 0, 1]` (blue) | 10 | + + +- Groups A vs D: same position, different color (color-only diff) +- Groups A vs B: same color, different position (position-only diff) +- Groups B vs E: same position, different color +- Within each group: 10 exact duplicates (any pairing is correct) + +Build snapshots in order A,B,C,D,E. Build tree in **reversed** order E,D,C,B,A. + +**Assertions:** + +1. All 50 matched, 0 unmatched on either side +2. For every matched pair: `Position` values match (within disk equality) +3. For every matched pair: `Color` values match (within disk equality) +4. Completes in < 5 seconds (depth limit prevents exponential blowup) + +**Test: `fifty_parts_position_only_variations`** + +50 Parts named `"Block"` -- same Color on all, 5 distinct Position values (10 duplicates each). Tree reversed. Verify all 50 pair correctly by Position. + +**Test: `fifty_parts_color_only_variations`** + +50 Parts named `"Block"` -- same Position on all, 5 distinct Color values (10 duplicates each). Tree reversed. Verify all 50 pair correctly by Color. + +**Test: `sixty_parts_with_near_float_values`** + +60 Parts named `"Segment"` with Transparency values chosen at `format_f32` boundaries: + + +| Group | Transparency | Count | Notes | +| ----- | ------------ | ----- | ------------------------------------------------ | +| F | `0.100000` | 12 | disk repr: `"0.1"` | +| G | `0.100001` | 12 | disk repr: `"0.100001"` -- different from F | +| H | `0.500000` | 12 | disk repr: `"0.5"` | +| I | `0.500001` | 12 | disk repr differs from H | +| J | `0.999999` | 12 | disk repr rounds to same or different from `1.0` | + + +The exact Transparency values will be chosen by testing `format_f32` output during implementation to find values that: + +- Are within fuzzy epsilon of each other but format differently (must NOT cross-pair) +- Are outside fuzzy epsilon but format the same (must cross-pair) + +This test specifically catches the bug where fuzzy equality would match Group F with Group G instances, causing 12 unnecessary file rewrites. + +#### 5c. Large ambiguous group matching -- syncback (`src/syncback/matching.rs`) + +Same test matrix as 5b but using `WeakDom` + `InstanceBuilder`: + +**Test: `fifty_same_name_parts_five_groups_syncback`** + +Same 5 variation groups (A-E) of 10 Parts each. `new_dom` in order, `old_dom` reversed. + +```rust +let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); +let new_root = new_dom.root_ref(); +for (pos, color) in &groups { + for _ in 0..10 { + new_dom.insert(new_root, InstanceBuilder::new("Part") + .with_name("Line") + .with_property("Position", Variant::Vector3(*pos)) + .with_property("Color", Variant::Color3(*color))); + } +} +``` + +Assertions identical to 5b: all 50 paired, Position+Color match on every pair. + +**Test: `sixty_parts_near_float_syncback`** + +Same boundary test as 5b's `sixty_parts_with_near_float_values`, but in syncback context. + +#### 5d. Disk boundary stress test + +**Test: `disk_representation_boundary_matching`** (in both matching files) + +Explicitly construct instances where fuzzy and disk comparison disagree, and verify matching picks the disk-correct pairing: + +2 Parts named `"Edge"`: + +- Snap/New A: Transparency = `1.00005` (format_f32 = `"1.00005"`) +- Snap/New B: Transparency = `1.00015` (format_f32 = `"1.00015"`) + +Tree/Old (reversed): + +- Tree/Old X: Transparency = `1.00015` +- Tree/Old Y: Transparency = `1.00005` + +With fuzzy equality: both (A,X) and (A,Y) score 0 (within epsilon) -- random pairing, 50% chance of wrong. +With disk equality: (A,Y) scores 0 and (A,X) scores 1 -- deterministically correct. + +Verify A matches Y and B matches X. + +#### 5e. Update existing tests + +Existing matching tests that assert `variant_eq` behavior in matching contexts will continue to pass because: + +- Most use exact integer/enum values (not affected by fuzzy vs disk) +- Float tests use values that are either identical or far apart + +Tests that use float values at fuzzy epsilon boundaries may need value adjustments if the old assertion relied on fuzzy tolerance. Specifically review: + +- `many_same_name_parts_matched_by_properties` (uses `i * 0.1` increments -- safe, these format distinctly) +- `ten_ambiguous_textures_stress` (uses Enum values -- safe, not float) +- `variant_eq_float32_with_new_epsilon` (tests `variant_eq` not `variant_eq_disk` -- unchanged) + +No existing test should break, but verify during implementation. \ No newline at end of file diff --git a/src/json.rs b/src/json.rs index cf8e1af0..3abeccfd 100644 --- a/src/json.rs +++ b/src/json.rs @@ -153,7 +153,12 @@ const F64_FLOAT_OPTIONS: Options = Options::builder() const F32_BUF_SIZE: usize = F32_FLOAT_OPTIONS.buffer_size_const::(); const F64_BUF_SIZE: usize = F64_FLOAT_OPTIONS.buffer_size_const::(); -fn format_f32(v: f32) -> String { +pub(crate) const F32_DISK_OPTIONS: &Options = &F32_FLOAT_OPTIONS; +pub(crate) const F64_DISK_OPTIONS: &Options = &F64_FLOAT_OPTIONS; +pub(crate) const F32_DISK_BUF_SIZE: usize = F32_BUF_SIZE; +pub(crate) const F64_DISK_BUF_SIZE: usize = F64_BUF_SIZE; + +pub(crate) fn format_f32(v: f32) -> String { let mut buffer = [0u8; F32_BUF_SIZE]; let digits = v.to_lexical_with_options::(&mut buffer, &F32_FLOAT_OPTIONS); std::str::from_utf8(digits) @@ -161,7 +166,7 @@ fn format_f32(v: f32) -> String { .into() } -fn format_f64(v: f64) -> String { +pub(crate) fn format_f64(v: f64) -> String { let mut buffer = [0u8; F64_BUF_SIZE]; let digits = v.to_lexical_with_options::(&mut buffer, &F64_FLOAT_OPTIONS); std::str::from_utf8(digits) diff --git a/src/resolution.rs b/src/resolution.rs index bb590d47..56dcb517 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -417,7 +417,7 @@ impl AmbiguousValue { } } -fn cleanup_f32(v: f32) -> f32 { +pub(crate) fn cleanup_f32(v: f32) -> f32 { let cleaned = format!("{:.6}", v).parse::().unwrap_or(v); if cleaned == 0.0 { 0.0 diff --git a/src/snapshot/matching.rs b/src/snapshot/matching.rs index c8f1c484..4d81b232 100644 --- a/src/snapshot/matching.rs +++ b/src/snapshot/matching.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use rbx_dom_weak::types::{Ref, Variant}; use rbx_dom_weak::Ustr; -use crate::variant_eq::variant_eq; +use crate::variant_eq::variant_eq_disk; use super::{InstanceSnapshot, InstanceWithMeta, RojoTree}; @@ -467,7 +467,7 @@ fn count_own_diffs(snap: &InstanceSnapshot, inst: &InstanceWithMeta, tree: &Rojo } else { let is_default = class_data .and_then(|cd| cd.default_properties.get(key.as_str())) - .is_some_and(|default| variant_eq(snap_val, default)); + .is_some_and(|default| variant_eq_disk(snap_val, default)); if !is_default { cost += count_variant_one_sided(snap_val); } @@ -478,7 +478,7 @@ fn count_own_diffs(snap: &InstanceSnapshot, inst: &InstanceWithMeta, tree: &Rojo if !snap_props.contains_key(key) { let is_default = class_data .and_then(|cd| cd.default_properties.get(key.as_str())) - .is_some_and(|default| variant_eq(inst_val, default)); + .is_some_and(|default| variant_eq_disk(inst_val, default)); if !is_default { cost += count_variant_one_sided(inst_val); } @@ -531,7 +531,7 @@ fn diff_variant_pair(a: &Variant, b: &Variant, tree: &RojoTree) -> u32 { count_attributes_diff(attrs_a, attrs_b) } _ => { - if variant_eq(a, b) { + if variant_eq_disk(a, b) { 0 } else { 1 @@ -578,7 +578,7 @@ fn count_attributes_diff( for (key, a_val) in a.iter() { match b.get(key.as_str()) { Some(b_val) => { - if !variant_eq(a_val, b_val) { + if !variant_eq_disk(a_val, b_val) { cost += 1; } } @@ -597,6 +597,7 @@ fn count_attributes_diff( mod tests { use super::*; use crate::snapshot::{InstanceMetadata, InstanceSnapshot, RojoTree}; + use crate::variant_eq::variant_eq; use rbx_dom_weak::{ustr, HashMapExt as _}; use std::borrow::Cow; @@ -1587,4 +1588,262 @@ mod tests { ); } } + + // ================================================================ + // Large ambiguous group tests (50+ same-named instances) + // ================================================================ + + #[test] + fn fifty_same_name_parts_five_variation_groups() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let groups: Vec<(Vector3, Color3)> = vec![ + (Vector3::new(0.0, 0.0, 0.0), Color3::new(1.0, 0.0, 0.0)), + (Vector3::new(0.0, 5.0, 0.0), Color3::new(1.0, 0.0, 0.0)), + (Vector3::new(0.0, 10.0, 0.0), Color3::new(0.0, 1.0, 0.0)), + (Vector3::new(0.0, 0.0, 0.0), Color3::new(0.0, 1.0, 0.0)), + (Vector3::new(0.0, 5.0, 0.0), Color3::new(0.0, 0.0, 1.0)), + ]; + + let mut snaps = Vec::new(); + for (pos, color) in &groups { + for _ in 0..10 { + snaps.push(make_snapshot_with_props( + "Line", + "Part", + vec![ + ("Position", Variant::Vector3(*pos)), + ("Color", Variant::Color3(*color)), + ], + )); + } + } + + let mut tree_children_snaps: Vec = Vec::new(); + for (pos, color) in groups.iter().rev() { + for _ in 0..10 { + tree_children_snaps.push(make_snapshot_with_props( + "Line", + "Part", + vec![ + ("Position", Variant::Vector3(*pos)), + ("Color", Variant::Color3(*color)), + ], + )); + } + } + + let (tree, tree_refs) = make_tree_from_snapshots(tree_children_snaps); + + let start = std::time::Instant::now(); + let result = match_forward(snaps, &tree_refs, &tree, &MatchingSession::new()); + let elapsed = start.elapsed(); + + assert_eq!(result.matched.len(), 50); + assert!(result.unmatched_snapshot.is_empty()); + assert!(result.unmatched_tree.is_empty()); + assert!(elapsed.as_secs() < 5, "took too long: {:?}", elapsed); + + for (snap, tree_ref) in &result.matched { + let inst = tree.get_instance(*tree_ref).unwrap(); + let snap_pos = snap.properties.get(&ustr("Position")).unwrap(); + let tree_pos = inst.properties().get(&ustr("Position")).unwrap(); + assert!( + variant_eq_disk(snap_pos, tree_pos), + "Position mismatch: snap={:?}, tree={:?}", + snap_pos, + tree_pos + ); + let snap_color = snap.properties.get(&ustr("Color")).unwrap(); + let tree_color = inst.properties().get(&ustr("Color")).unwrap(); + assert!( + variant_eq_disk(snap_color, tree_color), + "Color mismatch: snap={:?}, tree={:?}", + snap_color, + tree_color + ); + } + } + + #[test] + fn fifty_parts_position_only_variations() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let shared_color = Color3::new(0.5, 0.5, 0.5); + let positions = [ + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(10.0, 0.0, 0.0), + Vector3::new(0.0, 10.0, 0.0), + Vector3::new(0.0, 0.0, 10.0), + Vector3::new(5.0, 5.0, 5.0), + ]; + + let mut snaps = Vec::new(); + let mut tree_children_snaps = Vec::new(); + for pos in &positions { + for _ in 0..10 { + let props = vec![ + ("Position", Variant::Vector3(*pos)), + ("Color", Variant::Color3(shared_color)), + ]; + snaps.push(make_snapshot_with_props("Block", "Part", props.clone())); + tree_children_snaps.push(make_snapshot_with_props("Block", "Part", props)); + } + } + tree_children_snaps.reverse(); + + let (tree, tree_refs) = make_tree_from_snapshots(tree_children_snaps); + let result = match_forward(snaps, &tree_refs, &tree, &MatchingSession::new()); + + assert_eq!(result.matched.len(), 50); + assert!(result.unmatched_snapshot.is_empty()); + assert!(result.unmatched_tree.is_empty()); + + for (snap, tree_ref) in &result.matched { + let inst = tree.get_instance(*tree_ref).unwrap(); + let snap_pos = snap.properties.get(&ustr("Position")).unwrap(); + let tree_pos = inst.properties().get(&ustr("Position")).unwrap(); + assert!(variant_eq_disk(snap_pos, tree_pos), "Position mismatch"); + } + } + + #[test] + fn fifty_parts_color_only_variations() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let shared_pos = Vector3::new(0.0, 0.0, 0.0); + let colors = [ + Color3::new(1.0, 0.0, 0.0), + Color3::new(0.0, 1.0, 0.0), + Color3::new(0.0, 0.0, 1.0), + Color3::new(1.0, 1.0, 0.0), + Color3::new(0.0, 1.0, 1.0), + ]; + + let mut snaps = Vec::new(); + let mut tree_children_snaps = Vec::new(); + for color in &colors { + for _ in 0..10 { + let props = vec![ + ("Position", Variant::Vector3(shared_pos)), + ("Color", Variant::Color3(*color)), + ]; + snaps.push(make_snapshot_with_props("Block", "Part", props.clone())); + tree_children_snaps.push(make_snapshot_with_props("Block", "Part", props)); + } + } + tree_children_snaps.reverse(); + + let (tree, tree_refs) = make_tree_from_snapshots(tree_children_snaps); + let result = match_forward(snaps, &tree_refs, &tree, &MatchingSession::new()); + + assert_eq!(result.matched.len(), 50); + assert!(result.unmatched_snapshot.is_empty()); + assert!(result.unmatched_tree.is_empty()); + + for (snap, tree_ref) in &result.matched { + let inst = tree.get_instance(*tree_ref).unwrap(); + let snap_color = snap.properties.get(&ustr("Color")).unwrap(); + let tree_color = inst.properties().get(&ustr("Color")).unwrap(); + assert!(variant_eq_disk(snap_color, tree_color), "Color mismatch"); + } + } + + #[test] + fn sixty_parts_with_near_float_values() { + use rbx_dom_weak::types::Variant; + + let groups: Vec = vec![0.1, 0.100001, 0.5, 0.500001, 0.999999]; + let group_size = 12; + + let mut snaps = Vec::new(); + let mut tree_children_snaps = Vec::new(); + for &t in &groups { + for _ in 0..group_size { + snaps.push(make_snapshot_with_props( + "Segment", + "Part", + vec![("Transparency", Variant::Float32(t))], + )); + tree_children_snaps.push(make_snapshot_with_props( + "Segment", + "Part", + vec![("Transparency", Variant::Float32(t))], + )); + } + } + tree_children_snaps.reverse(); + + let (tree, tree_refs) = make_tree_from_snapshots(tree_children_snaps); + let result = match_forward(snaps, &tree_refs, &tree, &MatchingSession::new()); + + assert_eq!(result.matched.len(), 60); + assert!(result.unmatched_snapshot.is_empty()); + assert!(result.unmatched_tree.is_empty()); + + for (snap, tree_ref) in &result.matched { + let inst = tree.get_instance(*tree_ref).unwrap(); + let snap_t = snap.properties.get(&ustr("Transparency")).unwrap(); + let tree_t = inst.properties().get(&ustr("Transparency")).unwrap(); + assert!( + variant_eq_disk(snap_t, tree_t), + "Transparency mismatch: snap={:?}, tree={:?}", + snap_t, + tree_t + ); + } + } + + #[test] + fn disk_representation_boundary_matching() { + use rbx_dom_weak::types::Variant; + + let a_val: f32 = 1.00005; + let b_val: f32 = 1.00015; + + let snaps = vec![ + make_snapshot_with_props( + "Edge", + "Part", + vec![("Transparency", Variant::Float32(a_val))], + ), + make_snapshot_with_props( + "Edge", + "Part", + vec![("Transparency", Variant::Float32(b_val))], + ), + ]; + + let tree_children = vec![ + make_snapshot_with_props( + "Edge", + "Part", + vec![("Transparency", Variant::Float32(b_val))], + ), + make_snapshot_with_props( + "Edge", + "Part", + vec![("Transparency", Variant::Float32(a_val))], + ), + ]; + + let (tree, tree_refs) = make_tree_from_snapshots(tree_children); + let result = match_forward(snaps, &tree_refs, &tree, &MatchingSession::new()); + + assert_eq!(result.matched.len(), 2); + assert!(result.unmatched_snapshot.is_empty()); + assert!(result.unmatched_tree.is_empty()); + + for (snap, tree_ref) in &result.matched { + let inst = tree.get_instance(*tree_ref).unwrap(); + let snap_t = snap.properties.get(&ustr("Transparency")).unwrap(); + let tree_t = inst.properties().get(&ustr("Transparency")).unwrap(); + assert!( + variant_eq_disk(snap_t, tree_t), + "Boundary test failed: snap={:?}, tree={:?} (disk should pair exactly)", + snap_t, + tree_t + ); + } + } } diff --git a/src/syncback/matching.rs b/src/syncback/matching.rs index eb292aa1..14cde11c 100644 --- a/src/syncback/matching.rs +++ b/src/syncback/matching.rs @@ -18,7 +18,7 @@ use blake3::Hash; use rbx_dom_weak::types::{Ref, Variant}; use rbx_dom_weak::{Ustr, WeakDom}; -use crate::variant_eq::variant_eq; +use crate::variant_eq::variant_eq_disk; const UNMATCHED_PENALTY: u32 = 10_000; @@ -522,7 +522,7 @@ fn count_own_diffs(new_ref: Ref, old_ref: Ref, new_dom: &WeakDom, old_dom: &Weak } else { let is_default = class_data .and_then(|cd| cd.default_properties.get(key.as_str())) - .is_some_and(|default| variant_eq(new_val, default)); + .is_some_and(|default| variant_eq_disk(new_val, default)); if !is_default { cost += count_variant_one_sided(new_val); } @@ -533,7 +533,7 @@ fn count_own_diffs(new_ref: Ref, old_ref: Ref, new_dom: &WeakDom, old_dom: &Weak if !new_inst.properties.contains_key(key) { let is_default = class_data .and_then(|cd| cd.default_properties.get(key.as_str())) - .is_some_and(|default| variant_eq(old_val, default)); + .is_some_and(|default| variant_eq_disk(old_val, default)); if !is_default { cost += count_variant_one_sided(old_val); } @@ -589,7 +589,7 @@ fn diff_variant_pair(a: &Variant, b: &Variant, a_dom: &WeakDom, b_dom: &WeakDom) for (key, a_val) in attrs_a.iter() { match attrs_b.get(key.as_str()) { Some(b_val) => { - if !variant_eq(a_val, b_val) { + if !variant_eq_disk(a_val, b_val) { cost += 1; } } @@ -604,7 +604,7 @@ fn diff_variant_pair(a: &Variant, b: &Variant, a_dom: &WeakDom, b_dom: &WeakDom) cost } _ => { - if variant_eq(a, b) { + if variant_eq_disk(a, b) { 0 } else { 1 @@ -633,7 +633,7 @@ fn count_variant_one_sided(val: &Variant) -> u32 { #[cfg(test)] mod tests { use super::*; - use rbx_dom_weak::InstanceBuilder; + use rbx_dom_weak::{ustr, InstanceBuilder}; fn build_test_dom() -> (WeakDom, Ref, Ref, Ref) { let mut dom = WeakDom::new(InstanceBuilder::new("DataModel")); @@ -956,4 +956,376 @@ mod tests { ); } } + + // ================================================================ + // Large ambiguous group tests (50+ same-named instances) + // ================================================================ + + #[test] + fn fifty_same_name_parts_five_groups_syncback() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let groups: Vec<(Vector3, Color3)> = vec![ + (Vector3::new(0.0, 0.0, 0.0), Color3::new(1.0, 0.0, 0.0)), + (Vector3::new(0.0, 5.0, 0.0), Color3::new(1.0, 0.0, 0.0)), + (Vector3::new(0.0, 10.0, 0.0), Color3::new(0.0, 1.0, 0.0)), + (Vector3::new(0.0, 0.0, 0.0), Color3::new(0.0, 1.0, 0.0)), + (Vector3::new(0.0, 5.0, 0.0), Color3::new(0.0, 0.0, 1.0)), + ]; + + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + for (pos, color) in &groups { + for _ in 0..10 { + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("Line") + .with_property("Position", Variant::Vector3(*pos)) + .with_property("Color", Variant::Color3(*color)), + ); + } + } + + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + for (pos, color) in groups.iter().rev() { + for _ in 0..10 { + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("Line") + .with_property("Position", Variant::Vector3(*pos)) + .with_property("Color", Variant::Color3(*color)), + ); + } + } + + 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 start = std::time::Instant::now(); + let result = match_children( + &new_children, + &old_children, + &new_dom, + &old_dom, + None, + None, + &MatchingSession::new(), + ); + let elapsed = start.elapsed(); + + assert_eq!(result.matched.len(), 50); + assert!(result.unmatched_new.is_empty()); + assert!(result.unmatched_old.is_empty()); + assert!(elapsed.as_secs() < 5, "took too long: {:?}", elapsed); + + for (new_ref, old_ref) in &result.matched { + let new_inst = new_dom.get_by_ref(*new_ref).unwrap(); + let old_inst = old_dom.get_by_ref(*old_ref).unwrap(); + + let new_pos = new_inst.properties.get(&ustr("Position")).unwrap(); + let old_pos = old_inst.properties.get(&ustr("Position")).unwrap(); + assert!( + variant_eq_disk(new_pos, old_pos), + "Position mismatch: new={:?}, old={:?}", + new_pos, + old_pos + ); + let new_color = new_inst.properties.get(&ustr("Color")).unwrap(); + let old_color = old_inst.properties.get(&ustr("Color")).unwrap(); + assert!( + variant_eq_disk(new_color, old_color), + "Color mismatch: new={:?}, old={:?}", + new_color, + old_color + ); + } + } + + #[test] + fn fifty_parts_position_only_syncback() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let shared_color = Color3::new(0.5, 0.5, 0.5); + let positions = [ + Vector3::new(0.0, 0.0, 0.0), + Vector3::new(10.0, 0.0, 0.0), + Vector3::new(0.0, 10.0, 0.0), + Vector3::new(0.0, 0.0, 10.0), + Vector3::new(5.0, 5.0, 5.0), + ]; + + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + + for pos in &positions { + for _ in 0..10 { + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("Block") + .with_property("Position", Variant::Vector3(*pos)) + .with_property("Color", Variant::Color3(shared_color)), + ); + } + } + for pos in positions.iter().rev() { + for _ in 0..10 { + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("Block") + .with_property("Position", Variant::Vector3(*pos)) + .with_property("Color", Variant::Color3(shared_color)), + ); + } + } + + 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(), 50); + for (new_ref, old_ref) in &result.matched { + let new_pos = new_dom + .get_by_ref(*new_ref) + .unwrap() + .properties + .get(&ustr("Position")) + .unwrap(); + let old_pos = old_dom + .get_by_ref(*old_ref) + .unwrap() + .properties + .get(&ustr("Position")) + .unwrap(); + assert!(variant_eq_disk(new_pos, old_pos), "Position mismatch"); + } + } + + #[test] + fn fifty_parts_color_only_syncback() { + use rbx_dom_weak::types::{Color3, Variant, Vector3}; + + let shared_pos = Vector3::new(0.0, 0.0, 0.0); + let colors = [ + Color3::new(1.0, 0.0, 0.0), + Color3::new(0.0, 1.0, 0.0), + Color3::new(0.0, 0.0, 1.0), + Color3::new(1.0, 1.0, 0.0), + Color3::new(0.0, 1.0, 1.0), + ]; + + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + + for color in &colors { + for _ in 0..10 { + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("Block") + .with_property("Position", Variant::Vector3(shared_pos)) + .with_property("Color", Variant::Color3(*color)), + ); + } + } + for color in colors.iter().rev() { + for _ in 0..10 { + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("Block") + .with_property("Position", Variant::Vector3(shared_pos)) + .with_property("Color", Variant::Color3(*color)), + ); + } + } + + 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(), 50); + for (new_ref, old_ref) in &result.matched { + let new_color = new_dom + .get_by_ref(*new_ref) + .unwrap() + .properties + .get(&ustr("Color")) + .unwrap(); + let old_color = old_dom + .get_by_ref(*old_ref) + .unwrap() + .properties + .get(&ustr("Color")) + .unwrap(); + assert!(variant_eq_disk(new_color, old_color), "Color mismatch"); + } + } + + #[test] + fn sixty_parts_near_float_syncback() { + use rbx_dom_weak::types::Variant; + + let groups: Vec = vec![0.1, 0.100001, 0.5, 0.500001, 0.999999]; + let group_size = 12; + + let mut new_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let new_root = new_dom.root_ref(); + let mut old_dom = WeakDom::new(InstanceBuilder::new("DataModel")); + let old_root = old_dom.root_ref(); + + for &t in &groups { + for _ in 0..group_size { + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("Segment") + .with_property("Transparency", Variant::Float32(t)), + ); + } + } + for &t in groups.iter().rev() { + for _ in 0..group_size { + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("Segment") + .with_property("Transparency", Variant::Float32(t)), + ); + } + } + + 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(), 60); + assert!(result.unmatched_new.is_empty()); + assert!(result.unmatched_old.is_empty()); + + for (new_ref, old_ref) in &result.matched { + let new_t = new_dom + .get_by_ref(*new_ref) + .unwrap() + .properties + .get(&ustr("Transparency")) + .unwrap(); + let old_t = old_dom + .get_by_ref(*old_ref) + .unwrap() + .properties + .get(&ustr("Transparency")) + .unwrap(); + assert!( + variant_eq_disk(new_t, old_t), + "Transparency mismatch: new={:?}, old={:?}", + new_t, + old_t + ); + } + } + + #[test] + fn disk_representation_boundary_syncback() { + use rbx_dom_weak::types::Variant; + + let a_val: f32 = 1.00005; + let b_val: f32 = 1.00015; + + 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("Edge") + .with_property("Transparency", Variant::Float32(a_val)), + ); + new_dom.insert( + new_root, + InstanceBuilder::new("Part") + .with_name("Edge") + .with_property("Transparency", Variant::Float32(b_val)), + ); + + 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("Edge") + .with_property("Transparency", Variant::Float32(b_val)), + ); + old_dom.insert( + old_root, + InstanceBuilder::new("Part") + .with_name("Edge") + .with_property("Transparency", Variant::Float32(a_val)), + ); + + 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); + for (new_ref, old_ref) in &result.matched { + let new_t = new_dom + .get_by_ref(*new_ref) + .unwrap() + .properties + .get(&ustr("Transparency")) + .unwrap(); + let old_t = old_dom + .get_by_ref(*old_ref) + .unwrap() + .properties + .get(&ustr("Transparency")) + .unwrap(); + assert!( + variant_eq_disk(new_t, old_t), + "Boundary test failed: new={:?}, old={:?}", + new_t, + old_t + ); + } + } } diff --git a/src/variant_eq.rs b/src/variant_eq.rs index e506d28d..d6e6f852 100644 --- a/src/variant_eq.rs +++ b/src/variant_eq.rs @@ -1,5 +1,9 @@ +use lexical_write_float::{format::STANDARD, ToLexicalWithOptions}; use rbx_dom_weak::types::{PhysicalProperties, Variant, Vector3}; +use crate::json::{F32_DISK_BUF_SIZE, F32_DISK_OPTIONS, F64_DISK_BUF_SIZE, F64_DISK_OPTIONS}; +use crate::resolution::cleanup_f32; + const EPSILON_F32: f32 = 0.0001; const EPSILON_F64: f64 = 0.0001; @@ -213,6 +217,228 @@ fn vector_eq(a: &Vector3, b: &Vector3) -> bool { fuzzy_eq_f32(a.x, b.x) && fuzzy_eq_f32(a.y, b.y) && fuzzy_eq_f32(a.z, b.z) } +// ============================================================================ +// Disk-representation equality: "will these two values produce identical bytes +// when written to a .meta.json5 / .model.json5 file?" +// +// Used by the matching algorithms in src/snapshot/matching.rs and +// src/syncback/matching.rs where the cost metric is file modifications. +// ============================================================================ + +#[inline(always)] +fn disk_eq_f32(a: f32, b: f32) -> bool { + let a = if a == 0.0 { 0.0_f32 } else { a }; + let b = if b == 0.0 { 0.0_f32 } else { b }; + if a.to_bits() == b.to_bits() { + return true; + } + let mut buf_a = [0u8; F32_DISK_BUF_SIZE]; + let mut buf_b = [0u8; F32_DISK_BUF_SIZE]; + let repr_a = a.to_lexical_with_options::(&mut buf_a, F32_DISK_OPTIONS); + let repr_b = b.to_lexical_with_options::(&mut buf_b, F32_DISK_OPTIONS); + repr_a == repr_b +} + +#[inline(always)] +fn disk_eq_f32_cleaned(a: f32, b: f32) -> bool { + disk_eq_f32(cleanup_f32(a), cleanup_f32(b)) +} + +#[inline(always)] +fn disk_eq_f64(a: f64, b: f64) -> bool { + let a = if a == 0.0 { 0.0_f64 } else { a }; + let b = if b == 0.0 { 0.0_f64 } else { b }; + if a.to_bits() == b.to_bits() { + return true; + } + let mut buf_a = [0u8; F64_DISK_BUF_SIZE]; + let mut buf_b = [0u8; F64_DISK_BUF_SIZE]; + let repr_a = a.to_lexical_with_options::(&mut buf_a, F64_DISK_OPTIONS); + let repr_b = b.to_lexical_with_options::(&mut buf_b, F64_DISK_OPTIONS); + repr_a == repr_b +} + +#[inline(always)] +fn vector_eq_disk(a: &Vector3, b: &Vector3) -> bool { + disk_eq_f32_cleaned(a.x, b.x) && disk_eq_f32_cleaned(a.y, b.y) && disk_eq_f32_cleaned(a.z, b.z) +} + +/// Compares two variants by their on-disk representation. Two values are +/// "equal" iff serializing both through resolution.rs + json.rs produces +/// identical bytes in the output file. Used by matching algorithms where +/// cost = number of file modifications. +#[inline] +pub fn variant_eq_disk(variant_a: &Variant, variant_b: &Variant) -> bool { + if variant_a.ty() != variant_b.ty() { + return false; + } + + match (variant_a, variant_b) { + (Variant::Attributes(a), Variant::Attributes(b)) => { + if a.len() != b.len() { + return false; + } + for ((a_name, a_value), (b_name, b_value)) in a.iter().zip(b.iter()) { + if !(a_name == b_name && variant_eq_disk(a_value, b_value)) { + return false; + } + } + true + } + (Variant::Axes(a), Variant::Axes(b)) => a == b, + (Variant::BinaryString(a), Variant::BinaryString(b)) => a == b, + (Variant::Bool(a), Variant::Bool(b)) => a == b, + (Variant::BrickColor(a), Variant::BrickColor(b)) => a == b, + (Variant::CFrame(a), Variant::CFrame(b)) => { + vector_eq_disk(&a.position, &b.position) + && vector_eq_disk(&a.orientation.x, &b.orientation.x) + && vector_eq_disk(&a.orientation.y, &b.orientation.y) + && vector_eq_disk(&a.orientation.z, &b.orientation.z) + } + (Variant::Color3(a), Variant::Color3(b)) => { + disk_eq_f32_cleaned(a.r, b.r) + && disk_eq_f32_cleaned(a.g, b.g) + && disk_eq_f32_cleaned(a.b, b.b) + } + (Variant::Color3uint8(a), Variant::Color3uint8(b)) => a == b, + (Variant::ColorSequence(a), Variant::ColorSequence(b)) => { + if a.keypoints.len() != b.keypoints.len() { + return false; + } + let mut a_keypoints: Vec<_> = a.keypoints.iter().collect(); + let mut b_keypoints: Vec<_> = b.keypoints.iter().collect(); + a_keypoints.sort_unstable_by(|k1, k2| k1.time.partial_cmp(&k2.time).unwrap()); + b_keypoints.sort_unstable_by(|k1, k2| k1.time.partial_cmp(&k2.time).unwrap()); + + for (a_kp, b_kp) in a_keypoints.iter().zip(b_keypoints) { + if !(disk_eq_f32_cleaned(a_kp.time, b_kp.time) + && disk_eq_f32_cleaned(a_kp.color.r, b_kp.color.r) + && disk_eq_f32_cleaned(a_kp.color.g, b_kp.color.g) + && disk_eq_f32_cleaned(a_kp.color.b, b_kp.color.b)) + { + return false; + } + } + true + } + (Variant::Content(a), Variant::Content(b)) => a == b, + (Variant::ContentId(a), Variant::ContentId(b)) => a == b, + (Variant::Enum(a), Variant::Enum(b)) => a == b, + (Variant::EnumItem(a), Variant::EnumItem(b)) => a == b, + (Variant::Faces(a), Variant::Faces(b)) => a == b, + (Variant::Float32(a), Variant::Float32(b)) => disk_eq_f32(*a, *b), + (Variant::Float64(a), Variant::Float64(b)) => disk_eq_f64(*a, *b), + (Variant::Font(a), Variant::Font(b)) => { + a.weight == b.weight + && a.style == b.style + && a.family == b.family + && a.cached_face_id == b.cached_face_id + } + (Variant::Int32(a), Variant::Int32(b)) => a == b, + (Variant::Int64(a), Variant::Int64(b)) => a == b, + (Variant::MaterialColors(a), Variant::MaterialColors(b)) => a.encode() == b.encode(), + (Variant::NetAssetRef(a), Variant::NetAssetRef(b)) => a == b, + (Variant::NumberRange(a), Variant::NumberRange(b)) => { + disk_eq_f32_cleaned(a.max, b.max) && disk_eq_f32_cleaned(a.min, b.min) + } + (Variant::NumberSequence(a), Variant::NumberSequence(b)) => { + if a.keypoints.len() != b.keypoints.len() { + return false; + } + let mut a_keypoints: Vec<_> = a.keypoints.iter().collect(); + let mut b_keypoints: Vec<_> = b.keypoints.iter().collect(); + a_keypoints.sort_unstable_by(|k1, k2| k1.time.partial_cmp(&k2.time).unwrap()); + b_keypoints.sort_unstable_by(|k1, k2| k1.time.partial_cmp(&k2.time).unwrap()); + + for (a_kp, b_kp) in a_keypoints.iter().zip(b_keypoints) { + if !(disk_eq_f32_cleaned(a_kp.time, b_kp.time) + && disk_eq_f32_cleaned(a_kp.value, b_kp.value) + && disk_eq_f32_cleaned(a_kp.envelope, b_kp.envelope)) + { + return false; + } + } + true + } + (Variant::OptionalCFrame(a), Variant::OptionalCFrame(b)) => match (a, b) { + (Some(a), Some(b)) => { + vector_eq_disk(&a.position, &b.position) + && vector_eq_disk(&a.orientation.x, &b.orientation.x) + && vector_eq_disk(&a.orientation.y, &b.orientation.y) + && vector_eq_disk(&a.orientation.z, &b.orientation.z) + } + (None, None) => true, + _ => false, + }, + (Variant::PhysicalProperties(a), Variant::PhysicalProperties(b)) => match (a, b) { + (PhysicalProperties::Default, PhysicalProperties::Default) => true, + (PhysicalProperties::Custom(a2), PhysicalProperties::Custom(b2)) => { + disk_eq_f32_cleaned(a2.density(), b2.density()) + && disk_eq_f32_cleaned(a2.elasticity(), b2.elasticity()) + && disk_eq_f32_cleaned(a2.friction(), b2.friction()) + && disk_eq_f32_cleaned(a2.elasticity_weight(), b2.elasticity_weight()) + && disk_eq_f32_cleaned(a2.friction_weight(), b2.friction_weight()) + && disk_eq_f32_cleaned(a2.acoustic_absorption(), b2.acoustic_absorption()) + } + _ => false, + }, + (Variant::Ray(a), Variant::Ray(b)) => { + vector_eq_disk(&a.direction, &b.direction) && vector_eq_disk(&a.origin, &b.origin) + } + (Variant::Rect(a), Variant::Rect(b)) => { + disk_eq_f32_cleaned(a.max.x, b.max.x) + && disk_eq_f32_cleaned(a.max.y, b.max.y) + && disk_eq_f32_cleaned(a.min.x, b.min.x) + && disk_eq_f32_cleaned(a.min.y, b.min.y) + } + (Variant::Ref(a), Variant::Ref(b)) => a == b, + (Variant::Region3(a), Variant::Region3(b)) => { + vector_eq_disk(&a.max, &b.max) && vector_eq_disk(&a.min, &b.min) + } + (Variant::Region3int16(a), Variant::Region3int16(b)) => a == b, + (Variant::SecurityCapabilities(a), Variant::SecurityCapabilities(b)) => a == b, + (Variant::SharedString(a), Variant::SharedString(b)) => a == b, + (Variant::Tags(a), Variant::Tags(b)) => { + let mut a_sorted: Vec<&str> = a.iter().collect(); + let mut b_sorted: Vec<&str> = b.iter().collect(); + if a_sorted.len() == b_sorted.len() { + a_sorted.sort_unstable(); + b_sorted.sort_unstable(); + for (a_tag, b_tag) in a_sorted.into_iter().zip(b_sorted) { + if a_tag != b_tag { + return false; + } + } + true + } else { + false + } + } + (Variant::UDim(a), Variant::UDim(b)) => { + disk_eq_f32_cleaned(a.scale, b.scale) && a.offset == b.offset + } + (Variant::UDim2(a), Variant::UDim2(b)) => { + disk_eq_f32_cleaned(a.x.scale, b.x.scale) + && a.x.offset == b.x.offset + && disk_eq_f32_cleaned(a.y.scale, b.y.scale) + && a.y.offset == b.y.offset + } + (Variant::UniqueId(a), Variant::UniqueId(b)) => a == b, + (Variant::String(a), Variant::String(b)) => a == b, + (Variant::Vector2(a), Variant::Vector2(b)) => { + disk_eq_f32_cleaned(a.x, b.x) && disk_eq_f32_cleaned(a.y, b.y) + } + (Variant::Vector2int16(a), Variant::Vector2int16(b)) => a == b, + (Variant::Vector3(a), Variant::Vector3(b)) => vector_eq_disk(a, b), + (Variant::Vector3int16(a), Variant::Vector3int16(b)) => a == b, + (a, b) => panic!( + "unsupported variant disk comparison: {:?} and {:?}", + a.ty(), + b.ty() + ), + } +} + #[cfg(test)] mod tests { use super::*; @@ -259,4 +485,245 @@ mod tests { &Variant::Float32(1.0 + 0.00011) )); } + + // ================================================================ + // disk_eq / variant_eq_disk tests + // ================================================================ + + #[test] + fn disk_eq_f32_identical_bits() { + assert!(disk_eq_f32(1.0, 1.0)); + assert!(disk_eq_f32(0.0, 0.0)); + assert!(disk_eq_f32(-1.5, -1.5)); + } + + #[test] + fn disk_eq_f32_negative_zero() { + assert!(disk_eq_f32(0.0, -0.0)); + } + + #[test] + fn disk_eq_f32_nan() { + assert!(disk_eq_f32(f32::NAN, f32::NAN)); + assert!(!disk_eq_f32(f32::NAN, 0.0)); + assert!(!disk_eq_f32(0.0, f32::NAN)); + } + + #[test] + fn disk_eq_f32_infinity() { + assert!(disk_eq_f32(f32::INFINITY, f32::INFINITY)); + assert!(disk_eq_f32(f32::NEG_INFINITY, f32::NEG_INFINITY)); + assert!(!disk_eq_f32(f32::INFINITY, f32::NEG_INFINITY)); + assert!(!disk_eq_f32(f32::INFINITY, 0.0)); + } + + #[test] + fn disk_eq_f32_fuzzy_says_equal_but_disk_differs() { + let a: f32 = 10000.0; + let b: f32 = 10000.5; + assert!( + fuzzy_eq_f32(a, b), + "precondition: fuzzy should say equal (diff=0.5, relative threshold=1.0)" + ); + assert!( + !disk_eq_f32(a, b), + "disk representations differ: 6 sig digits gives '10000' vs '10000.5'" + ); + } + + #[test] + fn disk_eq_f64_basic() { + assert!(disk_eq_f64(1.0, 1.0)); + assert!(disk_eq_f64(0.0, -0.0)); + assert!(disk_eq_f64(f64::NAN, f64::NAN)); + assert!(!disk_eq_f64(1.0, 2.0)); + } + + #[test] + fn disk_eq_f32_cleaned_tiny_values_zeroed() { + assert!( + disk_eq_f32_cleaned(0.0000001, 0.0), + "cleanup_f32 zeros 1e-7 to 0.0" + ); + assert!( + !disk_eq_f32(0.0000001, 0.0), + "without cleanup, 1e-7 and 0 are different on disk" + ); + } + + #[test] + fn disk_eq_f32_cleaned_preserves_significant_values() { + assert!(!disk_eq_f32_cleaned(0.5, 0.6)); + assert!(disk_eq_f32_cleaned(0.5, 0.5)); + } + + #[test] + fn variant_eq_disk_vector3_tiny_component() { + assert!(variant_eq_disk( + &Variant::Vector3(Vector3::new(1.0, 2.0, 0.0000001)), + &Variant::Vector3(Vector3::new(1.0, 2.0, 0.0)), + )); + } + + #[test] + fn variant_eq_disk_vector3_different() { + assert!(!variant_eq_disk( + &Variant::Vector3(Vector3::new(1.0, 2.0, 3.0)), + &Variant::Vector3(Vector3::new(1.0, 2.0, 4.0)), + )); + } + + #[test] + fn variant_eq_disk_color3() { + use rbx_dom_weak::types::Color3; + assert!(variant_eq_disk( + &Variant::Color3(Color3::new(1.0, 0.0, 0.0)), + &Variant::Color3(Color3::new(1.0, 0.0, 0.0)), + )); + assert!(!variant_eq_disk( + &Variant::Color3(Color3::new(1.0, 0.0, 0.0)), + &Variant::Color3(Color3::new(0.0, 1.0, 0.0)), + )); + } + + #[test] + fn variant_eq_disk_float32_standalone() { + assert!(variant_eq_disk( + &Variant::Float32(0.5), + &Variant::Float32(0.5) + )); + assert!(!variant_eq_disk( + &Variant::Float32(0.5), + &Variant::Float32(0.6) + )); + } + + #[test] + fn variant_eq_disk_float32_diverges_from_fuzzy() { + let a: f32 = 10000.0; + let b: f32 = 10000.5; + assert!( + variant_eq(&Variant::Float32(a), &Variant::Float32(b)), + "fuzzy says equal (diff=0.5, relative threshold=1.0)" + ); + assert!( + !variant_eq_disk(&Variant::Float32(a), &Variant::Float32(b)), + "disk says different ('10000' vs '10000.5')" + ); + } + + #[test] + fn variant_eq_disk_non_float_types() { + assert!(variant_eq_disk(&Variant::Bool(true), &Variant::Bool(true))); + assert!(!variant_eq_disk( + &Variant::Bool(true), + &Variant::Bool(false) + )); + assert!(variant_eq_disk(&Variant::Int32(42), &Variant::Int32(42))); + assert!(!variant_eq_disk(&Variant::Int32(42), &Variant::Int32(43))); + assert!(variant_eq_disk( + &Variant::String("hello".into()), + &Variant::String("hello".into()), + )); + assert!(!variant_eq_disk( + &Variant::String("hello".into()), + &Variant::String("world".into()), + )); + } + + #[test] + fn variant_eq_disk_udim2_scale_cleaned() { + use rbx_dom_weak::types::{UDim, UDim2}; + assert!(variant_eq_disk( + &Variant::UDim2(UDim2::new(UDim::new(0.5, 0), UDim::new(0.0000001, 100))), + &Variant::UDim2(UDim2::new(UDim::new(0.5, 0), UDim::new(0.0, 100))), + )); + } + + #[test] + fn variant_eq_disk_number_range() { + use rbx_dom_weak::types::NumberRange; + assert!(variant_eq_disk( + &Variant::NumberRange(NumberRange::new(0.0, 1.0)), + &Variant::NumberRange(NumberRange::new(0.0, 1.0)), + )); + assert!(!variant_eq_disk( + &Variant::NumberRange(NumberRange::new(0.0, 1.0)), + &Variant::NumberRange(NumberRange::new(0.0, 2.0)), + )); + } + + #[test] + fn variant_eq_disk_tags_sorted() { + use rbx_dom_weak::types::Tags; + let mut tags_ab = Tags::new(); + tags_ab.push("A"); + tags_ab.push("B"); + let mut tags_ba = Tags::new(); + tags_ba.push("B"); + tags_ba.push("A"); + assert!(variant_eq_disk( + &Variant::Tags(tags_ab), + &Variant::Tags(tags_ba) + )); + } + + #[test] + fn variant_eq_disk_attributes_recursive() { + use rbx_dom_weak::types::Attributes; + let mut a = Attributes::new(); + a.insert("Health".into(), Variant::Float32(100.0)); + let mut b = Attributes::new(); + b.insert("Health".into(), Variant::Float32(100.0)); + assert!(variant_eq_disk( + &Variant::Attributes(a), + &Variant::Attributes(b), + )); + } + + #[test] + fn variant_eq_disk_cframe() { + use rbx_dom_weak::types::{CFrame, Matrix3}; + let identity = Matrix3::new( + Vector3::new(1.0, 0.0, 0.0), + Vector3::new(0.0, 1.0, 0.0), + Vector3::new(0.0, 0.0, 1.0), + ); + assert!(variant_eq_disk( + &Variant::CFrame(CFrame::new(Vector3::new(1.0, 2.0, 3.0), identity)), + &Variant::CFrame(CFrame::new(Vector3::new(1.0, 2.0, 3.0), identity)), + )); + assert!(!variant_eq_disk( + &Variant::CFrame(CFrame::new(Vector3::new(1.0, 2.0, 3.0), identity)), + &Variant::CFrame(CFrame::new(Vector3::new(1.0, 2.0, 4.0), identity)), + )); + } + + #[test] + fn variant_eq_disk_rect() { + use rbx_dom_weak::types::{Rect, Vector2}; + assert!(variant_eq_disk( + &Variant::Rect(Rect::new( + Vector2::new(0.0, 0.0), + Vector2::new(100.0, 100.0) + )), + &Variant::Rect(Rect::new( + Vector2::new(0.0, 0.0), + Vector2::new(100.0, 100.0) + )), + )); + } + + #[test] + fn variant_eq_disk_vector2() { + use rbx_dom_weak::types::Vector2; + assert!(variant_eq_disk( + &Variant::Vector2(Vector2::new(1.0, 2.0)), + &Variant::Vector2(Vector2::new(1.0, 2.0)), + )); + assert!(!variant_eq_disk( + &Variant::Vector2(Vector2::new(1.0, 2.0)), + &Variant::Vector2(Vector2::new(1.0, 3.0)), + )); + } }