From c074612acc61bd487ceb734c56afea8359c905e7 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Thu, 29 Oct 2026 18:53:45 +0800 Subject: [PATCH] Text: duplicate during a text edit copies the edited layer even after another is selected (#2226) --- crates/engine/src/cmd.rs | 4 ++-- crates/engine/src/cmd/edit.rs | 16 ++++++++++++--- crates/engine/src/cmd/params.rs | 7 ++++++- crates/engine/src/tests/text_edit.rs | 29 ++++++++++++++++++++++++++++ crates/engine/src/tests/three_d.rs | 1 + 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index cba6df6ef..eb308011d 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -163,8 +163,8 @@ pub use enabled::always; pub(crate) use enabled::{has_comp, has_keys, has_layers, has_project_selection, not_yet, not_yet_run}; pub use params::match_path_of; pub(crate) use params::{ - b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_p, merge_p, resolve_effect_path, resolve_layer, selectable, selected_leaf_props, str_p, - time_p, unlocked, visible_p, + b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_or, layers_p, merge_p, resolve_effect_path, resolve_layer, selectable, selected_leaf_props, + str_p, time_p, unlocked, visible_p, }; pub use schema::{accepted_params, check_params, params_schema}; diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index f89974c80..758a2a4a0 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -6,7 +6,7 @@ use effectcraft_project::{Comp, Expression, ItemId, ItemKind, Layer, LayerId, La use effectcraft_time::Tick; use serde_json::{Value, json}; -use super::{CommandSpec, has_comp, has_layers, layers_p, match_path_of, selected_leaf_props, str_p}; +use super::{CommandSpec, has_comp, has_layers, layers_or, layers_p, match_path_of, selected_leaf_props, str_p}; use crate::{EngineError, KeyRef, LinkClip, Result, Session, cmd}; fn can_undo(s: &Session) -> std::result::Result<(), String> { @@ -104,6 +104,10 @@ pub(crate) fn reid(layer: &mut Layer, next: &mut u64) { *next += 1; } +/// `edit.duplicate`: copies the targets in place. Without `layers` the targets are, in order: +/// during a text edit the edited layer (the edit is committed first, as `text.endEdit` does), +/// else the selected effects, shape items or text animators, else the selected layers. Layer +/// copies are selected and their ids returned; the other paths return their delegated result. fn duplicate(s: &mut Session, p: &Value) -> Result { let editing = s.state.text_edit.is_some(); // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). An open text edit @@ -132,9 +136,15 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { // A deselected layer never keeps an open text edit: the copies take the selection, so commit // an open edit first, exactly as `text.endEdit` (typed text kept, an empty Type-tool layer // discarded) and as Deselect All does — one call on the command path so every route (menu, - // shortcut, palette, MCP, script) ends the edit and leaves the same final state. + // shortcut, palette, MCP, script) ends the edit and leaves the same final state. Unnamed, the + // target is the edited layer even when an engine route selected another one meanwhile; it is + // read before the commit, which may discard an empty Type-tool layer and leave nothing to copy. + let edited = s.state.text_edit.as_ref().map(|e| e.layer); super::text_edit::end_edit(s, &Value::Null)?; - let (cid, ids) = layers_p(s, p)?; + let (cid, ids) = match edited { + Some(layer) => layers_or(s, p, &[layer])?, + None => layers_p(s, p)?, + }; let new = s.edit("Duplicate", None, |proj, st| { let mut next = proj.next_id; let comp = proj.comp_mut(cid).ok_or(crate::EngineError::NoComp)?; diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index 7c4e1c292..794d3484a 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -142,6 +142,11 @@ pub(crate) fn layer_p(s: &Session, p: &Value, cmd: &str) -> Result<(ItemId, Laye /// `layers` param (array) or the selection. pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> { + layers_or(s, p, &s.state.selected_layers) +} + +/// `layers` param (array), else `layer`, else those of `fallback` still in the comp. +pub(crate) fn layers_or(s: &Session, p: &Value, fallback: &[LayerId]) -> Result<(ItemId, Vec)> { let cid = comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; // Explicit references that match no layer are an error (agents otherwise get a silent no-op). @@ -153,7 +158,7 @@ pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> if let Some(v) = p.get("layer").filter(|v| !v.is_null()) { return Ok((cid, vec![resolve(v)?])); } - Ok((cid, s.state.selected_layers.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) + Ok((cid, fallback.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) } /// Whether a layer can be picked by the user (Select All, Ctrl+Up/Down, a Timeline click): not diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 176fe2ae6..dfd2596cd 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1312,6 +1312,35 @@ fn duplicate_during_an_edit_leaves_no_key_or_vertex_selected_on_the_original() { } } +#[test] +fn duplicate_during_an_edit_copies_the_edited_layer_after_another_is_selected() { + use effectcraft_project::LayerId; + // An engine route (MCP, control channel, script) can select another layer while the edit + // stays open. The implicit Duplicate still means the edited layer, as Clear does; a named + // target keeps its meaning. + let (mut s, t) = session("Hello"); + let other = s.execute("layer.newText", json!({"text": "Other"})).unwrap()["layer"].as_u64().unwrap(); + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + assert!(s.state.text_edit.as_ref().is_some_and(|e| e.layer == LayerId(t)), "selecting another layer leaves the edit open"); + let before = layer_count(&s); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit"); + assert_eq!(layer_count(&s), before + 1, "one layer is copied"); + assert_eq!(doc(&s, copy).text, "Hello", "the edited layer is copied, not the newly selected one"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "and its copy is selected"); + // Null parameters name no layer, as everywhere else layers are read. + for unnamed in [json!({"layers": null}), json!({"layer": null})] { + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + let copy = s.execute("edit.duplicate", unnamed.clone()).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, copy).text, "Hello", "{unnamed}: the edited layer is copied"); + } + begin_edit_at(&mut s, t, 5); + let named = s.execute("edit.duplicate", json!({"layer": other})).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, named).text, "Other", "a named layer is copied as named"); +} + #[test] fn duplicate_ends_the_open_edit_and_selects_the_copy() { let (mut s, t) = editing("Hello", 5); diff --git a/crates/engine/src/tests/three_d.rs b/crates/engine/src/tests/three_d.rs index dc28ae42a..f10c74043 100644 --- a/crates/engine/src/tests/three_d.rs +++ b/crates/engine/src/tests/three_d.rs @@ -26,6 +26,7 @@ fn auto_orient_command() { assert!(!effectcraft_render::three_d::camera::is_two_node(&s.active_comp().unwrap().layers[0])); } +/// Layers in the active comp. pub(crate) fn layer_count(s: &Session) -> usize { s.active_comp().unwrap().layers.len() }