diff --git a/crates/engine/src/session/comps.rs b/crates/engine/src/session/comps.rs index ed3810e79..4fda6628c 100644 --- a/crates/engine/src/session/comps.rs +++ b/crates/engine/src/session/comps.rs @@ -28,6 +28,9 @@ impl Session { /// Drop selections that no longer exist. Each comp keeps its own selection (#1001); a layer id /// counts only inside the comp that owns it, so the active comp never sees another comp's layers. + /// Validate property UIDs too: undo can remove a control and rewind the allocator, so keeping + /// its selection could silently select a different property when the UID is reused. Group + /// selections remain valid even though groups are not leaf properties. pub fn sanitize_state(&mut self) { let p = self.project.clone(); self.state.open_comps.retain(|c| p.comp(*c).is_some()); @@ -35,8 +38,8 @@ impl Session { let Some(c) = p.comp(*cid) else { return false }; let ids: Vec = c.layers.iter().map(|l| l.id).collect(); sel.layers.retain(|l| ids.contains(l)); - sel.props.retain(|(l, _)| ids.contains(l)); - sel.keys.retain(|k| ids.contains(&k.layer)); + sel.props.retain(|(lid, uid)| c.layer(*lid).is_some_and(|l| l.props.find(*uid).is_some() || l.props.find_group(*uid).is_some())); + sel.keys.retain(|k| c.layer(k.layer).is_some_and(|l| l.props.find(k.prop).is_some())); sel.vertices.retain(|v| ids.contains(&v.layer)); !sel.is_empty() }); @@ -49,11 +52,11 @@ impl Session { self.recall_selection(id); } } - if let Some(c) = self.active_comp() { + if let Some(c) = self.state.active_comp.and_then(|cid| p.comp(cid)) { let ids: Vec = c.layers.iter().map(|l| l.id).collect(); self.state.selected_layers.retain(|l| ids.contains(l)); - self.state.selected_props.retain(|(l, _)| ids.contains(l)); - self.state.selected_keys.retain(|k| ids.contains(&k.layer)); + self.state.selected_props.retain(|(lid, uid)| c.layer(*lid).is_some_and(|l| l.props.find(*uid).is_some() || l.props.find_group(*uid).is_some())); + self.state.selected_keys.retain(|k| c.layer(k.layer).is_some_and(|l| l.props.find(k.prop).is_some())); self.state.selected_vertices.retain(|v| ids.contains(&v.layer)); } if let Some((l, t)) = self.state.current_track diff --git a/crates/engine/src/tests/viewer.rs b/crates/engine/src/tests/viewer.rs index abce91877..ef2bd8acc 100644 --- a/crates/engine/src/tests/viewer.rs +++ b/crates/engine/src/tests/viewer.rs @@ -821,3 +821,42 @@ fn polystar_multi_property_expression_clear_resolves_targets_before_removing_con assert!(s.redo()); assert_eq!(group(&s), after); } + +/// Undo must discard removed controls before their UIDs can name newly allocated properties. +#[test] +fn polystar_undo_sanitizes_live_and_parked_property_selections_before_uid_reuse() { + for parked in [false, true] { + let mut s = comp(); + let cid = s.active_comp_id().unwrap(); + let lid = s.execute("layer.newShape", json!({"kind": "polygon"})).unwrap()["layer"].as_u64().unwrap(); + let polygon = layer(&s, lid).props.group("contents/group/contents/star").unwrap().clone(); + let type_uid = polygon.get("type").unwrap().uid; + s.execute("comp.new", json!({"width": 100, "height": 100, "duration": 2})).unwrap(); + let other = s.active_comp_id().unwrap(); + s.activate_comp(cid); + s.state.selected_props = vec![(LayerId(lid), type_uid)]; + s.execute("anim.addKeyframe", json!({})).unwrap(); + let inner = layer(&s, lid).props.group("contents/group/contents/star").unwrap().get("innerRadius").unwrap().uid; + let valid = vec![(LayerId(lid), type_uid), (LayerId(lid), polygon.uid)]; + s.state.selected_props = valid.iter().copied().chain([(LayerId(lid), inner)]).collect(); + s.state.selected_keys = vec![crate::KeyRef { layer: LayerId(lid), prop: inner, time: Tick::ZERO }]; + if parked { + s.activate_comp(other); + } + assert!(s.undo()); + let (props, keys) = if parked { + let selection = s.state.comp_selections.get(&cid).unwrap(); + (&selection.props, &selection.keys) + } else { + (&s.state.selected_props, &s.state.selected_keys) + }; + assert_eq!(props, &valid, "remove stale leaves while preserving property and group selections"); + assert!(keys.is_empty()); + s.activate_comp(cid); + s.execute("prop.separateDimensions", json!({"layer": lid, "value": true})).unwrap(); + let position_z = layer(&s, lid).props.prop("transform/positionZ").unwrap().clone(); + assert_eq!(position_z.uid, inner, "the removed control's UID was actually reused"); + s.execute("prop.setExpression", json!({"expression": "1"})).unwrap(); + assert_eq!(layer(&s, lid).props.prop("transform/positionZ").unwrap(), &position_z, "the reused UID must not become an implicit expression target"); + } +}