Merge pull request #862 from MortenHusted/fix/instance-main-modes

M3: instances follow the variable modes their main component sets

crates/doc/src/lib.rs

@@ -949,21 +949,39 @@ impl Document {
         self.styles.iter().find(|s| s.id == id || s.name == id)
     }
 
-    /// The effective variable modes for a node (walking up its ancestors).
+    /// The effective variable modes for a node: the ones it sets over [`Self::inherited_modes`].
     pub fn modes_for(&self, id: NodeId) -> BTreeMap<CollectionId, ModeId> {
-        let mut out = BTreeMap::new();
-        let mut chain = vec![id];
-        chain.extend(self.ancestors(id));
-        for c in chain.into_iter().rev() {
-            if let Some(n) = self.node(c) {
-                for (k, v) in &n.var_modes {
-                    out.insert(k.clone(), v.clone());
-                }
-            }
+        let mut out = self.inherited_modes(id);
+        if let Some(n) = self.node(id) {
+            out.extend(n.var_modes.iter().map(|(k, v)| (k.clone(), v.clone())));
         }
         out
     }
 
+    /// The modes a node is on where it sets none (Auto): its ancestors', and for an instance the
+    /// ones its main component sets, which are closer than any container's.
+    pub fn inherited_modes(&self, id: NodeId) -> BTreeMap<CollectionId, ModeId> {
+        let mut out = BTreeMap::new();
+        for c in self.ancestors(id).into_iter().rev() {
+            if let Some(n) = self.node(c) {
+                out.extend(self.main_modes(n).chain(&n.var_modes).map(|(k, v)| (k.clone(), v.clone())));
+            }
+        }
+        if let Some(n) = self.node(id) {
+            out.extend(self.main_modes(n).map(|(k, v)| (k.clone(), v.clone())));
+        }
+        out
+    }
+
+    /// The modes an instance's main component sets (none for other layers).
+    fn main_modes<'a>(&'a self, n: &Node) -> impl Iterator<Item = (&'a CollectionId, &'a ModeId)> {
+        let main = match n.kind {
+            NodeKind::Instance { component, .. } => self.node(component),
+            _ => None,
+        };
+        main.into_iter().flat_map(|m| &m.var_modes)
+    }
+
     /// Check structural invariants (used by tests and after loading untrusted files).
     pub fn validate(&self) -> Vec<String> {
         let mut issues = Vec::new();

crates/engine/src/cmd/component.rs

@@ -472,6 +472,7 @@ fn detach(s: &mut Session, p: &Value) -> Result<Value> {
         // Own layers lose their link (slot content stays as it is); nested instances stay
         // instances of their own mains.
         let nested = unlink_layers(doc, i, Some(i));
+        keep_main_modes(doc, i)?;
         let n = doc.get_mut(i)?;
         n.kind = NodeKind::Frame;
         n.source = None;
@@ -491,6 +492,19 @@ fn detach(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({"ids": out}))
 }
 
+/// Make the variable modes instance `i` takes from its main its own (it's being detached).
+fn keep_main_modes(doc: &mut Document, i: NodeId) -> Result<()> {
+    let modes = match doc.get(i)?.kind {
+        NodeKind::Instance { component, .. } => doc.node(component).map(|m| m.var_modes.clone()).unwrap_or_default(),
+        _ => return Ok(()),
+    };
+    let own = &mut doc.get_mut(i)?.var_modes;
+    for (c, m) in modes {
+        own.entry(c).or_insert(m);
+    }
+    Ok(())
+}
+
 fn reset_instance(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "component.resetInstance";
     let ids = req_targets(cmd, s, p)?;

crates/engine/src/components.rs

@@ -285,6 +285,8 @@ pub fn create_instance_at(doc: &mut Document, component: NodeId, parent: NodeId,
     root.dev_resources.clear();
     // The main's interactions play on its instances in prototypes; the instance's own start empty.
     root.interactions.clear();
+    // So do the variable modes the main sets (see `Document::inherited_modes`).
+    root.var_modes.clear();
     doc.insert(root, parent, index)?;
     sync_instance(doc, id);
     Ok(id)

crates/engine/src/instance_modes_tests.rs

@@ -0,0 +1,128 @@
+//! Variable modes on instances: an instance is on the modes its main component sets, under the
+//! ones set on the instance itself, so switching variants or changing the main's mode reaches it.
+
+use serde_json::{Value, json};
+
+use crate::Session;
+use crate::params::id_of;
+use figcraft_doc::NodeId;
+
+fn run(s: &mut Session, cmd: &str, p: Value) -> Value {
+    s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd} {p}: {e}"))
+}
+
+fn id(v: Value) -> NodeId {
+    id_of(&v["id"]).unwrap()
+}
+
+fn fill(s: &Session, n: NodeId) -> String {
+    s.doc().unwrap().node(n).unwrap().fills.first().and_then(|p| p.solid_color()).map(|c| c.to_hex()).unwrap_or_default()
+}
+
+fn set_mode(s: &mut Session, n: NodeId, mode: Value) {
+    run(s, "variable.setMode", json!({"ids": [n], "collection": "Tone", "mode": mode}));
+}
+
+/// A "Tone" collection whose variable `c` is red in mode A and blue in mode B.
+fn tone(s: &mut Session) {
+    run(s, "variable.createCollection", json!({"name": "Tone", "modes": ["A", "B"]}));
+    run(s, "variable.create", json!({"collection": "Tone", "name": "c", "type": "color", "values": {"A": "#FF0000", "B": "#0000FF"}}));
+}
+
+/// A component filled from `c`, on `mode` when one is given.
+fn swatch(s: &mut Session, name: &str, mode: Option<&str>) -> NodeId {
+    let f = id(run(s, "frame.create", json!({"x": 0, "y": 0, "width": 40, "height": 40, "name": name})));
+    run(s, "variable.bind", json!({"ids": [f], "field": "fill", "variable": "c"}));
+    if let Some(m) = mode {
+        set_mode(s, f, json!(m));
+    }
+    id(run(s, "component.create", json!({"ids": [f]})))
+}
+
+/// A set "Kind" whose variants A and B each set their own Tone mode.
+fn kinds(s: &mut Session) -> (NodeId, NodeId) {
+    tone(s);
+    let a = swatch(s, "Kind=A", Some("A"));
+    let b = swatch(s, "Kind=B", Some("B"));
+    let set = id(run(s, "component.combineVariants", json!({"ids": [a, b]})));
+    (set, b)
+}
+
+fn instance(s: &mut Session, component: NodeId) -> NodeId {
+    id(run(s, "instance.create", json!({"component": component, "x": 200, "y": 0})))
+}
+
+fn switch(s: &mut Session, i: NodeId, kind: &str) {
+    run(s, "instance.setProperties", json!({"ids": [i], "properties": {"Kind": kind}}));
+}
+
+#[test]
+fn switching_variants_takes_the_new_variants_mode() {
+    let mut s = Session::new();
+    let (set, _) = kinds(&mut s);
+    let i = instance(&mut s, set);
+    assert_eq!(fill(&s, i), "FF0000");
+    switch(&mut s, i, "B");
+    assert_eq!(fill(&s, i), "0000FF", "variant B sets mode B");
+    switch(&mut s, i, "A");
+    assert_eq!(fill(&s, i), "FF0000");
+    assert!(s.doc().unwrap().node(i).unwrap().var_modes.is_empty(), "the instance sets no mode of its own");
+}
+
+#[test]
+fn instances_follow_a_mode_their_main_sets_later() {
+    let mut s = Session::new();
+    tone(&mut s);
+    let main = swatch(&mut s, "Chip", None);
+    let i = instance(&mut s, main);
+    assert_eq!(fill(&s, i), "FF0000", "no mode set: the collection's default");
+    set_mode(&mut s, main, json!("B"));
+    assert_eq!(fill(&s, i), "0000FF");
+    set_mode(&mut s, main, Value::Null);
+    assert_eq!(fill(&s, i), "FF0000", "the main back on Auto");
+}
+
+#[test]
+fn a_mode_set_on_the_instance_wins_and_stays_through_variant_switches() {
+    let mut s = Session::new();
+    let (set, _) = kinds(&mut s);
+    let i = instance(&mut s, set);
+    set_mode(&mut s, i, json!("B"));
+    assert_eq!(fill(&s, i), "0000FF");
+    for kind in ["B", "A"] {
+        switch(&mut s, i, kind);
+        assert_eq!(fill(&s, i), "0000FF", "own mode B on variant {kind}");
+    }
+    set_mode(&mut s, i, Value::Null);
+    assert_eq!(fill(&s, i), "FF0000", "back on Auto: variant A's mode");
+}
+
+#[test]
+fn a_main_mode_is_closer_than_the_frame_around_the_instance() {
+    let mut s = Session::new();
+    let (_, b) = kinds(&mut s);
+    let frame = id(run(&mut s, "frame.create", json!({"x": 0, "y": 300, "width": 200, "height": 100})));
+    set_mode(&mut s, frame, json!("A"));
+    let i = id(run(&mut s, "instance.create", json!({"component": b, "x": 10, "y": 10, "parent": frame})));
+    assert_eq!(fill(&s, i), "0000FF", "the main's mode B, not the frame's A");
+    // Nested: an outer component on mode A holding an instance of B.
+    let outer_frame = id(run(&mut s, "frame.create", json!({"x": 0, "y": 500, "width": 100, "height": 100, "name": "Outer"})));
+    run(&mut s, "instance.create", json!({"component": b, "x": 0, "y": 0, "parent": outer_frame}));
+    set_mode(&mut s, outer_frame, json!("A"));
+    let outer = id(run(&mut s, "component.create", json!({"ids": [outer_frame]})));
+    let copy = instance(&mut s, outer);
+    let nested = s.doc().unwrap().node(copy).unwrap().children[0];
+    assert_eq!(fill(&s, nested), "0000FF");
+}
+
+#[test]
+fn detaching_keeps_the_mode_it_took_from_its_main() {
+    let mut s = Session::new();
+    let (set, b) = kinds(&mut s);
+    let i = instance(&mut s, set);
+    switch(&mut s, i, "B");
+    run(&mut s, "component.detach", json!({"ids": [i]}));
+    set_mode(&mut s, b, json!("A"));
+    assert_eq!(fill(&s, i), "0000FF", "the detached frame keeps mode B");
+    assert_eq!(s.doc().unwrap().node(i).unwrap().var_modes.len(), 1);
+}

crates/engine/src/lib.rs

@@ -1490,21 +1490,35 @@ fn affects_layout(before: &Document, doc: &Document, id: NodeId) -> bool {
 }
 
 /// The layers whose variable bindings an edit may have changed: every bound layer when a variable
-/// changed, else the touched layers and everything below those that moved, are new or chose
-/// another variable mode (modes are inherited).
+/// changed, else the touched layers and everything below those that moved, are new, chose
+/// another variable mode or became an instance of another main (modes are inherited, an
+/// instance's from its main too), and the instances of a main that chose another mode.
 fn binding_scope(doc: &Document, before: &Document, touched: &BTreeSet<NodeId>) -> Option<BTreeSet<NodeId>> {
     if doc.variables != before.variables {
         return None;
     }
+    let main = |n: &Node| match n.kind {
+        figcraft_doc::NodeKind::Instance { component, .. } => Some(component),
+        _ => None,
+    };
     let mut out = BTreeSet::new();
     for &id in touched {
         let Some(n) = doc.node(id) else { continue };
-        let inherited = before.node(id).is_none_or(|b| b.var_modes != n.var_modes || b.parent != n.parent);
-        if inherited {
+        let Some(b) = before.node(id) else {
+            out.extend(doc.subtree(id));
+            continue;
+        };
+        let modes = b.var_modes != n.var_modes;
+        if modes || b.parent != n.parent || main(b) != main(n) {
             out.extend(doc.subtree(id));
         } else {
             out.insert(id);
         }
+        if modes && matches!(n.kind, figcraft_doc::NodeKind::Component { .. }) {
+            for i in components::instances_of(doc, id) {
+                out.extend(doc.subtree(i));
+            }
+        }
     }
     Some(out)
 }
@@ -1682,6 +1696,8 @@ mod import_journal_tests;
 #[cfg(test)]
 mod inspect_behaviour_tests;
 #[cfg(test)]
+mod instance_modes_tests;
+#[cfg(test)]
 mod instance_resizing_tests;
 #[cfg(test)]
 mod interaction_rollback_tests;

crates/ui-egui/src/right.rs

@@ -961,8 +961,8 @@ fn variable_modes(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId])
     // The collection's name, then a wider mode picker ("Auto (Light)") across both field columns.
     let col = widgets::col_width(ui.available_width());
     let (name_w, pick_w) = ((col * 0.7).round(), (col * 1.3).round());
-    // Auto resolves from the parent of the first selected layer.
-    let parent_modes = sel.first().and_then(|i| doc.node(*i)).and_then(|n| n.parent).map(|p| doc.modes_for(p)).unwrap_or_default();
+    // Auto resolves as for the first selected layer: from its parent, or an instance's main.
+    let parent_modes = sel.first().map(|i| doc.inherited_modes(*i)).unwrap_or_default();
     let mut group = None;
     for r in &rows {
         let c = r.coll;