From bf65d8bfa8a35e264fc0bdbbaa9f369c32f82c30 Mon Sep 17 00:00:00 2001 From: Joe Date: Sat, 14 Feb 2026 10:56:25 -0600 Subject: [PATCH] initial work --- plugin/src/ChangeBatcher/createPatchSet.lua | 4 +- .../src/ChangeBatcher/encodePatchUpdate.lua | 34 ++++++- plugin/src/ChangeBatcher/propertyFilter.lua | 6 +- plugin/src/ServeSession.lua | 5 + src/rojo_ref.rs | 16 ++++ src/syncback/ref_properties.rs | 6 +- src/web/api.rs | 92 ++++++++++++++++--- 7 files changed, 143 insertions(+), 20 deletions(-) diff --git a/plugin/src/ChangeBatcher/createPatchSet.lua b/plugin/src/ChangeBatcher/createPatchSet.lua index 3207722b..b1354e87 100644 --- a/plugin/src/ChangeBatcher/createPatchSet.lua +++ b/plugin/src/ChangeBatcher/createPatchSet.lua @@ -33,13 +33,13 @@ return function(instanceMap, propertyChanges, syncSourceOnly) if syncSourceOnly then -- Filter to only Source property when server is in source-only mode if properties.Source then - local update = encodePatchUpdate(instance, instanceId, { Source = true }) + local update = encodePatchUpdate(instance, instanceId, { Source = true }, instanceMap) if update then table.insert(patch.updated, update) end end else - local update = encodePatchUpdate(instance, instanceId, properties) + local update = encodePatchUpdate(instance, instanceId, properties, instanceMap) if update then table.insert(patch.updated, update) end diff --git a/plugin/src/ChangeBatcher/encodePatchUpdate.lua b/plugin/src/ChangeBatcher/encodePatchUpdate.lua index 9f5c0441..55609a69 100644 --- a/plugin/src/ChangeBatcher/encodePatchUpdate.lua +++ b/plugin/src/ChangeBatcher/encodePatchUpdate.lua @@ -5,7 +5,9 @@ local RbxDom = require(Packages.RbxDom) local encodeProperty = require(script.Parent.encodeProperty) local UNENCODABLE_DATA_TYPES = require(script.Parent.propertyFilter) -return function(instance, instanceId, properties) +local NULL_REF = "00000000000000000000000000000000" + +return function(instance, instanceId, properties, instanceMap) local update = { id = instanceId, changedProperties = {}, @@ -26,6 +28,36 @@ return function(instance, instanceId, properties) continue end + -- Ref properties are encoded using the InstanceMap to resolve + -- Studio Instance references to server-side Ref IDs. + if descriptor.dataType == "Ref" then + local readSuccess, readResult = descriptor:read(instance) + if not readSuccess then + Log.debug("Could not read Ref property {:?}.{}: {}", instance, propertyName, readResult) + continue + end + + if readResult == nil then + -- Nil ref: encode as null ref + update.changedProperties[propertyName] = { Ref = NULL_REF } + elseif instanceMap then + local targetId = instanceMap.fromInstances[readResult] + if targetId then + update.changedProperties[propertyName] = { Ref = targetId } + else + Log.warn( + "Cannot sync Ref property {:?}.{}: target {:?} is not tracked by Atlas", + instance, + propertyName, + readResult + ) + end + else + Log.warn("Cannot encode Ref property {:?}.{}: no InstanceMap provided", instance, propertyName) + end + continue + end + local encodeSuccess, encodeResult = encodeProperty(instance, propertyName, descriptor) if not encodeSuccess then diff --git a/plugin/src/ChangeBatcher/propertyFilter.lua b/plugin/src/ChangeBatcher/propertyFilter.lua index a70a37c3..efa54fb0 100644 --- a/plugin/src/ChangeBatcher/propertyFilter.lua +++ b/plugin/src/ChangeBatcher/propertyFilter.lua @@ -4,12 +4,14 @@ Types listed here are skipped during property encoding because they cannot be serialized to JSON or are meaningless for syncback. + + Note: Ref properties are NOT blacklisted here. They are handled + specially in encodePatchUpdate.lua using the InstanceMap to resolve + Studio Instance references to server-side Ref IDs. ]] -- Property data types that cannot be encoded standalone. --- Matches the Variant::Ref / Variant::UniqueId filter in api.rs. local UNENCODABLE_DATA_TYPES = { - Ref = true, -- Instance references; need the InstanceMap, not encodable standalone UniqueId = true, -- Internal Roblox identifiers; not meaningful for syncback } diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 6ec59639..5523f08b 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -779,6 +779,11 @@ function ServeSession:__confirmAndApplyInitialPatch(catchUpPatch, serverInfo) local guid = HttpService:GenerateGUID(false) local tempRef = string.gsub(guid, "-", ""):lower() pullPatch.added[tempRef] = encoded + -- Note: We intentionally do NOT pre-insert into InstanceMap here. + -- The VFS watcher will process the written files and assign a + -- server-side Ref ID. Any Ref properties targeting this instance + -- will be deferred by the ChangeBatcher and resolved once the + -- server ID appears in the InstanceMap via forward-sync. -- Log at info level since this is a major file creation operation Log.info("[Pull] Create file: {} ({})", instancePath, instanceClass) end diff --git a/src/rojo_ref.rs b/src/rojo_ref.rs index e9a42a3c..bfa35227 100644 --- a/src/rojo_ref.rs +++ b/src/rojo_ref.rs @@ -1,5 +1,6 @@ use std::{fmt, sync::Arc}; +use rbx_dom_weak::{types::Ref, WeakDom}; use serde::{Deserialize, Serialize}; /// Legacy: ID attribute on target instances (kept for backwards compatibility) @@ -11,6 +12,21 @@ pub const REF_POINTER_ATTRIBUTE_PREFIX: &str = "Rojo_Target_"; /// No modification to the target instance is needed. pub const REF_PATH_ATTRIBUTE_PREFIX: &str = "Rojo_Ref_"; +/// Compute the `Rojo_Ref_*` attribute name for a given property name. +/// +/// Example: `ref_attribute_name("PrimaryPart")` returns `"Rojo_Ref_PrimaryPart"`. +pub fn ref_attribute_name(prop_name: &str) -> String { + format!("{REF_PATH_ATTRIBUTE_PREFIX}{prop_name}") +} + +/// Compute the path to a target instance for use in a `Rojo_Ref_*` attribute. +/// Returns the slash-separated path from the root (root name excluded). +/// +/// Example: an instance at `DataModel > Workspace > Part1` returns `"Workspace/Part1"`. +pub fn ref_target_path(dom: &WeakDom, target_ref: Ref) -> String { + dom.full_path_of(target_ref, "/") +} + // TODO add an internment strategy for RojoRefs // Something like what rbx-dom does for SharedStrings probably works diff --git a/src/syncback/ref_properties.rs b/src/syncback/ref_properties.rs index f1acac82..205378d4 100644 --- a/src/syncback/ref_properties.rs +++ b/src/syncback/ref_properties.rs @@ -10,8 +10,8 @@ use rbx_dom_weak::{ }; use crate::{ - syncback::snapshot::inst_path, REF_ID_ATTRIBUTE_NAME, REF_PATH_ATTRIBUTE_PREFIX, - REF_POINTER_ATTRIBUTE_PREFIX, + ref_attribute_name, syncback::snapshot::inst_path, REF_ID_ATTRIBUTE_NAME, + REF_PATH_ATTRIBUTE_PREFIX, REF_POINTER_ATTRIBUTE_PREFIX, }; pub struct RefLinks { @@ -232,7 +232,7 @@ pub fn link_referents(links: RefLinks, dom: &mut WeakDom) -> anyhow::Result<()> if let Some(path_refs) = links.path_links.get(&inst_id) { for link in path_refs { path_attrs.push(( - format!("{REF_PATH_ATTRIBUTE_PREFIX}{}", link.name), + ref_attribute_name(&link.name), Variant::String(link.path.clone()), )); } diff --git a/src/web/api.rs b/src/web/api.rs index 92c33582..6e76ddd7 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -2285,8 +2285,10 @@ impl ApiService { continue; } - // Skip Ref and UniqueId - they don't serialize to JSON (matches dedicated syncback) - if matches!(value, Variant::Ref(_) | Variant::UniqueId(_)) { + // Skip UniqueId - internal Roblox identifiers, not meaningful for syncback. + // Note: Ref properties are handled upstream in syncback_updated_properties() + // where they are converted to Rojo_Ref_* path-based attributes. + if matches!(value, Variant::UniqueId(_)) { continue; } @@ -2537,27 +2539,58 @@ impl ApiService { "ModuleScript" | "Script" | "LocalScript" ); - // Collect the non-Source properties as a HashMap - // for reuse with filter_properties_for_meta + // Extract Ref properties and convert to Rojo_Ref_* attributes. + // Non-Ref properties are collected separately for filter_properties_for_meta. + let mut ref_attributes: indexmap::IndexMap = + indexmap::IndexMap::new(); + let mut remove_attributes: Vec = Vec::new(); let mut props: HashMap = HashMap::new(); + for (key, value) in &update.changed_properties { if key == "Source" { continue; } - if let Some(v) = value { - props.insert(key.to_string(), v.clone()); + match value { + Some(Variant::Ref(target_ref)) => { + let attr_name = crate::ref_attribute_name(key); + if target_ref.is_none() { + // Null ref: mark attribute for removal + remove_attributes.push(attr_name); + } else if tree.get_instance(*target_ref).is_some() { + // Valid target: compute path and add as Rojo_Ref_* attribute + let path = crate::ref_target_path(tree.inner(), *target_ref); + ref_attributes.insert(attr_name, serde_json::Value::String(path)); + } else { + log::warn!( + "Cannot persist Ref property '{}' for instance {:?}: \ + target {:?} not found in tree", + key, + update.id, + target_ref + ); + } + } + Some(v) => { + props.insert(key.to_string(), v.clone()); + } + None => {} } } - if props.is_empty() { + if props.is_empty() && ref_attributes.is_empty() && remove_attributes.is_empty() { return Ok(()); } let skip_prop = if is_script { Some("Source") } else { None }; - let (properties, attributes) = + let (properties, mut attributes) = self.filter_properties_for_meta(class_name.as_str(), &props, skip_prop); - if properties.is_empty() && attributes.is_empty() { + // Merge Rojo_Ref_* attributes into the attributes map + for (k, v) in ref_attributes { + attributes.insert(k, v); + } + + if properties.is_empty() && attributes.is_empty() && remove_attributes.is_empty() { return Ok(()); } @@ -2567,7 +2600,13 @@ impl ApiService { let meta_path = inst_path.join("init.meta.json5"); // Read existing meta if present, merge with new properties - let meta = self.merge_or_build_meta(&meta_path, None, properties, attributes)?; + let meta = self.merge_or_build_meta( + &meta_path, + None, + properties, + attributes, + &remove_attributes, + )?; let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; self.suppress_path(&meta_path); @@ -2594,7 +2633,13 @@ impl ApiService { .unwrap_or(file_stem); let meta_path = parent_dir.join(format!("{}.meta.json5", base_name)); - let meta = self.merge_or_build_meta(&meta_path, None, properties, attributes)?; + let meta = self.merge_or_build_meta( + &meta_path, + None, + properties, + attributes, + &remove_attributes, + )?; let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize meta.json5")?; self.suppress_path(&meta_path); @@ -2620,6 +2665,7 @@ impl ApiService { Some(class_name.as_str()), properties, attributes, + &remove_attributes, )?; let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize model file")?; @@ -2637,7 +2683,13 @@ impl ApiService { let file_stem = inst_path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); let meta_path = parent_dir.join(format!("{}.meta.json5", file_stem)); - let meta = self.merge_or_build_meta(&meta_path, None, properties, attributes)?; + let meta = self.merge_or_build_meta( + &meta_path, + None, + properties, + attributes, + &remove_attributes, + )?; let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize meta.json5")?; self.suppress_path(&meta_path); @@ -2656,12 +2708,17 @@ impl ApiService { /// Reads an existing meta/model JSON5 file and merges new properties into it. /// If the file doesn't exist, builds a fresh meta object. + /// + /// `remove_attributes` is a list of attribute keys to remove from the existing + /// file (used for nil Ref cleanup, e.g., removing a stale `Rojo_Ref_PrimaryPart` + /// when PrimaryPart is set to nil). fn merge_or_build_meta( &self, existing_path: &std::path::Path, class_name: Option<&str>, new_properties: indexmap::IndexMap, new_attributes: indexmap::IndexMap, + remove_attributes: &[String], ) -> anyhow::Result { use anyhow::Context; @@ -2691,6 +2748,17 @@ impl ApiService { } } + // Remove attributes marked for deletion (e.g., stale Rojo_Ref_* entries) + if !remove_attributes.is_empty() { + if let Some(attrs) = obj.get_mut("attributes") { + if let Some(attrs_obj) = attrs.as_object_mut() { + for key in remove_attributes { + attrs_obj.remove(key); + } + } + } + } + // Merge attributes if !new_attributes.is_empty() { let attrs = obj