Merge remote-tracking branch 'origin/main' into fix/color-picker-dismissal-and-placement

# Conflicts:
#	ROADMAP.md
This commit is contained in:
Carbocode
2026-10-29 15:13:25 +01:00
15 changed files with 601 additions and 72 deletions
+4
View File
@@ -39,6 +39,7 @@ use figcraft_doc::{
use figcraft_geom::Affine;
use serde_json::Value;
mod cycles;
mod diff;
mod guard;
mod instances;
@@ -47,6 +48,9 @@ mod rebase;
mod sync;
mod variants;
pub use cycles::{
break_component_cycles, check_component_cycles, component_cycle, contains_itself_error, cycle_text, new_cycle, would_contain_itself,
};
pub use diff::{diff_groups, property_owners, set_boolean_default, variable_groups};
pub use guard::{check_override_edit, check_structure_edit, refused_override, unlink_layers};
pub(crate) use instances::copy_group;
@@ -0,0 +1,214 @@
//! A component can't contain its own instances, directly or through other components (#1084):
//! every instance would show itself without end. Each variant is a component of its own here: one
//! holding an instance of another variant of its set is fine unless that one leads back to it.
//! Commands that would close such a loop are refused ([`check_component_cycles`]);
//! files that hold one are broken on opening ([`break_component_cycles`]); and a rebuild never
//! expands an instance inside a copy of itself ([`expansion_layers`]), for loops that arrive some
//! other way (two edits merged in a live session).
use super::patch::rebuild_instances;
use super::{BTreeMap, BTreeSet, Document, NodeId, NodeKind, PROP_BINDING, PropDef, inside_instance, root_instance};
/// The components each main component holds instances of.
type Graph = BTreeMap<NodeId, BTreeSet<NodeId>>;
fn is_component(doc: &Document, id: NodeId) -> bool {
doc.node(id).is_some_and(|n| matches!(n.kind, NodeKind::Component { .. }))
}
/// The main components `id` is (in).
fn hosts(doc: &Document, id: NodeId) -> BTreeSet<NodeId> {
std::iter::once(id).chain(doc.ancestors(id)).filter(|a| is_component(doc, *a)).collect()
}
/// The components instance `id` shows: its main, and those its instance swap properties pick
/// (they swap a layer inside it when it's rebuilt).
fn shown(doc: &Document, id: NodeId) -> Vec<NodeId> {
let Some(NodeKind::Instance { component, props }) = doc.node(id).map(|n| &n.kind) else { return Vec::new() };
let Some(NodeKind::Component { props: defs, .. }) = doc.node(*component).map(|n| &n.kind) else { return Vec::new() };
let swaps =
defs.iter().filter(|(_, d)| matches!(d, PropDef::InstanceSwap { .. })).filter_map(|(k, _)| props.get(k).and_then(crate::params::id_of));
std::iter::once(*component).chain(swaps).filter(|c| is_component(doc, *c)).collect()
}
fn graph(doc: &Document) -> Graph {
let mut g = Graph::new();
for n in doc.nodes().filter(|n| matches!(n.kind, NodeKind::Instance { .. })) {
let shown = shown(doc, n.id);
for h in hosts(doc, n.id) {
g.entry(h).or_default().extend(shown.iter().copied());
}
}
g
}
/// A loop in `g`, its first component repeated last (`A, B, A`).
fn find_cycle(g: &Graph) -> Option<Vec<NodeId>> {
let next = |u: &NodeId| -> Vec<NodeId> { g.get(u).map(|s| s.iter().rev().copied().collect()).unwrap_or_default() };
let mut done: BTreeSet<NodeId> = BTreeSet::new();
for start in g.keys() {
if done.contains(start) {
continue;
}
// Depth first: the path so far, and the components each step on it has left to visit.
let mut path = vec![*start];
let mut todo = vec![next(start)];
while let Some(left) = todo.last_mut() {
match left.pop() {
Some(u) if path.contains(&u) => {
let at = path.iter().position(|p| *p == u)?;
let mut cycle = path.split_off(at);
cycle.push(u);
return Some(cycle);
}
Some(u) if done.contains(&u) => {}
Some(u) => {
todo.push(next(&u));
path.push(u);
}
None => {
todo.pop();
done.extend(path.pop());
}
}
}
}
None
}
/// A loop of components whose mains contain each other's instances, if the document has one.
pub fn component_cycle(doc: &Document) -> Option<Vec<NodeId>> {
find_cycle(&graph(doc))
}
/// A loop as its names: `Card → Button → Card`.
fn cycle_names(doc: &Document, cycle: &[NodeId]) -> String {
cycle.iter().map(|u| doc.node(*u).map_or_else(|| u.to_string(), |n| format!("\"{}\"", n.name))).collect::<Vec<_>>().join(" → ")
}
/// Whether an instance of `component` under `parent` would contain itself: `parent` is (in) a
/// main that instance shows, directly or through other components.
pub fn would_contain_itself(doc: &Document, component: NodeId, parent: NodeId) -> bool {
let hosts = hosts(doc, parent);
if hosts.is_empty() {
return false;
}
let g = graph(doc);
let mut seen = BTreeSet::new();
let mut todo = vec![component];
while let Some(u) = todo.pop() {
if hosts.contains(&u) {
return true;
}
if seen.insert(u) {
todo.extend(g.get(&u).into_iter().flatten().copied());
}
}
false
}
/// Why an instance of `component` can't go where it would contain itself.
pub fn contains_itself_error(doc: &Document, component: NodeId) -> String {
let name = doc.node(component).map(|n| n.name.clone()).unwrap_or_default();
format!("an instance of \"{name}\" can't go inside \"{name}\" or a component it contains: a component can't contain itself")
}
/// The loop `after` has and `before` didn't, if layers `touched` (edited, added or removed) closed
/// one: only layers in main components can.
pub fn new_cycle<'a>(before: &Document, after: &Document, mut touched: impl Iterator<Item = &'a NodeId>) -> Option<String> {
if !touched.any(|d| !hosts(after, *d).is_empty()) {
return None;
}
cycle_text(after).filter(|_| component_cycle(before).is_none())
}
/// The document's loop of components containing each other's instances ([`component_cycle`]),
/// as its names.
pub fn cycle_text(doc: &Document) -> Option<String> {
component_cycle(doc).map(|c| cycle_names(doc, &c))
}
/// Refuse an edit that makes components contain each other's instances (it is rolled back). A
/// document that already held a loop stays editable.
pub fn check_component_cycles(before: &Document, after: &Document) -> Result<(), String> {
match new_cycle(before, after, after.dirty().iter()) {
Some(names) => Err(format!("a component can't contain an instance of itself, directly or through other components: {names}")),
None => Ok(()),
}
}
/// The instance to detach to cut `from → to` out of a loop: one in a main of `from` showing `to`,
/// preferably one of the main's own (not a layer of another instance, else its outer instance).
fn cut_point(doc: &Document, from: NodeId, to: NodeId) -> Option<NodeId> {
let found: Vec<NodeId> = doc
.nodes()
.filter(|n| matches!(n.kind, NodeKind::Instance { .. }))
.map(|n| n.id)
.filter(|i| shown(doc, *i).contains(&to) && hosts(doc, *i).contains(&from))
.collect();
found.iter().copied().find(|i| !inside_instance(doc, *i)).or_else(|| found.first().and_then(|i| root_instance(doc, *i)))
}
/// Detach instance `top` and every instance inside it: its layers stay as they are, linked to no
/// main.
fn detach_all(doc: &mut Document, top: NodeId) {
for d in doc.subtree(top) {
let modes = match doc.node(d).map(|n| &n.kind) {
Some(NodeKind::Instance { component, .. }) => doc.node(*component).map(|m| m.var_modes.clone()),
_ => None,
};
let Some(n) = doc.node_mut(d) else { continue };
if let Some(modes) = modes {
n.kind = NodeKind::Frame;
n.bound.retain(|k, _| !k.starts_with(PROP_BINDING));
for (c, m) in modes {
n.var_modes.entry(c).or_insert(m);
}
}
n.source = None;
n.overrides.clear();
n.prop_refs.clear();
}
}
/// Break every loop of components containing each other's instances (a file from elsewhere, or
/// written before they were refused): an instance closing each loop is detached, and the instances
/// of its main rebuild without it. Returns a note for each.
pub fn break_component_cycles(doc: &mut Document) -> Vec<String> {
let mut notes = Vec::new();
// Each pass detaches an instance: never more passes than there are layers.
for _ in 0..doc.nodes().count() {
let Some(cycle) = component_cycle(doc) else { break };
let (Some(from), Some(to)) = (cycle.iter().rev().nth(1).copied(), cycle.last().copied()) else { break };
let Some(cut) = cut_point(doc, from, to) else { break };
let name = doc.node(cut).map(|n| n.name.clone()).unwrap_or_default();
detach_all(doc, cut);
rebuild_instances(doc, &BTreeSet::from([from]));
notes.push(format!("\"{name}\" was detached: components can't contain their own instances ({})", cycle_names(doc, &cycle)));
}
notes
}
/// The layers of `component` an instance `inst` of it copies, in pre-order: all of them but those
/// inside a nested instance of a component being expanded already (its own, or a main `inst` is
/// in), which would show itself again. `None` when `inst` itself is in its own main.
pub(super) fn expansion_layers(doc: &Document, inst: NodeId, component: NodeId) -> Option<Vec<NodeId>> {
let mut stops = hosts(doc, inst);
if !stops.insert(component) {
return None;
}
let repeats = |kind: &NodeKind| matches!(kind, NodeKind::Instance { component: c, .. } if stops.contains(c));
let mut out = Vec::new();
let mut stack: Vec<NodeId> = doc.node(component).map(|n| n.children.iter().rev().copied().collect()).unwrap_or_default();
while let Some(id) = stack.pop() {
let Some(n) = doc.node(id) else { continue };
out.push(id);
if out.len() > 5_000_000 {
break;
}
if !repeats(&n.kind) {
stack.extend(n.children.iter().rev());
}
}
Some(out)
}
@@ -1,3 +1,4 @@
use super::cycles::{contains_itself_error, would_contain_itself};
use super::diff::{binding_group, bindings_of};
use super::sync::{insert_linked, sync_instance_depth};
use super::{
@@ -16,8 +17,8 @@ pub fn create_instance_at(doc: &mut Document, component: NodeId, parent: NodeId,
if !matches!(comp.kind, NodeKind::Component { .. }) {
return Err(DocError::Invalid(format!("{component} is not a component")));
}
if doc.is_ancestor(component, parent) {
return Err(DocError::Invalid("an instance can't be placed inside its own main component".into()));
if would_contain_itself(doc, component, parent) {
return Err(DocError::Invalid(contains_itself_error(doc, component)));
}
let id = doc.alloc_id();
let mut root = comp.clone();
+1 -1
View File
@@ -500,7 +500,7 @@ fn sized_own(n: &Node) -> bool {
/// Rebuild every instance of `comps` (and of the components that hold those instances, and so on
/// outwards), each outermost instance once, inner components first.
fn rebuild_instances(doc: &mut Document, comps: &BTreeSet<NodeId>) {
pub(super) fn rebuild_instances(doc: &mut Document, comps: &BTreeSet<NodeId>) {
if comps.is_empty() {
return;
}
+101 -60
View File
@@ -1,3 +1,4 @@
use super::cycles::expansion_layers;
use super::diff::diff_groups;
use super::instances::{Carry, apply_prop, carry_overrides, carry_prop_bindings, copy_group, field_group, follow_variant_resizing, path_map};
use super::{
@@ -23,26 +24,76 @@ pub(super) fn sync_instance_depth(doc: &mut Document, inst: NodeId, depth: usize
// Old layers (kept alive for their overrides), by (parent, source), by source alone (when only
// one layer mirrors it) and, only when those don't match, by name path.
let (mut old, by_source, by_src) = index_old_layers(doc, inst, spare);
// Never a copy of itself inside it (components containing each other).
let Some(src) = expansion_layers(doc, inst, component) else { return };
let Matched { mut map, used, edited, fresh } = match_layers(doc, inst, component, &src, &mut old, &by_source, &by_src);
for c in root.children.clone() {
let _ = doc.remove(c);
}
for s in fresh {
let nid = doc.alloc_id();
map.insert(s, nid);
}
let Copies { nodes: new_nodes, nested, mut remeasure } = copy_layers(doc, inst, &src, &map, &old, &edited, eff_props);
for n in new_nodes {
let (id, parent) = (n.id, n.parent);
if insert_linked(doc, n).is_err()
&& let Some(p) = parent.and_then(|p| doc.node_mut(p))
{
p.children.retain(|c| *c != id);
}
}
let fresh = fresh_root(doc, &mut root, &comp, inst, component, &map);
if let Some(r) = doc.node_mut(inst) {
*r = fresh;
}
// A resized instance: children follow their constraints (auto layout re-flows later).
let (iw, ih) = doc.node(inst).map(|n| (n.width, n.height)).unwrap_or((comp.width, comp.height));
if (iw - comp.width).abs() > 1e-9 || (ih - comp.height).abs() > 1e-9 {
figcraft_layout::apply_constraints(doc, inst, comp.width, comp.height);
remeasure = map.values().copied().collect();
}
follow_geometry_overrides(doc, &src, &map, &old, &mut remeasure);
remeasure_text(doc, remeasure);
rebuild_nested(doc, nested, &old, &used, depth);
}
/// Which instance layer each main layer a rebuild copies becomes.
struct Matched {
/// Main layer → its copy's id (the matching old layer's, so a layer the main moved to another
/// parent keeps its id and overrides); the main itself → the instance.
map: BTreeMap<NodeId, NodeId>,
/// The old layers reused.
used: BTreeSet<NodeId>,
/// Slots holding the instance's own content (their main layers' default content isn't copied).
edited: BTreeSet<NodeId>,
/// Main layers no old layer matches: their copies get new ids.
fresh: Vec<NodeId>,
}
/// Match the main layers `src` of instance `inst`'s main `component` to its old layers: by
/// (parent, source), by source alone (when only one layer mirrors it) and, only when those don't
/// match, by name path. Old layers matched across variants drop what the two variants differ in.
fn match_layers(
doc: &Document,
inst: NodeId,
component: NodeId,
src: &[NodeId],
old: &mut BTreeMap<NodeId, std::sync::Arc<Node>>,
by_source: &BTreeMap<(NodeId, NodeId), NodeId>,
by_src: &BTreeMap<NodeId, Option<NodeId>>,
) -> Matched {
let mut by_path: Option<BTreeMap<String, NodeId>> = None;
let mut main_paths: Option<BTreeMap<NodeId, String>> = None;
// Ids for the copies: reuse the matching old layer's id (a layer the main moved to another
// parent keeps its id and overrides).
let src: Vec<NodeId> = doc.subtree(component).into_iter().filter(|i| *i != component).collect();
let mut map: BTreeMap<NodeId, NodeId> = BTreeMap::new();
map.insert(component, inst);
let mut used: BTreeSet<NodeId> = BTreeSet::new();
// Slots holding the instance's own content (new ids), and main layers not copied: the
// default content of those slots.
let mut edited: BTreeSet<NodeId> = BTreeSet::new();
let mut m = Matched { map: BTreeMap::from([(component, inst)]), used: BTreeSet::new(), edited: BTreeSet::new(), fresh: Vec::new() };
let mut skipped: BTreeSet<NodeId> = BTreeSet::new();
let mut fresh_ids = Vec::new();
for s in &src {
for s in src {
let Some(sn) = doc.node(*s) else { continue };
if sn.parent.is_some_and(|p| skipped.contains(&p) || map.get(&p).is_some_and(|pn| edited.contains(pn))) {
if sn.parent.is_some_and(|p| skipped.contains(&p) || m.map.get(&p).is_some_and(|pn| m.edited.contains(pn))) {
skipped.insert(*s);
continue;
}
let parent_new = sn.parent.and_then(|p| map.get(&p).copied());
let parent_new = sn.parent.and_then(|p| m.map.get(&p).copied());
let mut found = parent_new.and_then(|pn| by_source.get(&(pn, *s)).copied()).or_else(|| by_src.get(s).copied().flatten());
if found.is_none() {
let paths = by_path.get_or_insert_with(|| path_map(doc, inst, true).into_iter().map(|(k, v)| (v, k)).collect());
@@ -50,37 +101,47 @@ pub(super) fn sync_instance_depth(doc: &mut Document, inst: NodeId, depth: usize
found = mains.get(s).and_then(|p| paths.get(p)).copied();
}
let nid = match found {
Some(o) if used.insert(o) => o,
Some(o) if m.used.insert(o) => o,
_ => {
fresh_ids.push(*s);
m.fresh.push(*s);
continue;
}
};
if let Some(m) = old.get(&nid).and_then(|o| variant_kept(doc, o, *s)) {
old.insert(nid, std::sync::Arc::new(m));
if let Some(kept) = old.get(&nid).and_then(|o| variant_kept(doc, o, *s)) {
old.insert(nid, std::sync::Arc::new(kept));
}
if old.get(&nid).is_some_and(|o| holds_content(o)) && is_slot(doc, *s) {
edited.insert(nid);
m.edited.insert(nid);
}
map.insert(*s, nid);
m.map.insert(*s, nid);
}
for c in root.children.clone() {
let _ = doc.remove(c);
}
for s in fresh_ids {
let nid = doc.alloc_id();
map.insert(s, nid);
}
// Build the copies (pre-order: parents first).
let mut enclosing: BTreeMap<NodeId, Vec<NodeId>> = BTreeMap::new();
enclosing.insert(inst, vec![inst]);
let mut inst_props: BTreeMap<NodeId, BTreeMap<String, Value>> = BTreeMap::new();
inst_props.insert(inst, eff_props);
let mut nested = Vec::new();
let mut new_nodes = Vec::new();
// Text layers showing other text or style than their main layer (to re-measure).
let mut remeasure = Vec::new();
for s in &src {
m
}
/// A rebuild's copies of the main's layers, pre-order (parents first).
struct Copies {
nodes: Vec<Node>,
/// Nested instances showing another main than their main layer (swapped): rebuilt from it.
nested: Vec<NodeId>,
/// Text layers showing other text or style than their main layer (to re-measure).
remeasure: Vec<NodeId>,
}
/// Copy main layers `src` into instance `inst` as `map` names them, with the overrides of the
/// `old` layers they replace and the component property values (`props`: the instance's).
fn copy_layers(
doc: &Document,
inst: NodeId,
src: &[NodeId],
map: &BTreeMap<NodeId, NodeId>,
old: &BTreeMap<NodeId, std::sync::Arc<Node>>,
edited: &BTreeSet<NodeId>,
props: BTreeMap<String, Value>,
) -> Copies {
let mut enclosing: BTreeMap<NodeId, Vec<NodeId>> = BTreeMap::from([(inst, vec![inst])]);
let mut inst_props: BTreeMap<NodeId, BTreeMap<String, Value>> = BTreeMap::from([(inst, props)]);
let mut out = Copies { nodes: Vec::new(), nested: Vec::new(), remeasure: Vec::new() };
for s in src {
let Some(orig) = doc.node(*s) else { continue };
let Some(&nid) = map.get(s) else { continue };
let Some(parent) = orig.parent.and_then(|p| map.get(&p).copied()) else { continue };
@@ -113,7 +174,7 @@ pub(super) fn sync_instance_depth(doc: &mut Document, inst: NodeId, depth: usize
e.push(nid);
enclosing.insert(nid, e);
if !matches!(orig.kind, NodeKind::Instance { component: oc, .. } if oc == *c) {
nested.push(nid);
out.nested.push(nid);
}
} else {
enclosing.insert(nid, enc);
@@ -121,31 +182,11 @@ pub(super) fn sync_instance_depth(doc: &mut Document, inst: NodeId, depth: usize
if let (NodeKind::Text(a), NodeKind::Text(b)) = (&orig.kind, &n.kind)
&& a != b
{
remeasure.push(nid);
out.remeasure.push(nid);
}
new_nodes.push(n);
out.nodes.push(n);
}
for n in new_nodes {
let (id, parent) = (n.id, n.parent);
if insert_linked(doc, n).is_err()
&& let Some(p) = parent.and_then(|p| doc.node_mut(p))
{
p.children.retain(|c| *c != id);
}
}
let fresh = fresh_root(doc, &mut root, &comp, inst, component, &map);
if let Some(r) = doc.node_mut(inst) {
*r = fresh;
}
// A resized instance: children follow their constraints (auto layout re-flows later).
let (iw, ih) = doc.node(inst).map(|n| (n.width, n.height)).unwrap_or((comp.width, comp.height));
if (iw - comp.width).abs() > 1e-9 || (ih - comp.height).abs() > 1e-9 {
figcraft_layout::apply_constraints(doc, inst, comp.width, comp.height);
remeasure = map.values().copied().collect();
}
follow_geometry_overrides(doc, &src, &map, &old, &mut remeasure);
remeasure_text(doc, remeasure);
rebuild_nested(doc, nested, &old, &used, depth);
out
}
/// Geometry overrides of children win over constraints.
+2 -2
View File
@@ -292,8 +292,8 @@ pub(crate) fn place(s: &mut Session, p: &Value) -> Result<Value> {
let doc = s.doc_mut()?;
let c = main_of(doc, c).ok_or_else(|| bad(cmd, "`component` must be a component or component set"))?;
let mut slot = crate::drop::slot(doc, page, pt);
// Never inside its own main component (or a copy of it).
if doc.is_ancestor(c, slot.parent) || slot.parent == c {
// Never inside its own main component, or one it contains (directly or through others).
if crate::components::would_contain_itself(doc, c, slot.parent) {
slot = crate::drop::DropSlot { parent: page, index: None, line: None };
}
let (w, h) = doc.node(c).map(|n| (n.width, n.height)).unwrap_or((0.0, 0.0));
+2 -2
View File
@@ -376,8 +376,8 @@ pub(super) fn swap(s: &mut Session, p: &Value) -> Result<Value> {
/// Make instance `i` show main component `c`, keeping the values of properties with the same names
/// (the layers' overrides follow by name when the instance is rebuilt).
pub(crate) fn swap_instance(doc: &mut Document, i: NodeId, c: NodeId) -> std::result::Result<(), String> {
if doc.is_ancestor(c, i) {
return Err("an instance can't show the component it is inside".into());
if crate::components::would_contain_itself(doc, c, i) {
return Err(crate::components::contains_itself_error(doc, c));
}
let defaults = doc.node(c).map(default_props).unwrap_or_default();
match doc.get_mut(i).map(|n| &mut n.kind) {
+13
View File
@@ -70,11 +70,24 @@ pub fn load_path(path: &str, variables: Option<&Value>) -> Result<(figcraft_form
/// Load from bytes. Returns the document with its notes (repairs of a native file, or import
/// warnings) and version history, and whether it was imported from REST API file JSON or a `.fig`
/// file (such a file is never saved over). `variables`: the local variables JSON for a REST API file JSON.
/// Components containing each other's instances are broken apart, with a note.
pub(super) fn load_with(
bytes: &[u8],
name_hint: &str,
variables: Option<&Value>,
images: figcraft_format::rest::ImageResolver,
) -> Result<(figcraft_format::Loaded, bool)> {
let (mut loaded, rest) = load_any(bytes, name_hint, variables, images)?;
let notes = crate::components::break_component_cycles(&mut loaded.doc);
loaded.issues.extend(notes);
Ok((loaded, rest))
}
fn load_any(
bytes: &[u8],
name_hint: &str,
variables: Option<&Value>,
images: figcraft_format::rest::ImageResolver,
) -> Result<(figcraft_format::Loaded, bool)> {
match ext(name_hint).as_str() {
"svg" => {
+234
View File
@@ -0,0 +1,234 @@
//! Components can't contain their own instances, directly, through other components or through
//! the variants of a set (#1084); files holding such a loop open with it broken.
use std::sync::Arc;
use base64::Engine;
use figcraft_doc::{Document, NodeId, NodeKind};
use serde_json::{Value, json};
use crate::Session;
use crate::components::component_cycle;
use crate::components::tests::{copy_of, ex, idv, kids, node};
/// A 100×50 component named `name` at (x, 0), holding a rectangle; returns its id.
fn component(s: &mut Session, name: &str, x: f64) -> NodeId {
let f = idv(&ex(s, "frame.create", json!({"x": x, "y": 0, "width": 100, "height": 50, "name": name})));
ex(s, "shape.rectangle", json!({"x": 5, "y": 5, "width": 10, "height": 10, "parent": f}));
idv(&ex(s, "component.create", json!({"ids": [f]})))
}
fn instance(s: &mut Session, component: NodeId, parent: NodeId) -> NodeId {
idv(&ex(s, "instance.create", json!({"component": component, "x": 20, "y": 20, "parent": parent})))
}
/// `cmd` is refused with the loop explained, and the document is left as it was.
fn refused(s: &mut Session, cmd: &str, p: Value) {
let before = s.doc().unwrap().clone();
let undo = s.ds().unwrap().history.undo.len();
let err = s.execute(cmd, &p).expect_err(&format!("{cmd} {p} should be refused")).to_string();
assert!(err.contains("can't contain"), "{cmd}: {err}");
assert_eq!(*s.doc().unwrap(), before, "{cmd} rolled back");
assert_eq!(s.ds().unwrap().history.undo.len(), undo, "{cmd} left no undo step");
}
/// Card holds a Badge instance; Badge holds an Icon instance. Returns (card, badge, icon, the
/// Icon instance in Badge).
fn chain(s: &mut Session) -> (NodeId, NodeId, NodeId, NodeId) {
let icon = component(s, "Icon", 0.0);
let badge = component(s, "Badge", 200.0);
let card = component(s, "Card", 400.0);
let in_badge = instance(s, icon, badge);
instance(s, badge, card);
(card, badge, icon, in_badge)
}
#[test]
fn direct_and_indirect_loops_are_refused() {
let mut s = Session::new();
let (card, badge, _, in_badge) = chain(&mut s);
// Directly, and through Badge: Card already shows Badge.
refused(&mut s, "instance.create", json!({"component": card, "parent": card}));
refused(&mut s, "instance.create", json!({"component": card, "parent": badge}));
refused(&mut s, "instance.swap", json!({"ids": [in_badge], "component": card}));
// A Card instance moved or pasted into Badge.
let page = s.page().unwrap();
let loose = instance(&mut s, card, page);
refused(&mut s, "node.reparent", json!({"ids": [loose], "parent": badge}));
ex(&mut s, "edit.copy", json!({"ids": [loose]}));
refused(&mut s, "edit.paste", json!({"parent": badge}));
// A copied main pastes as an instance of it.
ex(&mut s, "edit.copy", json!({"ids": [card]}));
refused(&mut s, "edit.paste", json!({"parent": badge}));
assert!(component_cycle(s.doc().unwrap()).is_none());
}
#[test]
fn dropping_an_instance_where_it_would_contain_itself_places_it_on_the_page() {
let mut s = Session::new();
let (card, badge, _, _) = chain(&mut s);
let r = ex(&mut s, "instance.place", json!({"component": card, "x": 250, "y": 25}));
assert_eq!(r["parent"], json!(s.page().unwrap()), "not into Badge: {r}");
assert!(!s.doc().unwrap().is_ancestor(badge, idv(&r)));
}
#[test]
fn an_instance_swap_property_can_not_close_a_loop() {
let mut s = Session::new();
let (card, badge, icon, in_badge) = chain(&mut s);
ex(&mut s, "component.addProperty", json!({"id": badge, "name": "Icon", "type": "instanceSwap", "default": icon, "layer": in_badge}));
let in_card = kids(&s, card).into_iter().find(|k| matches!(node(&s, *k).kind, NodeKind::Instance { .. })).unwrap();
refused(&mut s, "instance.setProperties", json!({"ids": [in_card], "properties": {"Icon": card}}));
// Another component is fine.
let star = component(&mut s, "Star", 600.0);
ex(&mut s, "instance.setProperties", json!({"ids": [in_card], "properties": {"Icon": star}}));
}
/// A set of two variants, "State=On" holding an instance of "State=Off". Returns (on, off, the Off
/// instance in On).
fn nested_variants(s: &mut Session) -> (NodeId, NodeId, NodeId) {
let on = component(s, "State=On", 0.0);
let off = component(s, "State=Off", 200.0);
ex(s, "component.combineVariants", json!({"ids": [on, off]}));
let held = instance(s, off, on);
(on, off, held)
}
#[test]
fn a_variant_holds_a_sibling_but_no_loop_back_through_it() {
let mut s = Session::new();
let (on, off, held) = nested_variants(&mut s);
assert!(component_cycle(s.doc().unwrap()).is_none(), "a sibling variant's instance is no loop");
// Off leading back to On, directly or through another component, is one.
refused(&mut s, "instance.create", json!({"component": on, "parent": off}));
let row = component(&mut s, "Row", 600.0);
instance(&mut s, on, row);
refused(&mut s, "instance.create", json!({"component": row, "parent": off}));
// So is switching the held instance to the variant holding it.
refused(&mut s, "instance.setProperties", json!({"ids": [held], "properties": {"State": "On"}}));
// Combining a component with one holding its instance is fine too.
let a = component(&mut s, "A", 0.0);
let b = component(&mut s, "B", 200.0);
instance(&mut s, a, b);
ex(&mut s, "component.combineVariants", json!({"ids": [a, b]}));
assert!(component_cycle(s.doc().unwrap()).is_none());
}
/// `doc` saved and opened again: the session and what opening it reported.
fn reopened(doc: &Document) -> (Session, Value) {
let data = base64::engine::general_purpose::STANDARD.encode(figcraft_format::save_bytes(doc, None).unwrap());
let mut s = Session::new();
let r = ex(&mut s, "file.open", json!({"data": data, "name": "variants.figcraft"}));
(s, r)
}
#[test]
fn a_file_with_nested_sibling_variants_opens_unchanged_and_a_loop_through_them_is_repaired() {
let mut s = Session::new();
let (on, off, held) = nested_variants(&mut s);
let doc = s.doc().unwrap().clone();
let (t, r) = reopened(&doc);
assert_eq!(r["repairs"], json!([]), "{r}");
assert_eq!(t.doc().unwrap().nodes().count(), doc.nodes().count());
assert!(matches!(node(&t, held).kind, NodeKind::Instance { component, .. } if component == off));
// On → Off → On: an On instance in Off, written directly as an older file could have it.
let spare = component(&mut s, "Spare", 600.0);
let back = instance(&mut s, spare, off);
let mut doc = s.doc().unwrap().clone();
if let NodeKind::Instance { component, .. } = &mut doc.get_mut(back).unwrap().kind {
*component = on;
}
assert!(component_cycle(&doc).is_some());
let (t, r) = reopened(&doc);
assert!(r["repairs"].as_array().is_some_and(|n| n.iter().any(|n| n.as_str().is_some_and(|n| n.contains("was detached")))), "{r}");
assert!(component_cycle(t.doc().unwrap()).is_none());
}
#[test]
fn components_around_instances_and_instances_in_slots_stay_allowed() {
let mut s = Session::new();
let (card, badge, ..) = chain(&mut s);
// A new component around a Card instance.
let page = s.page().unwrap();
let f = idv(&ex(&mut s, "frame.create", json!({"x": 0, "y": 300, "width": 200, "height": 100, "parent": page})));
instance(&mut s, card, f);
let holder = idv(&ex(&mut s, "component.create", json!({"ids": [f]})));
instance(&mut s, badge, holder);
// An instance in a slot of an instance of its own component: the slot holds that instance's
// own content, outside its main.
let slot = idv(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 50, "height": 20, "parent": card, "name": "Slot"})));
ex(&mut s, "component.makeSlot", json!({"id": slot}));
let outer = instance(&mut s, card, page);
let outer_slot = copy_of(&s, outer, slot);
instance(&mut s, card, outer_slot);
assert!(component_cycle(s.doc().unwrap()).is_none());
}
/// A document where Card holds a Badge instance and Badge a Card instance, made by editing the
/// layers directly (as an older file or a merge could have it). Returns it with (card, badge).
fn looped() -> (Document, NodeId, NodeId) {
let mut s = Session::new();
let (card, badge, _, in_badge) = chain(&mut s);
let mut doc = s.doc().unwrap().clone();
if let NodeKind::Instance { component, .. } = &mut doc.get_mut(in_badge).unwrap().kind {
*component = card;
}
crate::components::sync_instance(&mut doc, in_badge);
assert!(component_cycle(&doc).is_some());
(doc, card, badge)
}
/// Edit mains `a` and `b` `n` times, every instance rebuilt in full each time; the layer count
/// after each round.
fn edit_mains(s: &mut Session, a: NodeId, b: NodeId, n: usize) -> Vec<usize> {
crate::components::FULL_REBUILD.with(|f| f.set(true));
let sizes = (0..n)
.map(|i| {
ex(s, "node.set", json!({"id": a, "props": {"fill": format!("#{:02x}0000", i * 20)}}));
ex(s, "node.set", json!({"id": b, "props": {"fill": format!("#00{:02x}00", i * 20)}}));
s.doc().unwrap().nodes().count()
})
.collect();
crate::components::FULL_REBUILD.with(|f| f.set(false));
sizes
}
#[test]
fn a_file_with_a_loop_opens_with_it_broken_and_a_note() {
let (doc, card, badge) = looped();
let bytes = figcraft_format::save_bytes(&doc, None).unwrap();
let data = base64::engine::general_purpose::STANDARD.encode(&bytes);
let mut s = Session::new();
let r = ex(&mut s, "file.open", json!({"data": data, "name": "looped.figcraft"}));
let notes = r["repairs"].as_array().cloned().unwrap_or_default();
assert!(notes.iter().any(|n| n.as_str().is_some_and(|n| n.contains("was detached"))), "{r}");
let doc = s.doc().unwrap();
assert!(component_cycle(doc).is_none());
// The detached instance kept its layers; the other side is still an instance.
assert!(doc.contains(card) && doc.contains(badge));
let count = doc.nodes().count();
// Editing the mains no longer grows the file.
assert_eq!(edit_mains(&mut s, card, badge, 10), vec![count; 10]);
// Importing it into another file breaks it the same way.
let mut t = Session::new();
let r = ex(&mut t, "file.import", json!({"data": data, "name": "looped.figcraft"}));
assert!(r["warnings"].as_array().is_some_and(|w| !w.is_empty()), "{r}");
assert!(component_cycle(t.doc().unwrap()).is_none());
}
#[test]
fn a_loop_merged_in_stops_expanding_and_the_document_stays_editable() {
let (doc, card, badge) = looped();
let mut s = Session::new();
let uid = s.ds().unwrap().uid;
s.apply_remote(uid, Arc::new(doc));
assert!(s.message.as_deref().is_some_and(|m| m.contains("contain each other")), "{:?}", s.message);
let sizes = edit_mains(&mut s, card, badge, 12);
// A couple of rebuilds settle the copies; after that nothing grows.
assert!(sizes.windows(2).skip(2).all(|w| w[0] == w[1]), "{sizes:?}");
// Editing goes on: layers added to a main reach its instances.
ex(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 4, "height": 4, "parent": card}));
let page = s.page().unwrap();
let loose = instance(&mut s, badge, page);
assert!(!kids(&s, loose).is_empty());
}
+2
View File
@@ -229,6 +229,8 @@ mod comments_polish_tests;
#[cfg(test)]
mod component_codegen_tests;
#[cfg(test)]
mod component_cycle_tests;
#[cfg(test)]
mod components_behaviour_tests;
#[cfg(test)]
mod components_scale_tests;
+3 -1
View File
@@ -193,7 +193,8 @@ impl Session {
}
/// Instances can't gain, lose or reorder layers, nor move, resize or re-constrain the layers in
/// them, and library copies are read-only: refuse such an edit (it is rolled back).
/// them, library copies are read-only and components can't contain their own instances: refuse
/// such an edit (it is rolled back).
pub(crate) fn check_instance_structure(&self, id: &str, before: &Option<(Arc<Document>, Vec<NodeId>, NodeId, u64)>) -> Result<()> {
let Some((bdoc, _, _, uid)) = before else { return Ok(()) };
let Some(ds) = self.docs.iter().find(|d| d.uid == *uid) else { return Ok(()) };
@@ -208,6 +209,7 @@ impl Session {
library::check_read_only(effective, bdoc, &ds.doc)
.and_then(|()| components::check_structure_edit(bdoc, &ds.doc))
.and_then(|()| components::check_override_edit(bdoc, &ds.doc))
.and_then(|()| components::check_component_cycles(bdoc, &ds.doc))
.map_err(|m| EngineError::Disabled(id.to_string(), m))
}
+16 -1
View File
@@ -5,6 +5,12 @@ use super::{
ToolId, VIEW_ONLY, editing_hint, editing_of, sanitize,
};
/// What a live session's document holding components that contain each other's instances shows
/// (`names`: the loop): those instances stop expanding where they would show themselves again.
fn loop_notice(names: &str) -> String {
format!("Components contain each other's instances ({names}): those instances stop where they would show themselves")
}
impl Session {
/// The active document (`None`: no document is open).
pub(super) fn tab(&self) -> Option<u64> {
@@ -37,6 +43,12 @@ impl Session {
/// Install a document that came from the session (others' edits): the selection, page and view
/// stay where they can, and the undo history isn't touched.
pub fn apply_remote(&mut self, uid: u64, doc: Arc<Document>) {
let Some(old) = self.docs.iter().find(|d| d.uid == uid).map(|d| d.doc.clone()) else { return };
// Edits merged from others can close a loop of components containing each other.
let changed = crate::derived::changed_nodes(&doc, &old);
if let Some(names) = crate::components::new_cycle(&old, &doc, changed.iter()) {
self.message = Some(loop_notice(&names));
}
let Some(ds) = self.docs.iter_mut().find(|d| d.uid == uid) else { return };
ds.doc = doc;
if !ds.doc.pages.contains(&ds.page) {
@@ -355,7 +367,10 @@ impl Session {
(Some(Role::Guest), None) => {
// The session's file gets a tab of its own, in front.
let ds = DocState::new(client.document().clone(), None);
self.message = Some(format!("Joined {}", ds.doc.name));
self.message = Some(match crate::components::cycle_text(&ds.doc) {
Some(names) => format!("Joined {}. {}", ds.doc.name, loop_notice(&names)),
None => format!("Joined {}", ds.doc.name),
});
let opened = (ds.uid, Some(ds.doc.clone()));
self.active = self.docs.len();
self.docs.push(ds);