From 382c1733a64d77188fe8fa1712337babd26c7dc9 Mon Sep 17 00:00:00 2001
From: Filip Tibell <filip.tibell@gmail.com>
Date: Fri, 7 Jul 2023 08:48:12 +0200
Subject: [PATCH] Fix unique ids not being stripped completely from models

---
 CHANGELOG.md                                  |  6 +++
 packages/lib-roblox/src/document/mod.rs       | 23 +++------
 .../lib-roblox/src/document/postprocessing.rs | 47 +++++++++++++++++++
 3 files changed, 60 insertions(+), 16 deletions(-)
 create mode 100644 packages/lib-roblox/src/document/postprocessing.rs

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a42ddbd..0ca7b58 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- Fixed `roblox.serializeModel` still keeping some unique ids
+
 ## `0.7.3` - July 5th, 2023
 
 ### Changed
diff --git a/packages/lib-roblox/src/document/mod.rs b/packages/lib-roblox/src/document/mod.rs
index 8a741e1..94a3cd0 100644
--- a/packages/lib-roblox/src/document/mod.rs
+++ b/packages/lib-roblox/src/document/mod.rs
@@ -1,4 +1,4 @@
-use rbx_dom_weak::{types::Ref as DomRef, InstanceBuilder as DomInstanceBuilder, WeakDom};
+use rbx_dom_weak::{InstanceBuilder as DomInstanceBuilder, WeakDom};
 use rbx_xml::{
     DecodeOptions as XmlDecodeOptions, DecodePropertyBehavior as XmlDecodePropertyBehavior,
     EncodeOptions as XmlEncodeOptions, EncodePropertyBehavior as XmlEncodePropertyBehavior,
@@ -7,11 +7,14 @@ use rbx_xml::{
 mod error;
 mod format;
 mod kind;
+mod postprocessing;
 
 pub use error::*;
 pub use format::*;
 pub use kind::*;
 
+use postprocessing::*;
+
 use crate::instance::{data_model, Instance};
 
 pub type DocumentResult<T> = Result<T, DocumentError>;
@@ -249,6 +252,8 @@ impl Document {
             data_model_child.clone_into_external_dom(&mut dom);
         }
 
+        postprocess_dom_for_place(&mut dom);
+
         Ok(Self {
             kind: DocumentKind::Place,
             format: DocumentFormat::default(),
@@ -274,8 +279,7 @@ impl Document {
             instance.clone_into_external_dom(&mut dom);
         }
 
-        let root_ref = dom.root_ref();
-        recurse_strip_unique_ids(&mut dom, root_ref);
+        postprocess_dom_for_model(&mut dom);
 
         Ok(Self {
             kind: DocumentKind::Model,
@@ -284,16 +288,3 @@ impl Document {
         })
     }
 }
-
-fn recurse_strip_unique_ids(dom: &mut WeakDom, dom_ref: DomRef) {
-    let child_refs = match dom.get_by_ref_mut(dom_ref) {
-        Some(inst) => {
-            inst.properties.remove("UniqueId");
-            inst.children().to_vec()
-        }
-        None => Vec::new(),
-    };
-    for child_ref in child_refs {
-        recurse_strip_unique_ids(dom, child_ref)
-    }
-}
diff --git a/packages/lib-roblox/src/document/postprocessing.rs b/packages/lib-roblox/src/document/postprocessing.rs
new file mode 100644
index 0000000..9efb136
--- /dev/null
+++ b/packages/lib-roblox/src/document/postprocessing.rs
@@ -0,0 +1,47 @@
+use rbx_dom_weak::{
+    types::{Ref as DomRef, VariantType as DomType},
+    Instance as DomInstance, WeakDom,
+};
+
+use crate::shared::instance::class_is_a;
+
+pub fn postprocess_dom_for_place(_dom: &mut WeakDom) {
+    // Nothing here yet
+}
+
+pub fn postprocess_dom_for_model(dom: &mut WeakDom) {
+    let root_ref = dom.root_ref();
+    recurse_instances(dom, root_ref, |inst| {
+        // Get rid of some unique ids - roblox does not
+        // save these in model files, and we shouldn't either
+        remove_matching_prop(inst, DomType::UniqueId, "UniqueId");
+        remove_matching_prop(inst, DomType::UniqueId, "HistoryId");
+        // Similar story with ScriptGuid - this is used
+        // in the studio-only cloud script drafts feature
+        if class_is_a(&inst.class, "LuaSourceContainer").unwrap_or(false) {
+            inst.properties.remove("ScriptGuid");
+        }
+    });
+}
+
+fn recurse_instances<F>(dom: &mut WeakDom, dom_ref: DomRef, f: F)
+where
+    F: Fn(&mut DomInstance),
+{
+    let child_refs = match dom.get_by_ref_mut(dom_ref) {
+        Some(inst) => {
+            f(inst);
+            inst.children().to_vec()
+        }
+        None => Vec::new(),
+    };
+    for child_ref in child_refs {
+        recurse_instances(dom, child_ref, &f);
+    }
+}
+
+fn remove_matching_prop(inst: &mut DomInstance, ty: DomType, name: &'static str) {
+    if inst.properties.get(name).map_or(false, |u| u.ty() == ty) {
+        inst.properties.remove(name);
+    }
+}