diff --git a/src/commands/build.rs b/src/commands/build.rs index 5b745111..1ae1ee48 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -121,11 +121,8 @@ pub fn build(options: &BuildOptions) -> Result<(), BuildError> { // Place files don't contain an entry for the DataModel, but our // RbxTree representation does. - let top_level_ids = tree - .get_instance(root_id) - .unwrap() - .instance - .get_children_ids(); + let root_instance = tree.get_instance(root_id).unwrap(); + let top_level_ids = root_instance.children(); rbx_xml::to_writer(&mut file, tree.inner(), top_level_ids, xml_encode_config())?; } @@ -137,11 +134,8 @@ pub fn build(options: &BuildOptions) -> Result<(), BuildError> { log::warn!("Using the XML place format (rbxlx) is recommended instead."); log::warn!("For more info, see https://github.com/LPGhatguy/rojo/issues/180"); - let top_level_ids = tree - .get_instance(root_id) - .unwrap() - .instance - .get_children_ids(); + let root_instance = tree.get_instance(root_id).unwrap(); + let top_level_ids = root_instance.children(); rbx_binary::encode(tree.inner(), top_level_ids, &mut file)?; } diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index c231746a..4dface7b 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -11,23 +11,31 @@ pub struct InstanceMetadata { /// manage. pub ignore_unknown_instances: bool, - /// A complete view of where this snapshot came from. It should contain - /// enough information, if not None, to recreate this snapshot - /// deterministically assuming the source has not changed state. - pub source: Option, + // TODO: Make source_path a SmallVec in order to support meta + // files? Maybe we should use another member of the snapshot middleware to + // support damage-painting + /// The path that this file came from, if it's the top-level instance from a + /// model that came directly from disk. + /// + /// This path is used to make sure that file changes update all instances + /// that may need updates.. + pub source_path: Option, + + /// If this instance was defined in a project file, this is the name from + /// the project file and the node under it. + /// + /// This information is used to make sure the instance has the correct name, + /// project-added children, and metadata when it's updated in response to a + /// file change. + pub project_node: Option<(String, ProjectNode)>, } impl Default for InstanceMetadata { fn default() -> Self { InstanceMetadata { ignore_unknown_instances: false, - source: None, + source_path: None, + project_node: None, } } } - -#[derive(Debug, Clone, PartialEq)] -pub struct InstanceSource { - pub path: PathBuf, - pub project_node: Option<(String, ProjectNode)>, -} diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 3dbd1f62..78acc584 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -100,6 +100,10 @@ fn apply_update_child( tree: &mut RojoTree, patch: &PatchUpdateInstance, ) { + if let Some(metadata) = &patch.changed_metadata { + tree.update_metadata(patch.id, metadata.clone()); + } + let mut instance = tree .get_instance_mut(patch.id) .expect("Instance referred to by patch does not exist"); @@ -128,10 +132,7 @@ fn apply_update_child( ); } Some(value) => { - instance - .instance - .properties - .insert(key.clone(), value.clone()); + instance.properties_mut().insert(key.clone(), value.clone()); } None => { instance.properties_mut().remove(key); diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index bd87dc7d..13b308b9 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -3,11 +3,11 @@ use std::collections::{HashMap, HashSet}; -use rbx_dom_weak::{RbxId, RbxInstance, RbxValue}; +use rbx_dom_weak::{RbxId, RbxValue}; use super::{ patch::{PatchAddInstance, PatchSet, PatchUpdateInstance}, - InstanceSnapshot, RojoTree, + InstanceSnapshot, InstanceWithMeta, RojoTree, }; pub fn compute_patch_set<'a>( @@ -84,34 +84,40 @@ fn compute_patch_set_internal<'a>( .get_instance(id) .expect("Instance did not exist in tree"); - compute_property_patches(snapshot, &instance.instance, patch_set); + compute_property_patches(snapshot, &instance, patch_set); compute_children_patches(context, snapshot, tree, id, patch_set); } fn compute_property_patches( snapshot: &InstanceSnapshot, - instance: &RbxInstance, + instance: &InstanceWithMeta, patch_set: &mut PatchSet, ) { let mut visited_properties = HashSet::new(); let mut changed_properties = HashMap::new(); - let changed_name = if snapshot.name == instance.name { + let changed_name = if snapshot.name == instance.name() { None } else { Some(snapshot.name.clone().into_owned()) }; - let changed_class_name = if snapshot.class_name == instance.class_name { + let changed_class_name = if snapshot.class_name == instance.class_name() { None } else { Some(snapshot.class_name.clone().into_owned()) }; + let changed_metadata = if &snapshot.metadata == instance.metadata() { + None + } else { + Some(snapshot.metadata.clone()) + }; + for (name, snapshot_value) in &snapshot.properties { visited_properties.insert(name.as_str()); - match instance.properties.get(name) { + match instance.properties().get(name) { Some(instance_value) => { if snapshot_value != instance_value { changed_properties.insert(name.clone(), Some(snapshot_value.clone())); @@ -123,7 +129,7 @@ fn compute_property_patches( } } - for name in instance.properties.keys() { + for name in instance.properties().keys() { if visited_properties.contains(name.as_str()) { continue; } @@ -131,16 +137,20 @@ fn compute_property_patches( changed_properties.insert(name.clone(), None); } - if changed_properties.is_empty() && changed_name.is_none() { + if changed_properties.is_empty() + && changed_name.is_none() + && changed_class_name.is_none() + && changed_metadata.is_none() + { return; } patch_set.updated_instances.push(PatchUpdateInstance { - id: instance.get_id(), + id: instance.id(), changed_name, changed_class_name, changed_properties, - changed_metadata: None, + changed_metadata, }); } diff --git a/src/snapshot/tree.rs b/src/snapshot/tree.rs index 00fb8ba4..374def45 100644 --- a/src/snapshot/tree.rs +++ b/src/snapshot/tree.rs @@ -99,9 +99,38 @@ impl RojoTree { } } + /// Replaces the metadata associated with the given instance ID. + pub fn update_metadata(&mut self, id: RbxId, metadata: InstanceMetadata) { + use std::collections::hash_map::Entry; + + match self.metadata_map.entry(id) { + Entry::Occupied(mut entry) => { + let existing_metadata = entry.get(); + + // If this instance's source path changed, we need to update our + // path associations so that file changes will trigger updates + // to this instance correctly. + if existing_metadata.source_path != metadata.source_path { + if let Some(existing_path) = &existing_metadata.source_path { + self.path_to_ids.remove(existing_path, id); + } + + if let Some(new_path) = &metadata.source_path { + self.path_to_ids.insert(new_path.clone(), id); + } + } + + entry.insert(metadata); + } + Entry::Vacant(entry) => { + entry.insert(metadata); + } + } + } + fn insert_metadata(&mut self, id: RbxId, metadata: InstanceMetadata) { - if let Some(source) = &metadata.source { - self.path_to_ids.insert(source.path.clone(), id); + if let Some(source_path) = &metadata.source_path { + self.path_to_ids.insert(source_path.clone(), id); } self.metadata_map.insert(id, metadata); @@ -117,15 +146,16 @@ impl RojoTree { ) { let metadata = self.metadata_map.remove(&id).unwrap(); - if let Some(source) = &metadata.source { - self.path_to_ids.remove(&source.path, id); - path_to_ids.insert(source.path.clone(), id); + if let Some(source_path) = &metadata.source_path { + self.path_to_ids.remove(source_path, id); + path_to_ids.insert(source_path.clone(), id); } metadata_map.insert(id, metadata); } } +/// RojoTree's equivalent of `RbxInstanceProperties`. #[derive(Debug, Clone)] pub struct InstancePropertiesWithMeta { pub properties: RbxInstanceProperties, @@ -141,13 +171,22 @@ impl InstancePropertiesWithMeta { } } -#[derive(Debug)] +/// RojoTree's equivalent of `&'a RbxInstance`. +/// +/// This has to be a value type for RojoTree because the instance and metadata +/// are stored in different places. The mutable equivalent is +/// `InstanceWithMetaMut`. +#[derive(Debug, Clone, Copy)] pub struct InstanceWithMeta<'a> { - pub instance: &'a RbxInstance, - pub metadata: &'a InstanceMetadata, + instance: &'a RbxInstance, + metadata: &'a InstanceMetadata, } impl InstanceWithMeta<'_> { + pub fn id(&self) -> RbxId { + self.instance.get_id() + } + pub fn name(&self) -> &str { &self.instance.name } @@ -163,15 +202,28 @@ impl InstanceWithMeta<'_> { pub fn children(&self) -> &[RbxId] { self.instance.get_children_ids() } + + pub fn metadata(&self) -> &InstanceMetadata { + &self.metadata + } } +/// RojoTree's equivalent of `&'a mut RbxInstance`. +/// +/// This has to be a value type for RojoTree because the instance and metadata +/// are stored in different places. The immutable equivalent is +/// `InstanceWithMeta`. #[derive(Debug)] pub struct InstanceWithMetaMut<'a> { - pub instance: &'a mut RbxInstance, - pub metadata: &'a mut InstanceMetadata, + instance: &'a mut RbxInstance, + metadata: &'a mut InstanceMetadata, } impl InstanceWithMetaMut<'_> { + pub fn id(&self) -> RbxId { + self.instance.get_id() + } + pub fn name(&self) -> &str { &self.instance.name } @@ -199,4 +251,8 @@ impl InstanceWithMetaMut<'_> { pub fn children(&self) -> &[RbxId] { self.instance.get_children_ids() } + + pub fn metadata(&self) -> &InstanceMetadata { + &self.metadata + } }