Add support for external references in RojoTree and syncback process

- Introduced REF_EXTERNAL_ATTRIBUTE_PREFIX for attributes pointing to external instances.
- Enhanced PatchApplyContext to track external references and apply them during patch application.
- Implemented get_instance_by_path method in RojoTree for resolving paths to external instances.
- Updated collect_referents function to handle external references and preserve their paths during syncback.
- Refactored link_referents to include external reference handling in the linking process.

src/rojo_ref.rs

@@ -4,6 +4,9 @@ use serde::{Deserialize, Serialize};
 
 pub const REF_ID_ATTRIBUTE_NAME: &str = "Rojo_Id";
 pub const REF_POINTER_ATTRIBUTE_PREFIX: &str = "Rojo_Target_";
+/// Prefix for attributes storing paths to external references (instances outside
+/// the sync tree, like SoundGroups in SoundService).
+pub const REF_EXTERNAL_ATTRIBUTE_PREFIX: &str = "Rojo_External_";
 
 // TODO add an internment strategy for RojoRefs
 // Something like what rbx-dom does for SharedStrings probably works

src/snapshot/patch_apply.rs

@@ -14,7 +14,10 @@ use super::{
     patch::{AppliedPatchSet, AppliedPatchUpdate, PatchSet, PatchUpdate},
     InstanceSnapshot, RojoTree,
 };
-use crate::{multimap::MultiMap, RojoRef, REF_ID_ATTRIBUTE_NAME, REF_POINTER_ATTRIBUTE_PREFIX};
+use crate::{
+    multimap::MultiMap, RojoRef, REF_EXTERNAL_ATTRIBUTE_PREFIX, REF_ID_ATTRIBUTE_NAME,
+    REF_POINTER_ATTRIBUTE_PREFIX,
+};
 
 /// Consumes the input `PatchSet`, applying all of its prescribed changes to the
 /// tree and returns an `AppliedPatchSet`, which can be used to keep another
@@ -81,6 +84,10 @@ struct PatchApplyContext {
     /// properties.
     attribute_refs_to_rewrite: MultiMap<Ref, (Ustr, String)>,
 
+    /// Tracks external ref properties (references to instances outside the sync
+    /// tree, like SoundGroups in SoundService). These are stored as paths.
+    external_refs_to_rewrite: MultiMap<Ref, (Ustr, String)>,
+
     /// The current applied patch result, describing changes made to the tree.
     applied_patch_set: AppliedPatchSet,
 }
@@ -129,6 +136,34 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) -
         instance.properties_mut().extend(real_rewrites.drain(..));
     }
 
+    // Handle external path-based references (e.g., SoundGroup pointing to SoundService)
+    let mut external_rewrites = Vec::new();
+    for (id, map) in context.external_refs_to_rewrite {
+        for (prop_name, path) in map {
+            if let Some(target) = tree.get_instance_by_path(&path) {
+                log::debug!(
+                    "Resolved external reference {} -> {} (path: {})",
+                    prop_name,
+                    target,
+                    path
+                );
+                external_rewrites.push((prop_name, Variant::Ref(target)));
+            } else {
+                log::debug!(
+                    "Could not resolve external reference {} at path '{}' - instance not found in tree",
+                    prop_name,
+                    path
+                );
+            }
+        }
+        if !external_rewrites.is_empty() {
+            let mut instance = tree
+                .get_instance_mut(id)
+                .expect("Invalid instance ID in deferred external ref map");
+            instance.properties_mut().extend(external_rewrites.drain(..));
+        }
+    }
+
     context.applied_patch_set
 }
 
@@ -295,6 +330,30 @@ fn defer_ref_properties(tree: &mut RojoTree, id: Ref, context: &mut PatchApplyCo
                 )
             }
         }
+        // Handle external path-based references (for instances outside the sync tree)
+        if let Some(prop_name) = attr_name.strip_prefix(REF_EXTERNAL_ATTRIBUTE_PREFIX) {
+            if let Variant::String(path) = attr_value {
+                context
+                    .external_refs_to_rewrite
+                    .insert(id, (ustr(prop_name), path.clone()));
+            } else if let Variant::BinaryString(path) = attr_value {
+                if let Ok(str) = std::str::from_utf8(path.as_ref()) {
+                    context
+                        .external_refs_to_rewrite
+                        .insert(id, (ustr(prop_name), str.to_string()));
+                } else {
+                    log::error!(
+                        "External paths specified by referent property attributes must be valid UTF-8 strings."
+                    )
+                }
+            } else {
+                log::warn!(
+                    "Attribute {attr_name} is of type {:?} when it was \
+                    expected to be a String",
+                    attr_value.ty()
+                )
+            }
+        }
     }
     if let Some(specified_id) = attr_id {
         tree.set_specified_id(id, specified_id);

src/snapshot/tree.rs

@@ -231,6 +231,48 @@ impl RojoTree {
         self.specified_id_to_refs.insert(specified, id);
     }
 
+    /// Looks up an instance by its path in the tree (e.g., "SoundService/MySoundGroup").
+    /// The path is relative to the root of the tree, with segments separated by "/".
+    /// Returns the Ref if found, None otherwise.
+    pub fn get_instance_by_path(&self, path: &str) -> Option<Ref> {
+        if path.is_empty() {
+            return Some(self.get_root_id());
+        }
+
+        let segments: Vec<&str> = path.split('/').collect();
+        let mut current_ref = self.get_root_id();
+
+        // The root of the tree doesn't appear in the path, so we start with
+        // the root's children.
+        for (i, segment) in segments.iter().enumerate() {
+            let current = self.inner.get_by_ref(current_ref)?;
+
+            // For the first segment, check direct children of root
+            // For subsequent segments, check children of current
+            let children = if i == 0 {
+                current.children()
+            } else {
+                current.children()
+            };
+
+            let mut found = false;
+            for &child_ref in children {
+                let child = self.inner.get_by_ref(child_ref)?;
+                if child.name == *segment {
+                    current_ref = child_ref;
+                    found = true;
+                    break;
+                }
+            }
+
+            if !found {
+                return None;
+            }
+        }
+
+        Some(current_ref)
+    }
+
     fn insert_metadata(&mut self, id: Ref, metadata: InstanceMetadata) {
         for path in &metadata.relevant_paths {
             self.path_to_ids.insert(path.clone(), id);

src/syncback/mod.rs

@@ -14,7 +14,7 @@ use rbx_dom_weak::{
 };
 use serde::{Deserialize, Serialize};
 use std::{
-    collections::{HashMap, HashSet, VecDeque},
+    collections::{HashMap, VecDeque},
     env,
     path::Path,
     sync::OnceLock,
@@ -24,7 +24,7 @@ use crate::{
     glob::Glob,
     snapshot::{InstanceWithMeta, RojoTree},
     snapshot_middleware::Middleware,
-    syncback::ref_properties::{collect_referents, link_referents},
+    syncback::ref_properties::{collect_all_paths, collect_referents, link_referents},
     Project,
 };
 
@@ -65,6 +65,11 @@ pub fn syncback_loop(
         .map(|rules| rules.compile_tree_globs())
         .transpose()?;
 
+    // Collect all instance paths BEFORE pruning so we can track external references
+    // (references to instances that will be pruned, like SoundGroups in SoundService).
+    log::debug!("Collecting instance paths before pruning...");
+    let pre_prune_paths = collect_all_paths(&new_tree);
+
     // TODO: Add a better way to tell if the root of a project is a directory
     let skip_pruning = if let Some(path) = &project.tree.path {
         let middleware =
@@ -87,7 +92,7 @@ pub fn syncback_loop(
     }
 
     log::debug!("Collecting referents for new DOM...");
-    let deferred_referents = collect_referents(&new_tree);
+    let deferred_referents = collect_referents(&new_tree, &pre_prune_paths);
 
     // Remove any properties that are manually blocked from syncback via the
     // project file.

src/syncback/ref_properties.rs

@@ -9,13 +9,15 @@ use rbx_dom_weak::{
 };
 
 use crate::{
-    multimap::MultiMap, syncback::snapshot::inst_path, REF_ID_ATTRIBUTE_NAME,
-    REF_POINTER_ATTRIBUTE_PREFIX,
+    multimap::MultiMap, syncback::snapshot::inst_path, REF_EXTERNAL_ATTRIBUTE_PREFIX,
+    REF_ID_ATTRIBUTE_NAME, REF_POINTER_ATTRIBUTE_PREFIX,
 };
 
 pub struct RefLinks {
     /// A map of referents to each of their Ref properties.
     prop_links: MultiMap<Ref, RefLink>,
+    /// A map of referents to external ref properties (ones pointing outside the tree).
+    external_links: MultiMap<Ref, ExternalRefLink>,
     /// A set of referents that need their ID rewritten. This includes
     /// Instances that have no existing ID.
     need_rewrite: HashSet<Ref>,
@@ -29,13 +31,42 @@ struct RefLink {
     value: Ref,
 }
 
+#[derive(PartialEq, Eq)]
+struct ExternalRefLink {
+    /// The name of a property
+    name: Ustr,
+    /// The path to the external instance.
+    path: String,
+}
+
+/// Collects all instance paths in a WeakDom before any pruning occurs.
+/// Returns a map of Ref -> path for all instances.
+pub fn collect_all_paths(dom: &WeakDom) -> HashMap<Ref, String> {
+    let mut paths = HashMap::new();
+    let mut queue = VecDeque::new();
+    queue.push_back(dom.root_ref());
+
+    while let Some(inst_ref) = queue.pop_front() {
+        let inst = dom.get_by_ref(inst_ref).unwrap();
+        queue.extend(inst.children().iter().copied());
+        paths.insert(inst_ref, inst_path(dom, inst_ref));
+    }
+
+    paths
+}
+
 /// Iterates through a WeakDom and collects referent properties.
 ///
 /// They can be linked to a dom later using `link_referents`.
-pub fn collect_referents(dom: &WeakDom) -> RefLinks {
+///
+/// The `pre_prune_paths` parameter should contain paths for instances that may
+/// have been pruned from the DOM. This allows external references (references
+/// to instances outside the sync tree) to be preserved as path-based attributes.
+pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap<Ref, String>) -> RefLinks {
     let mut ids = HashMap::new();
     let mut need_rewrite = HashSet::new();
     let mut links = MultiMap::new();
+    let mut external_links = MultiMap::new();
 
     // Note that this is back-in, front-out. This is important because
     // VecDeque::extend is the equivalent to using push_back.
@@ -56,15 +87,31 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks {
             let target = match dom.get_by_ref(*prop_value) {
                 Some(inst) => inst,
                 None => {
-                    // Properties that are Some but point to nothing may as
-                    // well be `nil`. Roblox and us never produce these values
-                    // but syncback prunes trees without adjusting ref
-                    // properties for performance reasons.
-                    log::warn!(
-                        "Property {}.{} will be `nil` on the disk because the actual value is not being included in syncback",
-                        inst_path(dom, inst_ref),
-                        prop_name
-                    );
+                    // The target doesn't exist in the current DOM. Check if we
+                    // have its path from before pruning.
+                    if let Some(external_path) = pre_prune_paths.get(prop_value) {
+                        log::debug!(
+                            "Property {}.{} points to external instance at '{}', storing as path reference",
+                            inst_path(dom, inst_ref),
+                            prop_name,
+                            external_path
+                        );
+                        external_links.insert(
+                            inst_ref,
+                            ExternalRefLink {
+                                name: *prop_name,
+                                path: external_path.clone(),
+                            },
+                        );
+                    } else {
+                        // Properties that are Some but point to nothing may as
+                        // well be `nil`. This is a truly dangling reference.
+                        log::warn!(
+                            "Property {}.{} will be `nil` on the disk because the referenced instance does not exist",
+                            inst_path(dom, inst_ref),
+                            prop_name
+                        );
+                    }
                     continue;
                 }
             };
@@ -105,24 +152,46 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks {
     RefLinks {
         need_rewrite,
         prop_links: links,
+        external_links,
     }
 }
 
 pub fn link_referents(links: RefLinks, dom: &mut WeakDom) -> anyhow::Result<()> {
     write_id_attributes(&links, dom)?;
 
-    let mut prop_list = Vec::new();
+    // Convert MultiMaps to HashMaps for easier access
+    let prop_links: HashMap<Ref, Vec<RefLink>> = links.prop_links.into_iter().collect();
+    let external_links: HashMap<Ref, Vec<ExternalRefLink>> =
+        links.external_links.into_iter().collect();
 
-    for (inst_id, properties) in links.prop_links {
-        for ref_link in properties {
-            let prop_inst = match dom.get_by_ref(ref_link.value) {
-                Some(inst) => inst,
-                None => continue,
-            };
-            let id = get_existing_id(prop_inst)
-                .expect("all Instances that are pointed to should have an ID");
-            prop_list.push((ref_link.name, Variant::String(id.to_owned())));
+    // Collect all instance IDs that need attributes updated
+    let mut all_inst_ids: HashSet<Ref> = prop_links.keys().copied().collect();
+    all_inst_ids.extend(external_links.keys().copied());
+
+    let mut prop_list = Vec::new();
+    let mut external_prop_list = Vec::new();
+
+    for inst_id in all_inst_ids {
+        // Collect internal ref links
+        if let Some(properties) = prop_links.get(&inst_id) {
+            for ref_link in properties {
+                let prop_inst = match dom.get_by_ref(ref_link.value) {
+                    Some(inst) => inst,
+                    None => continue,
+                };
+                let id = get_existing_id(prop_inst)
+                    .expect("all Instances that are pointed to should have an ID");
+                prop_list.push((ref_link.name, Variant::String(id.to_owned())));
+            }
         }
+
+        // Collect external ref links
+        if let Some(external_properties) = external_links.get(&inst_id) {
+            for ext_link in external_properties {
+                external_prop_list.push((ext_link.name, Variant::String(ext_link.path.clone())));
+            }
+        }
+
         let inst = match dom.get_by_ref_mut(inst_id) {
             Some(inst) => inst,
             None => continue,
@@ -139,9 +208,13 @@ pub fn link_referents(links: RefLinks, dom: &mut WeakDom) -> anyhow::Result<()>
             }
         }
         .into_iter()
-        .filter(|(name, _)| !name.starts_with(REF_POINTER_ATTRIBUTE_PREFIX))
+        .filter(|(name, _)| {
+            !name.starts_with(REF_POINTER_ATTRIBUTE_PREFIX)
+                && !name.starts_with(REF_EXTERNAL_ATTRIBUTE_PREFIX)
+        })
         .collect();
 
+        // Add internal ref pointers
         for (prop_name, prop_value) in prop_list.drain(..) {
             attributes.insert(
                 format!("{REF_POINTER_ATTRIBUTE_PREFIX}{prop_name}"),
@@ -149,6 +222,14 @@ pub fn link_referents(links: RefLinks, dom: &mut WeakDom) -> anyhow::Result<()>
             );
         }
 
+        // Add external ref pointers (path-based)
+        for (prop_name, prop_value) in external_prop_list.drain(..) {
+            attributes.insert(
+                format!("{REF_EXTERNAL_ATTRIBUTE_PREFIX}{prop_name}"),
+                prop_value,
+            );
+        }
+
         inst.properties
             .insert("Attributes".into(), attributes.into());
     }