From 5669e02331c6f0f896ed4fa4702d01d60c5adae5 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 16 Oct 2026 23:21:11 -0700 Subject: [PATCH] Variables: a data set rewrites bound type on hidden layers dataset.select skipped every bound object inside a hidden layer or group. Type on a hidden layer kept the previous row's text, which showed when the layer was shown again, and dataset.list and the Variables panel reported every row as matching that type while the layer stayed hidden. dataset.select, dataset.next and dataset.prev now rewrite type and apply the other values on hidden objects and inside hidden layers and groups. Those objects stay hidden unless a visibility value shows them. Objects that are locked or inside a locked layer or group are still skipped and counted in `skipped`. dataset.list, dataset.capture and dataset.update read hidden objects as well. Binding still refuses art inside a hidden layer or group. The values now go in the order the variables are defined. The passes that applied visibility values before and after the other values are removed. Docs: docs/mcp.md, docs/target-app-parity.md. --- crates/doc/src/variables.rs | 18 +++++--- crates/engine/src/cmd/variablecmds.rs | 41 ++++------------- crates/engine/src/tests_variables.rs | 64 +++++++++++++++++++++++++++ docs/mcp.md | 14 +++--- docs/target-app-parity.md | 3 +- 5 files changed, 94 insertions(+), 46 deletions(-) diff --git a/crates/doc/src/variables.rs b/crates/doc/src/variables.rs index 1147268cd..360de81c9 100644 --- a/crates/doc/src/variables.rs +++ b/crates/doc/src/variables.rs @@ -184,20 +184,26 @@ impl Document { self.variables.bindings = bindings; } - /// Whether a variable may read and rewrite the object `id`: it is in the document, neither - /// it nor anything above it is locked, and nothing above it is hidden. Its own visibility - /// doesn't count, as that is what a visibility variable drives: a row that hid it must be - /// able to show it again, and a capture must record it hidden. + /// Whether a variable may be bound to the object `id`: it is in the document, neither it nor + /// anything above it is locked, and nothing above it is hidden. Its own visibility doesn't + /// count, as that is what a visibility variable drives. pub fn variable_target(&self, id: NodeId) -> bool { self.ancestry(id).is_some_and(|a| a.iter().all(|i| self.node(*i).is_some_and(|n| !n.locked && (n.visible || *i == id)))) } + /// Whether a data set reads and rewrites the bound object `id`: it is in the document and + /// neither it nor anything above it is locked. Hidden objects count, and so do objects in a + /// hidden layer or group. + pub fn dataset_target(&self, id: NodeId) -> bool { + self.ancestry(id).is_some_and(|a| a.iter().all(|i| self.node(*i).is_some_and(|n| !n.locked))) + } + /// What the object `id` holds for the variable of `kind`: its characters, whether it shows, /// or a linked image's path. `None` when the object is gone, is not of that kind, has no - /// link path yet (an embedded image), or is locked or on a hidden or locked layer (the + /// link path yet (an embedded image), or is locked or in a locked layer or group (the /// document's value is whatever it was left at). pub fn variable_value(&self, id: NodeId, kind: VariableKind) -> Option { - if !self.variable_target(id) { + if !self.dataset_target(id) { return None; } let node = self.node(id)?; diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index e57a5049e..3694f8e54 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -61,9 +61,8 @@ fn names_param(p: &Value, cmd: &str, fallback: Option, nothing: &str) -> } } -/// Refuse what a variable of `kind` can't drive: art that is locked or on a hidden or locked -/// layer is not the document's to rewrite; a text variable only drives type; an image variable -/// only drives an image object. +/// Refuse what a variable of `kind` can't be bound to: art that is locked or on a hidden or locked +/// layer; a text variable only drives type; an image variable only drives an image object. fn check_bindable(doc: &vectorcraft_doc::Document, ids: &[NodeId], kind: VariableKind, cmd: &str) -> Result<()> { for id in ids { match doc.node(*id) { @@ -369,8 +368,8 @@ fn dataset_capture(s: &mut Session, p: &Value) -> Result { } /// Update Data Set: write what the bound objects hold now into the row that is active, for each -/// variable the row holds. A variable whose objects report no value (locked, inside a hidden or -/// locked layer or group, or gone) keeps the row's value. With `all`, every other variable the +/// variable the row holds. A variable whose objects report no value (locked, inside a locked +/// layer or group, or gone) keeps the row's value. With `all`, every other variable the /// objects report a value for is added to the row. Without an active row there is nothing to /// update, so say so rather than picking one. fn dataset_update(s: &mut Session, p: &Value) -> Result { @@ -439,28 +438,6 @@ 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; // Collects the type objects whose text the row rewrote, for the reply's `overflow`. @@ -476,10 +453,10 @@ fn apply_dataset(s: &mut Session, name: &str) -> Result { continue; } }; - // Locked art, art on a locked or hidden layer, and text whose object is gone, can't - // be rewritten: skipped and counted, like the rest. A hidden object is rewritten: a - // row that hid it shows it again. - if !d.variable_target(id) { + // Locked art, art in a locked layer or group, and text whose object is gone, can't be + // rewritten: skipped and counted, like the rest. Hidden art is rewritten, in a hidden + // layer or group too, and stays hidden unless a visibility value shows it. + if !d.dataset_target(id) { skipped += 1; continue; } @@ -946,7 +923,7 @@ pub fn specs() -> Vec { "Update Data Set", ["Window", "Variables"], None, - "{all?: bool} write what the bound objects hold now into the active dataset, for each variable the dataset holds; a variable whose objects report no value (locked, inside a hidden or locked layer or group, or gone) keeps the dataset's value. `all: true` also adds every other variable the objects report a value for; `left` names the variables it didn't write → {values, left}", + "{all?: bool} write what the bound objects hold now into the active dataset, for each variable the dataset holds; a variable whose objects report no value (locked, inside a locked layer or group, or gone) keeps the dataset's value. `all: true` also adds every other variable the objects report a value for; `left` names the variables it didn't write → {values, left}", has_doc, dataset_update ), diff --git a/crates/engine/src/tests_variables.rs b/crates/engine/src/tests_variables.rs index b6824f497..62bfa9e40 100644 --- a/crates/engine/src/tests_variables.rs +++ b/crates/engine/src/tests_variables.rs @@ -688,6 +688,70 @@ fn a_hidden_object_comes_back_and_is_captured_hidden() { s.execute("variable.bind", &json!({"variable": "More", "ids": [r.0]})).unwrap(); } +/// A row selected while a layer is hidden rewrites the type on it and applies its other values, +/// and leaves the layer hidden: showing the layer shows that row's text. A layer that is locked +/// as well as hidden is still left alone. +#[test] +fn a_row_rewrites_type_on_a_hidden_layer_but_not_on_a_locked_one() { + let mut s = session(); + let layer = NodeId(s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap()); + let t = text(&mut s, "Alice"); + let r = rect(&mut s); + s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap(); + s.execute("variable.define", &json!({"name": "Show", "kind": "visibility"})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap(); + s.execute("variable.bind", &json!({"variable": "Show", "ids": [r.0]})).unwrap(); + s.execute("dataset.new", &json!({"name": "A", "values": {"Name": "Alice", "Show": true}})).unwrap(); + s.execute("dataset.new", &json!({"name": "B", "values": {"Name": "Bob", "Show": false}})).unwrap(); + s.execute("dataset.select", &json!({"name": "A"})).unwrap(); + + s.execute("layer.setProps", &json!({"id": layer.0, "visible": false})).unwrap(); + let v = s.execute("dataset.select", &json!({"name": "B"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(2), Some(0)), "{v}"); + let doc = &s.doc().unwrap().doc; + assert_eq!(plain(&s, t), "Bob", "the type on the hidden layer holds the row's text"); + assert!(!doc.node(r).unwrap().visible, "the row's visibility value applies on the hidden layer"); + assert!(!doc.node(layer).unwrap().visible, "the row leaves the layer hidden"); + 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!("B")], "only the row the hidden art holds matches: {sets}"); + s.execute("layer.setProps", &json!({"id": layer.0, "visible": true})).unwrap(); + assert_eq!(plain(&s, t), "Bob", "showing the layer shows the row's text"); + + // Locked and hidden: the row skips both objects and the type keeps its text. + s.execute("layer.setProps", &json!({"id": layer.0, "visible": false, "locked": true})).unwrap(); + let v = s.execute("dataset.select", &json!({"name": "A"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(0), Some(2)), "{v}"); + assert_eq!(plain(&s, t), "Bob", "the type on the locked layer keeps its text"); + assert!(!s.doc().unwrap().doc.node(r).unwrap().visible); +} + +/// Type inside a hidden group is rewritten too, and List and Capture Data Set read it. +#[test] +fn a_row_rewrites_type_in_a_hidden_group_and_capture_reads_it() { + 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("dataset.new", &json!({"name": "A", "values": {"Name": "Alice"}})).unwrap(); + s.execute("dataset.new", &json!({"name": "B", "values": {"Name": "Bob"}})).unwrap(); + s.execute("object.setProps", &json!({"ids": [g.0], "visible": false})).unwrap(); + + let v = s.execute("dataset.select", &json!({"name": "B"})).unwrap(); + assert_eq!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(1), Some(0)), "{v}"); + assert_eq!(plain(&s, t), "Bob"); + assert!(!s.doc().unwrap().doc.node(g).unwrap().visible, "the group stays hidden"); + + let sets = s.execute("dataset.list", &json!({})).unwrap(); + let matches: Vec<&Value> = sets["datasets"].as_array().unwrap().iter().map(|d| &d["matches"]).collect(); + assert_eq!(matches, [&json!(false), &json!(true)], "the art holds B's text, not A's: {sets}"); + let v = s.execute("dataset.capture", &json!({"name": "C"})).unwrap(); + let sets = s.execute("dataset.list", &json!({})).unwrap(); + assert_eq!(sets["datasets"][2]["values"], json!({"Name": {"text": "Bob"}}), "{v}: {sets}"); +} + /// A row that hides selected objects deselects them, the key object included. Undo shows them /// and selects them again with the key object. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 08f9031df..eef40077d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1150,11 +1150,11 @@ already walks Image Trace. Short agent entry points: `dataset.select`, `dataset.next` and `dataset.prev` reply `{name, applied, skipped}`. They add `overflow: [id…]` when area type or type on a path whose text they rewrote doesn't fit, as `text.setText` does (see [Fitting area type](#fitting-area-type-auto-size-shrink-text-to-fit-overflow)). 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. +locked or inside a locked layer or group, and count it in `skipped`. They rewrite hidden objects and objects inside +a hidden layer or group, which stay hidden unless one of the data set's visibility values shows them. When such a +layer is shown again, its type holds the data set's text. They apply the values in the order the variables are +defined. Of two variables of the same kind on one object, the one defined last wins. `dataset.list`, +`dataset.capture` and `dataset.update` read hidden objects and objects inside a hidden layer or group as well. ```json {"name":"run_command","arguments":{"command":"symbol.list","params":{}}} @@ -1168,8 +1168,8 @@ variables of the same kind on one object, the one defined last wins. ``` `dataset.update {all?}` writes what the bound objects hold now into the active data set, for each variable that the -data set holds. A variable whose objects report no value, because they are locked, inside a hidden or locked layer -or group, or deleted, keeps the data set's value. With `all: true` it also adds every other variable that the +data set holds. A variable whose objects report no value, because they are locked, inside a locked layer or group, +or deleted, keeps the data set's value. With `all: true` it also adds every other variable that the objects report a value for. The reply gives `values`, the number of values it wrote, and `left`, the variables it did not write. diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index f756ba8f6..fb8721c15 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -422,7 +422,8 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Architecture:** 19+ crates with enforced layering (`cargo xtask layers`). Every action is a command (~400 engine + ~50 UI). Undo is unlimited via structural sharing. `command.batch` runs several commands as one transaction. - **Automation:** - - Variables: Update Data Set writes what the art holds into the active data set for the variables that data set holds, and keeps the data set's value for a variable whose objects are locked or inside a hidden or locked layer or group; `dataset.update {all: true}` also adds the other variables that the art reports a value for. + - Variables: Update Data Set writes what the art holds into the active data set for the variables that data set holds, and keeps the data set's value for a variable whose objects are locked or inside a locked layer or group; `dataset.update {all: true}` also adds the other variables that the art reports a value for. + - Variables: `dataset.select`, `dataset.next` and `dataset.prev` rewrite type and apply the other values on hidden objects and in hidden layers and groups, which stay hidden unless a visibility value shows them, so a layer shown later holds the selected data set's text; objects that are locked or in a locked layer or group are skipped. - Variables: `dataset.select`, `dataset.next` and `dataset.prev` name the area type and type on a path whose text they rewrote and that doesn't fit, in the reply's `overflow` key, as `text.setText` does. - Actions panel (record/playback, persisted), generic parameter dialogs for every "…" command. - Batch… over data sets names each file after the document and the data set (`action.batch`). It writes `/`, `\` and `:` in the data set's name as `-`, and adds `-2`, `-3`… to a name an earlier data set used.