Merge PR #2909

crates/engine/src/cmd/animation.rs

@@ -555,7 +555,15 @@ pub fn specs() -> Vec<CommandSpec> {
             has_keys,
             exponential_scale
         ),
-        cmd!("text.addSelector", "Add Text Selector", [], None, "{layer?, animator?: uid|index, kind: range|wiggly|expression}", has_text_layer, add_selector),
+        cmd!(
+            "text.addSelector",
+            "Add Text Selector",
+            [],
+            None,
+            "{layer?, animator?: uid|index (1-based; a uid match wins over an index), kind: range|wiggly|expression}",
+            has_text_layer,
+            add_selector
+        ),
         cmd!("text.removeAllAnimators", "Remove All Text Animators", ["Animation"], None, "{layers?}", has_text_layer, remove_all_animators),
         cmd!("anim.reveal", "Reveal Properties", [], None, "{kind: keyframes|animation|modified}", has_comp, reveal),
     ]

crates/engine/src/cmd/text_anim.rs

@@ -49,8 +49,9 @@ fn per_char_on(l: &Layer) -> bool {
     l.props.prop("text/perChar3d").is_some_and(|p| p.value.as_bool())
 }
 
-/// The animator a command targets: `animator` (uid or 1-based index), else the animator holding
-/// a selected property, else the last one.
+/// The animator a command targets: `animator` (uid or 1-based index; a uid match wins, so an
+/// animator whose uid equals an index is picked over the one at that position), else the animator
+/// holding a selected property, else the last one.
 fn animator_uid(s: &Session, l: &Layer, p: &Value, cmd: &str) -> Result<u64> {
     let anims = l.props.group("text/animators").ok_or_else(|| bad(cmd, "not a text layer"))?;
     let list: Vec<&PropGroup> = anims.groups().collect();
@@ -59,7 +60,11 @@ fn animator_uid(s: &Session, l: &Layer, p: &Value, cmd: &str) -> Result<u64> {
         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 +459,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Add Property",
             [],
             None,
-            "{layer?, animator?: uid|index, property: (see layer.addTextAnimator)}",
+            "{layer?, animator?: uid|index (1-based; a uid match wins over an index), property: (see layer.addTextAnimator)}",
             has_animator,
             add_animator_prop
         ),
@@ -463,7 +468,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Add Text Selector",
             [],
             None,
-            "{layer?, animator?: uid|index, kind: range|wiggly|expression}",
+            "{layer?, animator?: uid|index (1-based; a uid match wins over an index), kind: range|wiggly|expression}",
             has_animator,
             add_selector
         ),
@@ -482,7 +487,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "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; a uid match wins over an index)}",
             has_text_layer,
             font_axes
         ),

crates/engine/src/tests/text.rs

@@ -49,6 +49,38 @@ fn animate_text_adds_animators_with_companion_properties() {
     let _ = s.render(cid, s.time(), Default::default());
 }
 
+/// `animator` counts from 1, as `#n` references do, and a uid match wins over an index: every
+/// command that takes it says 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.addSelector", "text.animatorFontAxes"] {
+        let spec = crate::cmd::command_specs().iter().find(|c| c.id == id).unwrap();
+        assert!(spec.params.contains("animator?: uid|index (1-based; a uid match wins over an index)"), "{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));
+    // A second animator whose uid is 1: `animator: 1` is that uid, not the first animator, and
+    // index 2 reaches the same animator.
+    s.execute("layer.addTextAnimator", json!({"layer": t, "property": "opacity"})).unwrap();
+    let cid = s.active_comp_id().unwrap();
+    let layer = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap().layer_mut(LayerId(t)).unwrap();
+    layer.props.group_mut("text/animators").unwrap().children[1].as_group_mut().unwrap().uid = 1;
+    for (id, animator) in [("layer.addTextSelector", 1), ("text.addSelector", 1), ("layer.addTextSelector", 2)] {
+        let r = s.execute(id, json!({"layer": t, "animator": animator, "kind": "wiggly"})).unwrap();
+        assert_eq!(r["animator"].as_u64(), Some(1), "{id} animator {animator}: {r}");
+    }
+}
+
 #[test]
 fn add_property_and_selectors_with_undo_redo() {
     let (mut s, t) = session();