From 9b601eb9fe657b386c635f461a104f4a201cf648 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Thu, 19 Sep 2019 10:56:10 -0700 Subject: [PATCH] Tidy up patch structs, add AppliedPatchSet --- src/snapshot/patch.rs | 56 +++++++++++++++++++++++++++++++---- src/snapshot/patch_apply.rs | 14 ++++----- src/snapshot/patch_compute.rs | 14 ++++----- 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/snapshot/patch.rs b/src/snapshot/patch.rs index c89ed885..c3438cca 100644 --- a/src/snapshot/patch.rs +++ b/src/snapshot/patch.rs @@ -7,15 +7,19 @@ use rbx_dom_weak::{RbxId, RbxValue}; use super::{InstanceMetadata, InstanceSnapshot}; /// A set of different kinds of patches that can be applied to an RbxTree. +/// +/// These patches shouldn't be persisted: there's no mechanism in place to make +/// sure that another patch wasn't applied before this one that could cause a +/// conflict! #[derive(Debug, Default, Clone, PartialEq)] pub struct PatchSet<'a> { pub removed_instances: Vec, - pub added_instances: Vec>, - pub updated_instances: Vec, + pub added_instances: Vec>, + pub updated_instances: Vec, } impl<'a> PatchSet<'a> { - pub fn new() -> PatchSet<'a> { + pub fn new() -> Self { PatchSet { removed_instances: Vec::new(), added_instances: Vec::new(), @@ -26,14 +30,14 @@ impl<'a> PatchSet<'a> { /// A patch containing an instance that was added to the tree. #[derive(Debug, Clone, PartialEq)] -pub struct PatchAddInstance<'a> { +pub struct PatchAdd<'a> { pub parent_id: RbxId, pub instance: InstanceSnapshot<'a>, } -/// A patch indicating that properties (or the name) of an instance changed. +/// A patch indicating that properties of an instance changed. #[derive(Debug, Clone, PartialEq)] -pub struct PatchUpdateInstance { +pub struct PatchUpdate { pub id: RbxId, pub changed_name: Option, pub changed_class_name: Option, @@ -45,3 +49,43 @@ pub struct PatchUpdateInstance { /// Changed Rojo-specific metadata, if any of it changed. pub changed_metadata: Option, } + +/// Applied patch sets have the same rough shape as PatchSet, but are +/// descriptive of the operation that happened instead of prescribing what +/// mutations to apply to the tree. +/// +/// Applied patch sets are generated by applying a patch to a tree, and are +/// suitable for sending over the network to a synchronized tree like the Rojo +/// Studio plugin. +/// +// TODO: Introduce machinery to detect conflicts, like keeping previous + +// current values in all fields. +pub struct AppliedPatchSet { + pub removed: Vec, + pub added: Vec, + pub updated: Vec, +} + +impl AppliedPatchSet { + pub fn new() -> Self { + AppliedPatchSet { + removed: Vec::new(), + added: Vec::new(), + updated: Vec::new(), + } + } +} + +pub struct AppliedPatchAdd { + pub instance_id: RbxId, +} + +pub struct AppliedPatchUpdate { + pub id: RbxId, + + // TODO: Store previous values in order to detect application conflicts + pub changed_name: Option, + pub changed_class_name: Option, + pub changed_properties: HashMap>, + pub changed_metadata: Option, +} diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 78acc584..986f9c17 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; use rbx_dom_weak::{RbxId, RbxInstanceProperties, RbxValue}; use super::{ - patch::{PatchSet, PatchUpdateInstance}, + patch::{PatchSet, PatchUpdate}, InstancePropertiesWithMeta, InstanceSnapshot, RojoTree, }; @@ -95,11 +95,7 @@ fn apply_add_child( } } -fn apply_update_child( - context: &PatchApplyContext, - tree: &mut RojoTree, - patch: &PatchUpdateInstance, -) { +fn apply_update_child(context: &PatchApplyContext, tree: &mut RojoTree, patch: &PatchUpdate) { if let Some(metadata) = &patch.changed_metadata { tree.update_metadata(patch.id, metadata.clone()); } @@ -150,7 +146,7 @@ mod test { use maplit::hashmap; use rbx_dom_weak::RbxValue; - use super::super::PatchAddInstance; + use super::super::PatchAdd; #[test] fn add_from_empty() { @@ -179,7 +175,7 @@ mod test { }; let patch_set = PatchSet { - added_instances: vec![PatchAddInstance { + added_instances: vec![PatchAdd { parent_id: root_id, instance: snapshot.clone(), }], @@ -217,7 +213,7 @@ mod test { let root_id = tree.get_root_id(); - let patch = PatchUpdateInstance { + let patch = PatchUpdate { id: root_id, changed_name: Some("Foo".to_owned()), changed_class_name: Some("NewClassName".to_owned()), diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index 13b308b9..f2405df4 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -6,7 +6,7 @@ use std::collections::{HashMap, HashSet}; use rbx_dom_weak::{RbxId, RbxValue}; use super::{ - patch::{PatchAddInstance, PatchSet, PatchUpdateInstance}, + patch::{PatchAdd, PatchSet, PatchUpdate}, InstanceSnapshot, InstanceWithMeta, RojoTree, }; @@ -33,7 +33,7 @@ struct ComputePatchContext { snapshot_id_to_instance_id: HashMap, } -fn rewrite_refs_in_updates(context: &ComputePatchContext, updates: &mut [PatchUpdateInstance]) { +fn rewrite_refs_in_updates(context: &ComputePatchContext, updates: &mut [PatchUpdate]) { for update in updates { for property_value in update.changed_properties.values_mut() { if let Some(RbxValue::Ref { value: Some(id) }) = property_value { @@ -47,7 +47,7 @@ fn rewrite_refs_in_updates(context: &ComputePatchContext, updates: &mut [PatchUp } } -fn rewrite_refs_in_additions(context: &ComputePatchContext, additions: &mut [PatchAddInstance]) { +fn rewrite_refs_in_additions(context: &ComputePatchContext, additions: &mut [PatchAdd]) { for addition in additions { rewrite_refs_in_snapshot(context, &mut addition.instance); } @@ -145,7 +145,7 @@ fn compute_property_patches( return; } - patch_set.updated_instances.push(PatchUpdateInstance { + patch_set.updated_instances.push(PatchUpdate { id: instance.id(), changed_name, changed_class_name, @@ -204,7 +204,7 @@ fn compute_children_patches<'a>( ); } None => { - patch_set.added_instances.push(PatchAddInstance { + patch_set.added_instances.push(PatchAdd { parent_id: id, instance: snapshot_child.clone(), }); @@ -269,7 +269,7 @@ mod test { let patch_set = compute_patch_set(&snapshot, &tree, root_id); let expected_patch_set = PatchSet { - updated_instances: vec![PatchUpdateInstance { + updated_instances: vec![PatchUpdate { id: root_id, changed_name: None, changed_class_name: None, @@ -330,7 +330,7 @@ mod test { let patch_set = compute_patch_set(&snapshot, &tree, root_id); let expected_patch_set = PatchSet { - added_instances: vec![PatchAddInstance { + added_instances: vec![PatchAdd { parent_id: root_id, instance: InstanceSnapshot { snapshot_id: None,