Ensure that pruned Instances aren't treated as existing in syncback (#1179)

Closes #1178.
This commit is contained in:
Micah
2025-11-29 21:21:48 -08:00
committed by GitHub
parent b89cc7f398
commit d08780fc14
10 changed files with 65 additions and 6 deletions

View File

@@ -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)

View File

@@ -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

View File

@@ -0,0 +1,7 @@
---
source: tests/tests/syncback.rs
expression: src/Pointer1.model.json
---
{
"className": "ObjectValue"
}

View File

@@ -0,0 +1,7 @@
---
source: tests/tests/syncback.rs
expression: src/Pointer2.model.json
---
{
"className": "ObjectValue"
}

View File

@@ -0,0 +1,7 @@
---
source: tests/tests/syncback.rs
expression: src/Pointer3.model.json
---
{
"className": "ObjectValue"
}

View File

@@ -0,0 +1,9 @@
{
"name": "ref_properties_pruned",
"tree": {
"$className": "DataModel",
"ServerScriptService": {
"$path": "src"
}
}
}

View File

@@ -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

View File

@@ -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"]
}