Serialize humanoid parent links last in rbx-binary

PR to merge this upstream is open, but this allows us to use it now.

rbx_binary/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Within `PRNT` chunks, `Humanoid` instances are now linked last to avoid potential load order problems. ([#411])
+
+[#411]: https://github.com/rojo-rbx/rbx-dom/pull/411
+
 ## 0.7.4 (2024-01-16)
 * Add the ability to specify a `ReflectionDatabase` to use for serializing and deserializing. This takes the form of `Deserializer::reflection_database` and `Serializer::reflection_database`. ([#375])
 

rbx_binary/Cargo.toml

@@ -23,6 +23,7 @@ lz4 = "1.23.3"
 thiserror = "1.0.31"
 serde = { version = "1.0.137", features = ["derive"], optional = true }
 profiling = "1.0.6"
+lazy_static = "1.4.0"
 
 [dev-dependencies]
 criterion = "0.3.5"

rbx_binary/src/serializer/mod.rs

@@ -1,7 +1,7 @@
 mod error;
 mod state;
 
-use std::io::Write;
+use std::{collections::HashSet, io::Write};
 
 use rbx_dom_weak::{types::Ref, WeakDom};
 use rbx_reflection::ReflectionDatabase;
@@ -10,6 +10,13 @@ use self::state::SerializerState;
 
 pub use self::error::Error;
 
+lazy_static::lazy_static! {
+    /// A list of classes that should be parented last in files to prevent them
+    /// potentially loading before their descendants do, which can cause
+    /// problems like reported in https://github.com/rojo-rbx/rbx-dom/issues/406.
+    pub static ref LOWERED_CLASSES: HashSet<&'static str> = ["Humanoid"].into();
+}
+
 /// A configurable serializer for Roblox binary models and places.
 ///
 /// ## Example

rbx_binary/src/serializer/state.rs

@@ -31,7 +31,7 @@ use crate::{
     Serializer,
 };
 
-use super::error::InnerError;
+use super::{error::InnerError, LOWERED_CLASSES};
 
 static FILE_FOOTER: &[u8] = b"</roblox>";
 
@@ -1246,29 +1246,40 @@ impl<'dom, 'db, W: Write> SerializerState<'dom, 'db, W> {
         chunk.write_u8(0)?; // PRNT version 0
         chunk.write_le_u32(self.relevant_instances.len() as u32)?;
 
-        let object_referents = self
-            .relevant_instances
-            .iter()
-            .map(|id| self.id_to_referent[id]);
-
-        let parent_referents = self.relevant_instances.iter().map(|id| {
-            let instance = self.dom.get_by_ref(*id).unwrap();
-
-            // If there's no parent set OR our parent is not one of the
-            // instances we're serializing, we use -1 to represent a null
-            // parent.
-            if instance.parent().is_some() {
-                self.id_to_referent
-                    .get(&instance.parent())
-                    .cloned()
-                    .unwrap_or(-1)
+        // We want certain classes to be parented after everything else to avoid
+        // problems with
+        let mut delayed = Vec::new();
+        let mut object_referents = Vec::with_capacity(self.relevant_instances.len());
+        let mut parent_referents = Vec::with_capacity(self.relevant_instances.len());
+
+        for referent in &self.relevant_instances {
+            let item = self.dom.get_by_ref(*referent).unwrap();
+            if LOWERED_CLASSES.contains(item.class.as_str()) {
+                delayed.push(*referent)
             } else {
-                -1
+                object_referents.push(self.id_to_referent[referent]);
+                let parent = item.parent();
+                if parent.is_some() {
+                    parent_referents.push(self.id_to_referent.get(&parent).copied().unwrap_or(-1));
+                } else {
+                    parent_referents.push(-1);
+                }
             }
-        });
+        }
+
+        for referent in delayed {
+            let item = self.dom.get_by_ref(referent).unwrap();
+            object_referents.push(self.id_to_referent[&referent]);
+            let parent = item.parent();
+            if parent.is_some() {
+                parent_referents.push(self.id_to_referent.get(&parent).copied().unwrap_or(-1));
+            } else {
+                parent_referents.push(-1);
+            }
+        }
 
-        chunk.write_referent_array(object_referents)?;
-        chunk.write_referent_array(parent_referents)?;
+        chunk.write_referent_array(object_referents.into_iter())?;
+        chunk.write_referent_array(parent_referents.into_iter())?;
 
         chunk.dump(&mut self.output)?;
 

rbx_binary/src/tests/serializer.rs

@@ -193,3 +193,22 @@ fn default_shared_string() {
     let decoded = DecodedModel::from_reader(buf.as_slice());
     insta::assert_yaml_snapshot!(decoded);
 }
+
+/// Test to assure that Humanoid links are generated last in the PRNT chunk.
+#[test]
+fn parent_humanoid_last() {
+    let mut tree = WeakDom::new(InstanceBuilder::new("Model"));
+    let humanoid = InstanceBuilder::new("Humanoid");
+    let humanoid_ref = tree.insert(Ref::none(), humanoid);
+    let descendant = InstanceBuilder::new("Motor6D");
+    let desc_ref = tree.insert(Ref::none(), descendant);
+
+    tree.transfer_within(humanoid_ref, tree.root_ref());
+    tree.transfer_within(desc_ref, humanoid_ref);
+
+    let mut buf = Vec::new();
+    to_writer(&mut buf, &tree, &[tree.root_ref()]).unwrap();
+
+    let decoded = DecodedModel::from_reader(buf.as_slice());
+    insta::assert_yaml_snapshot!(decoded);
+}

rbx_binary/src/tests/snapshots/rbx_binary__tests__serializer__parent_humanoid_last.snap

@@ -0,0 +1,55 @@
+---
+source: rbx_binary/src/tests/serializer.rs
+assertion_line: 213
+expression: decoded
+---
+num_types: 3
+num_instances: 3
+chunks:
+  - Inst:
+      type_id: 1
+      type_name: Humanoid
+      object_format: 0
+      referents:
+        - 1
+  - Inst:
+      type_id: 0
+      type_name: Model
+      object_format: 0
+      referents:
+        - 0
+  - Inst:
+      type_id: 2
+      type_name: Motor6D
+      object_format: 0
+      referents:
+        - 2
+  - Prop:
+      type_id: 1
+      prop_name: Name
+      prop_type: String
+      values:
+        - Humanoid
+  - Prop:
+      type_id: 0
+      prop_name: Name
+      prop_type: String
+      values:
+        - Model
+  - Prop:
+      type_id: 2
+      prop_name: Name
+      prop_type: String
+      values:
+        - Motor6D
+  - Prnt:
+      version: 0
+      links:
+        - - 0
+          - -1
+        - - 2
+          - 1
+        - - 1
+          - 0
+  - End
+