From ad3999066d80fe09ae36450d193d64bf69588d33 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Wed, 2 Jan 2019 15:16:23 -0800 Subject: [PATCH] Expand diagnostics and exploratively fix some edge cases --- plugin/src/Session.lua | 8 ++++++- server/src/path_map.rs | 4 ++-- server/src/rbx_session.rs | 45 +++++++++++--------------------------- server/src/rbx_snapshot.rs | 9 ++++++++ server/src/web.rs | 6 +++++ 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/plugin/src/Session.lua b/plugin/src/Session.lua index 7d82c659..79b70b30 100644 --- a/plugin/src/Session.lua +++ b/plugin/src/Session.lua @@ -21,6 +21,8 @@ local function makeInstanceMap() if instance ~= nil then self.fromIds[id] = nil self.fromInstances[instance] = nil + else + Logging.warn("Attempted to remove nonexistant ID %s", tostring(id)) end end @@ -30,12 +32,14 @@ local function makeInstanceMap() if id ~= nil then self.fromInstances[instance] = nil self.fromIds[id] = nil + else + Logging.warn("Attempted to remove nonexistant instance %s", tostring(instance)) end end function self:destroyId(id) - self:removeId(id) local instance = self.fromIds[id] + self:removeId(id) if instance ~= nil then local descendantsToDestroy = {} @@ -51,6 +55,8 @@ local function makeInstanceMap() end instance:Destroy() + else + Logging.warn("Attempted to destroy nonexistant ID %s", tostring(id)) end end diff --git a/server/src/path_map.rs b/server/src/path_map.rs index ebb23364..b32302e2 100644 --- a/server/src/path_map.rs +++ b/server/src/path_map.rs @@ -3,7 +3,7 @@ use std::{ collections::{HashMap, HashSet}, }; -#[derive(Debug)] +#[derive(Debug, Serialize)] struct PathMapNode { value: T, children: HashSet, @@ -11,7 +11,7 @@ struct PathMapNode { /// A map from paths to instance IDs, with a bit of additional data that enables /// removing a path and all of its child paths from the tree more quickly. -#[derive(Debug)] +#[derive(Debug, Serialize)] pub struct PathMap { nodes: HashMap>, } diff --git a/server/src/rbx_session.rs b/server/src/rbx_session.rs index 218aa8fb..dd5d1345 100644 --- a/server/src/rbx_session.rs +++ b/server/src/rbx_session.rs @@ -66,7 +66,9 @@ impl RbxSession { trace!("Snapshotting path {}", closest_path.display()); let snapshot = snapshot_instances_from_imfs(&imfs, &closest_path, &mut self.sync_point_names) - .expect("Could not generate instance snapshot"); + .unwrap_or_else(|| panic!("Could not generate instance snapshot for path {}", closest_path.display())); + + trace!("Snapshot: {:#?}", snapshot); reconcile_subtree(&mut self.tree, instance_id, &snapshot, &mut self.path_map, &mut self.instance_metadata_map, &mut changes); } @@ -106,40 +108,14 @@ impl RbxSession { pub fn path_removed(&mut self, path: &Path) { info!("Path removed: {}", path.display()); - - let instance_id = match self.path_map.remove(path) { - Some(id) => id, - None => { - // TODO: init.lua files won't be in the path map, that should be - // patched up! - trace!("Path was not in path map, ignoring removal."); - return; - }, - }; - - let removed_subtree = match self.tree.remove_instance(instance_id) { - Some(tree) => tree, - None => { - warn!("Rojo tried to remove an instance that was half cleaned-up. This is probably a bug in Rojo."); - return; - }, - }; - - let changes = InstanceChanges { - added: HashSet::new(), - removed: removed_subtree.iter_all_ids().collect(), - updated: HashSet::new(), - }; - - trace!("Pushing changes: {}", changes); - - self.message_queue.push_messages(&[changes]); + self.path_map.remove(path); + self.path_created_or_updated(path); } pub fn path_renamed(&mut self, from_path: &Path, to_path: &Path) { info!("Path renamed from {} to {}", from_path.display(), to_path.display()); - self.path_removed(from_path); - self.path_created(to_path); + self.path_created_or_updated(from_path); + self.path_created_or_updated(to_path); } pub fn get_tree(&self) -> &RbxTree { @@ -149,6 +125,10 @@ impl RbxSession { pub fn get_instance_metadata_map(&self) -> &HashMap { &self.instance_metadata_map } + + pub fn debug_get_path_map(&self) -> &PathMap { + &self.path_map + } } pub fn construct_oneoff_tree(project: &Project, imfs: &Imfs) -> RbxTree { @@ -291,6 +271,7 @@ fn snapshot_instances_from_imfs<'a>( }) }, ImfsItem::Directory(directory) => { + // TODO: Expand init support to handle server and client scripts let init_path = directory.path.join("init.lua"); let mut instance = if directory.children.contains(&init_path) { @@ -306,7 +287,7 @@ fn snapshot_instances_from_imfs<'a>( } }; - instance.name = if let Some(actual_name) = sync_point_names.get(imfs_path) { + instance.name = if let Some(actual_name) = sync_point_names.get(&directory.path) { Cow::Owned(actual_name.clone()) } else { Cow::Borrowed(directory.path.file_name()?.to_str()?) diff --git a/server/src/rbx_snapshot.rs b/server/src/rbx_snapshot.rs index 9cd43bb7..768c4c5b 100644 --- a/server/src/rbx_snapshot.rs +++ b/server/src/rbx_snapshot.rs @@ -55,6 +55,7 @@ impl InstanceChanges { } } +#[derive(Debug)] pub struct RbxSnapshotInstance<'a> { pub name: Cow<'a, str>, pub class_name: Cow<'a, str>, @@ -125,6 +126,14 @@ pub fn reconcile_subtree( instance_metadata_map: &mut HashMap, changes: &mut InstanceChanges, ) { + if let Some(source_path) = &snapshot.source_path { + path_map.insert(source_path.clone(), id); + } + + if let Some(metadata) = &snapshot.metadata { + instance_metadata_map.insert(id, metadata.clone()); + } + if reconcile_instance_properties(tree.get_instance_mut(id).unwrap(), snapshot) { changes.updated.insert(id); } diff --git a/server/src/web.rs b/server/src/web.rs index 5467b3cc..679c1881 100644 --- a/server/src/web.rs +++ b/server/src/web.rs @@ -178,6 +178,12 @@ impl Server { Response::svg(graphviz_to_svg(&dot_source)) }, + (GET) (/visualize/path_map) => { + let rbx_session = self.session.rbx_session.lock().unwrap(); + + Response::json(&rbx_session.debug_get_path_map()) + }, + _ => Response::empty_404() ) }