From d069f0fe9838f6f8ef9005a57090d32459866fa7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 15 Oct 2026 09:06:18 -0700 Subject: [PATCH] Variables: selecting a data set sets the objects inside a group that it shows dataset.select applied a row's values in the order the variables were defined, and it skips an object inside a hidden group. When a row showed a group and set an object inside it, and the object's variable was defined first, the object was skipped while the group was still hidden: one selection applied the row in part, and selecting the row again applied the rest. A text variable on type inside the group was skipped the same way, and a row that hid a group skipped the objects inside it whose variables were defined after the group's. dataset.select, dataset.next and dataset.prev now apply the visibility values that show objects first, outermost object first, then the other values in definition order, then the visibility values that hide objects, innermost object first. Of two variables of the same kind on one object, the one defined last still wins. Docs: docs/mcp.md. --- crates/doc/src/variables.rs | 7 +- crates/engine/src/cmd/variablecmds.rs | 22 +++++++ crates/engine/src/tests_variables.rs | 93 +++++++++++++++++++++++++++ docs/mcp.md | 7 ++ 4 files changed, 126 insertions(+), 3 deletions(-) diff --git a/crates/doc/src/variables.rs b/crates/doc/src/variables.rs index 276567b12..1e86ecbb4 100644 --- a/crates/doc/src/variables.rs +++ b/crates/doc/src/variables.rs @@ -84,9 +84,10 @@ impl DataSet { /// /// An object may be driven by several variables — the reference app binds the selected object /// to the selected variable, one click at a time, and never says a second binding replaces the -/// first — so a binding is a set. `dataset.select` applies them in the order the variables are -/// defined, which is what makes two text variables on one object deterministic: the last one -/// wins, and `applied` counts both. +/// first — so a binding is a set. `dataset.select` applies the text values in the order the +/// variables are defined, which is what makes two text variables on one object deterministic: +/// the last one wins, and `applied` counts both. Of two visibility variables on one object, the +/// last one defined wins too. #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct Variables { #[serde(default, skip_serializing_if = "Vec::is_empty")] diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 889ca0b07..009d48d05 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -413,6 +413,28 @@ fn apply_dataset(s: &mut Session, name: &str) -> Result { for v in &d.variables.variables { pairs.extend(d.variables.objects_of(&v.name).into_iter().map(|id| (id, v.name.clone()))); } + // The row's values go in three passes: the visibility values that show an object, + // outermost object first; the other values, in definition order; the visibility values + // that hide an object, innermost object first. An object inside a group is then rewritten + // after the row shows the group and before the row hides it. An object with several + // visibility variables goes in the pass of the last one's value, and each pass keeps + // definition order otherwise. + let mut shows = std::collections::HashMap::new(); + for (id, var) in &pairs { + let kind = d.variables.variable(var).map(|v| v.kind); + if let (Some(VariableKind::Visibility), Some(DataValue::Visible(show))) = (kind, ds.values.get(var)) { + shows.insert(*id, *show); + } + } + let depths = d.index_paths(shows.keys().copied()); + pairs.sort_by_cached_key(|(id, var)| { + let depth = depths.get(id).map_or(0, Vec::len); + match (d.variables.variable(var).map(|v| v.kind), shows.get(id)) { + (Some(VariableKind::Visibility), Some(true)) => (0, depth), + (Some(VariableKind::Visibility), Some(false)) => (2, usize::MAX - depth), + _ => (1, 0), + } + }); let mut applied = 0usize; let mut skipped = 0usize; for (id, var) in pairs { diff --git a/crates/engine/src/tests_variables.rs b/crates/engine/src/tests_variables.rs index aa05e7963..522e0e9ca 100644 --- a/crates/engine/src/tests_variables.rs +++ b/crates/engine/src/tests_variables.rs @@ -22,6 +22,13 @@ fn rect(s: &mut Session) -> NodeId { NodeId(r["id"].as_u64().unwrap()) } +/// A group of the objects `ids` → the group's id. +fn group(s: &mut Session, ids: &[NodeId]) -> NodeId { + s.execute("select.set", &json!({"ids": ids.iter().map(|i| i.0).collect::>()})).unwrap(); + let r = s.execute("object.group", &json!({})).unwrap(); + NodeId(r["id"].as_u64().unwrap()) +} + fn plain(s: &Session, id: NodeId) -> String { match &s.doc().unwrap().doc.node(id).unwrap().kind { vectorcraft_doc::NodeKind::Text(t) => t.plain_text(), @@ -671,6 +678,92 @@ fn a_row_that_hides_selected_objects_deselects_them() { assert_eq!((sel.objects, sel.key), (vec![r, q], Some(r)), "undo selects r again as the key object"); } +/// A row that shows or hides a group also sets the objects inside it in the same selection, +/// whichever variable was defined first. +#[test] +fn a_row_sets_the_objects_inside_a_group_it_shows_or_hides() { + let mut s = session(); + let (inner, other) = (rect(&mut s), rect(&mut s)); + let g = group(&mut s, &[inner, other]); + s.execute("variable.define", &json!({"name": "Inner", "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Inner", "ids": [inner.0]})).unwrap(); + s.execute("variable.define", &json!({"name": "Set", "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Set", "ids": [g.0]})).unwrap(); + s.execute("dataset.new", &json!({"name": "Hidden", "values": {"Inner": false, "Set": false}})).unwrap(); + s.execute("dataset.new", &json!({"name": "Shown", "values": {"Inner": true, "Set": true}})).unwrap(); + + let v = s.execute("dataset.select", &json!({"name": "Hidden"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + let v = s.execute("dataset.select", &json!({"name": "Shown"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + let doc = &s.doc().unwrap().doc; + assert!(doc.node(g).unwrap().visible && doc.node(inner).unwrap().visible, "the group and the object inside it show"); + let sets = s.execute("dataset.list", &json!({})).unwrap(); + let matching: Vec<&Value> = sets["datasets"].as_array().unwrap().iter().filter(|d| d["matches"] == json!(true)).map(|d| &d["name"]).collect(); + assert_eq!(matching, [&json!("Shown")], "{sets}"); + + // With the inner object's variable defined last, a row that hides both still hides it. + s.execute("variable.delete", &json!({"names": ["Inner"]})).unwrap(); + s.execute("variable.define", &json!({"name": "Inner", "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Inner", "ids": [inner.0]})).unwrap(); + s.execute("dataset.set", &json!({"name": "Hidden", "values": {"Inner": false, "Set": false}})).unwrap(); + let v = s.execute("dataset.select", &json!({"name": "Hidden"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + let doc = &s.doc().unwrap().doc; + assert!(!doc.node(g).unwrap().visible && !doc.node(inner).unwrap().visible, "both are hidden"); +} + +/// A row writes the text of type inside a group that it shows or hides, whichever variable was +/// defined first. +#[test] +fn a_row_writes_type_inside_a_group_it_shows_or_hides() { + let mut s = session(); + let t = text(&mut s, "Alice"); + let other = rect(&mut s); + let g = group(&mut s, &[t, other]); + s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap(); + s.execute("variable.define", &json!({"name": "Set", "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Set", "ids": [g.0]})).unwrap(); + s.execute("dataset.new", &json!({"name": "Hidden", "values": {"Name": "Bob", "Set": false}})).unwrap(); + s.execute("dataset.new", &json!({"name": "Shown", "values": {"Name": "Carol", "Set": true}})).unwrap(); + + s.execute("dataset.select", &json!({"name": "Hidden"})).unwrap(); + assert_eq!(plain(&s, t), "Bob"); + let v = s.execute("dataset.select", &json!({"name": "Shown"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + assert_eq!(plain(&s, t), "Carol", "the text is written although the group started hidden"); + + // With the text variable defined last, a row that hides the group still writes the text. + s.execute("variable.delete", &json!({"names": ["Name"]})).unwrap(); + s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap(); + s.execute("dataset.set", &json!({"name": "Hidden", "values": {"Name": "Bob", "Set": false}})).unwrap(); + let v = s.execute("dataset.select", &json!({"name": "Hidden"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + assert_eq!(plain(&s, t), "Bob", "the text is written before the group is hidden"); + assert!(!s.doc().unwrap().doc.node(g).unwrap().visible); +} + +/// Of two visibility variables on one object, the one defined later decides whether it shows, +/// as with two text variables. +#[test] +fn the_later_visibility_variable_on_an_object_wins() { + let mut s = session(); + let r = rect(&mut s); + for name in ["First", "Second"] { + s.execute("variable.define", &json!({"name": name, "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": name, "ids": [r.0]})).unwrap(); + } + s.execute("dataset.new", &json!({"name": "Hide", "values": {"First": true, "Second": false}})).unwrap(); + s.execute("dataset.new", &json!({"name": "Show", "values": {"First": false, "Second": true}})).unwrap(); + let v = s.execute("dataset.select", &json!({"name": "Hide"})).unwrap(); + assert_eq!(v["applied"], json!(2), "{v}"); + assert!(!s.doc().unwrap().doc.node(r).unwrap().visible); + s.execute("dataset.select", &json!({"name": "Show"})).unwrap(); + assert!(s.doc().unwrap().doc.node(r).unwrap().visible); +} + /// Variables and datasets are separate namespaces: renaming a variable leaves a dataset of /// the same name, and which dataset is active, alone. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 693fb2159..4b96e58c9 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -965,6 +965,13 @@ already walks Image Trace. Short agent entry points: - Image Trace: `imageTrace.presets`, `imageTrace.make {id?, preset?, params?, view?}` (or `imageTrace.makeAndExpand`), `imageTrace.setView`, `imageTrace.expand`, `imageTrace.release`. +`dataset.select`, `dataset.next` and `dataset.prev` reply `{name, applied, skipped}`. They skip an object that is +locked or inside a hidden or locked layer or group. They apply the data set's visibility values that show objects +first, outermost object first, then its other values in the order the variables are defined, then its visibility +values that hide objects, innermost object first. A data set that shows a group sets the objects inside it in the +same call, and one that hides a visible group sets the objects inside it before it hides the group. Of two +variables of the same kind on one object, the one defined last wins. + ```json {"name":"run_command","arguments":{"command":"symbol.list","params":{}}} {"name":"run_command","arguments":{"command":"brush.list","params":{}}}