Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actually include attribute-defined properties in patch computation #944

Merged
merged 4 commits into from
Aug 19, 2024
Merged

Actually include attribute-defined properties in patch computation #944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1994561
Add test for issue 929
Dekkonot Jul 19, 2024
6dcca65
Include attribute properties in patch computation
Dekkonot Jul 19, 2024
62f75e8
Merge remote-tracking branch 'master' into i-fixa-da-ref-props
Dekkonot Jul 20, 2024
ed7e3c2
Add a doc comment for the ref_properties_patch_update test
Dekkonot Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions ...est/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_all-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-10:
Children: []
ClassName: ObjectValue
Id: id-10
Metadata:
ignoreUnknownInstances: true
Name: ProjectPointer
Parent: id-9
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: project target
Value:
Ref: id-9
id-2:
Children:
- id-3
ClassName: DataModel
Id: id-2
Metadata:
ignoreUnknownInstances: true
Name: ref_properties
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children:
- id-4
- id-5
- id-7
- id-9
ClassName: Workspace
Id: id-3
Metadata:
ignoreUnknownInstances: true
Name: Workspace
Parent: id-2
Properties: {}
id-4:
Children: []
ClassName: ObjectValue
Id: id-4
Metadata:
ignoreUnknownInstances: true
Name: CrossFormatPointer
Parent: id-3
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-5:
Children:
- id-6
ClassName: Folder
Id: id-5
Metadata:
ignoreUnknownInstances: false
Name: FolderTarget
Parent: id-3
Properties: {}
id-6:
Children: []
ClassName: ObjectValue
Id: id-6
Metadata:
ignoreUnknownInstances: false
Name: FolderPointer
Parent: id-5
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-7:
Children:
- id-8
ClassName: Folder
Id: id-7
Metadata:
ignoreUnknownInstances: false
Name: ModelTarget
Parent: id-3
Properties: {}
id-8:
Children: []
ClassName: Model
Id: id-8
Metadata:
ignoreUnknownInstances: false
Name: ModelPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: model target
PrimaryPart:
Ref: id-7
id-9:
Children:
- id-10
ClassName: Folder
Id: id-9
Metadata:
ignoreUnknownInstances: true
Name: ProjectTarget
Parent: id-3
Properties: {}
messageCursor: 0
sessionId: id-1
120 changes: 120 additions & 0 deletions ...-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_all.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
---
source: tests/tests/serve.rs
expression: "read_response.intern_and_redact(&mut redactions, root_id)"
---
instances:
id-10:
Children: []
ClassName: ObjectValue
Id: id-10
Metadata:
ignoreUnknownInstances: true
Name: ProjectPointer
Parent: id-9
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: project target
Value:
Ref: id-9
id-2:
Children:
- id-3
ClassName: DataModel
Id: id-2
Metadata:
ignoreUnknownInstances: true
Name: ref_properties
Parent: "00000000000000000000000000000000"
Properties: {}
id-3:
Children:
- id-4
- id-5
- id-7
- id-9
ClassName: Workspace
Id: id-3
Metadata:
ignoreUnknownInstances: true
Name: Workspace
Parent: id-2
Properties: {}
id-4:
Children: []
ClassName: ObjectValue
Id: id-4
Metadata:
ignoreUnknownInstances: true
Name: CrossFormatPointer
Parent: id-3
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-5:
Children:
- id-6
ClassName: Folder
Id: id-5
Metadata:
ignoreUnknownInstances: false
Name: FolderTarget
Parent: id-3
Properties: {}
id-6:
Children: []
ClassName: ObjectValue
Id: id-6
Metadata:
ignoreUnknownInstances: false
Name: FolderPointer
Parent: id-5
Properties:
Attributes:
Attributes:
Rojo_Target_Value:
String: folder target
Value:
Ref: id-5
id-7:
Children:
- id-8
ClassName: Folder
Id: id-7
Metadata:
ignoreUnknownInstances: false
Name: ModelTarget
Parent: id-3
Properties: {}
id-8:
Children: []
ClassName: Model
Id: id-8
Metadata:
ignoreUnknownInstances: false
Name: ModelPointer
Parent: id-7
Properties:
Attributes:
Attributes:
Rojo_Target_PrimaryPart:
String: model target
PrimaryPart:
Ref: id-7
id-9:
Children:
- id-10
ClassName: Folder
Id: id-9
Metadata:
ignoreUnknownInstances: true
Name: ProjectTarget
Parent: id-3
Properties: {}
messageCursor: 0
sessionId: id-1
12 changes: 12 additions & 0 deletions ...test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_info.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: tests/tests/serve.rs
expression: redactions.redacted_yaml(info)
---
expectedPlaceIds: ~
gameId: ~
placeId: ~
projectName: ref_properties
protocolVersion: 4
rootInstanceId: id-2
serverVersion: "[server-version]"
sessionId: id-1
71 changes: 70 additions & 1 deletion src/snapshot/patch_compute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use std::{

use rbx_dom_weak::types::{Ref, Variant};

use crate::{RojoRef, REF_POINTER_ATTRIBUTE_PREFIX};

use super::{
patch::{PatchAdd, PatchSet, PatchUpdate},
InstanceSnapshot, InstanceWithMeta, RojoTree,
Expand Down Expand Up @@ -87,18 +89,21 @@ fn compute_patch_set_internal(
.get_instance(id)
.expect("Instance did not exist in tree");

compute_property_patches(&mut snapshot, &instance, patch_set);
compute_property_patches(&mut snapshot, &instance, patch_set, tree);
compute_children_patches(context, &mut snapshot, tree, id, patch_set);
}

fn compute_property_patches(
snapshot: &mut InstanceSnapshot,
instance: &InstanceWithMeta,
patch_set: &mut PatchSet,
tree: &RojoTree,
) {
let mut visited_properties = HashSet::new();
let mut changed_properties = HashMap::new();

let attribute_ref_properties = compute_ref_properties(snapshot, tree);

let changed_name = if snapshot.name == instance.name() {
None
} else {
Expand Down Expand Up @@ -140,6 +145,24 @@ fn compute_property_patches(
changed_properties.insert(name.clone(), None);
}

for (name, ref_value) in attribute_ref_properties {
match (&ref_value, instance.properties().get(&name)) {
(Some(referent), Some(instance_value)) => {
if referent != instance_value {
changed_properties.insert(name, ref_value);
} else {
changed_properties.remove(&name);
}
}
(Some(_), None) | (None, Some(_)) => {
changed_properties.insert(name, ref_value);
}
(None, None) => {
changed_properties.remove(&name);
}
}
}

if changed_properties.is_empty()
&& changed_name.is_none()
&& changed_class_name.is_none()
Expand Down Expand Up @@ -224,6 +247,52 @@ fn compute_children_patches(
}
}

fn compute_ref_properties(
snapshot: &InstanceSnapshot,
tree: &RojoTree,
) -> HashMap<String, Option<Variant>> {
let mut map = HashMap::new();
let attributes = match snapshot.properties.get("Attributes") {
Some(Variant::Attributes(attrs)) => attrs,
_ => return map,
};

for (attr_name, attr_value) in attributes.iter() {
let prop_name = match attr_name.strip_prefix(REF_POINTER_ATTRIBUTE_PREFIX) {
Some(str) => str,
None => continue,
};
let rojo_ref = match attr_value {
Variant::String(str) => RojoRef::new(str.clone()),
Variant::BinaryString(bytes) => {
if let Ok(str) = std::str::from_utf8(bytes.as_ref()) {
RojoRef::new(str.to_string())
} else {
log::warn!(
"IDs specified by referent property attributes must be valid UTF-8 strings"
);
continue;
}
}
_ => {
log::warn!(
"Attribute {attr_name} is of type {:?} when it was \
expected to be a String",
attr_value.ty()
);
continue;
}
};
if let Some(target_id) = tree.get_specified_id(&rojo_ref) {
map.insert(prop_name.to_string(), Some(Variant::Ref(target_id)));
} else {
map.insert(prop_name.to_string(), None);
}
}

map
}

#[cfg(test)]
mod test {
use super::*;
Expand Down
Loading
Loading