diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index b2198972..3af877d3 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -44,7 +44,8 @@ fn with_prop( ) -> Result { 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( 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> { + 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 { diff --git a/crates/engine/src/tests_viewer.rs b/crates/engine/src/tests_viewer.rs index be3edf46..5ed0848b 100644 --- a/crates/engine/src/tests_viewer.rs +++ b/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)); +}