From 23f0d323b0b192d48442c887248b4863817123b7 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:54:08 +0000 Subject: [PATCH] Accept whole-number floats and reject invalid counts for polygon, star and spiral Found while testing crates/engine/src/cmd/create.rs:344 (polygon) and :358 (star; shape.spiral `segments` at :507 reads the same way) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/create.rs | 23 +++++++++++++++++------ crates/engine/src/tests/draw2.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index 8c4e71f1f..d5588ff44 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -339,11 +339,22 @@ fn ellipse(s: &mut Session, p: &Value) -> Result { add_art_turned(s, "Ellipse", path_kind(live.to_path(), Some(live)), f64_or(p, "rotation", 0.0)) } +/// The whole-number count `p` gives for `key` (`5` or `5.0`), or `default` when it is left out or +/// null. A fraction, a negative, a non-finite or a non-number is an error rather than the default. +fn count_or(p: &Value, key: &str, default: u64, cmd: &str) -> Result { + let n = number_or(p, key, default as f64, cmd)?; + if n.is_finite() && n >= 0.0 && n.fract() == 0.0 { + Ok(n as u64) + } else { + Err(bad(cmd, format!("`{key}` must be a whole number of at least 0, not {n}"))) + } +} + fn polygon(s: &mut Session, p: &Value) -> Result { let c = Point::new(f64_req(p, "cx", "shape.polygon")?, f64_req(p, "cy", "shape.polygon")?); let r = f64_req(p, "radius", "shape.polygon")?.abs(); - check_numbers(p, &["sides", "rotation"], "shape.polygon")?; - let sides = p.get("sides").and_then(Value::as_u64).unwrap_or(6).clamp(3, 1000) as u32; + check_numbers(p, &["rotation"], "shape.polygon")?; + let sides = count_or(p, "sides", 6, "shape.polygon")?.clamp(3, 1000) as u32; let rot = f64_or(p, "rotation", 0.0); let xf = Affine::translate(c.to_vec2()) * vectorcraft_geom::rotate_deg(rot); let live = LiveShape::Polygon { radius: r, sides, xf, radii: vec![], kinds: vec![], radii_relative: false }; @@ -353,9 +364,9 @@ fn polygon(s: &mut Session, p: &Value) -> Result { fn star(s: &mut Session, p: &Value) -> Result { let c = Point::new(f64_req(p, "cx", "shape.star")?, f64_req(p, "cy", "shape.star")?); let r1 = f64_req(p, "radius1", "shape.star")?.abs(); - check_numbers(p, &["radius2", "points", "rotation"], "shape.star")?; + check_numbers(p, &["radius2", "rotation"], "shape.star")?; let r2 = f64_or(p, "radius2", r1 / 2.0).abs(); - let n = p.get("points").and_then(Value::as_u64).unwrap_or(5).clamp(2, 1000) as u32; + let n = count_or(p, "points", 5, "shape.star")?.clamp(2, 1000) as u32; let rot = f64_or(p, "rotation", 0.0); let turn = vectorcraft_geom::rotate_deg(rot); // A quarter or whole turn (a Constrain Angle of 90°, say) turns the upright star exactly. @@ -500,12 +511,12 @@ fn line(s: &mut Session, p: &Value) -> Result { fn spiral(s: &mut Session, p: &Value) -> Result { let c = Point::new(f64_req(p, "cx", "shape.spiral")?, f64_req(p, "cy", "shape.spiral")?); - check_numbers(p, &["segments"], "shape.spiral")?; + let segments = count_or(p, "segments", 10, "shape.spiral")?.clamp(2, 1000) as u32; let path = shapes::spiral( c, f64_req(p, "radius", "shape.spiral")?, number_or(p, "decay", 80.0, "shape.spiral")?, - p.get("segments").and_then(Value::as_u64).unwrap_or(10).clamp(2, 1000) as u32, + segments, switch_or(p, "clockwise", true, "shape.spiral")?, ); add_art(s, "Spiral", path_kind(path, None), None) diff --git a/crates/engine/src/tests/draw2.rs b/crates/engine/src/tests/draw2.rs index 5cefe68f3..08ceb0354 100644 --- a/crates/engine/src/tests/draw2.rs +++ b/crates/engine/src/tests/draw2.rs @@ -2812,3 +2812,33 @@ fn pencil_with_live_preview_off_draws_the_filled_path_on_release() { assert!(n.path_data().is_some() && !n.appearance.fill_paint().is_none(), "{:?}", n.appearance); assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1, "one undo step"); } + +fn shape_anchors(cmd: &str, params: &serde_json::Value) -> usize { + let mut s = session(); + let r = s.execute(cmd, params).unwrap(); + let doc = &s.doc().unwrap().doc; + doc.node(NodeId(r["id"].as_u64().unwrap())).unwrap().path_data().unwrap().anchors().count() +} + +#[test] +fn shape_counts_written_as_whole_floats_are_honoured() { + let poly = |v: serde_json::Value| shape_anchors("shape.polygon", &json!({"cx": 100, "cy": 100, "radius": 50, "sides": v})); + assert_eq!(poly(json!(5.0)), 5); + assert_eq!(poly(json!(5)), 5); + let star = |v: serde_json::Value| shape_anchors("shape.star", &json!({"cx": 100, "cy": 100, "radius1": 50, "points": v})); + assert_eq!(star(json!(7.0)), star(json!(7))); + assert_ne!(star(json!(7.0)), star(json!(5))); + let spiral = |v: serde_json::Value| shape_anchors("shape.spiral", &json!({"cx": 100, "cy": 100, "radius": 50, "segments": v})); + assert_eq!(spiral(json!(4.0)), spiral(json!(4))); + assert_ne!(spiral(json!(4.0)), spiral(json!(10))); +} + +#[test] +fn shape_counts_that_are_not_whole_numbers_are_errors() { + for bad in [json!(5.5), json!(-3), json!(-3.0), json!("5")] { + let mut s = session(); + assert!(s.execute("shape.polygon", &json!({"cx": 1, "cy": 1, "radius": 5, "sides": bad})).is_err(), "{bad}"); + assert!(s.execute("shape.star", &json!({"cx": 1, "cy": 1, "radius1": 5, "points": bad})).is_err(), "{bad}"); + assert!(s.execute("shape.spiral", &json!({"cx": 1, "cy": 1, "radius": 5, "segments": bad})).is_err(), "{bad}"); + } +}