diff --git a/.cursor/plans/live_syncback_float_precision_1be79350.plan.md b/.cursor/plans/live_syncback_float_precision_1be79350.plan.md new file mode 100644 index 00000000..cf4c3eed --- /dev/null +++ b/.cursor/plans/live_syncback_float_precision_1be79350.plan.md @@ -0,0 +1,65 @@ +--- +name: Live syncback fixes +overview: "Fix five issues: lost Workspace attributes, spurious ClockTime, Rojo_Ref_CurrentCamera leak, and float precision noise via f32 truncation." +todos: + - id: fix-attributes-encoding + content: Add explicit Attributes/Tags encoding to encodeService.lua (mirroring encodeInstance.lua) + status: completed + - id: fix-clocktime + content: Filter non-serializable properties server-side in build_dom_from_chunks using the Rust reflection DB + status: completed + - id: fix-currentcamera-ref + content: "Stop Rojo_Ref_CurrentCamera from leaking: filter deferred_referents after camera destruction in syncback_loop" + status: completed + - id: fix-float-rounding + content: Truncate all float values to f32 in UnresolvedValue::from_variant for rbxl parity + status: completed +isProject: false +--- + +# Live Syncback Fixes + +## Issue 1: Workspace Attributes/Properties Lost + +**Root cause:** `encodeService.lua` iterates `classDescriptor.properties` but the Luau RbxDom database may not list `Attributes` and `Tags` as enumerable properties. `encodeInstance.lua` has explicit `encodeAttributes()` / `encodeTags()` helpers using `instance:GetAttributes()` / `instance:GetTags()` directly. + +**Fix:** Add explicit Attributes and Tags encoding to `[plugin/src/ChangeBatcher/encodeService.lua](plugin/src/ChangeBatcher/encodeService.lua)` after the property loop. Add `if propertyName == "Attributes" or propertyName == "Tags" then continue end` to the main loop to avoid double-encoding. + +## Issue 2: Lighting.ClockTime Appearing + +**Root cause:** Luau DB marks ClockTime as serializable; Rust DB marks it as DoesNotSerialize. Plugin sends it, CLI syncback filters it. + +**Fix:** In `[src/cli/serve.rs](src/cli/serve.rs)` `build_dom_from_chunks`, filter each plugin-sent property through `should_property_serialize` from the Rust reflection DB before applying it to the builder. + +## Issue 3: Rojo_Ref_CurrentCamera Leaking (pre-existing bug) + +**Root cause:** `collect_referents` captures `Workspace.CurrentCamera` -> Camera ref before camera is destroyed. `link_referents` writes `Rojo_Ref_CurrentCamera` without checking target still exists. + +**Fix:** In `[src/syncback/mod.rs](src/syncback/mod.rs)`, after camera destruction (line 222), filter `deferred_referents.path_links` to remove entries whose target ref no longer exists. + +## Issue 4: Float Precision + +**Root cause:** Live syncback sends Float64 from Lua; rbxl stores Float32. Tiny diffs cause properties to fail default comparison. + +**Fix:** In `[src/resolution.rs](src/resolution.rs)` `UnresolvedValue::from_variant`, truncate ALL float values to f32 precision. This is the single chokepoint for json5 writes: + +- **Float32(n)** (line 67): already f32, `n as f64` is fine (no change needed) +- **Float64(n)** (line 68): change to `(n as f32) as f64` to truncate +- **Vector2** (line 82): `(vector.x as f32) as f64` etc +- **Vector3** (line 85): same +- **CFrame** (lines 90-103): all 12 components +- **Color3** (line 88): already f32, fine +- **UDim/UDim2**: fall through to `FullyQualified` -- add a `round_variant` helper that truncates float components to f32 before wrapping + +Helper: + +```rust +fn truncate_to_f32(v: f64) -> f64 { + (v as f32) as f64 +} +``` + +## Deferred: Matching Parity + +Matching differences between live and CLI syncback are likely downstream of the above fixes (extra properties inflating scores, missing attributes, float diffs). Revisit after these four fixes land. If matching is still off, consider reusing the previously-served tree instead of creating a fresh `ServeSession::new_oneshot` in `run_live_syncback`. + diff --git a/plugin/src/ChangeBatcher/encodeService.lua b/plugin/src/ChangeBatcher/encodeService.lua index a726849b..71c90083 100644 --- a/plugin/src/ChangeBatcher/encodeService.lua +++ b/plugin/src/ChangeBatcher/encodeService.lua @@ -21,6 +21,9 @@ return function(service: Instance) if SKIP_PROPERTIES[propertyName] then continue end + if propertyName == "Attributes" or propertyName == "Tags" then + continue + end local isReadable = propertyMeta.scriptability == "ReadWrite" or propertyMeta.scriptability == "Read" @@ -53,6 +56,32 @@ return function(service: Instance) end end + local attrOk, attrMap = pcall(function() + return service:GetAttributes() + end) + if attrOk and attrMap and next(attrMap) then + local descriptor = RbxDom.findCanonicalPropertyDescriptor(service.ClassName, "Attributes") + if descriptor then + local encodeOk, encoded = encodeProperty(service, "Attributes", descriptor) + if encodeOk and encoded ~= nil then + properties.Attributes = encoded + end + end + end + + local tagOk, tagList = pcall(function() + return service:GetTags() + end) + if tagOk and tagList and #tagList > 0 then + local descriptor = RbxDom.findCanonicalPropertyDescriptor(service.ClassName, "Tags") + if descriptor then + local encodeOk, encoded = encodeProperty(service, "Tags", descriptor) + if encodeOk and encoded ~= nil then + properties.Tags = encoded + end + end + end + local chunk = { className = service.ClassName, childCount = #service:GetChildren(), diff --git a/src/cli/serve.rs b/src/cli/serve.rs index 421b45ba..9ebf2ceb 100644 --- a/src/cli/serve.rs +++ b/src/cli/serve.rs @@ -164,6 +164,9 @@ fn build_dom_from_chunks(payload: SyncbackPayload) -> anyhow::Result { for chunk in &payload.services { let mut builder = InstanceBuilder::new(&chunk.class_name); for (key, value) in &chunk.properties { + if !crate::syncback::should_property_serialize(&chunk.class_name, key) { + continue; + } builder = builder.with_property(key.as_str(), value.clone()); } let service_ref = dom.insert(root_ref, builder); diff --git a/src/resolution.rs b/src/resolution.rs index d1a92e26..d7f7048f 100644 --- a/src/resolution.rs +++ b/src/resolution.rs @@ -10,6 +10,68 @@ use serde::{Deserialize, Serialize}; use crate::REF_PATH_ATTRIBUTE_PREFIX; +/// Truncates float components of Variant types to f32 precision, matching the +/// rbxl binary format which stores most float properties as Float32. +fn truncate_variant(v: Variant) -> Variant { + #[inline] + fn t(v: f32) -> f32 { + v as f64 as f32 + } + + match v { + Variant::CFrame(cf) => Variant::CFrame(CFrame { + position: Vector3::new(t(cf.position.x), t(cf.position.y), t(cf.position.z)), + orientation: Matrix3::new( + Vector3::new( + t(cf.orientation.x.x), + t(cf.orientation.x.y), + t(cf.orientation.x.z), + ), + Vector3::new( + t(cf.orientation.y.x), + t(cf.orientation.y.y), + t(cf.orientation.y.z), + ), + Vector3::new( + t(cf.orientation.z.x), + t(cf.orientation.z.y), + t(cf.orientation.z.z), + ), + ), + }), + Variant::OptionalCFrame(Some(cf)) => Variant::OptionalCFrame(Some(CFrame { + position: Vector3::new(t(cf.position.x), t(cf.position.y), t(cf.position.z)), + orientation: Matrix3::new( + Vector3::new( + t(cf.orientation.x.x), + t(cf.orientation.x.y), + t(cf.orientation.x.z), + ), + Vector3::new( + t(cf.orientation.y.x), + t(cf.orientation.y.y), + t(cf.orientation.y.z), + ), + Vector3::new( + t(cf.orientation.z.x), + t(cf.orientation.z.y), + t(cf.orientation.z.z), + ), + ), + })), + Variant::Vector3(v) => Variant::Vector3(Vector3::new(t(v.x), t(v.y), t(v.z))), + Variant::Vector2(v) => Variant::Vector2(Vector2::new(t(v.x), t(v.y))), + Variant::UDim(u) => Variant::UDim(rbx_dom_weak::types::UDim::new(t(u.scale), u.offset)), + Variant::UDim2(u) => Variant::UDim2(rbx_dom_weak::types::UDim2::new( + rbx_dom_weak::types::UDim::new(t(u.x.scale), u.x.offset), + rbx_dom_weak::types::UDim::new(t(u.y.scale), u.y.offset), + )), + Variant::Float32(n) => Variant::Float32(n as f64 as f32), + Variant::Float64(n) => Variant::Float64(n as f32 as f64), + other => other, + } +} + /// A user-friendly version of `Variant` that supports specifying ambiguous /// values. Ambiguous values need a reflection database to be resolved to a /// usable value. @@ -41,6 +103,7 @@ impl UnresolvedValue { /// Creates an `UnresolvedValue` from a variant, using a class and property /// name to potentially allow for ambiguous Enum variants. pub fn from_variant(variant: Variant, class_name: &str, prop_name: &str) -> Self { + let variant = truncate_variant(variant); let descriptor = find_descriptor(class_name, prop_name); if descriptor.is_some() { // We can only use an ambiguous syntax if the property is known @@ -116,6 +179,7 @@ impl UnresolvedValue { /// Creates an `UnresolvedValue` from a variant, only returning ambiguous /// values if they're able to be resolved in a context-free environment. pub fn from_variant_unambiguous(variant: Variant) -> Self { + let variant = truncate_variant(variant); match variant { Variant::String(str) => Self::Ambiguous(AmbiguousValue::String(str)), Variant::Float64(number) => Self::Ambiguous(AmbiguousValue::Number(number)), diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index ed6ba733..26362623 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -207,18 +207,21 @@ pub fn syncback_loop_with_stats( .unwrap_or(false); if !sync_current_camera { log::debug!("Removing CurrentCamera from new DOM"); - let mut camera_ref = None; + let mut workspace_ref = None; + let mut camera_target = None; for child_ref in new_tree.root().children() { let inst = new_tree.get_by_ref(*child_ref).unwrap(); if inst.class == "Workspace" { - camera_ref = inst.properties.get(&ustr("CurrentCamera")); + workspace_ref = Some(*child_ref); + camera_target = inst.properties.get(&ustr("CurrentCamera")).cloned(); break; } } - if let Some(Variant::Ref(camera_ref)) = camera_ref { - if new_tree.get_by_ref(*camera_ref).is_some() { - new_tree.destroy(*camera_ref); + if let (Some(ws_ref), Some(Variant::Ref(cam_ref))) = (workspace_ref, camera_target) { + if new_tree.get_by_ref(cam_ref).is_some() { + new_tree.destroy(cam_ref); } + deferred_referents.remove_ref(ws_ref, "CurrentCamera"); } } diff --git a/src/syncback/ref_properties.rs b/src/syncback/ref_properties.rs index e7f878fd..cc54a3a2 100644 --- a/src/syncback/ref_properties.rs +++ b/src/syncback/ref_properties.rs @@ -149,6 +149,17 @@ pub struct RefLinks { pub placeholder_to_source_and_target: HashMap, } +impl RefLinks { + pub fn remove_ref(&mut self, source: Ref, prop_name: &str) { + if let Some(links) = self.path_links.get_mut(&source) { + links.retain(|link| link.name.as_str() != prop_name); + } + if let Some(links) = self.id_links.get_mut(&source) { + links.retain(|link| link.name.as_str() != prop_name); + } + } +} + #[derive(PartialEq, Eq)] struct PathRefLink { name: Ustr,