diff --git a/CHANGELOG.md b/CHANGELOG.md index 410a7cb0..7843cb44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ Making a new release? Simply add the new header with the version and date undern --> ## Unreleased +* Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179]) + +[#1179]: https://github.com/rojo-rbx/rojo/pull/1179 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_pruned-stdout.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_pruned-stdout.snap new file mode 100644 index 00000000..3a9c2502 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_pruned-stdout.snap @@ -0,0 +1,7 @@ +--- +source: tests/rojo_test/syncback_util.rs +expression: "String::from_utf8_lossy(&output.stdout)" +--- +Writing src/Pointer1.model.json +Writing src/Pointer2.model.json +Writing src/Pointer3.model.json diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json.snap new file mode 100644 index 00000000..aeee8256 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json.snap @@ -0,0 +1,7 @@ +--- +source: tests/tests/syncback.rs +expression: src/Pointer1.model.json +--- +{ + "className": "ObjectValue" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json.snap new file mode 100644 index 00000000..87da5680 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json.snap @@ -0,0 +1,7 @@ +--- +source: tests/tests/syncback.rs +expression: src/Pointer2.model.json +--- +{ + "className": "ObjectValue" +} diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json.snap new file mode 100644 index 00000000..9c5e3b94 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json.snap @@ -0,0 +1,7 @@ +--- +source: tests/tests/syncback.rs +expression: src/Pointer3.model.json +--- +{ + "className": "ObjectValue" +} diff --git a/rojo-test/syncback-tests/ref_properties_pruned/input-project/default.project.json b/rojo-test/syncback-tests/ref_properties_pruned/input-project/default.project.json new file mode 100644 index 00000000..9656479a --- /dev/null +++ b/rojo-test/syncback-tests/ref_properties_pruned/input-project/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "ref_properties_pruned", + "tree": { + "$className": "DataModel", + "ServerScriptService": { + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/syncback-tests/ref_properties_pruned/input-project/src/.gitkeep b/rojo-test/syncback-tests/ref_properties_pruned/input-project/src/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/rojo-test/syncback-tests/ref_properties_pruned/input.rbxl b/rojo-test/syncback-tests/ref_properties_pruned/input.rbxl new file mode 100644 index 00000000..1513ff11 Binary files /dev/null and b/rojo-test/syncback-tests/ref_properties_pruned/input.rbxl differ diff --git a/src/syncback/ref_properties.rs b/src/syncback/ref_properties.rs index 4a8385f8..054f61b8 100644 --- a/src/syncback/ref_properties.rs +++ b/src/syncback/ref_properties.rs @@ -8,7 +8,10 @@ use rbx_dom_weak::{ ustr, Instance, Ustr, WeakDom, }; -use crate::{multimap::MultiMap, REF_ID_ATTRIBUTE_NAME, REF_POINTER_ATTRIBUTE_PREFIX}; +use crate::{ + multimap::MultiMap, syncback::snapshot::inst_path, REF_ID_ATTRIBUTE_NAME, + REF_POINTER_ATTRIBUTE_PREFIX, +}; pub struct RefLinks { /// A map of referents to each of their Ref properties. @@ -50,6 +53,22 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks { continue; } + let target = match dom.get_by_ref(*prop_value) { + Some(inst) => inst, + None => { + // Properties that are Some but point to nothing may as + // well be `nil`. Roblox and us never produce these values + // but syncback prunes trees without adjusting ref + // properties for performance reasons. + log::warn!( + "Property {}.{} will be `nil` on the disk because the actual value is not being included in syncback", + inst_path(dom, inst_ref), + prop_name + ); + continue; + } + }; + links.insert( inst_ref, RefLink { @@ -58,10 +77,6 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks { }, ); - let target = dom - .get_by_ref(*prop_value) - .expect("Refs in DOM should point to valid Instances"); - // 1. Check if target has an ID if let Some(id) = get_existing_id(target) { // If it does, we need to check whether that ID is a duplicate diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index 851b4e4d..31553496 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -71,8 +71,13 @@ syncback_tests! { ref_properties_conflict => ["src/Pointer_2.model.json", "src/Target_2.model.json"], // Ensures that having multiple pointers that are aimed at the same target doesn't trigger ref rewrites. ref_properties_duplicate => [], + // Ensures that ref properties that point to nothing after the prune both + // do not leave any trace of themselves + ref_properties_pruned => ["src/Pointer1.model.json", "src/Pointer2.model.json", "src/Pointer3.model.json"], // Ensures that the old middleware is respected during syncback respect_old_middleware => ["default.project.json", "src/model_json.model.json", "src/rbxm.rbxm", "src/rbxmx.rbxmx"], + // Ensures that the `$schema` field roundtrips with syncback + schema_roundtrip => ["default.project.json", "src/model.model.json", "src/init/init.meta.json", "src/adjacent.meta.json"], // Ensures that StringValues inside project files are written to the // project file, but only if they don't have `$path` set string_value_project => ["default.project.json"], @@ -81,5 +86,4 @@ syncback_tests! { sync_rules => ["src/module.modulescript", "src/text.text"], // Ensures that the `syncUnscriptable` setting works unscriptable_properties => ["default.project.json"], - schema_roundtrip => ["default.project.json", "src/model.model.json", "src/init/init.meta.json", "src/adjacent.meta.json"] }