From fd1c6ed0cf33f396c5559ac3b97a524eb0c2992f Mon Sep 17 00:00:00 2001
From: Nova <technobaboo@gmail.com>
Date: Fri, 13 Jun 2025 18:42:59 -0700
Subject: [PATCH] fix: make spatial parenting more stable

---
 src/nodes/root.rs         |  2 +-
 src/nodes/spatial/mod.rs  | 36 ++++++++++--------------------------
 src/nodes/spatial/zone.rs |  5 ++++-
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/src/nodes/root.rs b/src/nodes/root.rs
index aae066c..1f7d089 100644
--- a/src/nodes/root.rs
+++ b/src/nodes/root.rs
@@ -47,7 +47,7 @@ impl Root {
 
 	pub fn set_transform(&self, transform: Mat4) {
 		let spatial = self.node.get_aspect::<Spatial>().unwrap();
-		spatial.set_spatial_parent(None).unwrap();
+		// spatial.set_spatial_parent(None).unwrap();
 		spatial.set_local_transform(transform);
 	}
 	pub async fn save_state(&self) -> Result<ClientState> {
diff --git a/src/nodes/spatial/mod.rs b/src/nodes/spatial/mod.rs
index 6bb2de9..d78bf04 100644
--- a/src/nodes/spatial/mod.rs
+++ b/src/nodes/spatial/mod.rs
@@ -192,45 +192,29 @@ impl Spatial {
 	fn get_parent(&self) -> Option<Arc<Spatial>> {
 		self.parent.lock().clone()
 	}
-	fn set_parent(self: &Arc<Self>, new_parent: Option<&Arc<Spatial>>) {
+	fn set_parent(self: &Arc<Self>, new_parent: &Arc<Spatial>) {
 		if let Some(parent) = self.get_parent() {
 			parent.children.remove(self);
 		}
-		if let Some(new_parent) = &new_parent {
-			new_parent.children.add_raw(self);
-		}
+		new_parent.children.add_raw(self);
 
-		*self.parent.lock() = new_parent.cloned();
+		*self.parent.lock() = Some(new_parent.clone());
 	}
 
-	pub fn set_spatial_parent(self: &Arc<Self>, parent: Option<&Arc<Spatial>>) -> Result<()> {
-		let is_ancestor = parent
-			.as_ref()
-			.map(|parent| self.is_ancestor_of((*parent).clone()))
-			.unwrap_or(false);
-		if is_ancestor {
+	pub fn set_spatial_parent(self: &Arc<Self>, parent: &Arc<Spatial>) -> Result<()> {
+		if self.is_ancestor_of(parent.clone()) {
 			bail!("Setting spatial parent would cause a loop");
 		}
 		self.set_parent(parent);
 
 		Ok(())
 	}
-	pub fn set_spatial_parent_in_place(
-		self: &Arc<Self>,
-		parent: Option<&Arc<Spatial>>,
-	) -> Result<()> {
-		let is_ancestor = parent
-			.as_ref()
-			.map(|parent| self.is_ancestor_of((*parent).clone()))
-			.unwrap_or(false);
-		if is_ancestor {
+	pub fn set_spatial_parent_in_place(self: &Arc<Self>, parent: &Arc<Spatial>) -> Result<()> {
+		if self.is_ancestor_of(parent.clone()) {
 			bail!("Setting spatial parent would cause a loop");
 		}
 
-		self.set_local_transform(Spatial::space_to_space_matrix(
-			Some(self),
-			parent.map(AsRef::as_ref),
-		));
+		self.set_local_transform(Spatial::space_to_space_matrix(Some(self), Some(parent)));
 		self.set_parent(parent);
 
 		Ok(())
@@ -282,7 +266,7 @@ impl SpatialAspect for Spatial {
 		let this_spatial = node.get_aspect::<Spatial>()?;
 		let parent = parent.get_aspect::<Spatial>()?;
 
-		this_spatial.set_spatial_parent(Some(&parent))?;
+		this_spatial.set_spatial_parent(&parent)?;
 		Ok(())
 	}
 
@@ -294,7 +278,7 @@ impl SpatialAspect for Spatial {
 		let this_spatial = node.get_aspect::<Spatial>()?;
 		let parent = parent.get_aspect::<Spatial>()?;
 
-		this_spatial.set_spatial_parent_in_place(Some(&parent))?;
+		this_spatial.set_spatial_parent_in_place(&parent)?;
 		Ok(())
 	}
 
diff --git a/src/nodes/spatial/zone.rs b/src/nodes/spatial/zone.rs
index 4743797..a4a75a3 100644
--- a/src/nodes/spatial/zone.rs
+++ b/src/nodes/spatial/zone.rs
@@ -45,7 +45,10 @@ pub fn release(spatial: &Spatial) {
 	};
 	let spatial = spatial_node.get_aspect::<Spatial>().unwrap();
 
-	let _ = spatial.set_spatial_parent_in_place(spatial.old_parent.lock().take().as_ref());
+	let Some(old_parent) = spatial.old_parent.lock().take() else {
+		return;
+	};
+	let _ = spatial.set_spatial_parent_in_place(&old_parent);
 	let mut spatial_zone = spatial.zone.lock();
 
 	if let Some(spatial_zone) = spatial_zone.upgrade() {