diff --git a/crates/engine/src/cmd/points.rs b/crates/engine/src/cmd/points.rs index e8385d39..38fd22b9 100644 --- a/crates/engine/src/cmd/points.rs +++ b/crates/engine/src/cmd/points.rs @@ -355,8 +355,8 @@ fn refit(sh: &mut Shape) { /// The one shape whose points are edited: `id`, or the single selected shape. fn target(s: &Session, p: &Value, cmd: &str) -> Result { let st = s.doc()?; - let id = match p.get("id").and_then(Value::as_u64) { - Some(i) => ShapeId(i as u32), + let id = match id_param(p, "id", cmd)? { + Some(i) => i, None => match st.selection.shapes.as_slice() { [one] => *one, _ => return Err(bad(cmd, "select one shape")), @@ -429,8 +429,8 @@ pub fn slide_vertices(doc: &deckcraft_model::Presentation, sel: &crate::Selectio fn points(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; - let id = match p.get("id").and_then(Value::as_u64) { - Some(i) => ShapeId(i as u32), + let id = match id_param(p, "id", "shape.points")? { + Some(i) => i, None => *st.selection.shapes.first().ok_or_else(|| bad("shape.points", "select a shape"))?, }; let sh = st.shape(id).ok_or_else(|| bad("shape.points", "no such shape"))?; diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 5b0dc5be..e3bf3920 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -1150,19 +1150,17 @@ fn media_options(s: &mut Session, p: &Value) -> Result { Ok(json!({})) } -fn shape_id(p: &Value, key: &str) -> Option { - p.get(key).and_then(Value::as_u64).map(|v| deckcraft_model::ShapeId(v as u32)) -} - fn sites(s: &mut Session, p: &Value) -> Result { - let id = shape_id(p, "id").ok_or_else(|| bad("shape.sites", "missing `id`"))?; + let id = id_param(p, "id", "shape.sites")?.ok_or_else(|| bad("shape.sites", "missing `id`"))?; let st = s.doc()?; let sh = st.shape(id).ok_or_else(|| bad("shape.sites", "no such shape"))?; Ok(json!(crate::connect::sites(&st.doc, &st.selection, sh).iter().map(|q| [q.x, q.y]).collect::>())) } fn connect(s: &mut Session, p: &Value) -> Result { - let (Some(from), Some(to)) = (shape_id(p, "from"), shape_id(p, "to")) else { return Err(bad("shape.connect", "missing `from` / `to`")) }; + let (Some(from), Some(to)) = (id_param(p, "from", "shape.connect")?, id_param(p, "to", "shape.connect")?) else { + return Err(bad("shape.connect", "missing `from` / `to`")); + }; let st = s.doc()?; let site_list = |id| st.shape(id).map(|sh| crate::connect::sites(&st.doc, &st.selection, sh)).unwrap_or_default(); let (a, b) = (site_list(from), site_list(to)); @@ -1186,7 +1184,7 @@ fn connect(s: &mut Session, p: &Value) -> Result { } }; let (p0, p1) = (a[ia], b[ib]); - let id = match shape_id(p, "id") { + let id = match id_param(p, "id", "shape.connect")? { Some(id) => id, None => { let preset = str_param(p, "preset").unwrap_or("straightConnector1"); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index cc872e66..a333cb86 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3557,3 +3557,22 @@ fn an_exhausted_id_space_refuses_growth() { sorted.dedup(); assert_eq!(sorted.len(), ids.len(), "no id repeats"); } + +#[test] +fn shape_ids_beyond_u32_are_rejected_not_wrapped() { + let (mut s, a, b) = two_shapes(); + let wrap = |id: deckcraft_model::ShapeId| u64::from(id.0) + (1u64 << 32); + let shapes_before = s.doc().unwrap().shapes().len(); + let r = s.execute("shape.connect", &json!({"from": wrap(a), "to": wrap(b)})); + assert!(r.is_err(), "connect must reject ids beyond u32, got {r:?}"); + let r = s.execute("shape.sites", &json!({"id": wrap(a)})); + assert!(r.is_err(), "sites must reject ids beyond u32, got {r:?}"); + let r = s.execute("shape.points", &json!({"id": wrap(a)})); + assert!(r.is_err(), "points must reject ids beyond u32, got {r:?}"); + let r = s.execute("shape.movePoint", &json!({"id": wrap(a), "index": 0, "x": 1.0, "y": 1.0})); + assert!(r.is_err_and(|e| e.to_string().contains("not a valid shape id")), "movePoint must reject ids beyond u32"); + assert_eq!(s.doc().unwrap().shapes().len(), shapes_before, "no connector was created"); + // In-range ids still work. + assert!(s.execute("shape.sites", &json!({"id": a.0})).is_ok()); + assert!(s.execute("shape.connect", &json!({"from": a.0, "to": b.0})).is_ok()); +}