Merge pull request #1785

crates/engine/src/commands/prop.rs

@@ -44,7 +44,8 @@ fn with_prop<T>(
 ) -> Result<T> {
     let t = s.time_of(cid);
     let linear = s.prefs.general.default_spatial_linear;
-    s.edit(label, merge, |proj, _| {
+    s.edit(label, merge, |proj, state| {
+        let mut next = proj.next_id;
         let l = layer_mut(proj, cid, lid)?;
         let lt = l.layer_time(t);
         let pr = l.props.find_mut(uid).ok_or_else(|| bad("prop", "property vanished"))?;
@@ -59,10 +60,61 @@ fn with_prop<T>(
                 k.spatial_out = [0.0; 3];
             }
         }
+        if pr.match_id == "type" {
+            let removed = sync_polystar_type(l, uid, lt, &mut next)?;
+            if !removed.is_empty() {
+                state.selected_props.retain(|(layer, prop)| *layer != lid || !removed.contains(prop));
+                state.selected_keys.retain(|key| key.layer != lid || !removed.contains(&key.prop));
+            }
+        }
+        proj.next_id = next;
         Ok(r)
     })
 }
 
+/// A static Polygon has no inner controls; switching to Star creates them in schema order
+/// with AE's switch defaults (#1685). Animated/expression-driven types need the controls at
+/// every time, including when the current value is Polygon.
+fn sync_polystar_type(layer: &mut Layer, uid: Uid, time: Tick, next: &mut Uid) -> Result<Vec<Uid>> {
+    if !matches!(layer.source, effectcraft_project::LayerSource::Shape) {
+        return Ok(vec![]);
+    }
+    let Some(group) =
+        layer.props.parent_of_mut(uid).filter(|g| g.match_id == "star" && matches!(g.kind, GroupKind::Indexed) && g.get("type").is_some_and(|p| p.uid == uid))
+    else {
+        return Ok(vec![]);
+    };
+    let Some(kind) = group.get("type") else { return Ok(vec![]) };
+    let needs_inner = kind.is_animated() || kind.has_expression() || kind.value_at(time) == effectcraft_keyframe::Value::Enum(0);
+    if !needs_inner {
+        let mut removed = vec![];
+        group.children.retain(|node| {
+            if let effectcraft_project::Node::Prop(p) = node
+                && matches!(p.match_id.as_str(), "innerRadius" | "innerRoundness")
+            {
+                removed.push(p.uid);
+                return false;
+            }
+            true
+        });
+        return Ok(removed);
+    }
+    for (inner, name, outer, value, ui) in [
+        ("innerRadius", "Inner Radius", "outerRadius", 50.0, effectcraft_project::ParamUi::Pixels),
+        ("innerRoundness", "Inner Roundness", "outerRoundness", 0.0, effectcraft_project::ParamUi::Percent),
+    ] {
+        if group.get(inner).is_none() {
+            let uid = *next;
+            *next = next.checked_add(1).ok_or_else(|| bad("prop", "property ids exhausted"))?;
+            let prop = Property::new(uid, inner, name, effectcraft_keyframe::Value::Scalar(value)).with_ui(ui);
+            let index =
+                group.children.iter().position(|n| matches!(n, effectcraft_project::Node::Prop(p) if p.match_id == outer)).unwrap_or(group.children.len());
+            group.children.insert(index, effectcraft_project::Node::Prop(prop));
+        }
+    }
+    Ok(vec![])
+}
+
 /// Explicit command key times default to layer time; readers and the CTI use comp time.
 #[derive(Clone, Copy)]
 pub(crate) enum KeyTimeBase {

crates/engine/src/tests_viewer.rs

@@ -637,3 +637,63 @@ fn each_comp_keeps_its_resolution_outside_the_undo_history_and_in_the_file() {
     s.execute_checked("view.res.auto", json!({})).unwrap();
     assert_eq!(s.active_comp().unwrap().resolution, Resolution::Auto);
 }
+
+/// #1685: changing Type updates the property tree in the same undo step, including when
+/// addressed by uid (as in the UI) rather than by path (scripts/control channel).
+#[test]
+fn polystar_type_switch_updates_inner_controls_and_undoes() {
+    let mut s = comp();
+    let lid = s.execute("layer.newShape", json!({"kind": "polygon"})).unwrap()["layer"].as_u64().unwrap();
+    let group = |s: &Session| layer(s, lid).props.group("contents/group/contents/star").unwrap().clone();
+    let polygon = group(&s);
+    let type_uid = polygon.get("type").unwrap().uid;
+    assert!(polygon.get("innerRadius").is_none());
+    s.execute("prop.set", json!({"layer": lid, "prop": type_uid, "value": 0})).unwrap();
+    let star = group(&s);
+    assert_eq!(star.get("innerRadius").unwrap().value, KV::Scalar(50.0));
+    assert_eq!(star.get("innerRoundness").unwrap().value, KV::Scalar(0.0));
+    assert_eq!(star.get("innerRadius").unwrap().ui, effectcraft_project::ParamUi::Pixels);
+    assert_eq!(star.get("innerRoundness").unwrap().ui, effectcraft_project::ParamUi::Percent);
+    let names: Vec<_> = star.children.iter().filter_map(|n| n.as_prop()).map(|p| p.match_id.as_str()).collect();
+    assert_eq!(names, ["direction", "type", "points", "position", "rotation", "innerRadius", "outerRadius", "innerRoundness", "outerRoundness"]);
+    assert_eq!(star.get("outerRadius"), polygon.get("outerRadius"));
+    assert!(s.undo());
+    assert_eq!(group(&s), polygon);
+    assert!(s.redo());
+    assert_eq!(group(&s), star);
+    // Reapplying the same Type must not reset custom inner values or allocate new ids.
+    s.execute("prop.set", json!({"layer": lid, "path": "contents/group/contents/star/innerRadius", "value": 37})).unwrap();
+    let before = group(&s);
+    let next = s.project.next_id;
+    s.execute("prop.set", json!({"layer": lid, "path": "contents/group/contents/star/type", "value": 0})).unwrap();
+    assert_eq!(group(&s), before);
+    assert_eq!(s.project.next_id, next);
+    let inner_uid = before.get("innerRadius").unwrap().uid;
+    s.state.selected_props = vec![(LayerId(lid), inner_uid)];
+    s.state.selected_keys = vec![crate::KeyRef { layer: LayerId(lid), prop: inner_uid, time: Tick::ZERO }];
+    s.execute("prop.set", json!({"layer": lid, "path": "contents/group/contents/star/type", "value": 1})).unwrap();
+    assert_eq!(group(&s).get("type").unwrap().value, KV::Enum(1));
+    assert!(group(&s).get("innerRadius").is_none());
+    assert!(group(&s).get("innerRoundness").is_none());
+    assert!(s.state.selected_props.is_empty());
+    assert!(s.state.selected_keys.is_empty());
+    assert!(s.undo());
+    assert_eq!(group(&s), before, "undo restores the original inner values and ids");
+    assert!(s.redo());
+    assert!(group(&s).get("innerRadius").is_none());
+    s.execute("prop.set", json!({"layer": lid, "prop": type_uid, "value": 0})).unwrap();
+    assert_eq!(group(&s).get("innerRadius").unwrap().value, KV::Scalar(50.0));
+}
+
+#[test]
+fn polystar_animated_type_keeps_the_inner_controls_needed_by_other_frames() {
+    let mut s = comp();
+    let lid = s.execute("layer.newShape", json!({"kind": "star"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("prop.set", json!({"layer": lid, "path": "contents/group/contents/star/innerRadius", "value": 27})).unwrap();
+    s.execute("prop.toggleAnimation", json!({"layer": lid, "path": "contents/group/contents/star/type", "value": true})).unwrap();
+    s.execute("prop.set", json!({"layer": lid, "path": "contents/group/contents/star/type", "time": 1, "value": 1})).unwrap();
+    let star = layer(&s, lid).props.group("contents/group/contents/star").unwrap().clone();
+    assert_eq!(star.get("type").unwrap().value_at(Tick::ZERO), KV::Enum(0));
+    assert_eq!(star.get("type").unwrap().value_at(Tick::from_seconds_f64(1.0)), KV::Enum(1));
+    assert_eq!(star.get("innerRadius").unwrap().value, KV::Scalar(27.0));
+}