Skip to content

Commit d08780f

Browse files
authored
Ensure that pruned Instances aren't treated as existing in syncback (#1179)
Closes #1178.
1 parent b89cc7f commit d08780f

10 files changed

+65
-6
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ Making a new release? Simply add the new header with the version and date undern
3030
-->
3131

3232
## Unreleased
33+
* Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179])
34+
35+
[#1179]: https://github.com/rojo-rbx/rojo/pull/1179
3336

3437
## [7.7.0-rc.1] (November 27th, 2025)
3538

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
source: tests/rojo_test/syncback_util.rs
3+
expression: "String::from_utf8_lossy(&output.stdout)"
4+
---
5+
Writing src/Pointer1.model.json
6+
Writing src/Pointer2.model.json
7+
Writing src/Pointer3.model.json
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
source: tests/tests/syncback.rs
3+
expression: src/Pointer1.model.json
4+
---
5+
{
6+
"className": "ObjectValue"
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
source: tests/tests/syncback.rs
3+
expression: src/Pointer2.model.json
4+
---
5+
{
6+
"className": "ObjectValue"
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
source: tests/tests/syncback.rs
3+
expression: src/Pointer3.model.json
4+
---
5+
{
6+
"className": "ObjectValue"
7+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "ref_properties_pruned",
3+
"tree": {
4+
"$className": "DataModel",
5+
"ServerScriptService": {
6+
"$path": "src"
7+
}
8+
}
9+
}

rojo-test/syncback-tests/ref_properties_pruned/input-project/src/.gitkeep

Whitespace-only changes.
47.4 KB
Binary file not shown.

src/syncback/ref_properties.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ use rbx_dom_weak::{
88
ustr, Instance, Ustr, WeakDom,
99
};
1010

11-
use crate::{multimap::MultiMap, REF_ID_ATTRIBUTE_NAME, REF_POINTER_ATTRIBUTE_PREFIX};
11+
use crate::{
12+
multimap::MultiMap, syncback::snapshot::inst_path, REF_ID_ATTRIBUTE_NAME,
13+
REF_POINTER_ATTRIBUTE_PREFIX,
14+
};
1215

1316
pub struct RefLinks {
1417
/// A map of referents to each of their Ref properties.
@@ -50,6 +53,22 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks {
5053
continue;
5154
}
5255

56+
let target = match dom.get_by_ref(*prop_value) {
57+
Some(inst) => inst,
58+
None => {
59+
// Properties that are Some but point to nothing may as
60+
// well be `nil`. Roblox and us never produce these values
61+
// but syncback prunes trees without adjusting ref
62+
// properties for performance reasons.
63+
log::warn!(
64+
"Property {}.{} will be `nil` on the disk because the actual value is not being included in syncback",
65+
inst_path(dom, inst_ref),
66+
prop_name
67+
);
68+
continue;
69+
}
70+
};
71+
5372
links.insert(
5473
inst_ref,
5574
RefLink {
@@ -58,10 +77,6 @@ pub fn collect_referents(dom: &WeakDom) -> RefLinks {
5877
},
5978
);
6079

61-
let target = dom
62-
.get_by_ref(*prop_value)
63-
.expect("Refs in DOM should point to valid Instances");
64-
6580
// 1. Check if target has an ID
6681
if let Some(id) = get_existing_id(target) {
6782
// If it does, we need to check whether that ID is a duplicate

tests/tests/syncback.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,13 @@ syncback_tests! {
7171
ref_properties_conflict => ["src/Pointer_2.model.json", "src/Target_2.model.json"],
7272
// Ensures that having multiple pointers that are aimed at the same target doesn't trigger ref rewrites.
7373
ref_properties_duplicate => [],
74+
// Ensures that ref properties that point to nothing after the prune both
75+
// do not leave any trace of themselves
76+
ref_properties_pruned => ["src/Pointer1.model.json", "src/Pointer2.model.json", "src/Pointer3.model.json"],
7477
// Ensures that the old middleware is respected during syncback
7578
respect_old_middleware => ["default.project.json", "src/model_json.model.json", "src/rbxm.rbxm", "src/rbxmx.rbxmx"],
79+
// Ensures that the `$schema` field roundtrips with syncback
80+
schema_roundtrip => ["default.project.json", "src/model.model.json", "src/init/init.meta.json", "src/adjacent.meta.json"],
7681
// Ensures that StringValues inside project files are written to the
7782
// project file, but only if they don't have `$path` set
7883
string_value_project => ["default.project.json"],
@@ -81,5 +86,4 @@ syncback_tests! {
8186
sync_rules => ["src/module.modulescript", "src/text.text"],
8287
// Ensures that the `syncUnscriptable` setting works
8388
unscriptable_properties => ["default.project.json"],
84-
schema_roundtrip => ["default.project.json", "src/model.model.json", "src/init/init.meta.json", "src/adjacent.meta.json"]
8589
}

0 commit comments

Comments
 (0)