From 5643f39769ab561b04fc43acab4321d8d5c06581 Mon Sep 17 00:00:00 2001 From: JonRC Date: Mon, 2 Nov 2026 20:22:13 -0300 Subject: [PATCH] Fix #2907: say that text animator indexes count from 1 layer.addTextAnimatorProperty, layer.addTextSelector and text.animatorFontAxes take `animator: uid|index`, counted from 1 as the code and `#n` references do, but the parameters didn't say so and `animator: 0` failed with only "no animator 0". The parameters now say 1-based, and the error says what the value can be. --- crates/engine/src/cmd/text_anim.rs | 12 ++++++++---- crates/engine/src/tests/text.rs | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/text_anim.rs b/crates/engine/src/cmd/text_anim.rs index 296dc4c37..397e697c4 100644 --- a/crates/engine/src/cmd/text_anim.rs +++ b/crates/engine/src/cmd/text_anim.rs @@ -59,7 +59,11 @@ fn animator_uid(s: &Session, l: &Layer, p: &Value, cmd: &str) -> Result { if let Some(g) = list.iter().find(|g| g.uid == n) { return Ok(g.uid); } - return list.get((n as usize).wrapping_sub(1)).map(|g| g.uid).ok_or_else(|| bad(cmd, format!("no animator {n}"))); + let count = if list.len() == 1 { "1 animator".to_string() } else { format!("{} animators", list.len()) }; + return list + .get((n as usize).wrapping_sub(1)) + .map(|g| g.uid) + .ok_or_else(|| bad(cmd, format!("no animator {n}: `animator` is a uid or a 1-based index, and the layer has {count}"))); } for (lid, uid) in &s.state.selected_props { if *lid != l.id { @@ -454,7 +458,7 @@ pub fn specs() -> Vec { "Add Property", [], None, - "{layer?, animator?: uid|index, property: (see layer.addTextAnimator)}", + "{layer?, animator?: uid|index (1-based), property: (see layer.addTextAnimator)}", has_animator, add_animator_prop ), @@ -463,7 +467,7 @@ pub fn specs() -> Vec { "Add Text Selector", [], None, - "{layer?, animator?: uid|index, kind: range|wiggly|expression}", + "{layer?, animator?: uid|index (1-based), kind: range|wiggly|expression}", has_animator, add_selector ), @@ -482,7 +486,7 @@ pub fn specs() -> Vec { "Variable Font Axes", ["Animation", "Animate Text"], None, - "{layer?, axis?: tag|name (omit to list the font's axes), animator?: uid|index}", + "{layer?, axis?: tag|name (omit to list the font's axes), animator?: uid|index (1-based)}", has_text_layer, font_axes ), diff --git a/crates/engine/src/tests/text.rs b/crates/engine/src/tests/text.rs index 3980c7bb5..132bfc416 100644 --- a/crates/engine/src/tests/text.rs +++ b/crates/engine/src/tests/text.rs @@ -49,6 +49,27 @@ fn animate_text_adds_animators_with_companion_properties() { let _ = s.render(cid, s.time(), Default::default()); } +/// `animator` counts from 1, as `#n` references do: the commands that take it say so, and 0 fails +/// with an error that says so (agents reading `uid|index` tried 0). +#[test] +fn animator_indices_count_from_1_and_say_so() { + let (mut s, t) = session(); + let uid = s.execute("layer.addTextAnimator", json!({"layer": t, "property": "position"})).unwrap()["animator"].as_u64().unwrap(); + for id in ["layer.addTextAnimatorProperty", "layer.addTextSelector", "text.animatorFontAxes"] { + let spec = crate::cmd::command_specs().iter().find(|c| c.id == id).unwrap(); + assert!(spec.params.contains("animator?: uid|index (1-based)"), "{id}: {}", spec.params); + } + for (id, p) in [ + ("layer.addTextAnimatorProperty", json!({"layer": t, "animator": 0, "property": "fillHue"})), + ("layer.addTextSelector", json!({"layer": t, "animator": 0, "kind": "wiggly"})), + ] { + let e = s.execute(id, p).unwrap_err().to_string(); + assert!(e.contains("no animator 0") && e.contains("1-based"), "{id}: {e}"); + } + let r = s.execute("layer.addTextSelector", json!({"layer": t, "animator": 1, "kind": "wiggly"})).unwrap(); + assert_eq!(r["animator"].as_u64(), Some(uid)); +} + #[test] fn add_property_and_selectors_with_undo_redo() { let (mut s, t) = session();