From 70e6e3c5e8f8af511414822ae957b77efd4369b2 Mon Sep 17 00:00:00 2001 From: Floyd Horng Date: Fri, 3 Apr 2026 16:55:17 -0500 Subject: [PATCH] Fix syncback not removing stale $properties at engine defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a property in a project file's $properties has a non-default value and Studio resets it to the engine default, syncback didn't remove the stale entry. This happened because the Roblox binary format omits default-valued properties, so they're absent from the deserialized instance. filter_properties correctly excludes them, but project_node_property_syncback only inserted/updated — never removed. The fix adds a retain pass after the insertion loop that removes entries from node.properties where the property is not in the filtered set AND has a known default in rbx_reflection_database. Unknown properties are conservatively kept. Also updates the reserialization logic to detect when stale properties were removed, since project_node_should_reserialize only compares forward through current properties. Fixes #1243 --- CHANGELOG.md | 2 + src/snapshot_middleware/project.rs | 70 +++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c090788..d042de3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) +* Fixed syncback not removing stale `$properties` entries when Studio resets a property to its engine default. ([#1244]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -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 +[#1244]: https://github.com/rojo-rbx/rojo/pull/1244 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index c114ac5d..09d286de 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -344,6 +344,11 @@ pub fn syncback_project<'sync>( let mut new_child_map = HashMap::new(); let mut node_changed_map = Vec::new(); + // Tracks whether any stale default-valued properties were removed from + // project nodes. If so, we must reserialize even if + // project_node_should_reserialize wouldn't otherwise detect a change + // (it only compares node properties forward, not in reverse). + let mut removed_stale_properties = false; let mut node_queue = VecDeque::with_capacity(1); node_queue.push_back((&mut project.tree, old_inst, snapshot.new_inst())); @@ -402,10 +407,12 @@ pub fn syncback_project<'sync>( // We only want to set properties if it needs it. if !middleware.handles_own_properties() { - project_node_property_syncback_path(snapshot, new_inst, node); + removed_stale_properties |= + project_node_property_syncback_path(snapshot, new_inst, node); } } else { - project_node_property_syncback_no_path(snapshot, new_inst, node); + removed_stale_properties |= + project_node_property_syncback_no_path(snapshot, new_inst, node); } for child_ref in new_inst.children() { @@ -507,12 +514,18 @@ pub fn syncback_project<'sync>( } let mut fs_snapshot = FsSnapshot::new(); - for (node_properties, node_attributes, old_inst) in node_changed_map { - if project_node_should_reserialize(node_properties, node_attributes, old_inst)? { - fs_snapshot.add_file(project_path, serde_json::to_vec_pretty(&project)?); - break; + let mut needs_reserialize = removed_stale_properties; + if !needs_reserialize { + for (node_properties, node_attributes, old_inst) in node_changed_map { + if project_node_should_reserialize(node_properties, node_attributes, old_inst)? { + needs_reserialize = true; + break; + } } } + if needs_reserialize { + fs_snapshot.add_file(project_path, serde_json::to_vec_pretty(&project)?); + } Ok(SyncbackReturn { fs_snapshot, @@ -521,15 +534,18 @@ pub fn syncback_project<'sync>( }) } +/// Syncs properties from the new instance into the project node. +/// Returns `true` if any stale properties were removed (i.e. properties +/// that existed in the project node but are now at their engine default). fn project_node_property_syncback( _snapshot: &SyncbackSnapshot, filtered_properties: UstrMap<&Variant>, new_inst: &Instance, node: &mut ProjectNode, -) { +) -> bool { let properties = &mut node.properties; let mut attributes = BTreeMap::new(); - for (name, value) in filtered_properties { + for (&name, &value) in &filtered_properties { match value { Variant::Attributes(attrs) => { for (attr_name, attr_value) in attrs.iter() { @@ -552,14 +568,48 @@ fn project_node_property_syncback( } } } + + // Remove stale properties: entries that exist in the project node's + // $properties but are no longer in the filtered (non-default) properties + // from the instance. This handles the case where Studio resets a property + // to its engine default — filter_properties won't include it, so we need + // to clean up the now-stale project entry. + let class_data = rbx_reflection_database::get() + .ok() + .and_then(|db| db.classes.get(new_inst.class.as_str())); + let len_before = properties.len(); + properties.retain(|prop_name, _| { + if filtered_properties.contains_key(prop_name) { + return true; + } + // Only remove if the property has a known default value in the + // reflection database. If there's no default, the property might be + // absent from the instance for other reasons (e.g. unknown property), + // so we conservatively keep it. + if let Some(data) = &class_data { + if data.default_properties.contains_key(prop_name.as_str()) { + log::debug!( + "Removing stale property '{}' from project node for class '{}': \ + value has been reset to engine default", + prop_name, + new_inst.class + ); + return false; + } + } + true + }); + let removed_stale = properties.len() < len_before; + node.attributes = attributes; + removed_stale } fn project_node_property_syncback_path( snapshot: &SyncbackSnapshot, new_inst: &Instance, node: &mut ProjectNode, -) { +) -> bool { let filtered_properties = snapshot .get_path_filtered_properties(new_inst.referent()) .unwrap(); @@ -570,7 +620,7 @@ fn project_node_property_syncback_no_path( snapshot: &SyncbackSnapshot, new_inst: &Instance, node: &mut ProjectNode, -) { +) -> bool { let filtered_properties = filter_properties(snapshot.project(), new_inst); project_node_property_syncback(snapshot, filtered_properties, new_inst, node) }