From be9e72f74d34fdcaaf256d078ee6dcad3691bf40 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Mon, 19 Oct 2026 09:15:34 +0200 Subject: [PATCH] M3: instances follow the variable modes their main component sets An instance copied its main's variable modes when it was made and kept the copy as modes of its own. Switching it to another variant kept the old variant's modes, and a mode set on the main later never reached it. Instances now take the modes their main sets through the link (`Document::inherited_modes`), closer than any frame around them. Modes set on the instance itself still win and stay through variant switches. Bindings are re-applied for the instances of a main that chose another mode and for an instance that switched mains. Detaching keeps the modes the instance took from its main, and the Variable modes panel shows them as the instance's Auto value. Instances made before this change keep their copy as modes of their own; setting one back to Auto lets it follow its main. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/lib.rs | 38 +++++-- crates/engine/src/cmd/component.rs | 14 +++ crates/engine/src/components.rs | 2 + crates/engine/src/instance_modes_tests.rs | 128 ++++++++++++++++++++++ crates/engine/src/lib.rs | 24 +++- crates/ui-egui/src/right.rs | 4 +- 6 files changed, 194 insertions(+), 16 deletions(-) create mode 100644 crates/engine/src/instance_modes_tests.rs diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 07700e8c..d131b2f0 100644 --- a/crates/doc/src/lib.rs +++ b/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 { - 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 { + 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 { + 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 { let mut issues = Vec::new(); diff --git a/crates/engine/src/cmd/component.rs b/crates/engine/src/cmd/component.rs index ff7a56df..6840bcf4 100644 --- a/crates/engine/src/cmd/component.rs +++ b/crates/engine/src/cmd/component.rs @@ -472,6 +472,7 @@ fn detach(s: &mut Session, p: &Value) -> Result { // 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 { 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 { let cmd = "component.resetInstance"; let ids = req_targets(cmd, s, p)?; diff --git a/crates/engine/src/components.rs b/crates/engine/src/components.rs index 56efc5c9..ea67bb07 100644 --- a/crates/engine/src/components.rs +++ b/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) diff --git a/crates/engine/src/instance_modes_tests.rs b/crates/engine/src/instance_modes_tests.rs new file mode 100644 index 00000000..79007622 --- /dev/null +++ b/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); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 312c2e87..3145c8a9 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1460,21 +1460,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) -> Option> { 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) } @@ -1652,6 +1666,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; diff --git a/crates/ui-egui/src/right.rs b/crates/ui-egui/src/right.rs index 53eaf953..5fd7dfc4 100644 --- a/crates/ui-egui/src/right.rs +++ b/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;