From 824b984a64cef6054a92bc60e766ef8e42afbd13 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Mon, 9 Sep 2019 13:50:39 -0700 Subject: [PATCH] First pass converting snapshot code over to RojoTree from RbxTree --- src/commands/build.rs | 37 ++++++++++----- src/commands/serve.rs | 15 +++--- src/commands/upload.rs | 17 ++++--- src/snapshot/patch_apply.rs | 89 ++++++++++++++++++++--------------- src/snapshot/patch_compute.rs | 42 ++++++++++------- src/snapshot/tree.rs | 31 ++++++++---- 6 files changed, 143 insertions(+), 88 deletions(-) diff --git a/src/commands/build.rs b/src/commands/build.rs index 453b42f6..5b745111 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -6,11 +6,11 @@ use std::{ }; use failure::Fail; -use rbx_dom_weak::{RbxInstanceProperties, RbxTree}; +use rbx_dom_weak::RbxInstanceProperties; use crate::{ imfs::new::{FsError, Imfs, RealFetcher, WatchMode}, - snapshot::{apply_patch_set, compute_patch_set}, + snapshot::{apply_patch_set, compute_patch_set, InstancePropertiesWithMeta, RojoTree}, snapshot_middleware::snapshot_from_imfs, }; @@ -78,10 +78,13 @@ pub fn build(options: &BuildOptions) -> Result<(), BuildError> { log::info!("Hoping to generate file of type {:?}", output_kind); - let mut tree = RbxTree::new(RbxInstanceProperties { - name: "ROOT".to_owned(), - class_name: "Folder".to_owned(), - properties: HashMap::new(), + let mut tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "ROOT".to_owned(), + class_name: "Folder".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); @@ -112,25 +115,35 @@ pub fn build(options: &BuildOptions) -> Result<(), BuildError> { // Model files include the root instance of the tree and all its // descendants. - rbx_xml::to_writer(&mut file, &tree, &[root_id], xml_encode_config())?; + rbx_xml::to_writer(&mut file, tree.inner(), &[root_id], xml_encode_config())?; } OutputKind::Rbxlx => { // 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().get_children_ids(); - rbx_xml::to_writer(&mut file, &tree, top_level_ids, xml_encode_config())?; + let top_level_ids = tree + .get_instance(root_id) + .unwrap() + .instance + .get_children_ids(); + + rbx_xml::to_writer(&mut file, tree.inner(), top_level_ids, xml_encode_config())?; } OutputKind::Rbxm => { - rbx_binary::encode(&tree, &[root_id], &mut file)?; + rbx_binary::encode(tree.inner(), &[root_id], &mut file)?; } OutputKind::Rbxl => { log::warn!("Support for building binary places (rbxl) is still experimental."); 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().get_children_ids(); - rbx_binary::encode(&tree, top_level_ids, &mut file)?; + let top_level_ids = tree + .get_instance(root_id) + .unwrap() + .instance + .get_children_ids(); + + rbx_binary::encode(tree.inner(), top_level_ids, &mut file)?; } } diff --git a/src/commands/serve.rs b/src/commands/serve.rs index ac2dab5a..9d6c89d5 100644 --- a/src/commands/serve.rs +++ b/src/commands/serve.rs @@ -1,13 +1,13 @@ use std::{collections::HashMap, path::PathBuf, sync::Arc}; use failure::Fail; -use rbx_dom_weak::{RbxInstanceProperties, RbxTree}; +use rbx_dom_weak::RbxInstanceProperties; use crate::{ imfs::new::{Imfs, RealFetcher, WatchMode}, project::{Project, ProjectLoadError}, serve_session::ServeSession, - snapshot::{apply_patch_set, compute_patch_set}, + snapshot::{apply_patch_set, compute_patch_set, InstancePropertiesWithMeta, RojoTree}, snapshot_middleware::snapshot_from_imfs, web::LiveServer, }; @@ -48,10 +48,13 @@ pub fn serve(options: &ServeOptions) -> Result<(), ServeError> { println!("Rojo server listening on port {}", port); - let mut tree = RbxTree::new(RbxInstanceProperties { - name: "ROOT".to_owned(), - class_name: "Folder".to_owned(), - properties: HashMap::new(), + let mut tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "ROOT".to_owned(), + class_name: "Folder".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); diff --git a/src/commands/upload.rs b/src/commands/upload.rs index 166f293b..5909e202 100644 --- a/src/commands/upload.rs +++ b/src/commands/upload.rs @@ -1,13 +1,13 @@ use std::{collections::HashMap, path::PathBuf}; use failure::Fail; -use rbx_dom_weak::{RbxInstanceProperties, RbxTree}; +use rbx_dom_weak::RbxInstanceProperties; use reqwest::header::{ACCEPT, CONTENT_TYPE, COOKIE, USER_AGENT}; use crate::{ auth_cookie::get_auth_cookie, imfs::new::{Imfs, RealFetcher, WatchMode}, - snapshot::{apply_patch_set, compute_patch_set}, + snapshot::{apply_patch_set, compute_patch_set, InstancePropertiesWithMeta, RojoTree}, snapshot_middleware::snapshot_from_imfs, }; @@ -45,10 +45,13 @@ pub fn upload(options: UploadOptions) -> Result<(), UploadError> { .or_else(get_auth_cookie) .ok_or(UploadError::NeedAuthCookie)?; - let mut tree = RbxTree::new(RbxInstanceProperties { - name: "ROOT".to_owned(), - class_name: "Folder".to_owned(), - properties: HashMap::new(), + let mut tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "ROOT".to_owned(), + class_name: "Folder".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); @@ -77,7 +80,7 @@ pub fn upload(options: UploadOptions) -> Result<(), UploadError> { let config = rbx_xml::EncodeOptions::new() .property_behavior(rbx_xml::EncodePropertyBehavior::WriteUnknown); - rbx_xml::to_writer(&mut buffer, &tree, &[root_id], config)?; + rbx_xml::to_writer(&mut buffer, tree.inner(), &[root_id], config)?; let url = format!( "https://data.roblox.com/Data/Upload.ashx?assetid={}", diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 3c52da95..ed97940c 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -2,14 +2,14 @@ use std::collections::HashMap; -use rbx_dom_weak::{RbxId, RbxInstanceProperties, RbxTree, RbxValue}; +use rbx_dom_weak::{RbxId, RbxInstanceProperties, RbxValue}; use super::{ patch::{PatchSet, PatchUpdateInstance}, - InstanceSnapshot, + InstancePropertiesWithMeta, InstanceSnapshot, RojoTree, }; -pub fn apply_patch_set(tree: &mut RbxTree, patch_set: &PatchSet) { +pub fn apply_patch_set(tree: &mut RojoTree, patch_set: &PatchSet) { let mut context = PatchApplyContext::default(); for removed_id in &patch_set.removed_instances { @@ -42,7 +42,7 @@ struct PatchApplyContext { /// The remaining Ref properties need to be handled during patch application, /// where we build up a map of snapshot IDs to instance IDs as they're created, /// then apply properties all at once at the end. -fn apply_deferred_properties(context: PatchApplyContext, tree: &mut RbxTree) { +fn apply_deferred_properties(context: PatchApplyContext, tree: &mut RojoTree) { for (id, mut properties) in context.properties_to_apply { let instance = tree .get_instance_mut(id) @@ -58,23 +58,26 @@ fn apply_deferred_properties(context: PatchApplyContext, tree: &mut RbxTree) { } } - instance.properties = properties; + instance.instance.properties = properties; } } fn apply_add_child( context: &mut PatchApplyContext, - tree: &mut RbxTree, + tree: &mut RojoTree, parent_id: RbxId, snapshot: &InstanceSnapshot, ) { - let properties = RbxInstanceProperties { - name: snapshot.name.clone().into_owned(), - class_name: snapshot.class_name.clone().into_owned(), + let properties = InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: snapshot.name.clone().into_owned(), + class_name: snapshot.class_name.clone().into_owned(), - // Property assignment is deferred until after we know about all - // instances in this patch. - properties: HashMap::new(), + // Property assignment is deferred until after we know about all + // instances in this patch. + properties: HashMap::new(), + }, + metadata: Default::default(), // TODO }; let id = tree.insert_instance(properties, parent_id); @@ -94,7 +97,7 @@ fn apply_add_child( fn apply_update_child( context: &PatchApplyContext, - tree: &mut RbxTree, + tree: &mut RojoTree, patch: &PatchUpdateInstance, ) { let instance = tree @@ -102,11 +105,11 @@ fn apply_update_child( .expect("Instance referred to by patch does not exist"); if let Some(name) = &patch.changed_name { - instance.name = name.clone(); + instance.instance.name = name.clone(); } if let Some(class_name) = &patch.changed_class_name { - instance.class_name = class_name.clone(); + instance.instance.class_name = class_name.clone(); } for (key, property_entry) in &patch.changed_properties { @@ -117,7 +120,7 @@ fn apply_update_child( Some(RbxValue::Ref { value: Some(id) }) => { let new_id = context.snapshot_id_to_instance_id.get(id).unwrap_or(id); - instance.properties.insert( + instance.instance.properties.insert( key.clone(), RbxValue::Ref { value: Some(*new_id), @@ -125,10 +128,13 @@ fn apply_update_child( ); } Some(value) => { - instance.properties.insert(key.clone(), value.clone()); + instance + .instance + .properties + .insert(key.clone(), value.clone()); } None => { - instance.properties.remove(key); + instance.instance.properties.remove(key); } } } @@ -149,10 +155,13 @@ mod test { fn add_from_empty() { let _ = env_logger::try_init(); - let mut tree = RbxTree::new(RbxInstanceProperties { - name: "Folder".to_owned(), - class_name: "Folder".to_owned(), - properties: HashMap::new(), + let mut tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "Folder".to_owned(), + class_name: "Folder".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); @@ -179,27 +188,33 @@ mod test { apply_patch_set(&mut tree, &patch_set); let root_instance = tree.get_instance(root_id).unwrap(); - let child_id = root_instance.get_children_ids()[0]; + let child_id = root_instance.instance.get_children_ids()[0]; let child_instance = tree.get_instance(child_id).unwrap(); - assert_eq!(child_instance.name.as_str(), &snapshot.name); - assert_eq!(child_instance.class_name.as_str(), &snapshot.class_name); - assert_eq!(&child_instance.properties, &snapshot.properties); - assert!(child_instance.get_children_ids().is_empty()); + assert_eq!(child_instance.instance.name.as_str(), &snapshot.name); + assert_eq!( + child_instance.instance.class_name.as_str(), + &snapshot.class_name + ); + assert_eq!(&child_instance.instance.properties, &snapshot.properties); + assert!(child_instance.instance.get_children_ids().is_empty()); } #[test] fn update_existing() { let _ = env_logger::try_init(); - let mut tree = RbxTree::new(RbxInstanceProperties { - name: "OldName".to_owned(), - class_name: "OldClassName".to_owned(), - properties: hashmap! { - "Foo".to_owned() => RbxValue::Int32 { value: 7 }, - "Bar".to_owned() => RbxValue::Int32 { value: 3 }, - "Unchanged".to_owned() => RbxValue::Int32 { value: -5 }, + let mut tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "OldName".to_owned(), + class_name: "OldClassName".to_owned(), + properties: hashmap! { + "Foo".to_owned() => RbxValue::Int32 { value: 7 }, + "Bar".to_owned() => RbxValue::Int32 { value: 3 }, + "Unchanged".to_owned() => RbxValue::Int32 { value: -5 }, + }, }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); @@ -235,8 +250,8 @@ mod test { }; let root_instance = tree.get_instance(root_id).unwrap(); - assert_eq!(root_instance.name, "Foo"); - assert_eq!(root_instance.class_name, "NewClassName"); - assert_eq!(root_instance.properties, expected_properties); + assert_eq!(root_instance.instance.name, "Foo"); + assert_eq!(root_instance.instance.class_name, "NewClassName"); + assert_eq!(root_instance.instance.properties, expected_properties); } } diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index a5227c57..2fa9bb61 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -3,16 +3,16 @@ use std::collections::{HashMap, HashSet}; -use rbx_dom_weak::{RbxId, RbxInstance, RbxTree, RbxValue}; +use rbx_dom_weak::{RbxId, RbxInstance, RbxValue}; use super::{ patch::{PatchAddInstance, PatchSet, PatchUpdateInstance}, - InstanceSnapshot, + InstanceSnapshot, RojoTree, }; pub fn compute_patch_set<'a>( snapshot: &'a InstanceSnapshot, - tree: &RbxTree, + tree: &RojoTree, id: RbxId, ) -> PatchSet<'a> { let mut patch_set = PatchSet::new(); @@ -72,7 +72,7 @@ fn rewrite_refs_in_snapshot(context: &ComputePatchContext, snapshot: &mut Instan fn compute_patch_set_internal<'a>( context: &mut ComputePatchContext, snapshot: &'a InstanceSnapshot, - tree: &RbxTree, + tree: &RojoTree, id: RbxId, patch_set: &mut PatchSet<'a>, ) { @@ -84,7 +84,7 @@ fn compute_patch_set_internal<'a>( .get_instance(id) .expect("Instance did not exist in tree"); - compute_property_patches(snapshot, instance, patch_set); + compute_property_patches(snapshot, &instance.instance, patch_set); compute_children_patches(context, snapshot, tree, id, patch_set); } @@ -147,7 +147,7 @@ fn compute_property_patches( fn compute_children_patches<'a>( context: &mut ComputePatchContext, snapshot: &'a InstanceSnapshot, - tree: &RbxTree, + tree: &RojoTree, id: RbxId, patch_set: &mut PatchSet<'a>, ) { @@ -155,7 +155,7 @@ fn compute_children_patches<'a>( .get_instance(id) .expect("Instance did not exist in tree"); - let instance_children = instance.get_children_ids(); + let instance_children = instance.instance.get_children_ids(); let mut paired_instances = vec![false; instance_children.len()]; @@ -173,8 +173,8 @@ fn compute_children_patches<'a>( .get_instance(**instance_child_id) .expect("Instance did not exist in tree"); - if snapshot_child.name == instance_child.name - && snapshot_child.class_name == instance_child.class_name + if snapshot_child.name == instance_child.instance.name + && snapshot_child.class_name == instance_child.instance.class_name { paired_instances[*instance_index] = true; return true; @@ -220,16 +220,21 @@ mod test { use maplit::hashmap; use rbx_dom_weak::RbxInstanceProperties; + use super::super::InstancePropertiesWithMeta; + /// This test makes sure that rewriting refs in instance update patches to /// instances that already exists works. We should be able to correlate the /// snapshot ID and instance ID during patch computation and replace the /// value before returning from compute_patch_set. #[test] fn rewrite_ref_existing_instance_update() { - let tree = RbxTree::new(RbxInstanceProperties { - name: "foo".to_owned(), - class_name: "foo".to_owned(), - properties: HashMap::new(), + let tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "foo".to_owned(), + class_name: "foo".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); @@ -277,10 +282,13 @@ mod test { /// one. #[test] fn rewrite_ref_existing_instance_addition() { - let tree = RbxTree::new(RbxInstanceProperties { - name: "foo".to_owned(), - class_name: "foo".to_owned(), - properties: HashMap::new(), + let tree = RojoTree::new(InstancePropertiesWithMeta { + properties: RbxInstanceProperties { + name: "foo".to_owned(), + class_name: "foo".to_owned(), + properties: HashMap::new(), + }, + metadata: Default::default(), }); let root_id = tree.get_root_id(); diff --git a/src/snapshot/tree.rs b/src/snapshot/tree.rs index 3df2e3dc..e91afec1 100644 --- a/src/snapshot/tree.rs +++ b/src/snapshot/tree.rs @@ -32,7 +32,7 @@ pub struct RojoTree { impl RojoTree { pub fn new(root: InstancePropertiesWithMeta) -> RojoTree { let mut tree = RojoTree { - inner: RbxTree::new(root.inner), + inner: RbxTree::new(root.properties), metadata_map: HashMap::new(), path_to_ids: MultiMap::new(), }; @@ -41,25 +41,29 @@ impl RojoTree { tree } + pub fn inner(&self) -> &RbxTree { + &self.inner + } + pub fn get_root_id(&self) -> RbxId { self.inner.get_root_id() } pub fn get_instance(&self, id: RbxId) -> Option { - if let Some(inner) = self.inner.get_instance(id) { + if let Some(instance) = self.inner.get_instance(id) { let metadata = self.metadata_map.get(&id).unwrap(); - Some(InstanceWithMeta { inner, metadata }) + Some(InstanceWithMeta { instance, metadata }) } else { None } } pub fn get_instance_mut(&mut self, id: RbxId) -> Option { - if let Some(inner) = self.inner.get_instance_mut(id) { + if let Some(instance) = self.inner.get_instance_mut(id) { let metadata = self.metadata_map.get_mut(&id).unwrap(); - Some(InstanceWithMetaMut { inner, metadata }) + Some(InstanceWithMetaMut { instance, metadata }) } else { None } @@ -70,7 +74,7 @@ impl RojoTree { properties: InstancePropertiesWithMeta, parent_id: RbxId, ) -> RbxId { - let id = self.inner.insert_instance(properties.inner, parent_id); + let id = self.inner.insert_instance(properties.properties, parent_id); self.insert_metadata(id, properties.metadata); id } @@ -124,18 +128,27 @@ impl RojoTree { #[derive(Debug, Clone)] pub struct InstancePropertiesWithMeta { - pub inner: RbxInstanceProperties, + pub properties: RbxInstanceProperties, pub metadata: InstanceMetadata, } +impl InstancePropertiesWithMeta { + pub fn new(properties: RbxInstanceProperties, metadata: InstanceMetadata) -> Self { + InstancePropertiesWithMeta { + properties, + metadata, + } + } +} + #[derive(Debug)] pub struct InstanceWithMeta<'a> { - pub inner: &'a RbxInstance, + pub instance: &'a RbxInstance, pub metadata: &'a InstanceMetadata, } #[derive(Debug)] pub struct InstanceWithMetaMut<'a> { - pub inner: &'a mut RbxInstance, + pub instance: &'a mut RbxInstance, pub metadata: &'a mut InstanceMetadata, }