From f635cdc1635ced349a5fb64cce62ce0f14f22346 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Wed, 11 Sep 2024 11:30:10 -0700 Subject: [PATCH 01/20] Filter default properties in json middlewares instead of filter fn --- ...o_test__syncback_util__all_middleware.snap | 4 + ...jo_test__syncback_util__child_but_not.snap | 4 +- ..._syncback_util__ignore_paths_removing.snap | 4 +- ...ojo_test__syncback_util__ignore_trees.snap | 1 + ..._syncback_util__nested_projects_weird.snap | 3 +- ...syncback_util__project_all_middleware.snap | 3 + ...ojo_test__syncback_util__project_init.snap | 1 + ...t__syncback_util__project_reserialize.snap | 1 + ...jo_test__syncback_util__rbxm_fallback.snap | 1 + ...o_test__syncback_util__ref_properties.snap | 3 +- ..._syncback_util__ref_properties_update.snap | 3 +- src/snapshot_middleware/json_model.rs | 36 +++------ src/snapshot_middleware/meta_file.rs | 74 +++++-------------- src/snapshot_middleware/mod.rs | 59 +++++++++++++++ src/snapshot_middleware/project.rs | 34 ++------- src/syncback/property_filter.rs | 26 ++----- 16 files changed, 121 insertions(+), 136 deletions(-) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap index 3db4bb1f6..79c03d271 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap @@ -11,15 +11,19 @@ added_files: - src/dir/init_server_script/init.server.luau - src/dir/module_script.luau - src/dir/server_script.server.luau + - src/dir_with_meta/init.meta.json - src/model_json.model.json - src/project_json.project.json - src/rbxm.rbxm - src/rbxmx.rbxmx - src/text.txt added_dirs: + - src - src/csv_init + - src/dir - src/dir/init_client_script - src/dir/init_module_script - src/dir/init_server_script + - src/dir_with_meta removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__child_but_not.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__child_but_not.snap index 06a661b0c..280232eb3 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__child_but_not.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__child_but_not.snap @@ -5,6 +5,8 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" added_files: - OnlyOneCopy/child_of_one.luau - ReplicatedStorage/child_replicated_storage.luau -added_dirs: [] +added_dirs: + - OnlyOneCopy + - ReplicatedStorage removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap index c2a0b917e..d4bfc84dc 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap @@ -1,9 +1,9 @@ --- source: tests/rojo_test/syncback_util.rs -assertion_line: 48 expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- -added_files: [] +added_files: + - default.project.json added_dirs: - src removed_files: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees.snap index dcdb221c8..3730bf053 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees.snap @@ -5,6 +5,7 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" added_files: - src/IncludeMe/.gitkeep added_dirs: + - src - src/IncludeMe removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__nested_projects_weird.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__nested_projects_weird.snap index f45539f41..a3a522539 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__nested_projects_weird.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__nested_projects_weird.snap @@ -5,6 +5,7 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" added_files: - src/modules/ClientModule.luau - src/modules/ServerModule.luau -added_dirs: [] +added_dirs: + - src/modules removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap index 4050efd19..e00aba349 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap @@ -3,6 +3,7 @@ source: tests/rojo_test/syncback_util.rs expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: + - default.project.json - src/client_script.client.luau - src/csv.csv - src/csv_init/init.csv @@ -17,6 +18,8 @@ added_files: - src/text.txt added_dirs: - src/csv_init + - src/dir + - src/dir_with_meta - src/init_client_script - src/init_module_script - src/init_server_script diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap index 320b704fe..dd0ed67a7 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap @@ -3,6 +3,7 @@ source: tests/rojo_test/syncback_util.rs expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: + - default.project.json - src/init.luau added_dirs: - src diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap index 441e0b7b9..5f6b67b4e 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap @@ -4,6 +4,7 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: - attribute_mismatch.luau + - default.project.json - property_mismatch.project.json added_dirs: [] removed_files: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__rbxm_fallback.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__rbxm_fallback.snap index 6b308f1d7..daf6a8e58 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__rbxm_fallback.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__rbxm_fallback.snap @@ -6,6 +6,7 @@ added_files: - ReplicatedStorage/ChildWithDuplicates.rbxm - ReplicatedStorage/ChildWithoutDuplicates/Child/.gitkeep added_dirs: + - ReplicatedStorage - ReplicatedStorage/ChildWithoutDuplicates - ReplicatedStorage/ChildWithoutDuplicates/Child removed_files: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties.snap index 77d69e6e7..a67fbf168 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties.snap @@ -5,6 +5,7 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" added_files: - src/pointer.model.json - src/target.model.json -added_dirs: [] +added_dirs: + - src removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_update.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_update.snap index 77d69e6e7..a67fbf168 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_update.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ref_properties_update.snap @@ -5,6 +5,7 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" added_files: - src/pointer.model.json - src/target.model.json -added_dirs: [] +added_dirs: + - src removed_files: [] removed_dirs: [] diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index c176066c3..3fdf4d018 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -17,6 +17,8 @@ use crate::{ RojoRef, }; +use super::filter_default_property; + pub fn snapshot_json_model( context: &InstanceContext, vfs: &Vfs, @@ -97,32 +99,14 @@ fn json_model_from_pair<'sync>( let mut properties = BTreeMap::new(); let mut attributes = BTreeMap::new(); for (name, value) in prop_buffer.drain(..) { - match value { - Variant::Attributes(attrs) => { - for (attr_name, attr_value) in attrs.iter() { - // We (probably) don't want to preserve internal attributes, - // only user defined ones. - if attr_name.starts_with("RBX") { - continue; - } - attributes.insert( - attr_name.clone(), - UnresolvedValue::from_variant_unambiguous(attr_value.clone()), - ); - } - } - Variant::SharedString(_) => { - log::warn!( - "Rojo cannot serialize the property {}.{name} in model.json files.\n\ - If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", new_inst.class, snapshot.get_new_inst_path(new)) - } - _ => { - properties.insert( - name.to_owned(), - UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), - ); - } - } + filter_default_property( + snapshot, + new_inst, + name, + value, + &mut attributes, + &mut properties, + ) } let mut children = Vec::with_capacity(new_inst.children().len()); diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 3004ad2aa..325a03e31 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -6,13 +6,15 @@ use std::{ use anyhow::{format_err, Context}; use memofs::{IoResultExt as _, Vfs}; -use rbx_dom_weak::types::{Attributes, Variant}; +use rbx_dom_weak::types::Attributes; use serde::{Deserialize, Serialize}; use crate::{ resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, RojoRef, }; +use super::filter_default_property; + /// Represents metadata in a sibling file with the same basename. /// /// As an example, hello.meta.json next to hello.lua would allow assigning @@ -80,34 +82,15 @@ impl AdjacentMetadata { .map(|inst| inst.metadata().ignore_unknown_instances) .unwrap_or_default(); - let class = &snapshot.new_inst().class; for (name, value) in snapshot.get_path_filtered_properties(snapshot.new).unwrap() { - match value { - Variant::Attributes(attrs) => { - for (attr_name, attr_value) in attrs.iter() { - // We (probably) don't want to preserve internal - // attributes, only user defined ones. - if attr_name.starts_with("RBX") { - continue; - } - attributes.insert( - attr_name.clone(), - UnresolvedValue::from_variant_unambiguous(attr_value.clone()), - ); - } - } - Variant::SharedString(_) => { - log::warn!( - "Rojo cannot serialize the property {}.{name} in meta.json files.\n\ - If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", class, snapshot.get_new_inst_path(snapshot.new)) - } - _ => { - properties.insert( - name.to_owned(), - UnresolvedValue::from_variant(value.clone(), class, name), - ); - } - } + filter_default_property( + snapshot, + snapshot.new_inst(), + name, + value, + &mut attributes, + &mut properties, + ); } Ok(Some(Self { @@ -266,34 +249,15 @@ impl DirectoryMetadata { .map(|inst| inst.metadata().ignore_unknown_instances) .unwrap_or_default(); - let class = &snapshot.new_inst().class; for (name, value) in snapshot.get_path_filtered_properties(snapshot.new).unwrap() { - match value { - Variant::Attributes(attrs) => { - for (name, value) in attrs.iter() { - // We (probably) don't want to preserve internal - // attributes, only user defined ones. - if name.starts_with("RBX") { - continue; - } - attributes.insert( - name.to_owned(), - UnresolvedValue::from_variant_unambiguous(value.clone()), - ); - } - } - Variant::SharedString(_) => { - log::warn!( - "Rojo cannot serialize the property {}.{name} in meta.json files.\n\ - If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", class, snapshot.get_new_inst_path(snapshot.new)) - } - _ => { - properties.insert( - name.to_owned(), - UnresolvedValue::from_variant(value.clone(), class, name), - ); - } - } + filter_default_property( + snapshot, + snapshot.new_inst(), + name, + value, + &mut attributes, + &mut properties, + ) } Ok(Some(Self { diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 4b1c00f76..a170b7518 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -19,17 +19,21 @@ mod txt; mod util; use std::{ + collections::BTreeMap, path::{Path, PathBuf}, sync::OnceLock, }; use anyhow::Context; use memofs::{IoResultExt, Vfs}; +use rbx_dom_weak::{types::Variant, Instance}; use serde::{Deserialize, Serialize}; use crate::{ glob::Glob, + resolution::UnresolvedValue, syncback::{SyncbackReturn, SyncbackSnapshot}, + variant_eq::variant_eq, }; use crate::{ snapshot::{InstanceContext, InstanceSnapshot, SyncRule}, @@ -388,3 +392,58 @@ pub fn default_sync_rules() -> &'static [SyncRule] { ] }) } + +fn filter_default_property( + snapshot: &SyncbackSnapshot, + new_inst: &Instance, + name: &str, + value: &Variant, + attributes: &mut BTreeMap, + properties: &mut BTreeMap, +) { + let db = rbx_reflection_database::get(); + let class_descriptor = db.classes.get(new_inst.class.as_str()); + + match value { + Variant::Attributes(attrs) => { + for (attr_name, attr_value) in attrs.iter() { + // We (probably) don't want to preserve internal attributes, + // only user defined ones. + if attr_name.starts_with("RBX") { + continue; + } + attributes.insert( + attr_name.clone(), + UnresolvedValue::from_variant_unambiguous(attr_value.clone()), + ); + } + } + Variant::SharedString(_) => { + log::warn!( + "Rojo cannot serialize the property {}.{name} in JSON files.\n\ + If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", + new_inst.class, snapshot.get_new_inst_path(new_inst.referent()) + ) + } + _ => { + let new_prop_is_default = if let Some(class_descriptor) = class_descriptor { + if let Some(default) = db.find_default_property(class_descriptor, name) { + variant_eq(value, default) + } else { + false + } + } else { + false + }; + + if new_prop_is_default { + properties.remove(name); + } else { + properties.insert( + name.to_owned(), + UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), + ); + } + } + } +} diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 0cd0e3599..5afb921cb 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -25,7 +25,7 @@ use crate::{ RojoRef, }; -use super::{emit_legacy_scripts_default, snapshot_from_vfs}; +use super::{emit_legacy_scripts_default, filter_default_property, snapshot_from_vfs}; pub fn snapshot_project( context: &InstanceContext, @@ -534,33 +534,7 @@ fn project_node_property_syncback<'inst>( let properties = &mut node.properties; let mut attributes = BTreeMap::new(); for (name, value) in filtered_properties { - match value { - Variant::Attributes(attrs) => { - for (attr_name, attr_value) in attrs.iter() { - // We (probably) don't want to preserve internal attributes, - // only user defined ones. - if attr_name.starts_with("RBX") { - continue; - } - attributes.insert( - attr_name.clone(), - UnresolvedValue::from_variant_unambiguous(attr_value.clone()), - ); - } - } - Variant::SharedString(_) => { - log::warn!( - "Rojo cannot serialize the property {}.{name} in project files.\n\ - If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", new_inst.class, snapshot.get_new_inst_path(new_inst.referent()) - ); - } - _ => { - properties.insert( - name.to_string(), - UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), - ); - } - } + filter_default_property(snapshot, new_inst, name, value, &mut attributes, properties) } node.attributes = attributes; } @@ -590,6 +564,10 @@ fn project_node_should_reserialize( node_attributes: &BTreeMap, instance: InstanceWithMeta, ) -> anyhow::Result { + if node_properties.len() != instance.properties().len() { + return Ok(true); + } + for (prop_name, unresolved_node_value) in node_properties { if let Some(inst_value) = instance.properties().get(prop_name) { let node_value = unresolved_node_value diff --git a/src/syncback/property_filter.rs b/src/syncback/property_filter.rs index fdbcd1dad..a07776185 100644 --- a/src/syncback/property_filter.rs +++ b/src/syncback/property_filter.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use rbx_dom_weak::{types::Variant, Instance}; use rbx_reflection::{PropertyKind, PropertySerialization, Scriptability}; -use crate::{variant_eq::variant_eq, Project}; +use crate::Project; /// Returns a map of properties from `inst` that are both allowed under the /// user-provided settings, are not their default value, and serialize. @@ -54,27 +54,11 @@ pub fn filter_properties_preallocated<'inst>( false }; - if let Some(class_data) = class_data { - let defaults = &class_data.default_properties; - for (name, value) in &inst.properties { - if predicate(name, value) { - continue; - } - if let Some(default) = defaults.get(name.as_str()) { - if !variant_eq(value, default) { - allocation.push((name, value)); - } - } else { - allocation.push((name, value)); - } - } - } else { - for (name, value) in &inst.properties { - if predicate(name, value) { - continue; - } - allocation.push((name, value)); + for (name, value) in &inst.properties { + if predicate(name, value) { + continue; } + allocation.push((name, value)); } } From 6b9e515d875be2179c003212aed95202804ca0ce Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Tue, 17 Sep 2024 13:49:46 -0700 Subject: [PATCH 02/20] Update ignore_trees_removing test, accept snaphot --- ...o_test__syncback_util__ignore_trees_removing.snap | 4 ++-- .../expected/default.project.json | 12 ------------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees_removing.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees_removing.snap index 9fa9aa7a2..46b76d56a 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees_removing.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_trees_removing.snap @@ -1,9 +1,9 @@ --- source: tests/rojo_test/syncback_util.rs -assertion_line: 45 expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: [] -added_dirs: [] +added_dirs: + - src removed_files: [] removed_dirs: [] diff --git a/rojo-test/syncback-tests/ignore_paths_removing/expected/default.project.json b/rojo-test/syncback-tests/ignore_paths_removing/expected/default.project.json index deefcff22..89b0cb041 100644 --- a/rojo-test/syncback-tests/ignore_paths_removing/expected/default.project.json +++ b/rojo-test/syncback-tests/ignore_paths_removing/expected/default.project.json @@ -4,18 +4,6 @@ "$className": "DataModel", "Workspace": { "$properties": { - "CSGAsyncDynamicCollision": { - "Enum": 0 - }, - "DecreaseMinimumPartDensityMode": { - "Enum": 0 - }, - "MoverConstraintRootBehavior": { - "Enum": 0 - }, - "RenderingCacheOptimizations": { - "Enum": 0 - }, "SignalBehavior": "Deferred", "StreamOutBehavior": "Opportunistic", "StreamingEnabled": true, From 579f55c82d12f728d2db7fa13c1e2106f7c59041 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Tue, 17 Sep 2024 17:30:22 -0700 Subject: [PATCH 03/20] Add remove_default_properties test --- ...__syncback_util__remove_default_props.snap | 16 ++++++++++ .../AdjacentMetadataScript.meta.json | 5 +++ .../AdjacentMetadataScript.server.lua | 0 .../DirectoryMetadataPart/Part/init.meta.json | 6 ++++ .../expected/Part.model.json | 10 ++++++ .../expected/default.project.json | 23 ++++++++++++++ .../remove_default_props/input.rbxl | Bin 0 -> 47058 bytes .../AdjacentMetadataScript.meta.json | 7 +++++ .../AdjacentMetadataScript.server.lua | 0 .../DirectoryMetadataPart/Part/init.meta.json | 8 +++++ .../output/Part.model.json | 12 ++++++++ .../output/default.project.json | 29 ++++++++++++++++++ tests/tests/syncback.rs | 1 + 13 files changed, 117 insertions(+) create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__remove_default_props.snap create mode 100644 rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.meta.json create mode 100644 rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.server.lua create mode 100644 rojo-test/syncback-tests/remove_default_props/expected/DirectoryMetadataPart/Part/init.meta.json create mode 100644 rojo-test/syncback-tests/remove_default_props/expected/Part.model.json create mode 100644 rojo-test/syncback-tests/remove_default_props/expected/default.project.json create mode 100644 rojo-test/syncback-tests/remove_default_props/input.rbxl create mode 100644 rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.meta.json create mode 100644 rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.server.lua create mode 100644 rojo-test/syncback-tests/remove_default_props/output/DirectoryMetadataPart/Part/init.meta.json create mode 100644 rojo-test/syncback-tests/remove_default_props/output/Part.model.json create mode 100644 rojo-test/syncback-tests/remove_default_props/output/default.project.json diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__remove_default_props.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__remove_default_props.snap new file mode 100644 index 000000000..cd0063571 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__remove_default_props.snap @@ -0,0 +1,16 @@ +--- +source: tests/rojo_test/syncback_util.rs +expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" +--- +added_files: + - AdjacentMetadataScript/AdjacentMetadataScript.meta.json + - AdjacentMetadataScript/AdjacentMetadataScript.server.lua + - DirectoryMetadataPart/Part/init.meta.json + - Part.model.json + - default.project.json +added_dirs: + - AdjacentMetadataScript + - DirectoryMetadataPart + - DirectoryMetadataPart/Part +removed_files: [] +removed_dirs: [] diff --git a/rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.meta.json b/rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.meta.json new file mode 100644 index 000000000..cfadea686 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.meta.json @@ -0,0 +1,5 @@ +{ + "properties": { + "Disabled": true + } +} diff --git a/rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.server.lua b/rojo-test/syncback-tests/remove_default_props/expected/AdjacentMetadataScript/AdjacentMetadataScript.server.lua new file mode 100644 index 000000000..e69de29bb diff --git a/rojo-test/syncback-tests/remove_default_props/expected/DirectoryMetadataPart/Part/init.meta.json b/rojo-test/syncback-tests/remove_default_props/expected/DirectoryMetadataPart/Part/init.meta.json new file mode 100644 index 000000000..138a460e9 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/expected/DirectoryMetadataPart/Part/init.meta.json @@ -0,0 +1,6 @@ +{ + "properties": { + "Anchored": true + }, + "className": "Part" +} diff --git a/rojo-test/syncback-tests/remove_default_props/expected/Part.model.json b/rojo-test/syncback-tests/remove_default_props/expected/Part.model.json new file mode 100644 index 000000000..78bdb8435 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/expected/Part.model.json @@ -0,0 +1,10 @@ +{ + "className": "Part", + "properties": { + "Size": [ + 1.0, + 2.0, + 3.0 + ] + } +} diff --git a/rojo-test/syncback-tests/remove_default_props/expected/default.project.json b/rojo-test/syncback-tests/remove_default_props/expected/default.project.json new file mode 100644 index 000000000..8adb932da --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/expected/default.project.json @@ -0,0 +1,23 @@ +{ + "name": "remove_default_props", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "AdjacentMetadataScript": { + "$path": "AdjacentMetadataScript" + }, + "DirectoryMetadataPart": { + "$path": "DirectoryMetadataPart" + }, + "JsonModelPart": { + "$path": "Part.model.json" + }, + "ProjectFilePart": { + "$className": "Part", + "$properties": { + "Transparency": 1.0 + } + } + } + } +} diff --git a/rojo-test/syncback-tests/remove_default_props/input.rbxl b/rojo-test/syncback-tests/remove_default_props/input.rbxl new file mode 100644 index 0000000000000000000000000000000000000000..9f20f6b05c1083e99966a61da3f735875eff9e34 GIT binary patch literal 47058 zcmcJYU5p&pmEU`2n!|636h(@pWT_?7GG&Vr$)RLgmP8I`L~3MGGoBfeiV>_`(_J&u zR!`Tsx`*V@vP8+Yc2`)$+Jz8U`()T_EH+pq*=*KAo|4CPVj$WL^00w|7(ueZZuX&U z%S-@e$(-MbFus%yFo>;-D3tNVZM$N9SF-n#eJGrqUb@~%Jh+jn+uJG8A*IbEq# z&dO!e+}!-^9=W+c`eKIvLz^nwhAZY{W$LB5`I>z906$0y?&9wbxyCykw>#(hSDSTr zXoznsmCHlmKO>)xn0vt=G7Eq;t{qi*z=}cArR4YHD zO11&JOm>gS4>W=l+_y>oQUJKA_Uc;qtmEf4;el-9sRk8;_DZwe@UG=j-Vf!c1oug~ zNWnduK$F>N8;h5L%y4Snod33Xe?qNaJU8?MsYtrdDb6wAOQ#8JzIV6Y!a*=|q+vP6|nRFNYX2-Jp z1%v#Ae1D%@q+kd71@fHNYPfy{MF>;FND(eQ6nzuiy;DA+mFGp=GfpR6`*SA$2L(VG zq+l0qr8X~gyQ`^+ujW*T+-dOCinHWia2DK_RjR`Xj;fG9DcB8HsM1usIcsvK zp=cB-*oKr5oDg)KcHOmkUm$M`6E2d1?RU#xpck4;%iU&s$?~vkvVBB;L5WCb_R62C zH1&d4cUsLG#@13zecDiIO+b6#&IPYwM62!ls-e*B-6v?;dtuGF;x43$__86;^1}Y$ z=tZaN`c0>mA~|44o)o~CT%_PICPb+(dF@$uwbiT(X`{$3Y%`g_V=MGu8AZh8XB@v< z84)gvhx_XA7Pt)O&~H+(4>CACFL4xk2bZ>Hs}yZ-YlYb{4CDaDS*458K-lmh<{62_Kvw}Bp$AD)nlbcWW8795!I zy}H}!SQX2!<3Bd^+I!>o5tw9fy}8otuFHp&)wEh)HV9*zYmH|yYnX@&MGb)|HqV=!wHKm2c*Z$5 z^1!wlFBx#Jxdoe0A8d|%T|X>iN9BW_&aq{vwDHh>Q>Tzhx3+Bv@bI=X`sFqX#iK^m zzufFJ7n&`R%bv>jZS`&#x|*PpUK_2$tIm0A?MABYqXw_#PO0#Ct}E8!`qF>-PF+5P z4eMAopM=enVgra^CpwO^j402qHEq34nR2uzrc^v}gr0HguUPm~2Cn(_S%DgUh1CiT zz!DrY1hmw8!C*gKIX=ziZ%i%Kh!fdH92JtVTE4j1%T8-8cFU7l^y7jKo4nR&diYIS zrN>Q`w8CPXo8+2zot4;jUo}8&L0B>tG%xzM>Sg+pXAFt%Vvh?P&oJ-$zSE3@t6}P_ z8Gk}hv~b?-q^8> zzb)~|klaXm1g>hA@p<_gO}I35-gg>Jw;emAhXL!}{AmH=UoK6}IE{wrna%h}Hsdcu z*vqr2M$VWDYo&ii!1$ue&4%kuIIG>YOnkN_hwRyy%cj`n6M)->iUTAj2`L0hj9=y9UVh2AKPJ2lK{k!z1 z5{O8wIYg~`M?t`A@?ByAaud2MZ_Tf}20PV2xk!dPR28Ks1;hL$`*~-n6SG4*1t3Nl z+s&`9xl-RsW#r`O$8XOc*Pq@`643i3WHj(o|dvylOe=a21p?N`6LWiJY%n}C!Q9Ds~4BO#mj zowjI`@3!mfn|}X2(!V(kp;kT!kxDqaoQwvQ)i-}8ANBw-;kx_ADI7bw(eBXIu(OHoY!RHYEC%&_}4mK`=g=Cr;k^LHXo_{%cB?FMsscD zaj&~*`DTseR+VD3k%*_7kuO8O(OJ?llY;vJBT6&y2s`Mx?pmWWy|^fqG8P1oL`k}D zgc$#M(YYQQZ~o?QsjBYLAV6;*m=m5@mvjWE_BR%KsmI^kO#7RBuW?z% zzcN&7^@yML8nJ7NVfl6=KclS^iW1go3+US`;8my-=~fkZfx>H-2RPOkGy9S=UzJrC z$jYdTEvBGv0Fv$-D#fy*1Kf!7#3^UwBK=|%3dUO#=X~h|v81P5FVD-c#q7j{oRBAR(^Z1a`RhfDitm`pyMBMf&&v~o^Eb!;cGI{MP-aL)Q$lxDjs6z z*H>L!qRCV%2EY=E0h}WHDu#ADz;Qe0nr^Exy?D9lUaNjb#$z-NPmm0QKoT(bATN52 zq}I9{1I?J=oL67#>_(0zy>TYw1^yJxA_bcO!;2SOry*YU72ACW0Kll{1 z{@yqK1v%_9rH|%~r33zS!3E)fbp!MGy=F&xD{lz6630 z{(<95iYd@s?N|Y$xnFeG7d*#r$p4ARv6=|^G5L*%Fq+^0Cm?T7%F zVIu|iQWETvAhzxH;8+7j@SD00JbPjtCC}&9?#r?fTv;Zp0NKj}v71R*;aX zAzn!NGh+qX@7t>NMxd`3djfi#jDMJC*c3ynqK1Tbh) za1e^v5irWa5#&6Q&6+cmWz5-@q|8~HCy-L+3BiEo2^39E>}(+OgbYni$Obe|TeV*E zd@XIBHOV>gW{}`m?Eo<=I~{d`hh0~ijqbA6A*~kZ{6==KyI3en*r6?;uN~@;40wS~ zAC{kK04dlDIMz_uBXZM^{F6LEFdgXy zcgd-*OYki7 zdkyR}X^}i4u2v);EOKz`5og{TkS_Oi}jJNB{o?Q02ECD-^?W}R$eB1JTJ#w`EYwe3z)^4X*JW`|2P4(H~Qe?%E)s*AGcDk5?@3 z&wvxt-4xjE1ZpY#AiL$}N@&l?x3ibj;y;*J4yC{7g;lx$Z9R@U1W!CG; z$jNILh#NaGQ$y<6FkUEma7uQ_E|f`PQ27;&C3n_ccCI!(9|^{eazGlS7}Ml?$xJy$ zk}*-%IAK#P`nQI$Dnh=own(PrE zwr+Il&*kRNv7SD83Ql&*bh@l$O_+!ozmN}1n%`p>xSmO@ zNiN!iaY%zk6}E9Isz7wst^!k4lvy4{yM_li(`x0W{51(qWV=KtdWTPMrsEK_0cS%B zhN%ZLcxts!wpSrk`SMh|;a-nBL0vRG8qynYwPeqdtR8F2Q?rgN=)uBUQRk+;5+syl zNJ8*A-!(455@=n9fZ7+WE=1Z^gMwX9l!#?*0RzUezzbs8n*5w72w=3?JXkb+S?_+I zY@YzWj6&J`l|p4@lva-;Y4y-7M^VbL9EuVww*?Hya^M9lr@b_Q^a0o&jt9f%WKDt(K(aSMvyd)#Wr=jBdh=!o)W!y|Q1V9vU?(bqdx66n6EV&$Hy68KIqP+0 z&XYXiM16EO2pwHW58b@Cnjy;84IcGXGwO9%E>f_MGVo^4x34yR&palaY%VUYbzI#C z=+O^U#Faf%bE|G$wrR;WnBE0d+AJZX8-4!zs@IW~_Y<n-`t z>T}l1h za=A3kZiDQvRMTtShUfYH3{MY^EXRbX#omE1M$#c97)<#~vOz+2U2$sydL6>>wF!$1 zd*-~w(9T|c@4errt~L2N=}7>W@loIrj`C4iAz|Z*TYzXJzifoW9-ZmMNoRcsz8*jM z^w_bdj~xS_)ol>H5y^78%ONbN$wfMYm5PFG9lVo=qR{WaIvW$%{73tG0Vw52Fk%<4s{Q2f#c;Km5X$1JNYx>n2_nRm1dnGYR43@V_#*) z+(CnP{H$E0|FZ)c6Y$BNTA9>YOo(>w?;)ZQ#8sqUZG(t0>V(H>AhPGK9qvcleotk_ z+)t%1wtF24u_dCrkTGwP;7CiFA6z_sgyS@7nZ z*9;qsWpe{G--6D|^46y7o2;>*=)QnJw&pdXnj3=f9)f&}kiD>-P@S*{a|bxfOB%}b zRJ##3G`zKWC*dUxEpTx<9s*2Fr|O~hjD(-bl^oG)(nnhX61ZJ(oyGN}+S~S4X5wn+ zb5SU|%RO9-?kOVQlrKogmFPdF9P58RV=g+rm`u!=@YXU0Eg%KPEMZKd^DsTu`FzG) zTnjl4W8|}COigY{!5$bBcM=$I>humM^38U}xhl&>+My3ZHSHIW{A-eb61fmeYPE^rJW=b7v`Pk^?E&!z@6# zI0uTtSVTA07SOlT(<3h61&(@5em)@=>1SZ!g%SEgTctG{9r%;g2=XYS5w^?%jR2C; zh~YbFjX-S}I*@`*%o{-xKJAwsdbx|zK^4hLem03#YTpNC7@LYSlCxP>#;dQr_F9M@ z3YP_KKyWr85VEWuQ33%r&;IAu{6$&9#3VwTp!l@h+0HZA4+TMF4~5xjlOfX`1!SmS zI9u4KF&mO(n{!z5wDCf)mr_uRu#`-Ls?{nDIwA-fMGCeTQ|)Ysr}%!;u8@RcgX%e`(ES327+j zz9@5$ibd6=l%sNyZksZ*Cf~bau3sJHv=}p~W%GSZ@;ha6tyKQra5y{*5py)sY5^f;SYcCgCG3p zM?V6Nf}#tgGoWQUYUjk7^mQxYIap3h4z*fkv%1?MJc1}|JlJWj%L@szm11lZu5vop zT5C4Wd9rLwW=cb$p_nzfNWmW4hI2AG%~2eA=DZO_)&TNt#*~rKf2jt|M*kn7PLhIq zsTQs1CCRtwvx4Fx{le&f#^ncxrl%|Ic4c(5^6|%&x8JVZxKVlk{mQ<56*-RYz4t2b zzFWC{yYk_Ol^>fUOtJ#NMtGdwhd&z-WZ9F?P)g{>oqTuKLy^xMdIjqPgAtgDbBK0s z%O@D0Z^Rd6+$3?N%tZH7GWr-BZbW5>hDGovCb^zwF5Ad+qMrPI+uB zkJ=kh#fdsSEq9-fz6l`e2j7p)cAo$-@n z`|t1Dw{O?3U2@RTyYIex`}XY*Km72=p)Vw$elp7OPMl*Oq0&Yao0Nn-i=MwCCj`k_ z^I7Mbbw(1ChP{zq(FkuC(sh=d=$H-ivRHzM-9I@5C!|{&DUSA zjE#*{es=QYO-T_pC&a`X6@|ZWQ}?S-WaE~ognW44ejB(Rs*ro8-%7$Z0tu{=6x;(C zA7{oYa5Q^dC?(Y7m&6Vr%CRx|R+4#;NOkFRNx`r!BsMX!)Sdx|Rk>d>cdR4JcpnR&usw(JO`5I`Qc)o<2uXn9Ew7o zYrX*IIZ71Sda@u+;@L0&mEk9R(Y9o3)K^GaX)HWEr&f;Nm1$bp$i~?VLsG0MAX_60 z!&0!r!aPDv2(48&_~mSV%~Q0FMq44$DyLmg6l#~&7hAx9`U1Q_UwTJUAApUeFQcbjGVtktx^`A%(3cUJ zEw4aiK++;Rn--at04h{Y0#KBYfGwb}1SmZTb%0|^)~CkHs;D;URuxErkZSTX5K?d# zpU54fHfNT1qBYUjbPcKCtg%*7Z~%NPRntM_9(kS>Cxm6hX$vY4Cy=x_zg8?xw7;A< zp(r6vTR>lNQfVuzU>LZP)t-tRtC$4SS{Xsd%x7Gd906}mjn8r&iZWccB@}QSNSf>0 zi@9DCl5$*!q6F7%0e!iS}Ux*X`=7-BNz8;d7SJXt?+YV7E-`f>N@$;Gj;qn~dy>PJr;Ym6<{Wvlzi!r+Tg7hslxk$ktNEk9i!@H=qnm^iCGw7q<(YByj zSDe8DQ&EPOI(M+;lmUto%3urVs|?!l0Y@3^YYf7NI@O#1?AcWmL5GpHDwE!-Y|c;Z zTGKiMv_NOjMhHm3F2J$Q&{m1%;@3)14Z0(gm=xR%oVZg?36TABJRp|YwH}vkhWhz6 z=fG}naZ&HLt4UsolOgvqT4*a@poKtES~&cRw9}vl=oKm035n4{ueG)!C%iT5bIh{5U@ccBl>|L7_pWE_QSr{%g3ae6+-*oO}bcNhuOo6Td zN$bj?w64?yq-#MNFE`CQu_o61j_0dWPPA@eDdltviW0hI3m8zhfNzznk8WuJ0giQx z#}AQi@m9X9F@r86Ae(!EfPkb0bTBO-t>OeTsCal#g;~|t^W-7N^+K?bgmhm71+Jav zQ!lQq#7CEz%pa2m=%3!))Z>_Q7 zRXLV?&4Ne{6p%OP`Bs48{u)I~OpK_W*-t<^yDZ97F15Ja!gd%onVzj2c+Pj zEi-K*+{`BAm!&AQnP3=UF&kxzjn1{PnF;O^{PGZx>@Rh z`8DrJm#+%WY9dOi_+?%#%D1MEz@~5kq+oxZY-W!?883T}HU7B3pD;v1R)dW0y0;uN zESh!NOYT{(vChe7(e(aJ^U8^hwK2&6Rlav@taG{|@AJaNQ8F2jf(Iy_!A^eZp_jjR zadGkV;^JF|d`!SJfD}9oc}=c-a+cpqm?@U4s=u*jCDk{t29WxX#r4ncD5aUL?y~f% ztFjIw?p*Dg1ND%YN*BaVS*J(snUIWon(@_DR~EO(%Lzl5M2^}>;i)3I!154TUDcB9 za^j<6WytSBWUGE$7!PrLUK!xWL^a7hiDfChDwmlCHIZBITa=M`euoaeAIGvKn2ZNJ zBiGibAS48m4havWLqa`7f`ph|`gwV)x+m|LwR6Kc;UJE0qi|67_E7AQYr+}mg|xPS zzQKW3I^YFLr(Fr)SfX^fHeff4jQS44AuA&$TTFqNfTYEAH7zEsUO1O>oQGnETmy36 z7BC>^ffsPTCO;>(1UTmWPPqvWyb~Apg40>Xma=$YgPtd7^DkgIkTlD$r&<2AfKJIp z3Lb@IlU(M7=HWx!Q%^nRG)_4ujvib1+}P2R3-#kiKmWO7PaSoRJI<-b;;FI4g(uBJ zTrE6&b2;HbQ9^jOfWE?`Em%4!xF0zEa%kzH=F_7(3uo6Q3wEAOw{f&#z^`o%;sVKQ z?*cfMJmV-{ZyLV!Ju7y=JHZU-5N+lKVh55I`~I}pwXo>Q%LxmL62h_t^c9xoH}C>} z6Sy+UBL$lQ$NWZl=Ur!|lVvryGOV`w7O)yfn$@oqv-+5jlw&m%C0K0>7?9P#3s|jV z7{D>BcgxMZoUBM>R_pr+2$C{fw?!0i9Y~t%52d+Y6Hwxf2{h^P$Ln8TlWnI&l5*h1 zve$?sA-!G)JVnI2vvUFiFdIoyPpm+JLty{2-Q^UG05$;1_9mf2r}a8xa@-!}*t@R@ zqz+0zBC*`P*T5OUPzzG94LC}t_gqJn-4L2oS3g%HSCm2Rmqr9;GFT6r!19yACVQ}` zGV)Cxus|>>SM=WDVfh#*(0B}_ObRxEMi#PJ+6=7NR1GR$4+Ir$sV>As8>+!%xnF3* z{VJF2rY6(FQeiwdDcDOBcF7eF3^RMF^DBZHXqspbHSnw=qF(~{e7kr02WHoFwl28k z{@bR>JinEbZr$_EbAeC$N%t$fBbH%-dOyv z|KorD@4vtE?yFx}UtRd(i!c7po6C0&Ow7#BzxCGDpZ?{4``TM)?%cV3=a>KPyYKua zr7;J~Mf&+xDurBZY02*EcjC?0P-QtC_sE-td|oz@$Z_Z(kI1dfKe(6tG397C+<9MC zY0|pk5&07*=yabS)Aeq~j`8v#zfu&H-i%#vUW`?BT6CQ8T#KuGwC90C}= z?q7S2>Y%Oaguph6xpjX41x)AJ ztSn^D-8u-SFgnwGww34`k!TkU9B;_0fBdF-{xl@FIT8U$?VrCchlqFGD@}Qst(iNJ ziswyBfG_Cfv#cZjn(B-lJGa`IU-sqj^_G5{{b%L(N97^~hsd6mPj#X`qvxD+IZ-~! zAo5PxHY5M3&RolNSIx6`+h1z~rI5G-gH7{t=wZ)jGKWiPCEUWekQD0NnVK-$AY>?Bzxr={48rI3KdQ>-4-w)(}5Q-UH3JBW2V#a z{(D}4mf^O|x`5k2(%hCKUf#a({=QxBy?gsZbFh^*?fZPUNz-{e1 z0LR=W)-V$!CIahm@iqA&QF3U?u-%qXz;+;Mwm(_S_NRoT9NVEN!FF4~fNTd|z;HI{v#IG;cAt}RkTSNiZfuy;27y4!2*+k!ARHiR;e4xDIG-1ia>9Y4P&kBWPs>I6 zXb3pbsF~T?KP(elkK8;hEE=wC*)7L(od%Vak*!u=8Bu4vJt-IlPV}q7J{>m!DG;L` zTLO;7$N>4$luU>=SX@qB56U<9%0&vc07l(%-Z%Zsyh*YqUnad1uri7P zp2{l4KnRSGN&nAO(Ku;Y2PB8N_-?ztz7JV^KNJ8c8m2vtt#9wPhBu1U3AvY3CnyTl zNgJ;%ps!+RsRJ*Nx}JRi97`RlOH(sWqaiDxvepY0Aw_`j36ga>yTR*GC3r?15uUkd zh0eATrbK8ncc|c6UCA6Rjo>n4rNA}$DN!$5^FfybfX}FxRumvA2zhzr&S!rsCQffGTUaJli+W09(do*}Rp@(mDksty9N~b?O-*DW_9Vln}iwpsy^o zWPle)<`Mb%uw0~IAK;kXwBoWH!tG5stLSZC#zR$x@wSu##sf()e)x7VEi+V!YXAWrmO!owl#hla}h5bU|udKIE1WFdHR%M-XI)q z%<}wbZn?JuAzos+6)C7u3KfWJW0pQlI$dXoZb%NeuHKQfS#*+lL3n<@d`~Y#3bvC0 zDkA&!B;e|XTUTJJS&4E~J}?(f3hoE{fLyXpK_(kJ?!~5@T(`D@kRrhUS73`2VigK!Fmu}eXEsG{K^Lc*Gk+tA$ zn$eMG6S6dXJ=%=*n;(TUuDa8v5Tc}>H5IWA=($;tXnyz#&@0V$gU&uqNP}(^ohJpG zAYh0far{;6Z#Ty49QP+@aJ$L&q^t~>^EKfKcWW~)-nuxoYpby(* zkC{B#v~rO*BXu)HW!oS32k62T&woV*O?8*{=Ud)OvM;Y=j*`FFT#D8i8OMc%qco)8 z5KX7B8F@{Y98bx}LiPttt*rV|OL>K~Gf=|GBBo`kpXQ$*NUPPPmHfY$a z*r~rHRqWXklg|RPCn!1<<8>()&3Qon8#9vsp*zJm=bB4$#H*FX32HD5x+Wi>AO&|( zf9$OPn5`ziYWyDX#=K-4VGD@!&+0s#T5OJXBE<+V-+p5(=hQCoE&yweW{pI?{l-Mn z!G#lbBU0?me9wZu#{~0+^)byB!&CH&c9Mc&xP;}Z80!=A1^$y1>;M8mw1nx3+iG;q zHLu&}#@Ut{;o*_d;i1Yq)#0kVykhgF-PRSz*rkj literal 0 HcmV?d00001 diff --git a/rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.meta.json b/rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.meta.json new file mode 100644 index 000000000..650d48392 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.meta.json @@ -0,0 +1,7 @@ +{ + "properties": { + "Disabled": true, + "RunContext": "Client", + "Archivable": false + } +} diff --git a/rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.server.lua b/rojo-test/syncback-tests/remove_default_props/output/AdjacentMetadataScript/AdjacentMetadataScript.server.lua new file mode 100644 index 000000000..e69de29bb diff --git a/rojo-test/syncback-tests/remove_default_props/output/DirectoryMetadataPart/Part/init.meta.json b/rojo-test/syncback-tests/remove_default_props/output/DirectoryMetadataPart/Part/init.meta.json new file mode 100644 index 000000000..d79c6a7b8 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/output/DirectoryMetadataPart/Part/init.meta.json @@ -0,0 +1,8 @@ +{ + "properties": { + "CanCollide": false, + "Massless": false, + "Anchored": true + }, + "className": "Part" +} diff --git a/rojo-test/syncback-tests/remove_default_props/output/Part.model.json b/rojo-test/syncback-tests/remove_default_props/output/Part.model.json new file mode 100644 index 000000000..49e050081 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/output/Part.model.json @@ -0,0 +1,12 @@ +{ + "className": "Part", + "properties": { + "Size": [ + 1.0, + 2.0, + 3.0 + ], + "Anchored": true, + "Massless": true + } +} diff --git a/rojo-test/syncback-tests/remove_default_props/output/default.project.json b/rojo-test/syncback-tests/remove_default_props/output/default.project.json new file mode 100644 index 000000000..226de3725 --- /dev/null +++ b/rojo-test/syncback-tests/remove_default_props/output/default.project.json @@ -0,0 +1,29 @@ +{ + "name": "remove_default_props", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "AdjacentMetadataScript": { + "$path": "AdjacentMetadataScript" + }, + "DirectoryMetadataPart": { + "$path": "DirectoryMetadataPart" + }, + "JsonModelPart": { + "$path": "Part.model.json" + }, + "ProjectFilePart": { + "$className": "Part", + "$properties": { + "Size": [ + 1.0, + 2.0, + 3.0 + ], + "Transparency": 1.0, + "CanCollide": false + } + } + } + } +} diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index 1d9e937c0..ca2ed0f02 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -32,4 +32,5 @@ syncback_basic_test! { ignore_trees, ignore_paths_removing, ignore_trees_removing, + remove_default_props, } From b54eee1b90fedf0e3421fea78c17ec4ef1375f63 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Fri, 20 Sep 2024 16:21:39 -0700 Subject: [PATCH 04/20] Add comment explaining property map removals --- src/snapshot_middleware/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index a170b7518..089c9052c 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -437,8 +437,12 @@ fn filter_default_property( }; if new_prop_is_default { + // This function is sometimes called with a property map that + // already contains properties. In that case, we need to remove + // the property from the map if it's at its default value. properties.remove(name); } else { + // Otherwise, we'll only insert non-default properties. properties.insert( name.to_owned(), UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), From 6eb04bff6adbe42b4f606e43e923b343229a3264 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Sat, 21 Sep 2024 16:29:18 -0700 Subject: [PATCH 05/20] Rename function, refactor so it produces attribute and prop maps --- src/snapshot_middleware/json_model.rs | 20 +++--- src/snapshot_middleware/meta_file.rs | 36 ++++------- src/snapshot_middleware/mod.rs | 89 +++++++++++++++------------ src/snapshot_middleware/project.rs | 11 ++-- 4 files changed, 72 insertions(+), 84 deletions(-) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 3fdf4d018..4e131b673 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -17,7 +17,7 @@ use crate::{ RojoRef, }; -use super::filter_default_property; +use super::populate_unresolved_properties; pub fn snapshot_json_model( context: &InstanceContext, @@ -96,18 +96,12 @@ fn json_model_from_pair<'sync>( filter_properties_preallocated(snapshot.project(), new_inst, prop_buffer); - let mut properties = BTreeMap::new(); - let mut attributes = BTreeMap::new(); - for (name, value) in prop_buffer.drain(..) { - filter_default_property( - snapshot, - new_inst, - name, - value, - &mut attributes, - &mut properties, - ) - } + let (properties, attributes) = { + let prop_buffer: &_ = prop_buffer; + populate_unresolved_properties(snapshot, new_inst, prop_buffer.into_iter().copied()) + }; + + prop_buffer.clear(); let mut children = Vec::with_capacity(new_inst.children().len()); diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 325a03e31..1815b0a3d 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -13,7 +13,7 @@ use crate::{ resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, RojoRef, }; -use super::filter_default_property; +use super::populate_unresolved_properties; /// Represents metadata in a sibling file with the same basename. /// @@ -57,8 +57,6 @@ impl AdjacentMetadata { snapshot: &SyncbackSnapshot, path: PathBuf, ) -> anyhow::Result> { - let mut properties = BTreeMap::new(); - let mut attributes = BTreeMap::new(); // TODO make this more granular. // I am breaking the cycle of bad TODOs. This is in reference to the fact // that right now, this will just not write any metadata at all for @@ -82,16 +80,11 @@ impl AdjacentMetadata { .map(|inst| inst.metadata().ignore_unknown_instances) .unwrap_or_default(); - for (name, value) in snapshot.get_path_filtered_properties(snapshot.new).unwrap() { - filter_default_property( - snapshot, - snapshot.new_inst(), - name, - value, - &mut attributes, - &mut properties, - ); - } + let (properties, attributes) = populate_unresolved_properties( + snapshot, + snapshot.new_inst(), + snapshot.get_path_filtered_properties(snapshot.new).unwrap(), + ); Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { @@ -224,8 +217,6 @@ impl DirectoryMetadata { snapshot: &SyncbackSnapshot, path: PathBuf, ) -> anyhow::Result> { - let mut properties = BTreeMap::new(); - let mut attributes = BTreeMap::new(); // TODO make this more granular. // I am breaking the cycle of bad TODOs. This is in reference to the fact // that right now, this will just not write any metadata at all for @@ -249,16 +240,11 @@ impl DirectoryMetadata { .map(|inst| inst.metadata().ignore_unknown_instances) .unwrap_or_default(); - for (name, value) in snapshot.get_path_filtered_properties(snapshot.new).unwrap() { - filter_default_property( - snapshot, - snapshot.new_inst(), - name, - value, - &mut attributes, - &mut properties, - ) - } + let (properties, attributes) = populate_unresolved_properties( + snapshot, + snapshot.new_inst(), + snapshot.get_path_filtered_properties(snapshot.new).unwrap(), + ); Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 089c9052c..458a1208e 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -393,61 +393,70 @@ pub fn default_sync_rules() -> &'static [SyncRule] { }) } -fn filter_default_property( +fn populate_unresolved_properties<'prop, I>( snapshot: &SyncbackSnapshot, new_inst: &Instance, - name: &str, - value: &Variant, - attributes: &mut BTreeMap, - properties: &mut BTreeMap, -) { + properties: I, +) -> ( + BTreeMap, + BTreeMap, +) +where + I: IntoIterator, +{ let db = rbx_reflection_database::get(); let class_descriptor = db.classes.get(new_inst.class.as_str()); + let mut new_properties = BTreeMap::new(); + let mut new_attributes = BTreeMap::new(); - match value { - Variant::Attributes(attrs) => { - for (attr_name, attr_value) in attrs.iter() { - // We (probably) don't want to preserve internal attributes, - // only user defined ones. - if attr_name.starts_with("RBX") { - continue; + for (name, value) in properties { + match value { + Variant::Attributes(attrs) => { + for (attr_name, attr_value) in attrs.iter() { + // We (probably) don't want to preserve internal attributes, + // only user defined ones. + if attr_name.starts_with("RBX") { + continue; + } + new_attributes.insert( + attr_name.clone(), + UnresolvedValue::from_variant_unambiguous(attr_value.clone()), + ); } - attributes.insert( - attr_name.clone(), - UnresolvedValue::from_variant_unambiguous(attr_value.clone()), - ); } - } - Variant::SharedString(_) => { - log::warn!( + Variant::SharedString(_) => { + log::warn!( "Rojo cannot serialize the property {}.{name} in JSON files.\n\ If this is not acceptable, resave the Instance at '{}' manually as an RBXM or RBXMX.", new_inst.class, snapshot.get_new_inst_path(new_inst.referent()) ) - } - _ => { - let new_prop_is_default = if let Some(class_descriptor) = class_descriptor { - if let Some(default) = db.find_default_property(class_descriptor, name) { - variant_eq(value, default) + } + _ => { + let new_prop_is_default = if let Some(class_descriptor) = class_descriptor { + if let Some(default) = db.find_default_property(class_descriptor, name) { + variant_eq(value, default) + } else { + false + } } else { false + }; + + if new_prop_is_default { + // This function is sometimes called with a property map that + // already contains properties. In that case, we need to remove + // the property from the map if it's at its default value. + new_properties.remove(name); + } else { + // Otherwise, we'll only insert non-default properties. + new_properties.insert( + name.to_owned(), + UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), + ); } - } else { - false - }; - - if new_prop_is_default { - // This function is sometimes called with a property map that - // already contains properties. In that case, we need to remove - // the property from the map if it's at its default value. - properties.remove(name); - } else { - // Otherwise, we'll only insert non-default properties. - properties.insert( - name.to_owned(), - UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), - ); } } } + + (new_properties, new_attributes) } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 5afb921cb..cc6fe5bbd 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -25,7 +25,7 @@ use crate::{ RojoRef, }; -use super::{emit_legacy_scripts_default, filter_default_property, snapshot_from_vfs}; +use super::{emit_legacy_scripts_default, populate_unresolved_properties, snapshot_from_vfs}; pub fn snapshot_project( context: &InstanceContext, @@ -531,11 +531,10 @@ fn project_node_property_syncback<'inst>( new_inst: &Instance, node: &mut ProjectNode, ) { - let properties = &mut node.properties; - let mut attributes = BTreeMap::new(); - for (name, value) in filtered_properties { - filter_default_property(snapshot, new_inst, name, value, &mut attributes, properties) - } + let (properties, attributes) = + populate_unresolved_properties(snapshot, new_inst, filtered_properties); + + node.properties = properties; node.attributes = attributes; } From b57f4739ca493b0363af0152e7442a406cb4b276 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Sat, 21 Sep 2024 16:41:25 -0700 Subject: [PATCH 06/20] Remove `true` branch for default props, since it is now unnecessary --- src/snapshot_middleware/mod.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 458a1208e..d7d57b6e2 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -442,12 +442,7 @@ where false }; - if new_prop_is_default { - // This function is sometimes called with a property map that - // already contains properties. In that case, we need to remove - // the property from the map if it's at its default value. - new_properties.remove(name); - } else { + if !new_prop_is_default { // Otherwise, we'll only insert non-default properties. new_properties.insert( name.to_owned(), From e335caa200d25794697992078fc225acfa49d143 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Sat, 21 Sep 2024 16:42:02 -0700 Subject: [PATCH 07/20] Add doc comment for populate_unresolved_properties --- src/snapshot_middleware/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index d7d57b6e2..337123fb4 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -393,6 +393,11 @@ pub fn default_sync_rules() -> &'static [SyncRule] { }) } +/// Generates and returns two maps of unresolved properties and attributes, +/// respectively, appropriate for producing JSON or other file representations +/// that use unresolved variants. This function ignores any properties that are +/// at their default values. It also ignores any reserved attributes (i.e. ones +/// with names starting with `RBX`). fn populate_unresolved_properties<'prop, I>( snapshot: &SyncbackSnapshot, new_inst: &Instance, From ea4af65927c699090be2c81e81050caa05908df9 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Fri, 11 Oct 2024 13:23:44 -0700 Subject: [PATCH 08/20] Remove outdated comment --- src/snapshot_middleware/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 337123fb4..28f00e687 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -448,7 +448,6 @@ where }; if !new_prop_is_default { - // Otherwise, we'll only insert non-default properties. new_properties.insert( name.to_owned(), UnresolvedValue::from_variant(value.clone(), &new_inst.class, name), From 09c4e2c19be0599b270e79c25f24f4c93f546b10 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Fri, 11 Oct 2024 13:24:02 -0700 Subject: [PATCH 09/20] Fix project_node_should_reserialize --- ..._syncback_util__ignore_paths_removing.snap | 3 +- ...syncback_util__project_all_middleware.snap | 1 - ...ojo_test__syncback_util__project_init.snap | 1 - ...t__syncback_util__project_reserialize.snap | 1 - src/snapshot_middleware/project.rs | 28 ++++++++++++++++--- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap index d4bfc84dc..46b76d56a 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__ignore_paths_removing.snap @@ -2,8 +2,7 @@ source: tests/rojo_test/syncback_util.rs expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- -added_files: - - default.project.json +added_files: [] added_dirs: - src removed_files: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap index e00aba349..ab0bb69cc 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_all_middleware.snap @@ -3,7 +3,6 @@ source: tests/rojo_test/syncback_util.rs expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: - - default.project.json - src/client_script.client.luau - src/csv.csv - src/csv_init/init.csv diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap index dd0ed67a7..320b704fe 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_init.snap @@ -3,7 +3,6 @@ source: tests/rojo_test/syncback_util.rs expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: - - default.project.json - src/init.luau added_dirs: - src diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap index 5f6b67b4e..441e0b7b9 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__project_reserialize.snap @@ -4,7 +4,6 @@ expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" --- added_files: - attribute_mismatch.luau - - default.project.json - property_mismatch.project.json added_dirs: [] removed_files: [] diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index cc6fe5bbd..bb4de2e5d 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -563,10 +563,6 @@ fn project_node_should_reserialize( node_attributes: &BTreeMap, instance: InstanceWithMeta, ) -> anyhow::Result { - if node_properties.len() != instance.properties().len() { - return Ok(true); - } - for (prop_name, unresolved_node_value) in node_properties { if let Some(inst_value) = instance.properties().get(prop_name) { let node_value = unresolved_node_value @@ -580,6 +576,30 @@ fn project_node_should_reserialize( } } + let db = rbx_reflection_database::get(); + for (prop_name, inst_value) in instance.properties() { + if prop_name == "Attributes" { + continue; + } + let Some(class_descriptor) = db.classes.get(instance.class_name()) else { + continue; + }; + let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { + continue; + }; + + if let Some(unresolved_node_value) = node_properties.get(prop_name) { + let node_value = unresolved_node_value + .clone() + .resolve(instance.class_name(), prop_name)?; + if variant_eq(&node_value, default_value) { + return Ok(true); + } + } else if !variant_eq(inst_value, default_value) { + return Ok(true); + } + } + match instance.properties().get("Attributes") { Some(Variant::Attributes(inst_attributes)) => { // This will also catch if one is empty but the other isn't From 7cdda8a0ab826816bb38d4a6b27e98eef77213f0 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Fri, 11 Oct 2024 17:08:31 -0700 Subject: [PATCH 10/20] Do class descriptor lookup only once --- src/snapshot_middleware/project.rs | 35 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index bb4de2e5d..5d1dedf8c 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -577,28 +577,27 @@ fn project_node_should_reserialize( } let db = rbx_reflection_database::get(); - for (prop_name, inst_value) in instance.properties() { - if prop_name == "Attributes" { - continue; - } - let Some(class_descriptor) = db.classes.get(instance.class_name()) else { - continue; - }; - let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { - continue; - }; + if let Some(class_descriptor) = db.classes.get(instance.class_name()) { + for (prop_name, inst_value) in instance.properties() { + if prop_name == "Attributes" { + continue; + } + let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { + continue; + }; - if let Some(unresolved_node_value) = node_properties.get(prop_name) { - let node_value = unresolved_node_value - .clone() - .resolve(instance.class_name(), prop_name)?; - if variant_eq(&node_value, default_value) { + if let Some(unresolved_node_value) = node_properties.get(prop_name) { + let node_value = unresolved_node_value + .clone() + .resolve(instance.class_name(), prop_name)?; + if variant_eq(&node_value, default_value) { + return Ok(true); + } + } else if !variant_eq(inst_value, default_value) { return Ok(true); } - } else if !variant_eq(inst_value, default_value) { - return Ok(true); } - } + }; match instance.properties().get("Attributes") { Some(Variant::Attributes(inst_attributes)) => { From ca488ac871c1883b641fd35c6849b7c04846816b Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Tue, 15 Oct 2024 16:17:08 -0700 Subject: [PATCH 11/20] Make old property pass more robust against unknowns, add comments --- src/snapshot_middleware/project.rs | 68 +++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 5d1dedf8c..703561ae9 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -576,28 +576,58 @@ fn project_node_should_reserialize( } } + // At this point, we know that the old instance contains at least all the + // properties specified by the new node, and that none of the properties + // specified by new node differ from their old values. + // + // Because node properties at default values are represented by omission, we + // still need to determine whether any properties missing from the new node + // are at non-default values on the old instance. Otherwise, we will fail to + // reserialize the node when properties change from non-default values to + // default values. let db = rbx_reflection_database::get(); - if let Some(class_descriptor) = db.classes.get(instance.class_name()) { - for (prop_name, inst_value) in instance.properties() { - if prop_name == "Attributes" { - continue; - } - let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { - continue; - }; + let maybe_class_descriptor = db.classes.get(instance.class_name()); + for (prop_name, inst_value) in instance.properties() { + // We skip attributes because they are compared later in this function. + if prop_name == "Attributes" { + continue; + } - if let Some(unresolved_node_value) = node_properties.get(prop_name) { - let node_value = unresolved_node_value - .clone() - .resolve(instance.class_name(), prop_name)?; - if variant_eq(&node_value, default_value) { - return Ok(true); - } - } else if !variant_eq(inst_value, default_value) { - return Ok(true); - } + // If the new node contains this property, then further checks are + // unnecessary, as we've already compared such properties in the + // previous loop. + if node_properties.contains_key(prop_name) { + continue; } - }; + + // If a class descriptor cannot be found, then the reflection database + // might be out of date. + // + // We should always reserialize the node in this case, otherwise we may + // fail to reproduce properties of classes unknown to the reflection + // database. + let Some(class_descriptor) = maybe_class_descriptor else { + return Ok(true); + }; + + // If a default value for this property cannot be found, then the + // reflection database might be out of date, or this property simply + // does not have a default value. + // + // We should always reserialize the node in this case, otherwise we may + // fail to reproduce properties that do not have defaults in the + // reflection database. + let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { + return Ok(true); + }; + + // If the old value for this property non-default, and the new node does + // not specify this property, it means that its value has changed to the + // default, and the node must be reserialized. + if !variant_eq(inst_value, default_value) { + return Ok(true); + } + } match instance.properties().get("Attributes") { Some(Variant::Attributes(inst_attributes)) => { From 0d8a36d1a909d446e027805517471fc9651f004e Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Tue, 15 Oct 2024 18:30:02 -0700 Subject: [PATCH 12/20] Use iter instead of into_iter for json model prop buffer --- src/snapshot_middleware/json_model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 4e131b673..bba6dc543 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -98,7 +98,7 @@ fn json_model_from_pair<'sync>( let (properties, attributes) = { let prop_buffer: &_ = prop_buffer; - populate_unresolved_properties(snapshot, new_inst, prop_buffer.into_iter().copied()) + populate_unresolved_properties(snapshot, new_inst, prop_buffer.iter().copied()) }; prop_buffer.clear(); From 583666a6340ccdac265c6ead4e68193f646b822c Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Tue, 15 Oct 2024 18:31:15 -0700 Subject: [PATCH 13/20] Fix some typos --- src/snapshot_middleware/project.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 703561ae9..d5daf9098 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -578,7 +578,7 @@ fn project_node_should_reserialize( // At this point, we know that the old instance contains at least all the // properties specified by the new node, and that none of the properties - // specified by new node differ from their old values. + // specified by the new node differ from their old values. // // Because node properties at default values are represented by omission, we // still need to determine whether any properties missing from the new node @@ -621,9 +621,9 @@ fn project_node_should_reserialize( return Ok(true); }; - // If the old value for this property non-default, and the new node does + // If the old value for this property is non-default, and the new node does // not specify this property, it means that its value has changed to the - // default, and the node must be reserialized. + // default, and the new node must be reserialized. if !variant_eq(inst_value, default_value) { return Ok(true); } From cae0c9b042ec7b344cdd679373ada644332a3d0e Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Wed, 13 Nov 2024 15:43:53 +0000 Subject: [PATCH 14/20] Move should_reserialize fn to snapshot_middleware module --- src/snapshot_middleware/mod.rs | 110 ++++++++++++++++++++++++++++ src/snapshot_middleware/project.rs | 113 ++--------------------------- 2 files changed, 115 insertions(+), 108 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 28f00e687..ad9d361f8 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -34,6 +34,7 @@ use crate::{ resolution::UnresolvedValue, syncback::{SyncbackReturn, SyncbackSnapshot}, variant_eq::variant_eq, + InstanceWithMeta, }; use crate::{ snapshot::{InstanceContext, InstanceSnapshot, SyncRule}, @@ -459,3 +460,112 @@ where (new_properties, new_attributes) } + +fn node_should_reserialize( + node_properties: &BTreeMap, + node_attributes: &BTreeMap, + instance: InstanceWithMeta, +) -> anyhow::Result { + for (prop_name, unresolved_node_value) in node_properties { + if let Some(inst_value) = instance.properties().get(prop_name) { + let node_value = unresolved_node_value + .clone() + .resolve(instance.class_name(), prop_name)?; + if !variant_eq(inst_value, &node_value) { + log::debug!("reserializing because different property on node"); + return Ok(true); + } + } else { + log::debug!("reserializing because property did no exist on node"); + return Ok(true); + } + } + + // At this point, we know that the old instance contains at least all the + // properties specified by the new node, and that none of the properties + // specified by the new node differ from their old values. + // + // Because node properties at default values are represented by omission, we + // still need to determine whether any properties missing from the new node + // are at non-default values on the old instance. Otherwise, we will fail to + // reserialize the node when properties change from non-default values to + // default values. + let db = rbx_reflection_database::get(); + let maybe_class_descriptor = db.classes.get(instance.class_name()); + for (prop_name, inst_value) in instance.properties() { + // We skip attributes because they are compared later in this function. + if prop_name == "Attributes" { + continue; + } + + // If the new node contains this property, then further checks are + // unnecessary, as we've already compared such properties in the + // previous loop. + if node_properties.contains_key(prop_name) { + continue; + } + + // If a class descriptor cannot be found, then the reflection database + // might be out of date. + // + // We should always reserialize the node in this case, otherwise we may + // fail to reproduce properties of classes unknown to the reflection + // database. + let Some(class_descriptor) = maybe_class_descriptor else { + log::debug!("reserializing because because no class descriptor"); + return Ok(true); + }; + + // If a default value for this property cannot be found, then the + // reflection database might be out of date, or this property simply + // does not have a default value. + // + // We should always reserialize the node in this case, otherwise we may + // fail to reproduce properties that do not have defaults in the + // reflection database. + let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { + log::debug!("reserializing because because no default value"); + return Ok(true); + }; + + // If the old value for this property is non-default, and the new node does + // not specify this property, it means that its value has changed to the + // default, and the new node must be reserialized. + if !variant_eq(inst_value, default_value) { + log::debug!("reserializing because non-default"); + return Ok(true); + } + } + + match instance.properties().get("Attributes") { + Some(Variant::Attributes(inst_attributes)) => { + // This will also catch if one is empty but the other isn't + if node_attributes.len() != inst_attributes.len() { + Ok(true) + } else { + for (attr_name, unresolved_node_value) in node_attributes { + if let Some(inst_value) = inst_attributes.get(attr_name.as_str()) { + let node_value = unresolved_node_value.clone().resolve_unambiguous()?; + if !variant_eq(inst_value, &node_value) { + log::debug!("reserializing because attribute different"); + return Ok(true); + } + } else { + log::debug!("reserializing because attribute DNE"); + return Ok(true); + } + } + Ok(false) + } + } + Some(_) => Ok(true), + None => { + if !node_attributes.is_empty() { + log::debug!("reserializing because non-empty attributes"); + Ok(true) + } else { + Ok(false) + } + } + } +} diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index d5daf9098..bf89d79e9 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -1,6 +1,6 @@ use std::{ borrow::Cow, - collections::{BTreeMap, HashMap, VecDeque}, + collections::{HashMap, VecDeque}, path::Path, }; @@ -14,14 +14,12 @@ use rbx_reflection::ClassTag; use crate::{ project::{PathNode, Project, ProjectNode}, - resolution::UnresolvedValue, snapshot::{ - InstanceContext, InstanceMetadata, InstanceSnapshot, InstanceWithMeta, InstigatingSource, - PathIgnoreRule, SyncRule, + InstanceContext, InstanceMetadata, InstanceSnapshot, InstigatingSource, PathIgnoreRule, + SyncRule, }, - snapshot_middleware::Middleware, + snapshot_middleware::{node_should_reserialize, Middleware}, syncback::{filter_properties, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, - variant_eq::variant_eq, RojoRef, }; @@ -512,7 +510,7 @@ 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)? { + if node_should_reserialize(node_properties, node_attributes, old_inst)? { fs_snapshot.add_file(project_path, serde_json::to_vec_pretty(&project)?); break; } @@ -558,107 +556,6 @@ fn project_node_property_syncback_no_path( project_node_property_syncback(snapshot, filtered_properties, new_inst, node) } -fn project_node_should_reserialize( - node_properties: &BTreeMap, - node_attributes: &BTreeMap, - instance: InstanceWithMeta, -) -> anyhow::Result { - for (prop_name, unresolved_node_value) in node_properties { - if let Some(inst_value) = instance.properties().get(prop_name) { - let node_value = unresolved_node_value - .clone() - .resolve(instance.class_name(), prop_name)?; - if !variant_eq(inst_value, &node_value) { - return Ok(true); - } - } else { - return Ok(true); - } - } - - // At this point, we know that the old instance contains at least all the - // properties specified by the new node, and that none of the properties - // specified by the new node differ from their old values. - // - // Because node properties at default values are represented by omission, we - // still need to determine whether any properties missing from the new node - // are at non-default values on the old instance. Otherwise, we will fail to - // reserialize the node when properties change from non-default values to - // default values. - let db = rbx_reflection_database::get(); - let maybe_class_descriptor = db.classes.get(instance.class_name()); - for (prop_name, inst_value) in instance.properties() { - // We skip attributes because they are compared later in this function. - if prop_name == "Attributes" { - continue; - } - - // If the new node contains this property, then further checks are - // unnecessary, as we've already compared such properties in the - // previous loop. - if node_properties.contains_key(prop_name) { - continue; - } - - // If a class descriptor cannot be found, then the reflection database - // might be out of date. - // - // We should always reserialize the node in this case, otherwise we may - // fail to reproduce properties of classes unknown to the reflection - // database. - let Some(class_descriptor) = maybe_class_descriptor else { - return Ok(true); - }; - - // If a default value for this property cannot be found, then the - // reflection database might be out of date, or this property simply - // does not have a default value. - // - // We should always reserialize the node in this case, otherwise we may - // fail to reproduce properties that do not have defaults in the - // reflection database. - let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { - return Ok(true); - }; - - // If the old value for this property is non-default, and the new node does - // not specify this property, it means that its value has changed to the - // default, and the new node must be reserialized. - if !variant_eq(inst_value, default_value) { - return Ok(true); - } - } - - match instance.properties().get("Attributes") { - Some(Variant::Attributes(inst_attributes)) => { - // This will also catch if one is empty but the other isn't - if node_attributes.len() != inst_attributes.len() { - Ok(true) - } else { - for (attr_name, unresolved_node_value) in node_attributes { - if let Some(inst_value) = inst_attributes.get(attr_name.as_str()) { - let node_value = unresolved_node_value.clone().resolve_unambiguous()?; - if !variant_eq(inst_value, &node_value) { - return Ok(true); - } - } else { - return Ok(true); - } - } - Ok(false) - } - } - Some(_) => Ok(true), - None => { - if !node_attributes.is_empty() { - Ok(true) - } else { - Ok(false) - } - } - } -} - fn infer_class_name(name: &str, parent_class: Option<&str>) -> Option> { // If className wasn't defined from another source, we may be able // to infer one. From bcfdc0c51a08e727772e95a1b992e7b79a2efb28 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Wed, 13 Nov 2024 15:46:03 +0000 Subject: [PATCH 15/20] Test whether json model should reserialize --- ..._rojo_test__syncback_util__json_model.snap | 10 +++ ...rojo_test__syncback_util__json_models.snap | 12 ++++ .../ChangedClassName.model.json | 3 + .../expected/ChangedProperties.model.json | 13 ++++ .../NoChangeShouldntReserialize.model.json | 13 ++++ .../json_models/expected/default.project.json | 17 +++++ .../syncback-tests/json_models/input.rbxl | Bin 0 -> 55724 bytes .../ChangedClassName.model.json | 3 + .../output/ChangedProperties.model.json | 3 + .../NoChangeShouldntReserialize.model.json | 13 ++++ .../json_models/output/default.project.json | 17 +++++ src/snapshot_middleware/json_model.rs | 62 +++++++++++++++--- tests/tests/syncback.rs | 1 + 13 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_model.snap create mode 100644 rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_models.snap create mode 100644 rojo-test/syncback-tests/json_models/expected/ChangedClassName/ChangedClassName.model.json create mode 100644 rojo-test/syncback-tests/json_models/expected/ChangedProperties.model.json create mode 100644 rojo-test/syncback-tests/json_models/expected/NoChangeShouldntReserialize.model.json create mode 100644 rojo-test/syncback-tests/json_models/expected/default.project.json create mode 100644 rojo-test/syncback-tests/json_models/input.rbxl create mode 100644 rojo-test/syncback-tests/json_models/output/ChangedClassName/ChangedClassName.model.json create mode 100644 rojo-test/syncback-tests/json_models/output/ChangedProperties.model.json create mode 100644 rojo-test/syncback-tests/json_models/output/NoChangeShouldntReserialize.model.json create mode 100644 rojo-test/syncback-tests/json_models/output/default.project.json diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_model.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_model.snap new file mode 100644 index 000000000..e945f5d7a --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_model.snap @@ -0,0 +1,10 @@ +--- +source: tests/rojo_test/syncback_util.rs +expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" +--- +added_files: + - ChangedProperties.model.json + - default.project.json +added_dirs: [] +removed_files: [] +removed_dirs: [] diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_models.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_models.snap new file mode 100644 index 000000000..61e4629e1 --- /dev/null +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__json_models.snap @@ -0,0 +1,12 @@ +--- +source: tests/rojo_test/syncback_util.rs +expression: "visualize_fs_snapshot(&fs_snapshot, &output_path)" +--- +added_files: + - ChangedClassName/ChangedClassName.model.json + - ChangedProperties.model.json + - default.project.json +added_dirs: + - ChangedClassName +removed_files: [] +removed_dirs: [] diff --git a/rojo-test/syncback-tests/json_models/expected/ChangedClassName/ChangedClassName.model.json b/rojo-test/syncback-tests/json_models/expected/ChangedClassName/ChangedClassName.model.json new file mode 100644 index 000000000..61a0557ce --- /dev/null +++ b/rojo-test/syncback-tests/json_models/expected/ChangedClassName/ChangedClassName.model.json @@ -0,0 +1,3 @@ +{ + "className": "RemoteFunction" +} diff --git a/rojo-test/syncback-tests/json_models/expected/ChangedProperties.model.json b/rojo-test/syncback-tests/json_models/expected/ChangedProperties.model.json new file mode 100644 index 000000000..9f3c95a32 --- /dev/null +++ b/rojo-test/syncback-tests/json_models/expected/ChangedProperties.model.json @@ -0,0 +1,13 @@ +{ + "className": "Part", + "properties": { + "BottomSurface": "Smooth", + "Size": [ + 4.0, + 1.0, + 2.0 + ], + "TopSurface": "Smooth", + "Transparency": 1.0 + } +} diff --git a/rojo-test/syncback-tests/json_models/expected/NoChangeShouldntReserialize.model.json b/rojo-test/syncback-tests/json_models/expected/NoChangeShouldntReserialize.model.json new file mode 100644 index 000000000..9f3c95a32 --- /dev/null +++ b/rojo-test/syncback-tests/json_models/expected/NoChangeShouldntReserialize.model.json @@ -0,0 +1,13 @@ +{ + "className": "Part", + "properties": { + "BottomSurface": "Smooth", + "Size": [ + 4.0, + 1.0, + 2.0 + ], + "TopSurface": "Smooth", + "Transparency": 1.0 + } +} diff --git a/rojo-test/syncback-tests/json_models/expected/default.project.json b/rojo-test/syncback-tests/json_models/expected/default.project.json new file mode 100644 index 000000000..0a13f50b8 --- /dev/null +++ b/rojo-test/syncback-tests/json_models/expected/default.project.json @@ -0,0 +1,17 @@ +{ + "name": "json_model", + "tree": { + "$className": "DataModel", + "ReplicatedStorage": { + "ChangedClassName": { + "$path": "ChangedClassName" + }, + "ChangedProperties": { + "$path": "ChangedProperties.model.json" + }, + "NoChangeShouldntReserialize": { + "$path": "NoChangeShouldntReserialize.model.json" + } + } + } +} diff --git a/rojo-test/syncback-tests/json_models/input.rbxl b/rojo-test/syncback-tests/json_models/input.rbxl new file mode 100644 index 0000000000000000000000000000000000000000..fbe9ca30a3e610205c4df99e9195f0dfdc885f8a GIT binary patch literal 55724 zcmcJYdyE~|ec$iyUXmh3Q52~cCE2=?X^DDJ;!D)Svc%<*ORdAldUr{=PG$RY@9gfi z?%rA7hs2d7ijr(4af4b3+|9-GaLM6D7zuR?8cDr70-s`;7 zT=d4q__b2`%oz9&>C>I&yYSmNrIss`y;Z-vw&HcXN`(GJz*KBo4{jLOx9fErZ~6Y} z$)zQ4v1id=0sWxP?bAUC_DtySZ90zkFJ5eUQ!Dk}l;2)zF84e2Uej+|%6(8mb)ODO za9dS>Z`Lt2f9gbY5pcb;p6bl4rZanlq76!L-6s9rtYfOa>UHY2fpHpmNIyTIgA&{U z=yn}qs@?Wl3-ycLT;2{r3h-7Plwivi{e?gJV+DJ=;pJidiUcUZo!99v+<}{Eul0M! z>z&*-?#oJUtLW$zw9hx&4gcj_%Dqs2RJf1mpaeIgN-So|Z?&WyNiHR}=M1Z^xKXJQ zx8Lr0mwJx6f))9hN2&l#kv8jK$kIN>eUTUPtqvqFM{T?V(-TRFEh z-Sk=w*P4GBBG*xnDr`*qt%ldB;0j@BRRN=dd!k>0UDOUYr(}q;^=?`;vxfP*1)vQ| zuw%FWqKM~uy|t87E*rSZ4*EjbGpqGw?{xj57kBt!*nt)5rvy81(O*PA)9!kmbW}KP zYRAcuJ0-XtbH{*BH&GxdD`kMxOzs_KGl;Bnj73NR-l6vzMF(FYqRMak^iB-n*D;;-VpNwLpK+!kK!{>on`F6iLmHJSMHLJ>>n1D%$n5}nul?fT5f_1fF!q9#C8JD00w?Kla z%zB;GX16PEOzrKex9W1f zlsJwWLIgl>D8aUx{u1y)!r1xlFwlcK@u&_;P(y0;XSU-ndfl!}tyga^dihK?Oa<<6 zK_{_0RdA`f+U%|C!|GbvTHi60HQ}rnf3gLw<2rNRs^9ZYzT~xIv3JIe*j+<{5&?$z z>3-V;JR30DS$D?RF{H#KrkkB^9RBYzJh_1wMH4a-65^eF)wA#xc*V_5k7VyBtu z`QJ1IZWJJnGg!=fQM9+UUoc>ph#hqkHi4s}cdrw+vAyzxfdt`F4o@h_@Dx(Hop{9% zzy&bMkGDOfzc_bfUKg6(=EY`9`n;*~4a?rEh7N5L$0)%jhR+Ed^DnMjReT9m)bv}D z-|W{n#+`Y;-)^KVc&Dk=#XlmfVSlaua+?q}n0joZe%A^yXd>EtPtNOgGy-?(i~10X zt7|!X21++MIv}cT3FWDN)Ashb;mZy8I1e@(wKH2^e9^*>8Mtd1hXgg<2-yn#(Gomm z2;8VlNJF-^e&hM6PCT9M#1WB%Y;_d&FVtK8I3GNlMH4&2F83QvpD=D&dLl;^M};s@!meVf_|JV`e9xV~-DvEbEL|bmiYEmp zuq}9YHigWzS1z zj~_O8;wRCX66`*vzhqSBPfbm}Bojz8_G^a59akoWC%-s9Gv#;MN~gy3MdcCt?1s4G z`WYKLKXa;6Z#0SR5%@vCZr(B_Ao1}0%xt~UkOf=pfo#PmBJ72^lu$lp7LFlUt^bzE^qS zJ7XWb|L*U?g)4JvqMgxY4n?8NT|)*=?9)8X?`*&P)vvx|517fvGVNL4{@S;{{oZTu zzyICWe)5y6S!r9whw9QDlYtYOPI&FEM)2xuhYug2QbraMS4uEpnLX92cRYk`tisg; z7*0J`e!hw4_y$ z>TCK4sHXpkWl^IUo<_7R%Rzx<0ZCid8^xA|!IrZuC<-mhb$;8xPz!MN1{@1G*{*95 zr!j6^%+(u^0=>C&bPJx*QbTA{uZtXQWO3Mw(J!80)||j)`2#nva94fnN}ghAvW#NE zQ>CNWEO&HyVWs1BSF|Efy`pb$=Y|%lN*lnNX#;prpnWi>m{`)-z52bTgA&|CN2 z2LytY65I|MaUvmG=+xU9s5)MIaedRfzfSp`XCZW*7D57kxFUEWS_rJZ`NL3lMd0qd z#z76})id#I|2k_05YvX zy@ujD7cXgj$m>1z=%Y*ez5D1=eO0q1ulwlnev_4^c}ez>&v*Uy2V<4{4_3yu?5q6a z{b#&Jv%mU?-&?Xm#uO-5t29&Kj{9}`A&@G;U4T)gS-c5n74yA*qZ>v_GdKgJ(2|>= zLrmyBQ@<2yv|GJ$B?P+JHb87zz!S;}SQn^%`JG{U#3Ii$+gby7#f#xzIu^pS&Vtm9 zmoRe}vepdmHJwhPJuH<83gBtU2@15xNJ9xW+g>izn=LczZFyn1yho=oS4yxG7*bo% zQfuUHuYT`)fB5R3{^@^s_0Rt7f557nEJJ|ZKtPzY+0H(oAJGZwrv$q!^!z|7^L9Gk z<(;X$(J1b}Z z)sf%3;p5+U=AEzK`+H-RD~JB!H@#F{Xkka3vHu9Q4E*H4v zg}H@ht2fzJy3=ae+%qQ@(W8x=!p(W0$jy1a_$$Bit6%)>-~K0G{Nq3V6U5`@JdhRS zJUVL%gsyc%Si-t(hlX0Ws|DbLnTZk6dHwjJVTZH>$X_Gv_~+x5kA`W-JNu6-)6Gvh zXrv$pKdjS?+?3!>zzmzu`C5lglE`QKJ+z<@Bz}#KE0cKn|?bjQszus?|mEkZqbW=#6v32u-CFHC%0xM&U zw%USN4I~|_Z!Xp#@QpO+DkY}q_xwd&+-rHdTsOF|aKEbjs18bSEAXEoR6|w}s>A8& z_ly;1*(+f)wl_m<#tqfLk+Rd_3;JPI2jv)uYUSF)|DPv_G*e)_Zc+po?L(?b_QuV+cp@;zg!#!Fr|&cfa}*#{0x2# zS%F_4Hs#;u_%(zj{MvSCM85`}@oRVG2M8MZf9}}sREK7m>OAMMZO5HLFeIUA$}xb| z%0C;j(C&`YU{UXLey910R;cQ&$yRf@y{Zdw%;$_ZklO9jK?(MRwGS-H5Cn3zv1MHu z$}+BOTXJ04bxj}}xTZT=Lx8SE@J*mUgKt7MqHnrcmSt_wIlq*4&YEgZA{itERlB;A z)tx(C1dq6$Z#H@>t_8Vf0cazycSlz!N_e4dV5k?m85wYxA7aycb;_E5up4lkW+5KM zRCl{%);6IjW1F@O$2MJk0>RV<^?5L@Pgm@?cR8^`Q9|stff2fR3Gr(I>r^8g0^7 zI|v*LDS|E9GB?te3EM(}GJ&L(xmv7D)UuH>T@^u5LPfTL5mf}dKt=An95DUW1+1B# zx7wXh%J%~V?T((nV%@MN67E^g`WHZCII2o)#RV#{EDjDPPZXgEJM3RLW_zT5iDLOCnf<}QM) z>cEvzovqhV-IJ<>gYp+Q0bvF~%Bg*2W2}G-To>-$rt1vdM|A-_*}?kAT6-DZ+}xFl zP|x684;h&ms2fIrrakzTw7IyuXdpqGpPp$qyi0O{$~Y{z2?`OZoJ~PdXj5)funml8 zCcp=0XGRpR)GPzj^-i)Y<%_@m>+jM#_xcx5ly9#3#Mh=@iLkEqfi1AU{EBDVrs_HE zbnJJlZ~ZV-T~-2dltFSQyyQNX;XI%92K#sUN?3JG^me2OZnXUyH7_}K+(^d{@c~Ut6QMT+R@++GVn=XZ7aEkWaE~`@1YN5!gM?X^X;WUt|9pJ9BQF}%?iEi8&1vu6XQ7v_w z7S-4Eb7Ct{_su)GUD{`d-EiA-y8s^hSaJzI>|!)gvG{18byh2&%V@>cUZ52q zX{~%dtrb*4Q5Ezo~1y(hGI^;fx?}Gj(yj3 zK8dC9!ReR+rEGvFvKoADW}EK~zkCP}0O z+d;{No1lJ-{J2zZRo0X_}y1HC93Zob|QgUpJ&~arPFui8HJacjmlqor}4S8}8K! zOobBc28w~RdpQbGTAuOS+Jn*ZVpiNS0~%PF^LqvxM~|$&@f5RK?QAn65K##w*u*E4 z7b+!jseDFb)gAX%>Mu3@4l0ae<%l$>F{a56l$mynDr2IovmvYu(fPHQB5UNV48#9-NLEc^&ymL!{2`os}~>gbz@S0ptFB~_YA31 zI6d>A+vD!MM25HlvZq|#Or?16yzbNXrgTBd>)h^`YyOyF;O@1Jnrd-fl7KYoEXa1w z$twn>8yHrxW$kM7j0_k)tCcJIYYLLc4vJ6=uIKz>IupSdlB^RxnCm*5kF&!{&K%xt z8WOZO*=p&@6WWa4);0XD_6OtOt!Q+s31FkrF(_m`*BOcsJKgb2K(Kh%k}X$7Ne;DJ z0}6IPQJBkS;Yerba7)ebM7dg}_JU-#rqhXiguRWjR5O$fK35?-B!DktPqu!?p0X-R zTgTC~b-3dI(GcsHT)l`!x4*V$&mH!F=W{xDMh7K$1~l@^Q_z0>P6AQWF?ak^pFP)U zE;rlt)`GXV($zKK`x_>8a>sB$QJv;a z^s2ra7y}Q@Q-Ynq7{N|?{&J_jw$fZYElzbKN^-AY)_!oQQ!W3Z?pau=>lr!SyqnXg zxxlgWyJVIDlTpXbdZ>|Zn1y2cW*?F=+^Wv?dkx?34D&qnWX5m(4<@Y0MAYJb+i4V!gO5(oub4w@c>to3E;E|^eA9(7(0q|Ml1~E7`w$sti9dIzqVew3- zj7_@k07YTs$-)bRfji((dx2fLkppmSax^lSLJwUYfvSu~Z7YsOUBLrM3;w}k!4td6 z2_A|Pg0~HfIMM?z5WMT_fMdba@%XVe85;&#M%uRO0%-$DOWQA&HV#-$+EA2`wryZU zX#+2ic1@>~FaS7~HnMb{^pp)7;LC{I)?XlUAZd}GO^e(eyO?m9)jd5vuiomJVZr2C z;dj_}LlFe!bOeRTmg-&G0weEn^OXkNxDOnmAMZ35UyQG3ns0#JxCOXS|MZq9K>6`? zz>)cs_5zy?!KVJLyDGEhTYmlUE`l{Lln%UyUxDw{w+D1ku3k_5W=TzJH>=G>=1X1E z#P(Y%v*sH#BySHvpz@d7p)pCH9&)1vhNXn)hFb@S=mfbH0YtZ^_S{~1lG(qNU~bQriQv@zLZZi>#Kgdp)J=@Z;-P2mfr-5oUZFRe2a!5lm3~n zz}?V^u#nBP-wZp9XY&PUz6a^~5sOW)W2(l3qVEL+y6wCfac&60?-1mBgdQQ&4b6#_ zQ1cCNR+e;>rIL0d?r8Yc)}2fp>1aWSBcd>DQUc>Xa5}RXTF*rInOdn4yQX}wm0HQ& zPJ8vG^@QzhyDPIX+xb!yMNiN1jHMWTr-^)5KCP@Pv431Sw*P#=oT+!@GO=L7uU0Tv z0VObQiC_|&N9eK5=L_acKa@Cv(Pt}|n!Zwkn-EMqNZ`P!r~5&X@3yn`m$av<9mXI` z)BXTSeotpTf>`zrS;ORBOaPz8!uY&dt0Xjo^kaMbOBVLa1U+Bt>*ZNro0Z@t`rp z9`n6=J`P_PnE6KSI8%0zm|xWr^hzXXBT6E%^j0MH`L_RZEOx$7e@*o#$qRN_CK<7I zT#$?aNhc$BrIV4YA{htHcfCUg4jr<-(f!nFE1VtYo7MBaAG#gd;S2rC4@l;0tKNPw zjH3i3^UYG!R0AcriKfcsIZzblB5r4G149Qrcgh94AW$FH=|egwKLm>`jMDGfs@`n$ zK+vp3P)8Yyux%Dt1dy~v>_}S#GxKDrfXh3OgcqN=^E{mFKB(XJ>7WFAKo#lA!1RY+ zy0H&RMoTUf>$x=b)yeAR%a=p^Shz`L1A^lTLDYhqFVg}EHZ9EeJ4?}qzp#mj;(qsjAt$Z$oLB&iH zv!MiAphBfn^|lrQTYBW(c%;dE(-Q8JeUE+Cp}vQr%yS4Ws-_n9>!AG9w3&7N!QETK zc+_JV&H9$D_hHrdgsHVs`70ysXkoXdCz1MXa{+9O!B$~|dvUL?%$onmA>WLQ5MtVy z#za)R`3IRBmJlEIY8Fus_jUd1i93G&H-6*izxmCd|IOdDI>II>!7ae)+f-kJ-fH-~ z+rEMAv93976Y;jp=}FC@rv#Uv@*eKXzaHyhKw_JeEHG}KzY(@kron&(aB z`SnH=Sx?A!9@9pVBVg2&;5LLsF9u2SUHh0&$8}JCY#bpI`+qbxSE+oLdHt*bL6WbqId6%3$;?BpqWU(X;2PKv zD;m;9Op!ICd>=F03WnJcgH6`Q21)V_X-HE%p{}DX6S^`nk(NMBKiKGeZ8JJV%-Z5{ z!E5=8qS&DkYCy(#ku{m0UI#uWr82`r6vjxAuqsNh%^FFZQ-vo&>#gZalE4GSjC9>^ zh%SopSxw|e0M$QC59q)h7?|KD1ZA|~?v#4{byXA=EcduRPDbB#4cd!YMq}q?^pj?Y<9HLbEh+$Kb zglNg{tmr|KsFGZxvGDNi7P_rMi?_P>lZTazso9(q z8VVf4Qg9c5xv-iLAV}+f!8int9o$r@igW!MR__j@pkqUz=UOMpzI_9G$aaM6YY?t^ ziz6x5)oIDh61rZ$q5U_D+7uAJhs7a!NH8KJCAb43BBL%JhYt&HSUJq1oAsIYk{=7M zCLpq)1UEuj#$v4`3M>`~X(_kBooxB-WwTKw91Tnkkqs=6q>a2*Y~(eOlrwTD3XL48 zAVx}XLrs4%Dc$T7zBVG37H;(Qu$GMX~+qy$@Q`Wu@o)8mK?q&v?53EgbRThi+jJT4!}ut#aYeHJwg8 z12DDY)#mL;-Ds~bPF+JQ1bEz)65I|x)~Xp)a$l~LRVRdH)M*Dy!Dv;nKF0tJkoBdFxPDbB~G!;HnOD zVQ8m54BpqvkJMk&?}^hwlA|5{s**FQvQ5Owp~@^5u2L#<-%b0HD|J%Fa#Dw)gw$;V zBT5~3fz-3i$5LlaBzdixc@tY!=u}@u=-_FgKJffq>IT}T8ROB<^A z2Dq$ogDoR&TX%uDfn28}S$eHZJXI|22LvxCZYWBK+cq$wxPfogG2|v>###)V5^M)d zDB;#1&8jq~vZ=92;_!8X5GA+;aI71;%vN{35oPf0m{~1BRYps;l>#jRNo(no#abef zEvF?YN@&S8Frt=#Z`CnGOEsNJG8VwG;EgN?U;ddDIruUnxAhl@97tN^bHySjoh~PG zC`yRjHZWA=q={s%lwc=t7J#2?>M<|&O^V$y1WQ4TZXyanfsQzhB~pSr_!jGk<(TL` zkmP-<(GOX5L0U##wy6Sj0ZFUtGaIXmR4$sqM%=pcakIdOg)XOHC<+x^6CkmB;H*MU z>Arn!2&}h``+DPKS67T*Zs$Ej&@wdCAYAtX2Z=Qsdd1YyBS(%LK6K!*Cl6s2?lpd@hz)8& zU-TkgP)sv9M+rW}Clsxd7yV1eU(9dgi6@R6KK%Gb)F;|QsULshv5lywdqb&5X#4ci zUx#|`uhsVXwY;_u9(wG+;iCu5^u8tuyJng1ulN6*9cPgf{*N{(!4`CcopHydxeOw0 z-nItL9gM*dpyvCz7IsLdSR|wb_W<6mL*OC~!0=Hiz2q(I^a?VqRaFLa76JXr{0=8TLfr4I74SOJro3Q}L=@byyT6S7* z=<)P=Tdpecw2PQ6cX`9IRTxH+911vUv(KLkuaZ=+y!UaVV|O-TJ8;5Z(-S&Uw+y@C z31~rh%JLjX3u6}pFXTq|?t%q%BN%PE(ap;}-5sur5d+zuSq91HoHqdZOH1y}&NbDQ zI0&`hpkverHh|y25`m;GaV%|#Zp4Pf3}}9u@3ITszz(X{^`Xs09TqB|CYMbGFuvESyV)$sH-wx=qL6{@sY-4S?G4B06|*uOw09 z^-+z3J6%A!Mg)>Iw1J(y$rcyju0;UH)ZG2K5wjaY6L#GILai8&v$r<(R5zsq%P9xz zvh3Of1e4X zV=AiNVn;ck^A$*_|6X!bZr% z5At1RaBFSs1~u`6WmAG1ZFdHtt{1o?3;2SOC97VXU~p?~VL*z3Nzd|SFt9N;?pW3( zu*3nR+b`$xuZx@pKA%V7lnbN0sSdt9hBUjymAu?5I@hpX-< zDYR^}P1Wq!O!S{`*@GKeV*{v(ZC6}4>LAp$fFV_<_q@`x?9hzZoV~94ZVMDOeQ}vF zDjV^^?I?(44l(pJ)Lbsky=(5oOl4KXFzSu$w`H4hkAFIj!vSaurRTb}F%}Ic*I?cY- zH?M@xPUk;OySDYHs@f>+bNdEG;ozTz7Y>VVz^C?tfY0LIJ{^=`58yaN5gN^tg7o$; zyNN1X@a@v=nocJU0%aKou`LzMK!K#q_);r)o&tiH~j@n?!&DUy>Qo;aOc`)Oi7-;M2MsW+acJYW3H!%gXwvg{k1i} zlRQ|drc;SF0TX+rZqIA$p<>$X5Ko({aN~vw@I(=o=T2za-}2YEy;~fGtkiVQt-Xw` zPLE~6k@dx0|pNh|37w1Qj{KO_=NJbd<{xg~F4*I&*&9)gWD zfW>qkX$( zfux^!6$DrjCD?5{G^M>Mow_!TecJa|?b1>8ZDCMFAx*RG&|Hpa+!3C3!nvzyCwfPu zsoWh(02Ugk>jA)#Z#)_xvhWRe)i=KX{qJX^6coGC6tn)zUZ?t{cf#(nl>=X<1UEy# zTsLk)QkqvZ@wD!)w{U;C5fbGaN=&4EqOfKf3a0{D0*9^V>#Y~{C_k@JeM^ngkF22R z`B2x)?}b8PsOxi!i>*G>X(zrstQr}ADZ!m|odE$NJt45KRk_Ge^loONsYA! zDl#I^65#zI!5LrAW@K)6ZjII3a8z+~7b^qq9hmoeJuPW=C%YOQXHIBY*0Up_X=-Fj zICLoYi=~I)SS7g2N-(EQ=B<;R)t{YM@1SysX%+oZf;(uL{6DAVS)P0DkdnS)Y?RjU>aw$O#GDht|y_lu;IbK(y>!0g9QX4l{xnf?fy0JAT6=2G@ z9X{NB))Lb)6;Of?(>jx={MN%Lf7{a1(z8oTZ=1HW!((kIIrsi5+mz=z>~CyY3H#y>y?W`?Ke`!hxED9T z#j1k=ZIPL)uji~+f-KhCEke*MN>ByAS;zcZM|o7$zLPN9ZJIAoF7>39d!<;pkBg+7 za-k@pT-(4<<+_>!KCu!-o%*t%y_2NfwV~TjAt@tu+eCrXfuyDWXjjLZQG?XYg1s}s2zcfGzZh(OJ zIk@|VGZfksuQrr|~^HD8V+6sCsa_OdMruI+2Y^gHYGP z^Vv1egg))HmwPMK%UUo&wC(^369rKnZe1ng3h~yOWD~{)z(fZ5j1KIqiA9jrF7`Np zbR_^C07@`Mzwq&B5006O=SS|MJvh{YSKG}sY9ri(^BMj4IUSS_j3)6f3o}m89Xly- zaKOL(%%}dwg|C18>tCtQ{F^`e2fy{T)?#DnKmFJL_CNpX+SgwF?E2cppZmloe)-Ln zYqw9$E-bwL_DkRYlmGnrw~t-B_S0)W{{64M^9!hqxf2KFM_XYEaog60?fA(`Iv$Zp zM;G*v4!uQ<`uFLptv|Sp`f=sx2}P$m&BhqL8=ughxIpKQ`7zz#!|`IgK0uOXil#Q+ z0Awpqw)G^oH>A#Zt59Nm`3^X^t0%U;n%K%Z;2_jBzadp`ya6cg+2F(O)VM83Ejxa} zr><~!Jo7T3=;~jzDKN{5nK_y#TpMkUi+F4k71#91?IlhUJ`DSUuQ#H0fC=fc=-{<=6+($EkDa_7XIa?-%W+Zp0KJ1?o zYz0m<Mx|BEWf-LecR`k(2}w>)pn?62Ty z}HehL{#~Gv2^#GKVk7i(Zi_B5JuhZ6%Oh)5w*FhJL-D6X1l2O zpL9>xmsut-C0_|;qMz-6Ghw|$mAe*T#%4VQOK%^J8sB@2#@)e|%FKl8oP0$BS_ZIrnFer^W=&w&uUl0D({mB4b3$M2pB{h0`{q? zt#+F?otjp%qS|osVMTSdN*g0PB;eTc%@`FjGid!aVUmFyiZZ5YTX0O%H8>z?gZogi z!96XKas~%Qp~1O>2k^~0!bvBDalvC;4Owo624h%dn4<*az!5t8HDb$h=T1Nhl;|3) ztxAfcSQVh=xl=<&xM)3v1G z(sS~Z%h9_nU6ZDrv`N35HfeY19TF_=(=+XccZtzt92VR_`*HnT&hDWow0qZ-Z39D1 z*|k&P1$OG16yP}6Af4gQnE_gCQm*Q4)sE_2l>kYrq>@&NEA3}QQcl`Xl#sSSc%PU29M zkhpDNM2Q10khtsEfMba>_dI`UD%`$2RO*nFkve#qD3CgkwA8!BQlAt_IjKWYLh3Qx z@93ETbM;2E-(`rQ+|J-~w>~7MYxx;bNx%z~u$v676OW~K~Ruu;>=BnSa5qa0gx zfpUPPmGeu*$~i8Qa>{|CP&rIZm|Rgl7z55M%w(9ij%z{pzAMj4MB|m~c2+)j1ur)F zkgFrh)=(Y6bCPNTC*N_VJ81k~9h9q8AO&g!j4CO?cEGV3Sr|G$qYD9gWQ|_gBSn&F zCJSVMF}E<=M5U$x1Vs8n2{r>nJ-SiROzUXD^u4D4iD3XMV;GiM$1qUBK^>I;a~%*9 zq_w$G4~g!0?Zx$5(8V`G1%QHIH{w`+2iM(xq1c?JR8cu|f}+rzT*I&p3^fc_>%a@N z?k3@YW35AVerC4bXlPq!)_cLCqzDjRIkiD&CwLS}{gmJa@XWR&Jl9|xbheiWB|@7u za|iC4E0trV5nKzf4!EXMiFsMhM}4LTc%e8@{#P{SRp^CAbMVONZ0-hIg*tyU_GrJ{P_$js==<-NZF%=wa|@}NLY}F&>&x;K#q*kQNfd>ujNol61%d~X7JM%)csKPpB@zPv zWV3U^Yn!c<{ilBQTUmAia=9aXD3b2YY3SNuZ(XkzwlUESL*O^kD7Fg~CD;uGsZJPg zl`3BLx))gq zS1-ID522a`FLw+A%c-FTexFVz!6Jt1IcS~cMP0yZaHkKi{@5g-DcKyv_UPoLHC__b zGxy0d21M)e_lqv8oawsmQE6i?U0qhwEi3EMO`^Z>L3pvi9d~JjL}%bxJh2+d80a@8 z*a;HjJ=O9r)?3dv+YQF+xWK==RSSYGI+TjP#IE7De=%=A4yXd>e?X`2*Fgzx2Mqr+ zKNk(>`lmi>S4ZZa`h4Y{=KlC|ODDWuz1fO8`k<=Zr-Ksgp%I4RGoEfWkQ~j8l9TPm z>(iqF7=8Pg>iJXrT+)Rg1 zjiH(E>9WwO{OGv1BE9&@?Mzv9BOsTEZySkC_rB_)yw!_dzuwCi;10S@W3%<%ir!qq;Pi`{v@C2__q<~y&U=&WO`L?x3QW-XJ@J?DL1j~yIftmSdVGBN0USD5p(`pl!@Ly~0B=i($Ii)T?J zbV*GgYy)xqSwCSci(S~RpBRPb`+SV$?u}($MQR0Owglz-d`xtvsXI7WpPNv#SCG#T z>D!>PVAK`UTxVp8P0~+FFpiY)b|86w$PN=Xl?35DR2^ey7 zA>kx%hUv=1-83Mv(yWw7(G0DZLl^EXai`BTm`eOA=^1=0x5Tq5@S`8Dm z2=O1@$(qirey3k&7p~~j>bU->JX<-VbN~Kp( snapshot: &SyncbackSnapshot<'sync>, ) -> anyhow::Result> { + fn serialize<'sync>(path: &Path, model: &JsonModel) -> anyhow::Result> { + println!("{}", path.display()); + Ok(SyncbackReturn { + fs_snapshot: FsSnapshot::new().with_added_file( + path, + serde_json::to_vec_pretty(model).context("failed to serialize new JSON Model")?, + ), + children: Vec::new(), + removed_children: Vec::new(), + }) + } let mut property_buffer = Vec::with_capacity(snapshot.new_inst().properties.len()); - let mut model = json_model_from_pair(snapshot, &mut property_buffer, snapshot.new); + let mut root_model = json_model_from_pair(snapshot, &mut property_buffer, snapshot.new); // We don't need the name on the root, but we do for children. - model.name = None; + root_model.name = None; + + let mut node_queue = VecDeque::new(); + node_queue.push_back((&root_model, snapshot.old_inst(), Some(snapshot.new_inst()))); + + while let Some((model, old_inst, new_inst)) = node_queue.pop_front() { + let Some(old_inst) = old_inst else { + return serialize(&snapshot.path, &root_model); + }; + + let Some(new_inst) = new_inst else { + return serialize(&snapshot.path, &root_model); + }; + + if old_inst.class_name() != new_inst.class { + return serialize(&snapshot.path, &root_model); + } + + if old_inst.name() != new_inst.name { + return serialize(&snapshot.path, &root_model); + } + + if node_should_reserialize(&model.properties, &model.attributes, old_inst)? == true { + return serialize(&snapshot.path, &root_model); + } + + for ((child_model, old_child_ref), new_child_ref) in model + .children + .iter() + .zip(old_inst.children().iter()) + .zip(new_inst.children().iter()) + { + node_queue.push_back(( + child_model, + snapshot.get_old_instance(*old_child_ref), + snapshot.get_new_instance(*new_child_ref), + )) + } + } Ok(SyncbackReturn { - fs_snapshot: FsSnapshot::new().with_added_file( - &snapshot.path, - serde_json::to_vec_pretty(&model).context("failed to serialize new JSON Model")?, - ), + fs_snapshot: FsSnapshot::new(), children: Vec::new(), removed_children: Vec::new(), }) diff --git a/tests/tests/syncback.rs b/tests/tests/syncback.rs index ca2ed0f02..7a816ce52 100644 --- a/tests/tests/syncback.rs +++ b/tests/tests/syncback.rs @@ -33,4 +33,5 @@ syncback_basic_test! { ignore_paths_removing, ignore_trees_removing, remove_default_props, + json_models, } From a854e8aac5f8f7d5a8b168c1cecee06c4bb04062 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Wed, 13 Nov 2024 17:55:14 +0000 Subject: [PATCH 16/20] Remove println --- src/snapshot_middleware/json_model.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 903f0b44c..5c5343c58 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -70,7 +70,6 @@ pub fn syncback_json_model<'sync>( snapshot: &SyncbackSnapshot<'sync>, ) -> anyhow::Result> { fn serialize<'sync>(path: &Path, model: &JsonModel) -> anyhow::Result> { - println!("{}", path.display()); Ok(SyncbackReturn { fs_snapshot: FsSnapshot::new().with_added_file( path, From 59d780c4a98066d6633c82b42ad582471f5c7bce Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Wed, 13 Nov 2024 17:57:02 +0000 Subject: [PATCH 17/20] Remove log::debugs --- src/snapshot_middleware/mod.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index ad9d361f8..881c0993b 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -472,11 +472,9 @@ fn node_should_reserialize( .clone() .resolve(instance.class_name(), prop_name)?; if !variant_eq(inst_value, &node_value) { - log::debug!("reserializing because different property on node"); return Ok(true); } } else { - log::debug!("reserializing because property did no exist on node"); return Ok(true); } } @@ -512,7 +510,6 @@ fn node_should_reserialize( // fail to reproduce properties of classes unknown to the reflection // database. let Some(class_descriptor) = maybe_class_descriptor else { - log::debug!("reserializing because because no class descriptor"); return Ok(true); }; @@ -524,7 +521,6 @@ fn node_should_reserialize( // fail to reproduce properties that do not have defaults in the // reflection database. let Some(default_value) = db.find_default_property(class_descriptor, prop_name) else { - log::debug!("reserializing because because no default value"); return Ok(true); }; @@ -532,7 +528,6 @@ fn node_should_reserialize( // not specify this property, it means that its value has changed to the // default, and the new node must be reserialized. if !variant_eq(inst_value, default_value) { - log::debug!("reserializing because non-default"); return Ok(true); } } @@ -547,11 +542,9 @@ fn node_should_reserialize( if let Some(inst_value) = inst_attributes.get(attr_name.as_str()) { let node_value = unresolved_node_value.clone().resolve_unambiguous()?; if !variant_eq(inst_value, &node_value) { - log::debug!("reserializing because attribute different"); return Ok(true); } } else { - log::debug!("reserializing because attribute DNE"); return Ok(true); } } @@ -561,7 +554,6 @@ fn node_should_reserialize( Some(_) => Ok(true), None => { if !node_attributes.is_empty() { - log::debug!("reserializing because non-empty attributes"); Ok(true) } else { Ok(false) From 3dd68565a952b86ef7bf9f125e6e25f82e1bc0a0 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Thu, 14 Nov 2024 15:41:36 +0000 Subject: [PATCH 18/20] Remove == true --- src/snapshot_middleware/json_model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 5c5343c58..18c8cd090 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -105,7 +105,7 @@ pub fn syncback_json_model<'sync>( return serialize(&snapshot.path, &root_model); } - if node_should_reserialize(&model.properties, &model.attributes, old_inst)? == true { + if node_should_reserialize(&model.properties, &model.attributes, old_inst)? { return serialize(&snapshot.path, &root_model); } From a4f10c61d134c1eccc6e04fe9b7feffee8051164 Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Thu, 14 Nov 2024 15:48:38 +0000 Subject: [PATCH 19/20] Test if meta files should reserialize --- src/snapshot_middleware/meta_file.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 1815b0a3d..efa617fa5 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -13,7 +13,7 @@ use crate::{ resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, RojoRef, }; -use super::populate_unresolved_properties; +use super::{node_should_reserialize, populate_unresolved_properties}; /// Represents metadata in a sibling file with the same basename. /// @@ -86,6 +86,12 @@ impl AdjacentMetadata { snapshot.get_path_filtered_properties(snapshot.new).unwrap(), ); + if let Some(old_inst) = snapshot.old_inst() { + if !node_should_reserialize(&properties, &attributes, old_inst)? { + return Ok(None); + }; + } + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) @@ -246,6 +252,12 @@ impl DirectoryMetadata { snapshot.get_path_filtered_properties(snapshot.new).unwrap(), ); + if let Some(old_inst) = snapshot.old_inst() { + if !node_should_reserialize(&properties, &attributes, old_inst)? { + return Ok(None); + }; + } + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) From b2fd639a3be711c158ea70f614f5aaf16bafc5ff Mon Sep 17 00:00:00 2001 From: kennethloeffler Date: Thu, 14 Nov 2024 15:50:09 +0000 Subject: [PATCH 20/20] Run tests --- .../end_to_end__rojo_test__syncback_util__all_middleware.snap | 1 - 1 file changed, 1 deletion(-) diff --git a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap index 79c03d271..f001225cd 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__all_middleware.snap @@ -11,7 +11,6 @@ added_files: - src/dir/init_server_script/init.server.luau - src/dir/module_script.luau - src/dir/server_script.server.luau - - src/dir_with_meta/init.meta.json - src/model_json.model.json - src/project_json.project.json - src/rbxm.rbxm