Variables: a visibility variable shows and hides locked art

variable.bind refused a locked layer, a locked object and objects
inside a locked layer or group for every kind of variable, and
dataset.select skipped them. A visibility variable bound before the
art was locked stopped working, and a locked layer could not be shown
or hidden by a data set.

A visibility variable now binds locked art and objects inside a locked
layer or group. dataset.select, dataset.next and dataset.prev show and
hide them and leave them locked. dataset.list, dataset.capture and
dataset.update read whether they show. Text, image and graph variables
are refused for locked art as before, and a row skips them. Binding
any variable still refuses art inside a hidden layer or group, and for
a visibility variable the error says the art is in a hidden group or
layer.

Docs: docs/mcp.md, docs/target-app-parity.md.

crates/doc/src/variables.rs

@@ -90,8 +90,9 @@ pub struct DataSet {
 
 impl DataSet {
     /// Whether the art, as [`Document::captured_values`] read it, still holds what this row
-    /// says. A value the art can't report (its object is locked or gone) isn't drift: the row
-    /// keeps it, and applying it again would skip it too.
+    /// says. A value the art can't report (its object is gone, or is locked or in a locked layer
+    /// or group and the variable isn't a visibility variable) isn't drift: the row keeps it, and
+    /// applying it again would skip it too.
     pub fn matches(&self, current: &BTreeMap<String, DataValue>) -> bool {
         self.values.iter().all(|(k, v)| current.get(k).is_none_or(|c| c == v))
     }
@@ -188,26 +189,29 @@ impl Document {
         self.variables.bindings = bindings;
     }
 
-    /// 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 variable of `kind` may be bound to the object `id`: it is in the document,
+    /// nothing above it is hidden, and, unless `kind` is visibility, neither it nor anything above
+    /// it is locked. Its own visibility doesn't count, as that is what a visibility variable
+    /// drives.
+    pub fn variable_target(&self, id: NodeId, kind: VariableKind) -> bool {
+        let locks = kind != VariableKind::Visibility;
+        self.ancestry(id).is_some_and(|a| a.iter().all(|i| self.node(*i).is_some_and(|n| !(locks && 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)))
+    /// Whether a data set reads and rewrites the bound object `id` for a variable of `kind`: it
+    /// is in the document and, unless `kind` is visibility, 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, kind: VariableKind) -> bool {
+        let locks = kind != VariableKind::Visibility;
+        self.ancestry(id).is_some_and(|a| a.iter().all(|i| self.node(*i).is_some_and(|n| !(locks && 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 in a locked layer or group (the
-    /// document's value is whatever it was left at).
+    /// link path yet (an embedded image), or, unless `kind` is visibility, 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<DataValue> {
-        if !self.dataset_target(id) {
+        if !self.dataset_target(id, kind) {
             return None;
         }
         let node = self.node(id)?;

crates/engine/src/cmd/variablecmds.rs

@@ -74,13 +74,14 @@ fn names_param(p: &Value, cmd: &str, fallback: Option<String>, nothing: &str) ->
     }
 }
 
-/// 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.
+/// Refuse what a variable of `kind` can't be bound to: art on a hidden layer or in a hidden group,
+/// and, unless `kind` is visibility, art that is locked or on a locked layer or in a locked group;
+/// 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) {
             None => return Err(EngineError::NoNode(*id)),
-            Some(_) if !doc.variable_target(*id) => return Err(bad(cmd, not_a_target(doc, *id))),
+            Some(_) if !doc.variable_target(*id, kind) => return Err(bad(cmd, not_a_target(doc, *id, kind))),
             Some(n) if kind == VariableKind::Text && !matches!(n.kind, NodeKind::Text(_)) => {
                 return Err(bad(cmd, format!("node {} is not type", id.0)));
             }
@@ -96,14 +97,17 @@ fn check_bindable(doc: &vectorcraft_doc::Document, ids: &[NodeId], kind: Variabl
     Ok(())
 }
 
-/// Why `variable_target` refuses `id`: the object is locked, a group or layer that holds it is
-/// locked, or one that holds it is hidden, checked in that order.
-fn not_a_target(doc: &vectorcraft_doc::Document, id: NodeId) -> String {
+/// Why `variable_target` refuses `id` for a variable of `kind`: the object is locked, a group or
+/// layer that holds it is locked, or one that holds it is hidden, checked in that order. For a
+/// visibility variable, the lock checks are skipped and the message says the object is in a
+/// hidden group or layer.
+fn not_a_target(doc: &vectorcraft_doc::Document, id: NodeId, kind: VariableKind) -> String {
     let holders: Vec<&vectorcraft_doc::Node> =
         doc.ancestry(id).unwrap_or_default().into_iter().filter(|i| *i != id).filter_map(|i| doc.node(i)).collect();
-    if doc.node(id).is_some_and(|n| n.locked) {
+    let locks = kind != VariableKind::Visibility;
+    if locks && doc.node(id).is_some_and(|n| n.locked) {
         format!("node {} is locked", id.0)
-    } else if holders.iter().any(|n| n.locked) {
+    } else if locks && holders.iter().any(|n| n.locked) {
         format!("node {} is in a locked group or layer", id.0)
     } else {
         format!("node {} is in a hidden group or layer", id.0)
@@ -395,8 +399,9 @@ fn dataset_capture(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 /// 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 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 (gone, or, for a variable
+/// other than a visibility variable, locked or inside a locked layer or group) 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<Value> {
@@ -480,10 +485,11 @@ fn apply_dataset(s: &mut Session, name: &str) -> Result<Value> {
                     continue;
                 }
             };
-            // 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) {
+            // Art that is gone, and art that is locked or in a locked layer or group, is skipped
+            // and counted, like the rest, except that a visibility value shows and hides locked
+            // art. 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, kind) {
                 skipped += 1;
                 continue;
             }
@@ -903,7 +909,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Bind Variable",
             ["Window", "Variables"],
             None,
-            "{variable, ids?} bind objects (default: the selection) to a variable; text needs type, image needs an image object, graph needs a graph root, locked art is refused. An object may hold several variables, and binding it to one it already has changes nothing → {bound, already}",
+            "{variable, ids?} bind objects (default: the selection) to a variable; text needs type, image needs an image object, graph needs a graph root, locked art and art in a locked layer or group are refused except by a visibility variable. An object may hold several variables, and binding it to one it already has changes nothing → {bound, already}",
             has_doc,
             bind
         ),
@@ -959,7 +965,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "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 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 (gone, or, for a variable other than a visibility variable, locked or inside a locked layer or group) 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
         ),

crates/engine/src/tests_variables.rs

@@ -271,7 +271,7 @@ fn unbind_reports_what_it_actually_removed() {
     assert!(v["variables"].get("bindings").is_none_or(|v| v == &json!({})), "{v}");
 }
 
-/// Locked art is the document's to rewrite only while it isn't locked.
+/// Locked type takes no new text variable binding, and a row leaves its text alone.
 #[test]
 fn locked_art_is_neither_bound_nor_rewritten() {
     let mut s = session();
@@ -327,6 +327,90 @@ fn binding_errors_say_whether_a_layer_or_group_is_hidden_or_locked() {
     assert!(e.contains(&format!("node {} is locked", t.0)), "{e}");
 }
 
+/// A visibility variable binds a locked layer and a locked object inside a locked group, and a row
+/// hides and shows them in one undo step without unlocking them. List, Capture and Update Data Set
+/// read whether they show.
+#[test]
+fn a_visibility_variable_shows_and_hides_locked_art() {
+    let mut s = session();
+    let badge = rect(&mut s);
+    let other = rect(&mut s);
+    let g = group(&mut s, &[badge, other]);
+    let layer = NodeId(s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap());
+    rect(&mut s);
+    s.execute("object.setProps", &json!({"ids": [badge.0], "locked": true})).unwrap();
+    s.execute("object.setProps", &json!({"ids": [g.0], "locked": true})).unwrap();
+    s.execute("layer.setProps", &json!({"id": layer.0, "locked": true})).unwrap();
+    s.execute("variable.define", &json!({"name": "Layer", "kind": "visibility"})).unwrap();
+    s.execute("variable.define", &json!({"name": "Badge", "kind": "visibility"})).unwrap();
+    s.execute("variable.bind", &json!({"variable": "Layer", "ids": [layer.0]})).unwrap();
+    s.execute("variable.bind", &json!({"variable": "Badge", "ids": [badge.0]})).unwrap();
+    s.execute("dataset.new", &json!({"name": "Shown", "values": {"Layer": true, "Badge": true}})).unwrap();
+    s.execute("dataset.new", &json!({"name": "Hidden", "values": {"Layer": false, "Badge": false}})).unwrap();
+    let shows = |s: &Session, id: NodeId| s.doc().unwrap().doc.node(id).unwrap().visible;
+    let locked = |s: &Session| [layer, g, badge].iter().all(|id| s.doc().unwrap().doc.node(*id).unwrap().locked);
+
+    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!(!shows(&s, layer) && !shows(&s, badge), "the row hides the locked layer and the locked object");
+    assert!(locked(&s), "the row leaves the locks as they were");
+    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!("Hidden")], "List reads the locked art: {sets}");
+    assert_eq!(s.execute("dataset.capture", &json!({"name": "Captured"})).unwrap()["values"], 2);
+    let sets = s.execute("dataset.list", &json!({})).unwrap();
+    assert_eq!(sets["datasets"][2]["values"], json!({"Badge": {"visible": false}, "Layer": {"visible": false}}), "{sets}");
+
+    // Undo puts back what the row hid.
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(shows(&s, layer) && shows(&s, badge), "undo shows them again");
+
+    let v = s.execute("dataset.select", &json!({"name": "Hidden"})).unwrap();
+    assert_eq!(v["applied"], 2, "{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}");
+    assert!(shows(&s, layer) && shows(&s, badge), "the row shows them");
+    assert!(locked(&s));
+
+    // Update writes the locked layer's visibility into the active row.
+    s.execute("layer.setProps", &json!({"id": layer.0, "visible": false})).unwrap();
+    let v = s.execute("dataset.update", &json!({})).unwrap();
+    assert_eq!((v["values"].clone(), v["left"].clone()), (json!(2), json!([])), "{v}");
+    let sets = s.execute("dataset.list", &json!({})).unwrap();
+    assert_eq!(sets["datasets"][0]["values"], json!({"Badge": {"visible": true}, "Layer": {"visible": false}}), "{sets}");
+}
+
+/// Text and graph variables refuse locked art and art in a locked layer. A visibility variable is
+/// refused for art on a hidden layer that is also locked, and the error says the art is in a hidden
+/// group or layer.
+#[test]
+fn locked_art_refuses_text_and_graph_variables_and_a_hidden_layer_refuses_visibility() {
+    let mut s = session();
+    let layer = NodeId(s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap());
+    let t = text(&mut s, "x");
+    let r = rect(&mut s);
+    s.execute("object.setProps", &json!({"ids": [r.0], "locked": true})).unwrap();
+    s.execute("layer.setProps", &json!({"id": layer.0, "locked": true})).unwrap();
+    s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap();
+    s.execute("variable.define", &json!({"name": "Chart", "kind": "graph"})).unwrap();
+    s.execute("variable.define", &json!({"name": "Show", "kind": "visibility"})).unwrap();
+    let bind = |s: &mut Session, var: &str, id: NodeId| s.execute("variable.bind", &json!({"variable": var, "ids": [id.0]}));
+
+    let e = bind(&mut s, "Name", t).unwrap_err().to_string();
+    assert!(e.contains(&format!("node {} is in a locked group or layer", t.0)), "{e}");
+    let e = bind(&mut s, "Chart", r).unwrap_err().to_string();
+    assert!(e.contains(&format!("node {} is locked", r.0)), "{e}");
+    bind(&mut s, "Show", t).unwrap();
+    assert!(s.doc().unwrap().doc.variables.objects_of("Name").is_empty());
+
+    s.execute("layer.setProps", &json!({"id": layer.0, "visible": false})).unwrap();
+    let e = bind(&mut s, "Show", r).unwrap_err().to_string();
+    assert!(e.contains(&format!("node {} is in a hidden group or layer", r.0)), "{e}");
+    // The hidden layer's own visibility is what the variable drives.
+    bind(&mut s, "Show", layer).unwrap();
+}
+
 /// Make Text Dynamic / Make Visibility Dynamic: what the panel's buttons run. One step from a
 /// selection to a bound variable, named after what it drives.
 #[test]
@@ -531,7 +615,7 @@ fn updating_writes_only_the_variables_the_row_holds() {
     assert_eq!(sets["datasets"][0]["values"]["Client"], json!({"text": "Typed by hand"}), "{sets}");
 }
 
-/// A variable whose object is locked reports no value, so Update keeps the row's value for it
+/// A text variable whose object is locked reports no value, so Update keeps the row's value for it
 /// rather than dropping it from the row.
 #[test]
 fn updating_keeps_a_value_the_art_does_not_report() {
@@ -762,8 +846,8 @@ fn a_hidden_object_comes_back_and_is_captured_hidden() {
 }
 
 /// 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.
+/// and leaves the layer hidden: showing the layer shows that row's text. On a layer that is locked
+/// as well as hidden, the type keeps its text and the row's visibility value applies.
 #[test]
 fn a_row_rewrites_type_on_a_hidden_layer_but_not_on_a_locked_one() {
     let mut s = session();
@@ -791,12 +875,12 @@ fn a_row_rewrites_type_on_a_hidden_layer_but_not_on_a_locked_one() {
     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.
+    // Locked and hidden: the row skips the type, which keeps its text, and shows the rectangle.
     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!((v["applied"].as_u64(), v["skipped"].as_u64()), (Some(1), Some(1)), "{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);
+    assert!(s.doc().unwrap().doc.node(r).unwrap().visible, "the row's visibility value applies on the locked layer");
 }
 
 /// Type inside a hidden group is rewritten too, and List and Capture Data Set read it.