diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 7e305fa2..c76ba5a1 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -44,6 +44,26 @@ pub use snapshot::{inst_path, SyncbackData, SyncbackSnapshot}; /// new files. const DEBUG_MODEL_FORMAT_VAR: &str = "ROJO_SYNCBACK_DEBUG"; +/// Services that are considered "visible" and will be included when +/// `ignoreHiddenServices` is enabled. All other services will be ignored. +const VISIBLE_SERVICES: &[&str] = &[ + "Lighting", + "MaterialService", + "NetworkClient", + "ReplicatedFirst", + "ReplicatedStorage", + "ServerScriptService", + "ServerStorage", + "SoundService", + "StarterGui", + "StarterPack", + "StarterPlayer", + "Teams", + "TextChatService", + "VoiceChatService", + "Workspace", +]; + /// A glob that can be used to tell if a path contains a `.git` folder. static GIT_IGNORE_GLOB: OnceLock = OnceLock::new(); @@ -91,6 +111,17 @@ pub fn syncback_loop( strip_unknown_root_children(&mut new_tree, old_tree); } + // If ignoreHiddenServices is enabled, filter to only visible services + if project + .syncback_rules + .as_ref() + .map(|rules| rules.ignore_hidden_services()) + .unwrap_or(false) + { + log::debug!("Filtering hidden services"); + strip_hidden_services(&mut new_tree); + } + log::debug!("Collecting referents for new DOM..."); let deferred_referents = collect_referents(&new_tree, &pre_prune_paths); @@ -408,6 +439,12 @@ pub struct SyncbackRules { /// Defaults to `false`. #[serde(skip_serializing_if = "Option::is_none")] encode_windows_invalid_chars: Option, + /// When enabled, only "visible" services will be synced back. This includes + /// commonly used services like Workspace, ReplicatedStorage, ServerScriptService, + /// etc., while ignoring internal/hidden services like Chat, HttpService, etc. + /// Defaults to `false`. + #[serde(skip_serializing_if = "Option::is_none")] + ignore_hidden_services: Option, } impl SyncbackRules { @@ -453,6 +490,14 @@ impl SyncbackRules { pub fn encode_windows_invalid_chars(&self) -> bool { self.encode_windows_invalid_chars.unwrap_or(true) } + + /// Returns whether hidden/internal services should be ignored during + /// syncback. When `true`, only visible services like Workspace, + /// ReplicatedStorage, etc. will be synced. Defaults to `false`. + #[inline] + pub fn ignore_hidden_services(&self) -> bool { + self.ignore_hidden_services.unwrap_or(false) + } } fn is_valid_path(globs: &Option>, base_path: &Path, path: &Path) -> bool { @@ -524,6 +569,29 @@ fn descendants(dom: &WeakDom, root_ref: Ref) -> Vec { ordered } +/// Removes root children (services) that are not in the `VISIBLE_SERVICES` list. +/// This is used when `ignoreHiddenServices` is enabled to filter out internal +/// services like Chat, HttpService, etc. +fn strip_hidden_services(dom: &mut WeakDom) { + let root_children = dom.root().children().to_vec(); + + for child_ref in root_children { + let child = dom + .get_by_ref(child_ref) + .expect("all children of the root should exist in the DOM"); + + // Check if this service is in the visible services list + if !VISIBLE_SERVICES.contains(&child.name.as_str()) { + log::trace!( + "Pruning hidden service {} of class {}", + child.name, + child.class + ); + dom.destroy(child_ref); + } + } +} + /// Removes the children of `new`'s root that are not also children of `old`'s /// root. /// diff --git a/src/syncback/ref_properties.rs b/src/syncback/ref_properties.rs index 9d251585..871e19d8 100644 --- a/src/syncback/ref_properties.rs +++ b/src/syncback/ref_properties.rs @@ -51,13 +51,60 @@ pub fn collect_all_paths(dom: &WeakDom) -> HashMap { paths } -/// Checks if a path is unique in the DOM by verifying no duplicates exist -/// at ANY level of the path (target, parent, grandparent, etc.). -fn is_path_unique(dom: &WeakDom, target_ref: Ref) -> bool { +/// Pre-computes which instances have duplicate-named siblings. +/// Returns a HashSet of Refs that have at least one sibling with the same name. +/// +/// This is O(N) where N is the number of instances, and allows subsequent +/// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings. +fn compute_refs_with_duplicate_siblings(dom: &WeakDom) -> HashSet { + let mut has_duplicate_siblings = HashSet::new(); + let mut queue = VecDeque::new(); + queue.push_back(dom.root_ref()); + + while let Some(inst_ref) = queue.pop_front() { + let inst = match dom.get_by_ref(inst_ref) { + Some(i) => i, + None => continue, + }; + + // Count children by name and collect their refs + let mut name_to_refs: HashMap<&str, Vec> = HashMap::new(); + for child_ref in inst.children() { + if let Some(child) = dom.get_by_ref(*child_ref) { + name_to_refs.entry(&child.name).or_default().push(*child_ref); + } + queue.push_back(*child_ref); + } + + // Mark refs that share a name with siblings + for (_name, refs) in name_to_refs { + if refs.len() > 1 { + for r in refs { + has_duplicate_siblings.insert(r); + } + } + } + } + + has_duplicate_siblings +} + +/// Checks if a path is unique using pre-computed duplicate sibling info. +/// This is O(d) where d is the depth of the instance. +fn is_path_unique_with_cache( + dom: &WeakDom, + target_ref: Ref, + has_duplicate_siblings: &HashSet, +) -> bool { let mut current_ref = target_ref; - // Walk up the tree checking each level for duplicate names among siblings + // Walk up the tree checking each level using the pre-computed cache loop { + // O(1) lookup instead of O(siblings) counting + if has_duplicate_siblings.contains(¤t_ref) { + return false; + } + let current = match dom.get_by_ref(current_ref) { Some(inst) => inst, None => return false, @@ -69,26 +116,6 @@ fn is_path_unique(dom: &WeakDom, target_ref: Ref) -> bool { return true; } - let parent = match dom.get_by_ref(parent_ref) { - Some(inst) => inst, - None => return true, - }; - - // Check if any sibling has the same name as current - let current_name = ¤t.name; - let mut count = 0; - for child_ref in parent.children() { - if let Some(child) = dom.get_by_ref(*child_ref) { - if &child.name == current_name { - count += 1; - if count > 1 { - // Duplicate found at this level - path is not unique - return false; - } - } - } - } - // Move up to parent and check the next level current_ref = parent_ref; } @@ -105,6 +132,13 @@ pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap) let mut id_links: HashMap> = HashMap::new(); let mut targets_needing_id: HashSet = HashSet::new(); + // Pre-compute duplicate sibling info in O(N) - this makes all subsequent + // path uniqueness checks O(d) instead of O(d × s) where d=depth, s=siblings + let has_duplicate_siblings = compute_refs_with_duplicate_siblings(dom); + + // Cache path uniqueness results to avoid re-checking the same target + let mut path_unique_cache: HashMap = HashMap::new(); + let mut queue = VecDeque::new(); queue.push_back(dom.root_ref()); @@ -122,8 +156,12 @@ pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap) // Check if target exists in current DOM if let Some(_target) = dom.get_by_ref(*target_ref) { - // Target exists - check if path is unique - if is_path_unique(dom, *target_ref) { + // Target exists - check if path is unique (with caching) + let is_unique = *path_unique_cache + .entry(*target_ref) + .or_insert_with(|| is_path_unique_with_cache(dom, *target_ref, &has_duplicate_siblings)); + + if is_unique { // Path is unique - use path-based system let target_path = inst_path(dom, *target_ref); path_links.entry(inst_ref).or_default().push(PathRefLink { diff --git a/src/web/api.rs b/src/web/api.rs index 2b32fac0..353438ab 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -221,13 +221,24 @@ impl ApiService { } // Process added instances (syncback: create files from Studio instances) - for added in request.added.values() { - if let Err(err) = self.syncback_added_instance(added) { - log::warn!( - "Failed to syncback added instance '{}': {}", - added.name, - err - ); + // We hold the tree lock during the entire processing to ensure the cache + // remains consistent with the tree state being checked. + if !request.added.is_empty() { + let tree = self.serve_session.tree(); + // Pre-compute duplicate sibling info in O(N) for efficient path uniqueness checks + // This makes all subsequent path checks O(d) instead of O(d × s) + let duplicate_siblings_cache = Self::compute_tree_refs_with_duplicate_siblings(&tree); + + for added in request.added.values() { + if let Err(err) = + self.syncback_added_instance(added, &tree, &duplicate_siblings_cache) + { + log::warn!( + "Failed to syncback added instance '{}': {}", + added.name, + err + ); + } } } @@ -257,13 +268,73 @@ impl ApiService { /// Syncback an added instance by creating a file in the filesystem. /// The file watcher will pick up the change and update Rojo's tree. /// - /// Checks if a path in the Rojo tree is unique by verifying no duplicates exist - /// at ANY level of the path (similar to ref_properties::is_path_unique). + /// Pre-computes which instances have duplicate-named siblings in a RojoTree. + /// Returns a HashSet of Refs that have at least one sibling with the same name. + /// + /// This is O(N) where N is the number of instances, and allows subsequent + /// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings. + fn compute_tree_refs_with_duplicate_siblings( + tree: &crate::snapshot::RojoTree, + ) -> HashSet { + use std::collections::VecDeque; + + let mut has_duplicate_siblings = HashSet::new(); + let mut queue = VecDeque::new(); + queue.push_back(tree.root().id()); + + while let Some(inst_ref) = queue.pop_front() { + let inst = match tree.get_instance(inst_ref) { + Some(i) => i, + None => continue, + }; + + // Count children by name and collect their refs + let mut name_to_refs: HashMap<&str, Vec> = HashMap::new(); + for child_ref in inst.children() { + if let Some(child) = tree.get_instance(*child_ref) { + name_to_refs.entry(child.name()).or_default().push(*child_ref); + } + queue.push_back(*child_ref); + } + + // Mark refs that share a name with siblings + for (_name, refs) in name_to_refs { + if refs.len() > 1 { + for r in refs { + has_duplicate_siblings.insert(r); + } + } + } + } + + has_duplicate_siblings + } + + /// Checks if a path in the Rojo tree is unique using pre-computed duplicate sibling info. /// Returns true if the path is unique, false if duplicates exist at any level. - fn is_tree_path_unique(&self, tree: &crate::snapshot::RojoTree, target_ref: Ref) -> bool { + /// This is O(d) where d is the depth of the instance. + fn is_tree_path_unique_with_cache( + tree: &crate::snapshot::RojoTree, + target_ref: Ref, + has_duplicate_siblings: &HashSet, + ) -> bool { let mut current_ref = target_ref; loop { + // O(1) lookup instead of O(siblings) counting + if has_duplicate_siblings.contains(¤t_ref) { + if let Some(current) = tree.get_instance(current_ref) { + if let Some(parent) = tree.get_instance(current.parent()) { + log::warn!( + "Syncback: Path to '{}' is not unique - duplicate-named siblings at '{}'", + current.name(), + parent.name() + ); + } + } + return false; + } + let current = match tree.get_instance(current_ref) { Some(inst) => inst, None => return false, @@ -275,31 +346,6 @@ impl ApiService { return true; } - let parent = match tree.get_instance(parent_ref) { - Some(inst) => inst, - None => return true, - }; - - // Check if any sibling has the same name as current - let current_name = current.name(); - let mut count = 0; - for child_ref in parent.children() { - if let Some(child) = tree.get_instance(*child_ref) { - if child.name() == current_name { - count += 1; - if count > 1 { - // Duplicate found at this level - path is not unique - log::warn!( - "Syncback: Path to '{}' is not unique - duplicate-named siblings at '{}'", - current_name, - parent.name() - ); - return false; - } - } - } - } - // Move up to parent and check the next level current_ref = parent_ref; } @@ -307,9 +353,14 @@ impl ApiService { /// This follows the same middleware selection logic as the dedicated syncback /// system in `src/syncback/mod.rs` to ensure consistent behavior. + /// + /// Takes the tree reference and duplicate siblings cache as parameters to ensure + /// consistency - the cache was computed from this exact tree state. fn syncback_added_instance( &self, added: &crate::web::interface::AddedInstance, + tree: &crate::snapshot::RojoTree, + duplicate_siblings_cache: &HashSet, ) -> anyhow::Result<()> { use crate::syncback::validate_file_name; use anyhow::Context; @@ -323,13 +374,13 @@ impl ApiService { })?; // Find the parent instance to get its filesystem path - let tree = self.serve_session.tree(); let parent_instance = tree .get_instance(added.parent) .context("Parent instance not found in Rojo tree")?; // Check if the parent path is unique (no duplicate-named siblings at any level) - if !self.is_tree_path_unique(&tree, added.parent) { + // Uses pre-computed cache for O(d) instead of O(d × s) complexity + if !Self::is_tree_path_unique_with_cache(tree, added.parent, duplicate_siblings_cache) { anyhow::bail!( "Cannot sync instance '{}' - parent path contains duplicate-named siblings", added.name