diff --git a/crates/engine/src/cmd/layer_params.rs b/crates/engine/src/cmd/layer_params.rs index af67bf36e7..c5abca3d7a 100644 --- a/crates/engine/src/cmd/layer_params.rs +++ b/crates/engine/src/cmd/layer_params.rs @@ -21,7 +21,10 @@ pub(crate) fn explicit_layer(s: &Session, p: &Value) -> Result> .map(|(_, _, l)| Some(l.id)) .ok_or_else(|| EngineError::Other(format!("no layer named \"{name}\"; {}", layer_listing(&d.doc)))) } - Some(v @ Value::Object(fields)) if fields.contains_key("index") || fields.contains_key("ordinal") => super::recorded_layer(s, v), + // A group end is no layer: fail rather than fall back to the selection. + Some(v @ Value::Object(fields)) if fields.contains_key("index") || fields.contains_key("ordinal") => { + super::recorded_layer(s, v)?.map(Some).ok_or_else(|| EngineError::Other("a group end is not a layer".into())) + } Some(Value::Null) | None => Ok(None), Some(other) => Err(EngineError::Other(format!("`layer` must be an id or a name, not {other}"))), } @@ -52,7 +55,6 @@ pub(crate) fn layer_listing(doc: &Document) -> String { pub(crate) fn layer_param(s: &Session, p: &Value) -> Result { match explicit_layer(s, p)? { Some(id) => Ok(id), - None if p.get("layer").is_some_and(Value::is_object) => Err(EngineError::Other("a group end is not a layer".into())), None => s.active().and_then(|d| d.active_layer).ok_or(EngineError::Other("no active layer".into())), } } diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index b939e73271..1a57be0981 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -189,7 +189,13 @@ impl Session { let d = self.active_mut()?; d.doc.layer(target)?; let active = d.active_layer.replace(target); - let r = self.precondition(spec, params); + // The checks see the resolved id: a relative ordinal (`Frwr`, `Bckw`) would otherwise + // resolve again from the swapped-in active layer and land one layer further. + let mut resolved = params.clone(); + if let Some(fields) = resolved.as_object_mut() { + fields.insert("layer".into(), Value::from(target.0)); + } + let r = self.precondition(spec, &resolved); if let Some(d) = self.active_mut() { d.active_layer = active; } diff --git a/crates/engine/tests/it/atn_reference_forms.rs b/crates/engine/tests/it/atn_reference_forms.rs index bc92415156..0e35280eb7 100644 --- a/crates/engine/tests/it/atn_reference_forms.rs +++ b/crates/engine/tests/it/atn_reference_forms.rs @@ -470,3 +470,42 @@ fn empty_comp_list_index_is_noop() { assert!(s.execute("layerComp.apply", params).is_err()); } } + +/// A group-end index names no layer, so a command acting on a layer fails and changes nothing +/// rather than falling back to the selected layers. +#[test] +fn group_end_index_never_targets_the_selection() { + let mut s = scene(true); + s.execute("layer.select", json!({"layer":layer(&s,"L1")})).unwrap(); + s.execute("layer.select", json!({"layer":layer(&s,"L2"),"mode":"add"})).unwrap(); + let past = s.active().unwrap().history.past_len(); + let doc = s.active().unwrap().doc.clone(); + assert!(s.execute("layer.translate", json!({"layer":{"index":3},"dx":1,"dy":0})).is_err()); + let offset = Descriptor::new("Ofst").with("Hrzn", D::UnitFloat { unit: *b"#Pxl", value: 1.0 }).with("Vrtc", D::UnitFloat { unit: *b"#Pxl", value: 0.0 }); + let action = + imported(AtnEvent::Code(*b"move"), Descriptor::new("null").with("null", D::Reference(vec![index("Lyr ", 3)])).with("T ", D::Descriptor(offset))); + s.actions.list = vec![action]; + let result = s.execute("actions.play", json!({"action":"Select"})).unwrap(); + assert!(result.get("failed").is_some(), "{result}"); + assert_eq!(s.active().unwrap().history.past_len(), past); + assert_eq!(s.active().unwrap().doc, doc); +} + +/// A relative ordinal is resolved once: the precondition checked on the named layer sees that +/// layer, not the one past it (here a type layer that would refuse a colour-channel edit). +#[test] +fn forward_ordinal_precondition_checks_the_resolved_layer() { + let mut s = Session::new(); + s.execute("file.new", json!({"width":4,"height":4,"background":"transparent"})).unwrap(); + s.execute("layer.renameLayer", json!({"name":"A"})).unwrap(); + s.execute("layer.new.layer", json!({"name":"B"})).unwrap(); + s.execute("edit.fill", json!({"contents":"color","color":"#000000"})).unwrap(); + s.execute("type.create", json!({"text":"C","size":2})).unwrap(); + s.execute("layer.renameLayer", json!({"name":"C"})).unwrap(); + s.execute("layer.select", json!({"layer":layer(&s,"A")})).unwrap(); + s.execute("channel.target", json!({"colors":[0]})).unwrap(); + let b = photocraft_doc::LayerId(layer(&s, "B")); + let before = s.active().unwrap().doc.layer(b).cloned(); + s.execute("edit.clear", json!({"layer":{"ordinal":"Frwr"}})).unwrap(); + assert_ne!(s.active().unwrap().doc.layer(b).cloned(), before); +}