From 33be2ed7168ff454fdf0c77077eaa971f7099bc0 Mon Sep 17 00:00:00 2001 From: Kenneth Loeffler Date: Sat, 8 Jul 2023 02:14:04 -0700 Subject: [PATCH] Start using new rbx_dom_weak instance cloning methods (#62) --- Cargo.lock | 12 ++-- packages/lib-roblox/Cargo.toml | 10 +-- packages/lib-roblox/src/instance/mod.rs | 96 +++---------------------- 3 files changed, 21 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2628328..6bcad3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1535,7 +1535,7 @@ dependencies = [ [[package]] name = "rbx_binary" version = "0.7.0" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "log", "lz4", @@ -1566,7 +1566,7 @@ dependencies = [ [[package]] name = "rbx_dom_weak" version = "2.4.0" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "rbx_types", "serde", @@ -1575,7 +1575,7 @@ dependencies = [ [[package]] name = "rbx_reflection" version = "4.2.0" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "rbx_types", "serde", @@ -1585,7 +1585,7 @@ dependencies = [ [[package]] name = "rbx_reflection_database" version = "0.2.6+roblox-572" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "lazy_static", "rbx_reflection", @@ -1596,7 +1596,7 @@ dependencies = [ [[package]] name = "rbx_types" version = "1.5.0" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "base64 0.13.1", "bitflags 1.3.2", @@ -1610,7 +1610,7 @@ dependencies = [ [[package]] name = "rbx_xml" version = "0.13.0" -source = "git+https://github.com/rojo-rbx/rbx-dom?rev=b6d255e0b5d96155f694ca224676b251059cf2de#b6d255e0b5d96155f694ca224676b251059cf2de" +source = "git+https://github.com/rojo-rbx/rbx-dom?rev=e7a813d569c3f8a54be8a8873c33f8976c37b8b1#e7a813d569c3f8a54be8a8873c33f8976c37b8b1" dependencies = [ "base64 0.13.1", "log", diff --git a/packages/lib-roblox/Cargo.toml b/packages/lib-roblox/Cargo.toml index d221c66..985181a 100644 --- a/packages/lib-roblox/Cargo.toml +++ b/packages/lib-roblox/Cargo.toml @@ -22,11 +22,11 @@ thiserror.workspace = true glam = "0.24" rand = "0.8" -rbx_binary = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "b6d255e0b5d96155f694ca224676b251059cf2de" } -rbx_dom_weak = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "b6d255e0b5d96155f694ca224676b251059cf2de" } -rbx_reflection = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "b6d255e0b5d96155f694ca224676b251059cf2de" } -rbx_reflection_database = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "b6d255e0b5d96155f694ca224676b251059cf2de" } -rbx_xml = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "b6d255e0b5d96155f694ca224676b251059cf2de" } +rbx_binary = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "e7a813d569c3f8a54be8a8873c33f8976c37b8b1" } +rbx_dom_weak = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "e7a813d569c3f8a54be8a8873c33f8976c37b8b1" } +rbx_reflection = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "e7a813d569c3f8a54be8a8873c33f8976c37b8b1" } +rbx_reflection_database = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "e7a813d569c3f8a54be8a8873c33f8976c37b8b1" } +rbx_xml = { git = "https://github.com/rojo-rbx/rbx-dom", rev = "e7a813d569c3f8a54be8a8873c33f8976c37b8b1" } [dev-dependencies] anyhow = "1.0" diff --git a/packages/lib-roblox/src/instance/mod.rs b/packages/lib-roblox/src/instance/mod.rs index ac63c1c..87675b9 100644 --- a/packages/lib-roblox/src/instance/mod.rs +++ b/packages/lib-roblox/src/instance/mod.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeMap, HashMap, VecDeque}, + collections::{BTreeMap, VecDeque}, fmt, hash::{Hash, Hasher}, sync::RwLock, @@ -140,18 +140,14 @@ impl Instance { root of the weak dom, and return its referent. */ pub fn clone_into_external_dom(self, external_dom: &mut WeakDom) -> DomRef { - let cloned = self.clone_instance(); - - let mut dom = INTERNAL_DOM + let dom = INTERNAL_DOM .try_write() .expect("Failed to get write access to document"); - let internal_dom_ref = cloned.dom_ref; - let external_root_ref = external_dom.root_ref(); + let cloned = dom.clone_into_external(self.dom_ref, external_dom); + external_dom.transfer_within(cloned, external_dom.root_ref()); - dom.transfer(internal_dom_ref, external_dom, external_root_ref); - - internal_dom_ref + cloned } /** @@ -167,91 +163,19 @@ impl Instance { pub fn clone_instance(&self) -> Instance { // NOTE: We create a new scope here to avoid deadlocking since // our clone implementation must have exclusive write access - let parent_ref = { - INTERNAL_DOM - .try_read() - .expect("Failed to get read access to document") - .get_by_ref(self.dom_ref) - .expect("Failed to find instance in document") - .parent() - }; - - // Keep track of a map from old ref -> new ref for each - // instance so that we can then transform properties that - // are instance refs into ones pointing at the new instances - let mut reference_map = HashMap::new(); - - let new_ref = Self::clone_inner(self.dom_ref, parent_ref, &mut reference_map); - let new_inst = Self::new(new_ref); - - { + let new_ref = { let mut dom = INTERNAL_DOM .try_write() .expect("Failed to get write access to document"); - let new_refs = reference_map.values().clone().collect::>(); - for new_ref in new_refs { - let new_inst = dom - .get_by_ref_mut(*new_ref) - .expect("Failed to find cloned instance in document"); - for prop_value in new_inst.properties.values_mut() { - if let DomValue::Ref(prop_ref) = prop_value { - // NOTE: It is possible to get None here if the ref points to - // something outside of the newly cloned instance hierarchy - if let Some(new) = reference_map.get(prop_ref) { - *prop_value = DomValue::Ref(*new); - } - } - } - } - } + dom.clone_within(self.dom_ref) + }; + + let new_inst = Self::new(new_ref); new_inst.set_parent(None); new_inst } - pub fn clone_inner( - dom_ref: DomRef, - parent_ref: DomRef, - reference_map: &mut HashMap, - ) -> DomRef { - // NOTE: We create a new scope here to avoid deadlocking since - // our clone implementation must have exclusive write access - let (new_ref, child_refs) = { - let mut dom = INTERNAL_DOM - .try_write() - .expect("Failed to get write access to document"); - - let (new_class, new_name, new_props, child_refs) = { - let instance = dom - .get_by_ref(dom_ref) - .expect("Failed to find instance in document"); - ( - instance.class.to_string(), - instance.name.to_string(), - instance.properties.clone(), - instance.children().to_vec(), - ) - }; - - let new_ref = dom.insert( - parent_ref, - DomInstanceBuilder::new(new_class) - .with_name(new_name) - .with_properties(new_props), - ); - - reference_map.insert(dom_ref, new_ref); - - (new_ref, child_refs) - }; - - for child_ref in child_refs { - Self::clone_inner(child_ref, new_ref, reference_map); - } - - new_ref - } - /** Destroys the instance, removing it completely from the weak dom with no way of recovering it.