From d9e7f04f96d4ab9ac8fb43c186199bcbd97ea21e Mon Sep 17 00:00:00 2001 From: ardrous Date: Fri, 13 Feb 2026 19:53:25 -0500 Subject: [PATCH 1/2] feat: add --base flag to upload command for merging with place files --- src/cli/upload.rs | 184 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 172 insertions(+), 12 deletions(-) diff --git a/src/cli/upload.rs b/src/cli/upload.rs index 3c3610fd..987f2f21 100644 --- a/src/cli/upload.rs +++ b/src/cli/upload.rs @@ -1,8 +1,14 @@ -use std::path::PathBuf; +use std::{ + io::BufReader, + mem::forget, + path::{Path, PathBuf}, +}; use anyhow::{bail, Context}; use clap::Parser; +use fs_err::File; use memofs::Vfs; +use rbx_dom_weak::{types::Ref, Instance, WeakDom}; use reqwest::{ header::{ACCEPT, CONTENT_TYPE, COOKIE, USER_AGENT}, StatusCode, @@ -19,6 +25,13 @@ pub struct UploadCommand { #[clap(default_value = "")] pub project: PathBuf, + /// Path to a base .rbxl or .rbxlx file to merge with the project before + /// uploading. When provided, the Rojo project tree is merged into this + /// file so that the upload contains both the base content (3D assets, + /// terrain, lighting, etc.) and the project's scripts. + #[clap(long)] + pub base: Option, + /// Authenication cookie to use. If not specified, Rojo will attempt to find one from the system automatically. #[clap(long)] pub cookie: Option, @@ -44,19 +57,38 @@ impl UploadCommand { let session = ServeSession::new(vfs, project_path)?; - let tree = session.tree(); - let inner_tree = tree.inner(); - let root = inner_tree.root(); - - let encode_ids = match root.class.as_str() { - "DataModel" => root.children().to_vec(), - _ => vec![root.referent()], - }; - let mut buffer = Vec::new(); - log::trace!("Encoding binary model"); - rbx_binary::to_writer(&mut buffer, tree.inner(), &encode_ids)?; + if let Some(base_path) = &self.base { + let base_path = resolve_path(base_path); + + log::trace!("Reading base file: {}", base_path.display()); + let base_dom = read_base_file(&base_path)?; + + log::trace!("Merging Rojo project into base file"); + let merged_dom = merge_rojo_into_base(base_dom, session.tree().inner())?; + + let encode_ids = merged_dom.root().children().to_vec(); + + log::trace!("Encoding merged binary model"); + rbx_binary::to_writer(&mut buffer, &merged_dom, &encode_ids)?; + } else { + let tree = session.tree(); + let inner_tree = tree.inner(); + let root = inner_tree.root(); + + let encode_ids = match root.class.as_str() { + "DataModel" => root.children().to_vec(), + _ => vec![root.referent()], + }; + + log::trace!("Encoding binary model"); + rbx_binary::to_writer(&mut buffer, inner_tree, &encode_ids)?; + } + + // Avoid dropping ServeSession: it's potentially very expensive to + // drop and we're about to exit anyway. + forget(session); match (self.cookie, self.api_key, self.universe_id) { (cookie, None, universe) => { @@ -90,6 +122,134 @@ impl UploadCommand { } } +/// Reads a base .rbxl or .rbxlx file into a WeakDom. +fn read_base_file(path: &Path) -> anyhow::Result { + let extension = path + .extension() + .and_then(|ext| ext.to_str()) + .context("Base file must have a .rbxl or .rbxlx extension")?; + + let content = BufReader::new( + File::open(path) + .with_context(|| format!("Could not open base file at {}", path.display()))?, + ); + + match extension { + "rbxl" => rbx_binary::from_reader(content).with_context(|| { + format!( + "Could not deserialize binary place file at {}", + path.display() + ) + }), + "rbxlx" => { + let config = rbx_xml::DecodeOptions::new() + .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); + rbx_xml::from_reader(content, config).with_context(|| { + format!( + "Could not deserialize XML place file at {}", + path.display() + ) + }) + } + _ => bail!("Base file must be .rbxl or .rbxlx, got .{}", extension), + } +} + +/// Merges the Rojo project tree into a base WeakDom. +/// +/// For each service that exists in the Rojo tree, finds the matching service +/// in the base DOM by name and className and recursively merges Rojo children +/// into it. Instances in the base that Rojo doesn't manage are left untouched, +/// preserving 3D assets, terrain, lighting, and other non-scripted content. +fn merge_rojo_into_base(mut base: WeakDom, rojo: &WeakDom) -> anyhow::Result { + let rojo_root = rojo.root(); + let base_root_ref = base.root_ref(); + + if rojo_root.class.as_str() != "DataModel" { + // Non-place project (model): clone the entire Rojo tree as a child + let cloned_ref = rojo.clone_into_external(rojo_root.referent(), &mut base); + base.transfer_within(cloned_ref, base_root_ref); + return Ok(base); + } + + merge_instances(&mut base, base_root_ref, rojo, rojo.root_ref()); + + Ok(base) +} + +/// Recursively merges the children of a Rojo instance into the corresponding +/// base instance. +/// +/// For each Rojo child: +/// - If a matching child (by name+class) exists in base: update its +/// properties and recurse into its children +/// - If no match: clone the entire Rojo subtree into base under the +/// current parent +/// +/// Base children that have no Rojo counterpart are never touched, preserving +/// 3D assets and other content that Rojo doesn't manage. +fn merge_instances( + base: &mut WeakDom, + base_parent_ref: Ref, + rojo: &WeakDom, + rojo_parent_ref: Ref, +) { + for &rojo_child_ref in rojo.get_by_ref(rojo_parent_ref).unwrap().children() { + let rojo_child = rojo.get_by_ref(rojo_child_ref).unwrap(); + + let base_match = find_child_by_name_and_class( + base, + base_parent_ref, + &rojo_child.name, + rojo_child.class.as_str(), + ); + + match base_match { + Some(base_child_ref) => { + update_properties(base, base_child_ref, rojo_child); + merge_instances(base, base_child_ref, rojo, rojo_child_ref); + } + None => { + let cloned_ref = rojo.clone_into_external(rojo_child_ref, base); + base.transfer_within(cloned_ref, base_parent_ref); + } + } + } +} + +/// Finds a child of `parent_ref` in the given DOM that matches both name and +/// className. +fn find_child_by_name_and_class( + dom: &WeakDom, + parent_ref: Ref, + name: &str, + class: &str, +) -> Option { + let parent = dom.get_by_ref(parent_ref)?; + for &child_ref in parent.children() { + let child = dom.get_by_ref(child_ref)?; + if child.name == name && child.class.as_str() == class { + return Some(child_ref); + } + } + None +} + +/// Overlays Rojo properties onto a base instance. Properties that exist only +/// in the base instance are preserved; properties from Rojo overwrite any +/// existing base values. +fn update_properties(base: &mut WeakDom, base_ref: Ref, rojo_instance: &Instance) { + let base_instance = base.get_by_ref_mut(base_ref).unwrap(); + + base_instance.name.clone_from(&rojo_instance.name); + + for (prop_name, prop_value) in &rojo_instance.properties { + base_instance + .properties + .insert(*prop_name, prop_value.clone()); + } +} + fn do_upload(buffer: Vec, asset_id: u64, cookie: &str) -> anyhow::Result<()> { let url = format!( "https://data.roblox.com/Data/Upload.ashx?assetid={}", From 3b0e7666362caa448a29afdbd62f349778661438 Mon Sep 17 00:00:00 2001 From: ardrous Date: Sat, 14 Feb 2026 14:47:43 -0500 Subject: [PATCH 2/2] feat: add support for merging with base Roblox files in build and upload commands --- CHANGELOG.md | 2 + ..._end__tests__build__base_merge_delete.snap | 29 ++ ...o_end__tests__build__base_merge_model.snap | 24 ++ ...o_end__tests__build__base_merge_place.snap | 35 ++ .../build-tests/base_merge_delete/base.rbxlx | 23 ++ .../base_merge_delete/default.project.json | 9 + .../base_merge_delete/src/Hello.luau | 1 + .../build-tests/base_merge_model/base.rbxmx | 7 + .../base_merge_model/default.project.json | 6 + .../base_merge_model/src/Child.luau | 1 + .../build-tests/base_merge_place/base.rbxlx | 23 ++ .../base_merge_place/default.project.json | 11 + .../base_merge_place/src/Hello.luau | 1 + src/cli/build.rs | 84 ++-- src/cli/upload.rs | 151 +------ src/lib.rs | 1 + src/merge_base.rs | 371 ++++++++++++++++++ tests/tests/build.rs | 67 ++++ 18 files changed, 681 insertions(+), 165 deletions(-) create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_delete.snap create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_model.snap create mode 100644 rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_place.snap create mode 100644 rojo-test/build-tests/base_merge_delete/base.rbxlx create mode 100644 rojo-test/build-tests/base_merge_delete/default.project.json create mode 100644 rojo-test/build-tests/base_merge_delete/src/Hello.luau create mode 100644 rojo-test/build-tests/base_merge_model/base.rbxmx create mode 100644 rojo-test/build-tests/base_merge_model/default.project.json create mode 100644 rojo-test/build-tests/base_merge_model/src/Child.luau create mode 100644 rojo-test/build-tests/base_merge_place/base.rbxlx create mode 100644 rojo-test/build-tests/base_merge_place/default.project.json create mode 100644 rojo-test/build-tests/base_merge_place/src/Hello.luau create mode 100644 src/merge_base.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c090788..e0cfd6cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Making a new release? Simply add the new header with the version and date undern ## Unreleased +* Added `--base` flag to `rojo build` and `rojo upload` for merging a Rojo project into an existing Roblox file ([#1226]) * `inf` and `nan` values in properties are now synced ([#1176]) * Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179]) * Fixed instance replacement fallback failing when too many instances needed to be replaced. ([#1192]) @@ -48,6 +49,7 @@ Making a new release? Simply add the new header with the version and date undern [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 +[#1226]: https://github.com/rojo-rbx/rojo/pull/1226 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_delete.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_delete.snap new file mode 100644 index 00000000..384a9cac --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_delete.snap @@ -0,0 +1,29 @@ +--- +source: tests/tests/build.rs +assertion_line: 112 +expression: contents +--- + + + + ReplicatedStorage + + + + Hello + + + + + + + Workspace + + + + PreservedPart + + + + diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_model.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_model.snap new file mode 100644 index 00000000..489dc9d5 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_model.snap @@ -0,0 +1,24 @@ +--- +source: tests/tests/build.rs +assertion_line: 112 +expression: contents +--- + + + + BaseFolder + + + + + base_merge_model + + + + Child + + + + + diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_place.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_place.snap new file mode 100644 index 00000000..6faa8a0e --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__base_merge_place.snap @@ -0,0 +1,35 @@ +--- +source: tests/tests/build.rs +assertion_line: 112 +expression: contents +--- + + + + ReplicatedStorage + + + + ExistingScript + -- existing + + + + + Hello + + + + + + + Workspace + + + + BasePart + + + + diff --git a/rojo-test/build-tests/base_merge_delete/base.rbxlx b/rojo-test/build-tests/base_merge_delete/base.rbxlx new file mode 100644 index 00000000..dad09fe1 --- /dev/null +++ b/rojo-test/build-tests/base_merge_delete/base.rbxlx @@ -0,0 +1,23 @@ + + + + ReplicatedStorage + + + + ShouldBeDeleted + + + + + + + Workspace + + + + PreservedPart + + + + diff --git a/rojo-test/build-tests/base_merge_delete/default.project.json b/rojo-test/build-tests/base_merge_delete/default.project.json new file mode 100644 index 00000000..fa017401 --- /dev/null +++ b/rojo-test/build-tests/base_merge_delete/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "base_merge_delete", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "$path": "src" + } + } +} diff --git a/rojo-test/build-tests/base_merge_delete/src/Hello.luau b/rojo-test/build-tests/base_merge_delete/src/Hello.luau new file mode 100644 index 00000000..50a7a965 --- /dev/null +++ b/rojo-test/build-tests/base_merge_delete/src/Hello.luau @@ -0,0 +1 @@ +-- hello from rojo diff --git a/rojo-test/build-tests/base_merge_model/base.rbxmx b/rojo-test/build-tests/base_merge_model/base.rbxmx new file mode 100644 index 00000000..ee6cc89b --- /dev/null +++ b/rojo-test/build-tests/base_merge_model/base.rbxmx @@ -0,0 +1,7 @@ + + + + BaseFolder + + + diff --git a/rojo-test/build-tests/base_merge_model/default.project.json b/rojo-test/build-tests/base_merge_model/default.project.json new file mode 100644 index 00000000..cf38f0db --- /dev/null +++ b/rojo-test/build-tests/base_merge_model/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "base_merge_model", + "tree": { + "$path": "src" + } +} diff --git a/rojo-test/build-tests/base_merge_model/src/Child.luau b/rojo-test/build-tests/base_merge_model/src/Child.luau new file mode 100644 index 00000000..9ca3dc51 --- /dev/null +++ b/rojo-test/build-tests/base_merge_model/src/Child.luau @@ -0,0 +1 @@ +-- model child diff --git a/rojo-test/build-tests/base_merge_place/base.rbxlx b/rojo-test/build-tests/base_merge_place/base.rbxlx new file mode 100644 index 00000000..28a42e64 --- /dev/null +++ b/rojo-test/build-tests/base_merge_place/base.rbxlx @@ -0,0 +1,23 @@ + + + + ReplicatedStorage + + + + ExistingScript + + + + + + + Workspace + + + + BasePart + + + + diff --git a/rojo-test/build-tests/base_merge_place/default.project.json b/rojo-test/build-tests/base_merge_place/default.project.json new file mode 100644 index 00000000..2ef86e23 --- /dev/null +++ b/rojo-test/build-tests/base_merge_place/default.project.json @@ -0,0 +1,11 @@ +{ + "name": "base_merge_place", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "Hello": { + "$path": "src/Hello.luau" + } + } + } +} diff --git a/rojo-test/build-tests/base_merge_place/src/Hello.luau b/rojo-test/build-tests/base_merge_place/src/Hello.luau new file mode 100644 index 00000000..50a7a965 --- /dev/null +++ b/rojo-test/build-tests/base_merge_place/src/Hello.luau @@ -0,0 +1 @@ +-- hello from rojo diff --git a/src/cli/build.rs b/src/cli/build.rs index c2ddebb3..c6d0d9d9 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -11,7 +11,7 @@ use memofs::Vfs; use roblox_install::RobloxStudio; use tokio::runtime::Runtime; -use crate::serve_session::ServeSession; +use crate::{merge_base, serve_session::ServeSession}; use super::resolve_path; @@ -39,6 +39,13 @@ pub struct BuildCommand { #[clap(long, short, conflicts_with = "output")] pub plugin: Option, + /// Path to a base Roblox file (.rbxl, .rbxlx, .rbxm, .rbxmx) to merge + /// the project into. The Rojo project tree is merged into this file so + /// that the output contains both the base content and the project's + /// scripts/configuration. + #[clap(long)] + pub base: Option, + /// Whether to automatically rebuild when any input files change. #[clap(long)] pub watch: bool, @@ -84,7 +91,7 @@ impl BuildCommand { let session = ServeSession::new(vfs, project_path)?; let mut cursor = session.message_queue().cursor(); - write_model(&session, &output_path, output_kind)?; + write_model(&session, &output_path, output_kind, self.base.as_deref())?; if self.watch { let rt = Runtime::new().unwrap(); @@ -94,7 +101,7 @@ impl BuildCommand { let (new_cursor, _patch_set) = rt.block_on(receiver).unwrap(); cursor = new_cursor; - write_model(&session, &output_path, output_kind)?; + write_model(&session, &output_path, output_kind, self.base.as_deref())?; } } @@ -155,39 +162,70 @@ fn write_model( session: &ServeSession, output: &Path, output_kind: OutputKind, + base_path: Option<&Path>, ) -> anyhow::Result<()> { println!("Building project '{}'", session.project_name()); let tree = session.tree(); - let root_id = tree.get_root_id(); log::trace!("Opening output file for write"); let mut file = BufWriter::new(File::create(output)?); - match output_kind { - OutputKind::Rbxm => { - rbx_binary::to_writer(&mut file, tree.inner(), &[root_id])?; + if let Some(base_path) = base_path { + let base_path = super::resolve_path(base_path); + + log::trace!("Reading base file: {}", base_path.display()); + let base_dom = merge_base::read_base_dom(&base_path)?; + + log::trace!("Merging Rojo project into base file"); + let merged_dom = merge_base::merge_rojo_into_base(base_dom, &tree)?; + + // The merged DOM's root is always the WeakDom synthetic root + // (DataModel). For both place and model output, we encode its + // children — the services/instances — not the root itself. + let encode_ids = merged_dom.root().children().to_vec(); + + match output_kind { + OutputKind::Rbxl | OutputKind::Rbxm => { + rbx_binary::to_writer(&mut file, &merged_dom, &encode_ids)?; + } + OutputKind::Rbxlx | OutputKind::Rbxmx => { + rbx_xml::to_writer(&mut file, &merged_dom, &encode_ids, xml_encode_config())?; + } } - OutputKind::Rbxl => { - let root_instance = tree.get_instance(root_id).unwrap(); - let top_level_ids = root_instance.children(); + } else { + let root_id = tree.get_root_id(); - rbx_binary::to_writer(&mut file, tree.inner(), top_level_ids)?; - } - OutputKind::Rbxmx => { - // Model files include the root instance of the tree and all its - // descendants. + match output_kind { + OutputKind::Rbxm => { + rbx_binary::to_writer(&mut file, tree.inner(), &[root_id])?; + } + OutputKind::Rbxl => { + let root_instance = tree.get_instance(root_id).unwrap(); + let top_level_ids = root_instance.children(); - rbx_xml::to_writer(&mut file, tree.inner(), &[root_id], xml_encode_config())?; - } - OutputKind::Rbxlx => { - // Place files don't contain an entry for the DataModel, but our - // WeakDom representation does. + rbx_binary::to_writer(&mut file, tree.inner(), top_level_ids)?; + } + OutputKind::Rbxmx => { + // Model files include the root instance of the tree and all its + // descendants. - let root_instance = tree.get_instance(root_id).unwrap(); - let top_level_ids = root_instance.children(); + rbx_xml::to_writer(&mut file, tree.inner(), &[root_id], xml_encode_config())?; + } + OutputKind::Rbxlx => { + // Place files don't contain an entry for the DataModel, but our + // WeakDom representation does. - rbx_xml::to_writer(&mut file, tree.inner(), top_level_ids, xml_encode_config())?; + let root_instance = tree.get_instance(root_id).unwrap(); + let top_level_ids = root_instance.children(); + + rbx_xml::to_writer( + &mut file, + tree.inner(), + top_level_ids, + xml_encode_config(), + )?; + } } } diff --git a/src/cli/upload.rs b/src/cli/upload.rs index 987f2f21..9d6a49c6 100644 --- a/src/cli/upload.rs +++ b/src/cli/upload.rs @@ -1,20 +1,14 @@ -use std::{ - io::BufReader, - mem::forget, - path::{Path, PathBuf}, -}; +use std::{mem::forget, path::PathBuf}; use anyhow::{bail, Context}; use clap::Parser; -use fs_err::File; use memofs::Vfs; -use rbx_dom_weak::{types::Ref, Instance, WeakDom}; use reqwest::{ header::{ACCEPT, CONTENT_TYPE, COOKIE, USER_AGENT}, StatusCode, }; -use crate::{auth_cookie::get_auth_cookie, serve_session::ServeSession}; +use crate::{auth_cookie::get_auth_cookie, merge_base, serve_session::ServeSession}; use super::resolve_path; @@ -25,10 +19,10 @@ pub struct UploadCommand { #[clap(default_value = "")] pub project: PathBuf, - /// Path to a base .rbxl or .rbxlx file to merge with the project before - /// uploading. When provided, the Rojo project tree is merged into this - /// file so that the upload contains both the base content (3D assets, - /// terrain, lighting, etc.) and the project's scripts. + /// Path to a base Roblox file (.rbxl, .rbxlx, .rbxm, .rbxmx) to merge + /// with the project before uploading. When provided, the Rojo project + /// tree is merged into this file so that the upload contains both the + /// base content and the project's scripts. #[clap(long)] pub base: Option, @@ -63,10 +57,11 @@ impl UploadCommand { let base_path = resolve_path(base_path); log::trace!("Reading base file: {}", base_path.display()); - let base_dom = read_base_file(&base_path)?; + let base_dom = merge_base::read_base_dom(&base_path)?; log::trace!("Merging Rojo project into base file"); - let merged_dom = merge_rojo_into_base(base_dom, session.tree().inner())?; + let tree = session.tree(); + let merged_dom = merge_base::merge_rojo_into_base(base_dom, &tree)?; let encode_ids = merged_dom.root().children().to_vec(); @@ -122,134 +117,6 @@ impl UploadCommand { } } -/// Reads a base .rbxl or .rbxlx file into a WeakDom. -fn read_base_file(path: &Path) -> anyhow::Result { - let extension = path - .extension() - .and_then(|ext| ext.to_str()) - .context("Base file must have a .rbxl or .rbxlx extension")?; - - let content = BufReader::new( - File::open(path) - .with_context(|| format!("Could not open base file at {}", path.display()))?, - ); - - match extension { - "rbxl" => rbx_binary::from_reader(content).with_context(|| { - format!( - "Could not deserialize binary place file at {}", - path.display() - ) - }), - "rbxlx" => { - let config = rbx_xml::DecodeOptions::new() - .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); - rbx_xml::from_reader(content, config).with_context(|| { - format!( - "Could not deserialize XML place file at {}", - path.display() - ) - }) - } - _ => bail!("Base file must be .rbxl or .rbxlx, got .{}", extension), - } -} - -/// Merges the Rojo project tree into a base WeakDom. -/// -/// For each service that exists in the Rojo tree, finds the matching service -/// in the base DOM by name and className and recursively merges Rojo children -/// into it. Instances in the base that Rojo doesn't manage are left untouched, -/// preserving 3D assets, terrain, lighting, and other non-scripted content. -fn merge_rojo_into_base(mut base: WeakDom, rojo: &WeakDom) -> anyhow::Result { - let rojo_root = rojo.root(); - let base_root_ref = base.root_ref(); - - if rojo_root.class.as_str() != "DataModel" { - // Non-place project (model): clone the entire Rojo tree as a child - let cloned_ref = rojo.clone_into_external(rojo_root.referent(), &mut base); - base.transfer_within(cloned_ref, base_root_ref); - return Ok(base); - } - - merge_instances(&mut base, base_root_ref, rojo, rojo.root_ref()); - - Ok(base) -} - -/// Recursively merges the children of a Rojo instance into the corresponding -/// base instance. -/// -/// For each Rojo child: -/// - If a matching child (by name+class) exists in base: update its -/// properties and recurse into its children -/// - If no match: clone the entire Rojo subtree into base under the -/// current parent -/// -/// Base children that have no Rojo counterpart are never touched, preserving -/// 3D assets and other content that Rojo doesn't manage. -fn merge_instances( - base: &mut WeakDom, - base_parent_ref: Ref, - rojo: &WeakDom, - rojo_parent_ref: Ref, -) { - for &rojo_child_ref in rojo.get_by_ref(rojo_parent_ref).unwrap().children() { - let rojo_child = rojo.get_by_ref(rojo_child_ref).unwrap(); - - let base_match = find_child_by_name_and_class( - base, - base_parent_ref, - &rojo_child.name, - rojo_child.class.as_str(), - ); - - match base_match { - Some(base_child_ref) => { - update_properties(base, base_child_ref, rojo_child); - merge_instances(base, base_child_ref, rojo, rojo_child_ref); - } - None => { - let cloned_ref = rojo.clone_into_external(rojo_child_ref, base); - base.transfer_within(cloned_ref, base_parent_ref); - } - } - } -} - -/// Finds a child of `parent_ref` in the given DOM that matches both name and -/// className. -fn find_child_by_name_and_class( - dom: &WeakDom, - parent_ref: Ref, - name: &str, - class: &str, -) -> Option { - let parent = dom.get_by_ref(parent_ref)?; - for &child_ref in parent.children() { - let child = dom.get_by_ref(child_ref)?; - if child.name == name && child.class.as_str() == class { - return Some(child_ref); - } - } - None -} - -/// Overlays Rojo properties onto a base instance. Properties that exist only -/// in the base instance are preserved; properties from Rojo overwrite any -/// existing base values. -fn update_properties(base: &mut WeakDom, base_ref: Ref, rojo_instance: &Instance) { - let base_instance = base.get_by_ref_mut(base_ref).unwrap(); - - base_instance.name.clone_from(&rojo_instance.name); - - for (prop_name, prop_value) in &rojo_instance.properties { - base_instance - .properties - .insert(*prop_name, prop_value.clone()); - } -} - fn do_upload(buffer: Vec, asset_id: u64, cookie: &str) -> anyhow::Result<()> { let url = format!( "https://data.roblox.com/Data/Upload.ashx?assetid={}", diff --git a/src/lib.rs b/src/lib.rs index f1947eaa..974c8fc4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ mod change_processor; mod glob; mod json; mod lua_ast; +mod merge_base; mod message_queue; mod multimap; mod path_serializer; diff --git a/src/merge_base.rs b/src/merge_base.rs new file mode 100644 index 00000000..aa67c334 --- /dev/null +++ b/src/merge_base.rs @@ -0,0 +1,371 @@ +//! Shared logic for merging a Rojo project tree into a base Roblox file. +//! +//! Used by both `rojo build --base` and `rojo upload --base`. + +use std::{io::BufReader, path::Path}; + +use anyhow::{bail, Context}; +use fs_err::File; +use rbx_dom_weak::{types::Ref, Instance, WeakDom}; + +use crate::snapshot::RojoTree; + +/// A pair of (base instance, Rojo instance) whose children still need merging. +struct MergePair { + base_ref: Ref, + rojo_ref: Ref, +} + +/// Reads a base Roblox file (.rbxl, .rbxlx, .rbxm, .rbxmx) into a WeakDom. +pub fn read_base_dom(path: &Path) -> anyhow::Result { + let extension = path + .extension() + .and_then(|ext| ext.to_str()) + .context("Base file must have a .rbxl, .rbxlx, .rbxm, or .rbxmx extension")?; + + let content = BufReader::new( + File::open(path) + .with_context(|| format!("Could not open base file at {}", path.display()))?, + ); + + match extension { + "rbxl" | "rbxm" => rbx_binary::from_reader(content) + .with_context(|| format!("Could not deserialize binary file at {}", path.display())), + "rbxlx" | "rbxmx" => { + let config = rbx_xml::DecodeOptions::new() + .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown); + rbx_xml::from_reader(content, config) + .with_context(|| format!("Could not deserialize XML file at {}", path.display())) + } + _ => bail!( + "Base file must be .rbxl, .rbxlx, .rbxm, or .rbxmx, got .{}", + extension + ), + } +} + +/// Merges the Rojo project tree into a base WeakDom. +/// +/// For place projects (root class is "DataModel"), iteratively merges Rojo's +/// children into matching base children by name and className. For model +/// projects, clones the entire Rojo tree as a child of the base root. +/// +/// The `ignore_unknown_instances` metadata on each Rojo instance controls +/// whether unmatched base children are preserved or deleted: +/// - `true` (default for project nodes without `$path`): preserve +/// - `false` (default when `$path` is set): delete +pub fn merge_rojo_into_base(mut base: WeakDom, rojo: &RojoTree) -> anyhow::Result { + let rojo_root_id = rojo.get_root_id(); + let rojo_inner = rojo.inner(); + let rojo_root = rojo_inner + .get_by_ref(rojo_root_id) + .expect("Rojo tree should have a root"); + let base_root_ref = base.root_ref(); + + // For model projects (root is not DataModel), clone entire Rojo tree + // as a child of the base root. + if rojo_root.class.as_str() != "DataModel" { + let cloned_ref = rojo_inner.clone_into_external(rojo_root_id, &mut base); + base.transfer_within(cloned_ref, base_root_ref); + return Ok(base); + } + + // For place projects: iteratively merge children using a stack. + let mut stack = vec![MergePair { + base_ref: base_root_ref, + rojo_ref: rojo_root_id, + }]; + + while let Some(pair) = stack.pop() { + let rojo_children: Vec = rojo_inner + .get_by_ref(pair.rojo_ref) + .expect("Rojo instance should exist") + .children() + .to_vec(); + + let base_children: Vec = base + .get_by_ref(pair.base_ref) + .expect("Base instance should exist") + .children() + .to_vec(); + + let mut base_matched = vec![false; base_children.len()]; + + for &rojo_child_ref in &rojo_children { + let rojo_child = rojo_inner + .get_by_ref(rojo_child_ref) + .expect("Rojo child should exist"); + + // Find the first unmatched base child with the same name+class. + let base_match = base_children + .iter() + .enumerate() + .find(|(idx, base_child_ref)| { + if base_matched[*idx] { + return false; + } + let base_child = base + .get_by_ref(**base_child_ref) + .expect("Base child should exist"); + base_child.name == rojo_child.name + && base_child.class.as_str() == rojo_child.class.as_str() + }); + + match base_match { + Some((idx, &base_child_ref)) => { + base_matched[idx] = true; + update_properties(&mut base, base_child_ref, rojo_child); + stack.push(MergePair { + base_ref: base_child_ref, + rojo_ref: rojo_child_ref, + }); + } + None => { + let cloned_ref = rojo_inner.clone_into_external(rojo_child_ref, &mut base); + base.transfer_within(cloned_ref, pair.base_ref); + } + } + } + + // Delete unmatched base children if ignore_unknown_instances is false. + let ignore_unknown = rojo + .get_metadata(pair.rojo_ref) + .map(|m| m.ignore_unknown_instances) + .unwrap_or(true); + + if !ignore_unknown { + for (idx, &base_child_ref) in base_children.iter().enumerate() { + if !base_matched[idx] { + base.destroy(base_child_ref); + } + } + } + } + + Ok(base) +} + +/// Overlays Rojo properties onto a base instance. Properties that exist only +/// in the base instance are preserved; properties from Rojo overwrite any +/// existing base values. +fn update_properties(base: &mut WeakDom, base_ref: Ref, rojo_instance: &Instance) { + let base_instance = base + .get_by_ref_mut(base_ref) + .expect("Base instance should exist"); + + base_instance.name.clone_from(&rojo_instance.name); + + for (prop_name, prop_value) in &rojo_instance.properties { + base_instance + .properties + .insert(*prop_name, prop_value.clone()); + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::snapshot::{InstanceMetadata, InstanceSnapshot, RojoTree}; + use rbx_dom_weak::{types::Variant, ustr, InstanceBuilder}; + + #[test] + fn merge_adds_new_children() { + let mut base = WeakDom::new(InstanceBuilder::new("DataModel")); + base.insert( + base.root_ref(), + InstanceBuilder::new("ReplicatedStorage").with_name("ReplicatedStorage"), + ); + + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("Game") + .class_name("DataModel") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("ReplicatedStorage") + .class_name("ReplicatedStorage") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("NewScript") + .class_name("ModuleScript")])]), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let root_children = merged.root().children(); + assert_eq!(root_children.len(), 1); + + let rs = merged.get_by_ref(root_children[0]).unwrap(); + assert_eq!(rs.name, "ReplicatedStorage"); + assert_eq!(rs.children().len(), 1); + + let child = merged.get_by_ref(rs.children()[0]).unwrap(); + assert_eq!(child.name, "NewScript"); + assert_eq!(child.class.as_str(), "ModuleScript"); + } + + #[test] + fn merge_deletes_unknown_when_not_ignored() { + let mut base = WeakDom::new(InstanceBuilder::new("DataModel")); + let rs_ref = base.insert( + base.root_ref(), + InstanceBuilder::new("ReplicatedStorage").with_name("ReplicatedStorage"), + ); + base.insert( + rs_ref, + InstanceBuilder::new("ModuleScript").with_name("OldScript"), + ); + + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("Game") + .class_name("DataModel") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("ReplicatedStorage") + .class_name("ReplicatedStorage") + .metadata(InstanceMetadata::new().ignore_unknown_instances(false)) + .children(vec![InstanceSnapshot::new() + .name("NewScript") + .class_name("ModuleScript")])]), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let root_children = merged.root().children(); + let rs = merged.get_by_ref(root_children[0]).unwrap(); + assert_eq!(rs.children().len(), 1, "OldScript should have been deleted"); + + let child = merged.get_by_ref(rs.children()[0]).unwrap(); + assert_eq!(child.name, "NewScript"); + } + + #[test] + fn merge_preserves_unknown_when_ignored() { + let mut base = WeakDom::new(InstanceBuilder::new("DataModel")); + let rs_ref = base.insert( + base.root_ref(), + InstanceBuilder::new("ReplicatedStorage").with_name("ReplicatedStorage"), + ); + base.insert( + rs_ref, + InstanceBuilder::new("ModuleScript").with_name("OldScript"), + ); + + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("Game") + .class_name("DataModel") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("ReplicatedStorage") + .class_name("ReplicatedStorage") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("NewScript") + .class_name("ModuleScript")])]), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let root_children = merged.root().children(); + let rs = merged.get_by_ref(root_children[0]).unwrap(); + assert_eq!( + rs.children().len(), + 2, + "Both OldScript and NewScript should be present" + ); + } + + #[test] + fn merge_preserves_unmanaged_services() { + let mut base = WeakDom::new(InstanceBuilder::new("DataModel")); + base.insert( + base.root_ref(), + InstanceBuilder::new("ReplicatedStorage").with_name("ReplicatedStorage"), + ); + base.insert( + base.root_ref(), + InstanceBuilder::new("Workspace").with_name("Workspace"), + ); + + // Rojo project only manages ReplicatedStorage, not Workspace + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("Game") + .class_name("DataModel") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("ReplicatedStorage") + .class_name("ReplicatedStorage") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true))]), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let root_children = merged.root().children(); + assert_eq!( + root_children.len(), + 2, + "Both ReplicatedStorage and Workspace should be present" + ); + } + + #[test] + fn merge_overlays_properties() { + let mut base = WeakDom::new(InstanceBuilder::new("DataModel")); + base.insert( + base.root_ref(), + InstanceBuilder::new("ModuleScript") + .with_name("Script") + .with_property("Source", Variant::String("-- base source".into())) + .with_property("Disabled", Variant::Bool(true)), + ); + + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("Game") + .class_name("DataModel") + .metadata(InstanceMetadata::new().ignore_unknown_instances(true)) + .children(vec![InstanceSnapshot::new() + .name("Script") + .class_name("ModuleScript") + .property(ustr("Source"), Variant::String("-- rojo source".into()))]), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let script = merged.get_by_ref(merged.root().children()[0]).unwrap(); + + // Rojo property overwrites base + assert_eq!( + script.properties.get(&ustr("Source")), + Some(&Variant::String("-- rojo source".into())) + ); + // Base-only property is preserved + assert_eq!( + script.properties.get(&ustr("Disabled")), + Some(&Variant::Bool(true)) + ); + } + + #[test] + fn merge_model_clones_into_base() { + let base = WeakDom::new(InstanceBuilder::new("DataModel")); + + let rojo = RojoTree::new( + InstanceSnapshot::new() + .name("MyModel") + .class_name("Folder"), + ); + + let merged = merge_rojo_into_base(base, &rojo).unwrap(); + + let root_children = merged.root().children(); + assert_eq!(root_children.len(), 1); + + let model = merged.get_by_ref(root_children[0]).unwrap(); + assert_eq!(model.name, "MyModel"); + assert_eq!(model.class.as_str(), "Folder"); + } +} diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 8c533115..fc57d787 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -67,6 +67,27 @@ gen_build_tests! { no_name_top_level_project, } +macro_rules! gen_build_base_tests { + ( $($test_name: ident, $base_file: expr, $output_ext: expr,)* ) => { + $( + paste::item! { + #[test] + fn []() { + let _ = env_logger::try_init(); + + run_build_base_test(stringify!($test_name), $base_file, $output_ext); + } + } + )* + }; +} + +gen_build_base_tests! { + base_merge_place, "base.rbxlx", "rbxlx", + base_merge_delete, "base.rbxlx", "rbxlx", + base_merge_model, "base.rbxmx", "rbxmx", +} + fn run_build_test(test_name: &str) { let working_dir = get_working_dir_path(); @@ -107,3 +128,49 @@ fn run_build_test(test_name: &str) { assert_snapshot!(test_name, contents); }); } + +fn run_build_base_test(test_name: &str, base_file: &str, output_ext: &str) { + let working_dir = get_working_dir_path(); + + let input_path = Path::new(BUILD_TESTS_PATH).join(test_name); + let base_path = input_path.join(base_file); + + let output_dir = tempdir().expect("couldn't create temporary directory"); + let output_path = output_dir + .path() + .join(format!("{}.{}", test_name, output_ext)); + + let output = Command::new(ROJO_PATH) + .args([ + "build", + input_path.to_str().unwrap(), + "-o", + output_path.to_str().unwrap(), + "--base", + base_path.to_str().unwrap(), + ]) + .env("RUST_LOG", "error") + .current_dir(working_dir) + .output() + .expect("Couldn't start Rojo"); + + print!("{}", String::from_utf8_lossy(&output.stdout)); + eprint!("{}", String::from_utf8_lossy(&output.stderr)); + + assert!(output.status.success(), "Rojo did not exit successfully"); + + let contents = fs::read_to_string(&output_path).expect("Couldn't read output file"); + + let mut settings = insta::Settings::new(); + + let snapshot_path = Path::new(BUILD_TESTS_PATH) + .parent() + .unwrap() + .join("build-test-snapshots"); + + settings.set_snapshot_path(snapshot_path); + + settings.bind(|| { + assert_snapshot!(test_name, contents); + }); +}