From 65fb1112ab2dcfbfa298143a1ed072314ccc001b Mon Sep 17 00:00:00 2001 From: jyviko <2501648+jyviko@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:32:44 -0400 Subject: [PATCH] Engine: EngineError::Param carries the schema Problem at its field's path The ~40 parameter checks give a ProblemKind instead of text; Session::execute looks the problem up in the command's form (a group's missing field becomes that field's own path). Replies still name keys; forms phrase the same problem by label. A test refuses every field of every form and checks each refusal names a field of that form. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo --- crates/engine/src/cmd/block.rs | 2 +- crates/engine/src/cmd/cells.rs | 4 +- crates/engine/src/cmd/contact.rs | 3 +- crates/engine/src/cmd/datum.rs | 3 +- crates/engine/src/cmd/forms.rs | 61 +++++++++++++++++++++++++++---- crates/engine/src/cmd/geometry.rs | 5 ++- crates/engine/src/cmd/hex.rs | 13 +++---- crates/engine/src/cmd/mesh.rs | 30 ++++++++------- crates/engine/src/cmd/mirror.rs | 13 ++++++- crates/engine/src/cmd/mod.rs | 37 +++++++++++++------ crates/engine/src/cmd/results.rs | 9 +++-- crates/engine/src/cmd/solve.rs | 7 ++-- crates/engine/src/cmd/tags.rs | 3 +- crates/engine/src/lib.rs | 26 +++++++++---- crates/ui-egui/src/form.rs | 15 ++++---- crates/ui-egui/src/lib.rs | 28 +++++++------- 16 files changed, 172 insertions(+), 87 deletions(-) diff --git a/crates/engine/src/cmd/block.rs b/crates/engine/src/cmd/block.rs index 00fc8b8..06a74b7 100644 --- a/crates/engine/src/cmd/block.rs +++ b/crates/engine/src/cmd/block.rs @@ -109,7 +109,7 @@ fn block_mesh(s: &mut Session, p: &Value) -> Result { let region = match (planar, &p.face) { (true, face) => GeometryRef::face(p.body.clone(), face.clone().unwrap_or_else(|| "area".into()))?, (false, None) => GeometryRef::body(p.body.clone())?, - (false, Some(_)) => return Err(EngineError::param("face", "names the region of 2D elements; a solid block fills its body")), + (false, Some(_)) => return Err(EngineError::invalid("face", "names the region of 2D elements; a solid block fills its body")), }; if planar && !p.faces.is_empty() { return Err(EngineError::Params("a 2D patch has no faces to name: name its edges".into())); diff --git a/crates/engine/src/cmd/cells.rs b/crates/engine/src/cmd/cells.rs index 40cf6cb..07f916e 100644 --- a/crates/engine/src/cmd/cells.rs +++ b/crates/engine/src/cmd/cells.rs @@ -87,14 +87,14 @@ fn cells(s: &mut Session, p: &Value) -> Result { Some(v) => match v.as_u64() { Some(1) => false, Some(2) => true, - _ => return Err(EngineError::param("order", "must be 1 or 2")), + _ => return Err(EngineError::invalid("order", "must be 1 or 2")), }, }; let default = p.get("default").map(Method::parse).transpose()?.unwrap_or(Method::Auto); let methods: BTreeMap = match p.get("methods") { None => BTreeMap::new(), Some(Value::Object(m)) => m.iter().map(|(k, v)| Ok((k.clone(), Method::parse(v)?))).collect::>()?, - Some(_) => return Err(EngineError::param("methods", "must map cell names to methods")), + Some(_) => return Err(EngineError::invalid("methods", "must map cell names to methods")), }; let design = s.design.clone().ok_or_else(|| EngineError::Params("open a design first".into()))?; let names: Vec = match p.get("bodies") { diff --git a/crates/engine/src/cmd/contact.rs b/crates/engine/src/cmd/contact.rs index e8e1c34..1a65efe 100644 --- a/crates/engine/src/cmd/contact.rs +++ b/crates/engine/src/cmd/contact.rs @@ -12,6 +12,7 @@ //! touch there. Each touching pair is reported with the faces of each side that are close to the //! other; with `create`, each becomes two face sets and a contact pair. +use crate::schema::ProblemKind; use std::collections::BTreeMap; use fieldcraft_fem::contact::{Grid, bounds, closest_point}; @@ -34,7 +35,7 @@ fn contact(p: &Value, surfaces: ContactSurfaces) -> Result { /// A required name-valued parameter. fn text<'a>(p: &'a Value, key: &str) -> Result<&'a str> { - p.get(key).and_then(Value::as_str).ok_or_else(|| EngineError::param(key, "must be a set name")) + p.get(key).and_then(Value::as_str).ok_or_else(|| EngineError::param(key, ProblemKind::NotAName)) } pub const PAIR: CommandSpec = CommandSpec { diff --git a/crates/engine/src/cmd/datum.rs b/crates/engine/src/cmd/datum.rs index 16bafab..40e8dcb 100644 --- a/crates/engine/src/cmd/datum.rs +++ b/crates/engine/src/cmd/datum.rs @@ -5,6 +5,7 @@ //! in order, whenever the design is opened (`geometry.open`, `file.open`, samples), so an edit //! of the design followed by a re-open replays them. +use crate::schema::ProblemKind; use std::sync::Arc; use fieldcraft_geometry::{Cut, Design}; @@ -126,7 +127,7 @@ fn plane_datum(by: PlaneBy) -> Result { Ok(match by { PlaneBy::Offset { plane, offset } => { if !offset.is_finite() { - return Err(EngineError::param("offset", "must be a finite number")); + return Err(EngineError::param("offset", ProblemKind::NotANumber)); } let normal = match plane { Principal::Yz => [1.0, 0.0, 0.0], diff --git a/crates/engine/src/cmd/forms.rs b/crates/engine/src/cmd/forms.rs index 59ec434..11530b1 100644 --- a/crates/engine/src/cmd/forms.rs +++ b/crates/engine/src/cmd/forms.rs @@ -1516,15 +1516,10 @@ mod tests { fill_required(&s, form, &mut p); } let missing: Vec = problems(form, &p).into_iter().filter(|p| p.kind == ProblemKind::Required).map(|p| p.path).collect(); - // The engine names a missing nested field by its path, its own key, or the group - // that holds it. - let is_missing = |key: &str| { - missing.iter().any(|m| m == key || m.rsplit('/').next() == Some(key) || m.strip_prefix(key).is_some_and(|r| r.starts_with('/'))) - }; match s.execute(c.id, &p) { - Err(EngineError::Param { key, .. }) if is_missing(&key) => {} - Err(EngineError::Param { key, problem }) => { - failures.push(format!("{}: {p} refused for `{key}` {problem}, but only {missing:?} were missing", c.id)); + Err(EngineError::Param(e)) if e.kind == ProblemKind::Required && missing.contains(&e.path) => {} + Err(EngineError::Param(e)) => { + failures.push(format!("{}: {p} refused: {}, but only {missing:?} were missing", c.id, e.for_agents())); } // State, not parameters: the meshers' default is the open design's bodies, the // bar has no explicit step; activation and probes need one of several fields. @@ -1541,6 +1536,56 @@ mod tests { assert!(checked > 20, "{checked}"); } + /// Every parameter the engine refuses is a field of the command's form: the problem's path + /// leads to a field, so a form can show it beside that field, by its label. Each field of + /// each form is given a value of the wrong kind in turn, over the form's starting values. + #[test] + fn every_param_error_names_a_field_of_its_form() { + let hostile = [json!("x"), json!(-1), json!(0), json!(1e300), json!([]), json!({}), json!(null), json!([1, "x"])]; + let (mut seen, mut failures) = (0, Vec::new()); + for c in crate::commands() { + let Some(form) = c.form else { continue }; + if c.id.starts_with("file.") || c.id.starts_with("edit.") || form.iter().any(|f| matches!(f.kind, Kind::Path { .. })) { + continue; // they replace the session's model or write files + } + let base = prefill(form, &json!({})); + let mut tries = vec![json!({}), base.clone()]; + for f in form.iter().filter(|f| !f.key.is_empty()) { + for v in &hostile { + let mut p = base.clone(); + if let Some(o) = p.as_object_mut() { + o.insert(f.key.into(), v.clone()); + } + tries.push(p); + } + } + for p in tries { + let limits = crate::Limits { max_steiner: 10, max_tets: 100, max_hex_elements: 100, threads: 1 }; + let mut s = Session { limits, ..Session::default() }; + crate::cmd::define::tests::defined_bar(&mut s); + if let Err(EngineError::Param(e)) = s.execute(c.id, &p) { + seen += 1; + if e.field.is_none() || field_at(form, &e.path).is_none() { + failures.push(format!("{}: {p} refused `{}`, not a field of the form ({})", c.id, e.path, e.for_agents())); + } + } + } + } + failures.sort(); + failures.dedup(); + assert!(failures.is_empty(), "{failures:#?}"); + assert!(seen > 50, "{seen}"); + } + + /// A required field left empty says what to do by its kind: a name, a path, else "required". + #[test] + fn required_fields_are_phrased_by_kind() { + let form = |fields: &'static [Field]| problems(fields, &json!({})).iter().map(Problem::for_form).collect::>(); + assert_eq!(form(RECOVER), ["Autosave: required"]); + assert_eq!(form(FILE_SAVE_AS), ["File: enter a path"]); + assert!(form(MATERIAL).contains(&"Name: enter a name".to_string()), "{:?}", form(MATERIAL)); + } + /// The commands whose form is the JSON text alone (no schema yet). None: keep it that way. #[test] fn every_command_has_a_form() { diff --git a/crates/engine/src/cmd/geometry.rs b/crates/engine/src/cmd/geometry.rs index 3c2b30a..d186de4 100644 --- a/crates/engine/src/cmd/geometry.rs +++ b/crates/engine/src/cmd/geometry.rs @@ -1,3 +1,4 @@ +use crate::schema::ProblemKind; use std::collections::BTreeSet; use std::sync::Arc; @@ -156,14 +157,14 @@ fn bodies(s: &mut Session, _: &Value) -> Result { } fn body<'a>(s: &'a Session, p: &Value) -> Result<&'a Shape> { - let name = p.get("body").and_then(Value::as_str).ok_or_else(|| EngineError::param("body", "must be a body name"))?; + let name = p.get("body").and_then(Value::as_str).ok_or_else(|| EngineError::param("body", ProblemKind::NotAName))?; s.design.as_deref().and_then(|d| d.body(name)).ok_or_else(|| EngineError::Params(format!("no body `{name}` (open a design first)"))) } /// The body named by `body`, tessellated within `tolerance`. pub(crate) fn body_mesh<'a>(s: &'a Session, p: &Value) -> Result<(&'a Shape, SurfaceMesh)> { let shape = body(s, p)?; - let tolerance = p.get("tolerance").and_then(Value::as_f64).ok_or_else(|| EngineError::param("tolerance", "must be a number (mm)"))?; + let tolerance = p.get("tolerance").and_then(Value::as_f64).ok_or_else(|| EngineError::param("tolerance", ProblemKind::NotANumber))?; Ok((shape, shape.tessellate(tolerance)?)) } diff --git a/crates/engine/src/cmd/hex.rs b/crates/engine/src/cmd/hex.rs index bd4801b..4f26e18 100644 --- a/crates/engine/src/cmd/hex.rs +++ b/crates/engine/src/cmd/hex.rs @@ -1,5 +1,6 @@ //! Structured hexahedral meshing of the open design's bodies or of imported surfaces. +use crate::schema::ProblemKind; use std::borrow::Cow; use fieldcraft_geometry::{FaceSurface, Shape, SurfaceMesh}; @@ -56,9 +57,7 @@ fn analytic(s: FaceSurface) -> Surface { pub(super) fn number(p: &Value, key: &str) -> Result> { match p.get(key) { None => Ok(None), - Some(v) => { - v.as_f64().filter(|x| x.is_finite() && *x > 0.0).map(Some).ok_or_else(|| EngineError::param(key, "must be a positive number (mm)")) - } + Some(v) => v.as_f64().filter(|x| x.is_finite() && *x > 0.0).map(Some).ok_or_else(|| EngineError::param(key, ProblemKind::NotPositive)), } } @@ -81,7 +80,7 @@ pub(super) fn inputs<'a>(s: &'a Session, p: &Value) -> Result>> { let tolerance = number(p, "tolerance")?; match (p.get("body"), p.get("surface")) { (Some(b), None) => { - let name = b.as_str().ok_or_else(|| EngineError::param("body", "must be a body name"))?; + let name = b.as_str().ok_or_else(|| EngineError::param("body", ProblemKind::NotAName))?; let shape = s .design .as_deref() @@ -155,14 +154,14 @@ pub(super) fn hex_blocks(s: &mut Session, p: &Value) -> Result { let shapes: Vec<&Shape> = match p.get("bodies") { None => design.bodies.iter().collect(), Some(v) => { - let names = v.as_array().ok_or_else(|| EngineError::param("bodies", "must be a list of body names"))?; + let names = v.as_array().ok_or_else(|| EngineError::param("bodies", ProblemKind::NotNames))?; // Bounded before the duplicate check, which is quadratic in the list. if names.len() > MAX_CELLS { return Err(EngineError::Params(format!("`bodies` lists {} bodies; at most {MAX_CELLS} are meshed together", names.len()))); } let mut out: Vec<&Shape> = Vec::with_capacity(names.len()); for n in names { - let name = n.as_str().ok_or_else(|| EngineError::param("bodies", "must be a list of body names"))?; + let name = n.as_str().ok_or_else(|| EngineError::param("bodies", ProblemKind::NotNames))?; let shape = design.body(name).ok_or_else(|| EngineError::Params(format!("no body `{name}`")))?; if out.iter().any(|b| b.name == shape.name) { return Err(EngineError::Params(format!("body `{name}` is listed twice"))); @@ -264,7 +263,7 @@ fn order(p: &Value) -> Result { Some(v) => match v.as_u64() { Some(1) => Ok(false), Some(2) => Ok(true), - _ => Err(EngineError::param("order", "must be 1 (hex8) or 2 (hex20)")), + _ => Err(EngineError::invalid("order", "must be 1 (hex8) or 2 (hex20)")), }, } } diff --git a/crates/engine/src/cmd/mesh.rs b/crates/engine/src/cmd/mesh.rs index cc938a6..8dda4b2 100644 --- a/crates/engine/src/cmd/mesh.rs +++ b/crates/engine/src/cmd/mesh.rs @@ -1,3 +1,4 @@ +use crate::schema::ProblemKind; use std::borrow::Cow; use std::collections::BTreeMap; @@ -123,12 +124,15 @@ const TET_MAX_PASSES: u64 = 200; pub(super) fn optional_count(p: &Value, key: &str, default: u64, range: std::ops::RangeInclusive) -> Result { let v = match p.get(key) { None => default, - Some(v) => v - .as_u64() - .filter(|n| range.contains(n)) - .ok_or_else(|| EngineError::param(key, format!("must be a whole number in {}..={}", range.start(), range.end())))?, + Some(v) => v.as_u64().filter(|n| range.contains(n)).ok_or_else(|| EngineError::param(key, whole_in(&range)))?, }; - usize::try_from(v).map_err(|_| EngineError::param(key, "is too large")) + usize::try_from(v).map_err(|_| EngineError::param(key, whole_in(&range))) +} + +/// A whole number in `range`, as a problem's kind. +fn whole_in(range: &std::ops::RangeInclusive) -> ProblemKind { + let int = |n: u64| i64::try_from(n).unwrap_or(i64::MAX); + ProblemKind::NotAWholeNumber { min: int(*range.start()), max: int(*range.end()) } } /// The boundary options of `mesh.surface.check`, `mesh.tet.generate` and the hex meshers. @@ -144,7 +148,7 @@ pub(super) fn surface_options(p: &Value, mesh: &SurfaceMesh) -> Result) -> Result { let v = optional_number(p, key, default)?; - if range.contains(&v) { Ok(v) } else { Err(EngineError::param(key, format!("must be a number in {}..={}", range.start(), range.end()))) } + if range.contains(&v) { Ok(v) } else { Err(EngineError::param(key, ProblemKind::NotInRange { min: *range.start(), max: *range.end() })) } } /// The quality options of `mesh.tet.generate`, and whether the quality stage runs at all. @@ -155,7 +159,7 @@ pub(super) fn quality_options(p: &Value, max_tets: u64) -> Result<(QualityOption Some(v) => Some( v.as_f64() .filter(|h| *h > 0.0 && h.is_finite() && *h <= fieldcraft_model::MAX_COORD) - .ok_or_else(|| EngineError::param("size", "must be a positive number (mm)"))?, + .ok_or_else(|| EngineError::param("size", ProblemKind::NotPositive))?, ), }; let order = match p.get("order") { @@ -163,7 +167,7 @@ pub(super) fn quality_options(p: &Value, max_tets: u64) -> Result<(QualityOption Some(v) => match v.as_str() { Some("linear") => Order::Linear, Some("quadratic") => Order::Quadratic, - _ => return Err(EngineError::param("order", "must be \"linear\" or \"quadratic\"")), + _ => return Err(EngineError::param("order", ProblemKind::NotAChoice)), }, }; let target = number_in(p, "quality_target", 40.0, 5.0..=60.0)?; @@ -395,14 +399,14 @@ fn planar(s: &mut Session, p: &Value) -> Result { .get("size") .and_then(Value::as_f64) .filter(|v| *v > 0.0 && v.is_finite()) - .ok_or_else(|| EngineError::param("size", "must be a positive number (the element size)"))?; + .ok_or_else(|| super::wrong(p, "size", ProblemKind::NotPositive))?; let kind = match p.get("kind") { None => ElementKind::Quad4, Some(k) => k .as_str() .and_then(ElementKind::from_name) .filter(|k| k.dimension() == 2) - .ok_or_else(|| EngineError::param("kind", "must be quad4, quad8, tri3 or tri6"))?, + .ok_or_else(|| EngineError::param("kind", ProblemKind::NotAChoice))?, }; let opts = fieldcraft_mesh::quad::QuadOptions { max_elements: optional_count(p, "max_elements", 1_000_000, 1..=PLANAR_MAX_ELEMENTS)?, @@ -424,7 +428,7 @@ fn planar(s: &mut Session, p: &Value) -> Result { } let name = match p.get("name") { None => "Plane".to_string(), - Some(n) => n.as_str().map(str::to_string).ok_or_else(|| EngineError::param("name", "must be a body name"))?, + Some(n) => n.as_str().map(str::to_string).ok_or_else(|| EngineError::param("name", ProblemKind::NotAName))?, }; GeometryRef::body(name.clone())?; let mut id = 0u32; @@ -444,7 +448,7 @@ fn planar(s: &mut Session, p: &Value) -> Result { (loops, name, "area".to_string(), names) } (None, Some(_)) => { - let face_name = p.get("face").and_then(Value::as_str).ok_or_else(|| EngineError::param("face", "must be a face name"))?; + let face_name = p.get("face").and_then(Value::as_str).ok_or_else(|| EngineError::param("face", ProblemKind::NotAName))?; let (shape, mesh) = super::geometry::body_mesh(s, p)?; let face = shape.face_index(face_name).ok_or_else(|| EngineError::Params(format!("body `{}` has no face `{face_name}`", shape.name)))?; let b = face_boundary(&mesh.positions, &mesh.triangles, &mesh.tri_face, face, mesh.weld_distance)?; @@ -534,7 +538,7 @@ fn number(v: f64) -> Value { fn quality_report(s: &mut Session, p: &Value) -> Result { let bins = match p.get("bins") { None => 10, - Some(v) => v.as_u64().and_then(|b| usize::try_from(b).ok()).ok_or_else(|| EngineError::param("bins", "must be a whole number"))?, + Some(v) => v.as_u64().and_then(|b| usize::try_from(b).ok()).ok_or_else(|| EngineError::invalid("bins", "must be a whole number"))?, }; let selection = match p.get("elements") { None => None, diff --git a/crates/engine/src/cmd/mirror.rs b/crates/engine/src/cmd/mirror.rs index e13a90a..e963912 100644 --- a/crates/engine/src/cmd/mirror.rs +++ b/crates/engine/src/cmd/mirror.rs @@ -6,6 +6,7 @@ //! on (found by position). Without such a body the image is `/mirror` with the source's //! face names. +use crate::schema::ProblemKind; use std::collections::{BTreeMap, BTreeSet}; use fieldcraft_geometry::Shape; @@ -34,6 +35,10 @@ const SAME_PLACE: f64 = 1e-4; /// Distance (relative to the mesh's bounding-box diagonal) within which nodes are on the plane. const ON_PLANE: f64 = 1e-7; +fn not_a_plane() -> EngineError { + EngineError::invalid("plane", r#"must be a datum plane's name or {"point": [x, y, z], "normal": [x, y, z]}"#) +} + /// The plane: a datum plane by name, or a point and a normal. fn plane(s: &Session, p: &Value) -> Result<([f64; 3], [f64; 3])> { match p.get("plane") { @@ -42,8 +47,12 @@ fn plane(s: &Session, p: &Value) -> Result<([f64; 3], [f64; 3])> { .datum(name) .and_then(|d| d.plane()) .ok_or_else(|| EngineError::Params(format!("no datum plane `{}`", name.chars().take(64).collect::()))), - Some(v @ Value::Object(_)) => Ok((super::xyz(v, "point")?, super::xyz(v, "normal")?)), - _ => Err(EngineError::param("plane", "must be a datum plane's name or {\"point\", \"normal\"}")), + Some(v @ Value::Object(_)) => match (super::xyz(v, "point"), super::xyz(v, "normal")) { + (Ok(point), Ok(normal)) => Ok((point, normal)), + _ => Err(not_a_plane()), + }, + None | Some(Value::Null) => Err(EngineError::param("plane", ProblemKind::Required)), + _ => Err(not_a_plane()), } } diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 1634b7c..a002db1 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -23,6 +23,7 @@ pub(crate) mod solve; mod symmetry; mod tags; +use crate::schema::ProblemKind; use fieldcraft_model::MAX_LABEL; use serde::Deserialize; use serde_json::{Value, json}; @@ -143,9 +144,18 @@ pub static ALL: &[CommandSpec] = &[ results::CLEAR, ]; +/// The problem with required parameter `key`: [`ProblemKind::Required`] when it is left out, +/// else `kind`. +pub(crate) fn wrong(p: &Value, key: &str, kind: ProblemKind) -> EngineError { + EngineError::param(key, if p.get(key).is_none_or(Value::is_null) { ProblemKind::Required } else { kind }) +} + /// A required label parameter (1..=u32::MAX; the model checks its own range). pub(crate) fn label(p: &Value, key: &str) -> Result { - p.get(key).and_then(Value::as_u64).and_then(|v| u32::try_from(v).ok()).ok_or_else(|| EngineError::param(key, "must be a positive whole number")) + p.get(key) + .and_then(Value::as_u64) + .and_then(|v| u32::try_from(v).ok()) + .ok_or_else(|| wrong(p, key, ProblemKind::NotAWholeNumber { min: 1, max: i64::from(u32::MAX) })) } /// The first node and element labels after the model's largest, for `nodes` new nodes and @@ -163,7 +173,7 @@ pub(crate) fn fresh_labels(model: &fieldcraft_model::Model, nodes: usize, elemen /// A required `[x, y, z]` parameter. pub(crate) fn xyz(p: &Value, key: &str) -> Result<[f64; 3]> { - p.get(key).and_then(xyz_value).ok_or_else(|| EngineError::param(key, "must be [x, y, z]")) + p.get(key).and_then(xyz_value).ok_or_else(|| wrong(p, key, ProblemKind::NotAVector { len: 3 })) } /// A JSON `[x, y, z]`. @@ -176,9 +186,12 @@ fn xyz_value(v: &Value) -> Option<[f64; 3]> { /// A required list of `[x, y, z]` points, at most `max_len` long. pub(crate) fn points(p: &Value, key: &str, max_len: usize) -> Result> { - let bad = |what: String| EngineError::param(key, format!("must be a list of at most {max_len} [x, y, z]{what}")); - let raw = p.get(key).and_then(Value::as_array).filter(|a| a.len() <= max_len).ok_or_else(|| bad(String::new()))?; - raw.iter().enumerate().map(|(i, v)| xyz_value(v).ok_or_else(|| bad(format!(" (entry {i} is not)")))).collect() + let raw = p.get(key).and_then(Value::as_array).filter(|a| a.len() <= max_len); + let raw = raw.ok_or_else(|| wrong(p, key, ProblemKind::NotAList { max: max_len, what: "[x, y, z]" }))?; + raw.iter() + .enumerate() + .map(|(i, v)| xyz_value(v).ok_or_else(|| EngineError::invalid(key, format!("must be a list of [x, y, z] (entry {i} is not)")))) + .collect() } /// A required list of labels (each in 1..=MAX_LABEL), at most `max_len` long. @@ -187,11 +200,11 @@ pub(crate) fn labels(p: &Value, key: &str, max_len: usize) -> Result> { .get(key) .and_then(Value::as_array) .filter(|a| a.len() <= max_len) - .ok_or_else(|| EngineError::param(key, format!("must be a list of at most {max_len} labels")))?; + .ok_or_else(|| wrong(p, key, ProblemKind::NotAList { max: max_len, what: "labels" }))?; raw.iter() .map(|v| v.as_u64().and_then(|n| u32::try_from(n).ok()).filter(|n| (1..=MAX_LABEL).contains(n))) .collect::>>() - .ok_or_else(|| EngineError::param(key, format!("must be labels in 1..={MAX_LABEL}"))) + .ok_or_else(|| EngineError::invalid(key, format!("must be labels in 1..={MAX_LABEL}"))) } /// A required path parameter, as given. @@ -199,7 +212,7 @@ fn path_param<'a>(p: &'a Value, key: &str) -> Result<&'a str> { p.get(key) .and_then(Value::as_str) .filter(|s| !s.trim().is_empty() && s.len() <= fieldcraft_io::MAX_PATH_LEN && !s.contains('\0')) - .ok_or_else(|| EngineError::param(key, "must be a file path")) + .ok_or_else(|| wrong(p, key, ProblemKind::NotAPath)) } /// A required path parameter naming a file (or folder) the command reads or writes: the path @@ -214,23 +227,23 @@ pub(crate) fn file_param(s: &Session, p: &Value, key: &str, access: Access) -> R pub(crate) fn optional_number(p: &Value, key: &str, default: f64) -> Result { match p.get(key) { None => Ok(default), - Some(v) => v.as_f64().ok_or_else(|| EngineError::param(key, "must be a number")), + Some(v) => v.as_f64().ok_or_else(|| EngineError::param(key, ProblemKind::NotANumber)), } } /// The `name` parameter. pub(crate) fn name(p: &Value) -> Result<&str> { - p.get("name").and_then(Value::as_str).ok_or_else(|| EngineError::param("name", "must be a name")) + p.get("name").and_then(Value::as_str).ok_or_else(|| wrong(p, "name", ProblemKind::NotAName)) } /// An optional parameter read as `T`'s JSON form; `None` when it is absent. pub(crate) fn opt<'a, T: Deserialize<'a>>(p: &'a Value, key: &str) -> Result> { - p.get(key).map(|v| T::deserialize(v).map_err(|e| EngineError::param(key, format!("is not valid: {e}")))).transpose() + p.get(key).map(|v| T::deserialize(v).map_err(|e| EngineError::invalid(key, format!("is not valid: {e}")))).transpose() } /// A required parameter read as `T`'s JSON form. pub(crate) fn req<'a, T: Deserialize<'a>>(p: &'a Value, key: &str) -> Result { - opt(p, key)?.ok_or_else(|| EngineError::param(key, "is required")) + opt(p, key)?.ok_or_else(|| EngineError::param(key, ProblemKind::Required)) } /// Entries listed in a report: the first ones, and how many there are. diff --git a/crates/engine/src/cmd/results.rs b/crates/engine/src/cmd/results.rs index 55a3407..d9712a3 100644 --- a/crates/engine/src/cmd/results.rs +++ b/crates/engine/src/cmd/results.rs @@ -2,6 +2,7 @@ //! animation) and reaction sums. The display is session state ([`ResultsView`]): these //! commands are not edits, so they are not journalled or undone. +use crate::schema::ProblemKind; use serde_json::{Value, json}; use super::{opt, xyz}; @@ -173,7 +174,7 @@ fn deformed(s: &mut Session, p: &Value) -> Result { fn contour(s: &mut Session, p: &Value) -> Result { let step = step_for(s, p, Want::Any)?; - let name = opt::(p, "field")?.ok_or_else(|| EngineError::param("field", "is required (a field id, or \"none\")"))?; + let name = opt::(p, "field")?.ok_or_else(|| EngineError::param("field", ProblemKind::Required))?; let field = match name.as_str() { "none" => None, id => Some(Field::parse(id).ok_or_else(|| { @@ -255,7 +256,7 @@ fn frame(s: &mut Session, p: &Value) -> Result { (None, Some(t)) if t.is_finite() => { times.iter().enumerate().min_by(|a, b| (a.1 - t).abs().total_cmp(&(b.1 - t).abs())).map_or(last, |(i, _)| i) } - (None, Some(_)) => return Err(EngineError::param("time", "must be a number")), + (None, Some(_)) => return Err(EngineError::param("time", ProblemKind::NotANumber)), (None, None) => last, }; let v = view_of(s, &step)?; @@ -272,7 +273,7 @@ fn reactions(s: &mut Session, p: &Value) -> Result { let step = step_for(s, p, Want::Any)?; let about = if p.get("about").is_some() { xyz(p, "about")? } else { [0.0; 3] }; if !about.iter().all(|c| c.is_finite()) { - return Err(EngineError::param("about", "must be a finite [x, y, z]")); + return Err(EngineError::param("about", ProblemKind::NotAVector { len: 3 })); } let Some((result, current)) = s.latest_result(&step) else { return Err(EngineError::Params(solved_list(s))) }; let Kind::Static(r) = result.kind() else { @@ -287,7 +288,7 @@ fn reactions(s: &mut Session, p: &Value) -> Result { .filter(|a| a.len() <= MAX_SETS) .ok_or_else(|| EngineError::Params(format!("`sets` must be a list of at most {MAX_SETS} set names")))?; list.iter() - .map(|n| n.as_str().map(str::to_string).ok_or_else(|| EngineError::param("sets", "must be set names"))) + .map(|n| n.as_str().map(str::to_string).ok_or_else(|| EngineError::param("sets", ProblemKind::NotNames))) .collect::>()? } None => { diff --git a/crates/engine/src/cmd/solve.rs b/crates/engine/src/cmd/solve.rs index 03887d7..f8b4d55 100644 --- a/crates/engine/src/cmd/solve.rs +++ b/crates/engine/src/cmd/solve.rs @@ -1,6 +1,7 @@ //! `solve.*`: run the model's steps with the implicit, the explicit or the lattice Boltzmann //! solver, keep the results in the session, and read them back. +use crate::schema::ProblemKind; use fieldcraft_explicit::{ExplicitResult, solve_explicit}; use fieldcraft_lbm::{FluidResult, Obstacle}; use fieldcraft_mesh::surface::{BoundarySurface, SurfaceOptions}; @@ -134,7 +135,7 @@ fn cap + TryFrom + std::fmt::Display>(p: &Value, key .as_u64() .filter(|n| (1..=most).contains(n)) .and_then(|n| T::try_from(n).ok()) - .ok_or_else(|| EngineError::param(key, format!("must be a whole number from 1 to {default}"))), + .ok_or_else(|| EngineError::param(key, ProblemKind::NotAWholeNumber { min: 1, max: i64::try_from(most).unwrap_or(i64::MAX) })), } } @@ -390,7 +391,7 @@ fn history(s: &mut Session, p: &Value) -> Result { let h = &r.history; let samples = h.time.len(); let every = match opt::(p, "every")? { - Some(0) => return Err(EngineError::param("every", "must be at least 1")), + Some(0) => return Err(EngineError::param("every", ProblemKind::Below(1.0))), Some(k) => usize::try_from(k).unwrap_or(usize::MAX), None => samples.div_ceil(MAX_HISTORY_SAMPLES).max(1), }; @@ -541,7 +542,7 @@ fn probe_fluid(r: &FluidResult, p: &Value) -> Result { } fn probe(s: &mut Session, p: &Value) -> Result { - let step = opt::(p, "step")?.ok_or_else(|| EngineError::param("step", "is required"))?; + let step = opt::(p, "step")?.ok_or_else(|| EngineError::param("step", ProblemKind::Required))?; let result = s.result(&step).ok_or_else(|| { EngineError::Params(if s.latest_result(&step).is_some() { format!("the result of step `{step}` is stale (the model changed since it was solved); solve it again") diff --git a/crates/engine/src/cmd/tags.rs b/crates/engine/src/cmd/tags.rs index f2426c5..8e0be2c 100644 --- a/crates/engine/src/cmd/tags.rs +++ b/crates/engine/src/cmd/tags.rs @@ -1,6 +1,7 @@ //! Geometry tags on the mesh: which body, face, edge or vertex each mesh entity lies on. Meshers //! write them; these commands set them by hand (for a mesh from elsewhere) and clear them. +use crate::schema::ProblemKind; use fieldcraft_model::{ElementFace, GeometryRef, MAX_ELEMENTS}; use serde_json::{Value, json}; @@ -38,7 +39,7 @@ struct ElementEdge { /// An optional list parameter of at most [`MAX_ELEMENTS`] entries. fn list<'a, T: serde::Deserialize<'a>>(p: &'a Value, key: &str) -> Result> { if p.get(key).and_then(Value::as_array).is_some_and(|a| a.len() > MAX_ELEMENTS) { - return Err(EngineError::param(key, format!("must be a list of at most {MAX_ELEMENTS} entries"))); + return Err(EngineError::param(key, ProblemKind::NotAList { max: MAX_ELEMENTS, what: "entries" })); } Ok(opt(p, key)?.unwrap_or_default()) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ff123ed..c4add2c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -42,11 +42,11 @@ pub enum EngineError { UnknownCommand(String), #[error("{0}")] Params(String), - /// One parameter is missing or wrong: its key (a JSON path, `elastic/young`) and what is - /// wrong with it. The message names the key (agents need it); a form shows the problem - /// beside the field, by its label. - #[error("`{key}` {problem}")] - Param { key: String, problem: String }, + /// One parameter is missing or wrong: the schema's [`schema::Problem`], at the field's JSON + /// path (`elastic/young`). The message names the key ([`schema::Problem::for_agents`]); a + /// form shows it beside the field, by its label ([`schema::Problem::for_form`]). + #[error("{}", .0.for_agents())] + Param(schema::Problem), #[error(transparent)] Model(#[from] fieldcraft_model::Error), #[error(transparent)] @@ -72,9 +72,15 @@ pub enum EngineError { } impl EngineError { - /// A problem with parameter `key`. - pub fn param(key: impl Into, problem: impl Into) -> Self { - Self::Param { key: key.into(), problem: problem.into() } + /// A problem with the parameter at `path` (`elastic/young`). + pub fn param(path: impl Into, kind: schema::ProblemKind) -> Self { + Self::Param(schema::Problem::at(path, kind)) + } + + /// A parameter problem with something wrong that the schema does not describe: what the + /// value must be or do instead. + pub fn invalid(path: impl Into, why: impl Into) -> Self { + Self::param(path, schema::ProblemKind::Invalid(why.into())) } } @@ -245,6 +251,10 @@ impl Session { let before = (spec.kind == CommandKind::Model).then(|| (self.snapshot(), self.document())); let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (spec.run)(self, params))); let result = match outcome { + Ok(Err(EngineError::Param(p))) => Err(EngineError::Param(match spec.form { + Some(form) => schema::refine(form, params, p), + None => p, + })), Ok(r) => r, Err(p) => { let message = p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); diff --git a/crates/ui-egui/src/form.rs b/crates/ui-egui/src/form.rs index 0411339..201cc55 100644 --- a/crates/ui-egui/src/form.rs +++ b/crates/ui-egui/src/form.rs @@ -33,7 +33,8 @@ pub struct Context<'a> { pub touched: &'a BTreeSet, /// OK was tried: every problem shows. pub attempted: bool, - /// The command refused a parameter: its path and the problem, shown beside the field. + /// The command refused a parameter: its path and the problem as the form phrases it, shown + /// beside the field. pub refused: Option<(&'a str, &'a str)>, /// What is missing or wrong in the parameters ([`messages`]). pub problems: &'a [(String, String)], @@ -146,9 +147,9 @@ pub fn show(ui: &mut Ui, t: &Tokens, cx: &Context, fields: &'static [Field], par impl Form<'_> { /// The problem with the field at `path`, under it: the command's refusal at once, the /// form's own once the field was edited or OK was tried. - fn problem_note(&self, ui: &mut Ui, path: &str, label: &str) { + fn problem_note(&self, ui: &mut Ui, path: &str) { let message = match self.cx.refused { - Some((key, problem)) if key == path => Some(format!("{label}: {problem}")), + Some((key, message)) if key == path => Some(message.to_string()), _ => self.problems.get(path).filter(|_| self.cx.attempted || self.cx.touched.contains(path)).cloned(), }; if let Some(message) = message { @@ -190,7 +191,7 @@ impl Form<'_> { *obj = schema::switch_variant(f, common, obj, &t); self.touch(&path); } - self.problem_note(ui, &path, f.label); + self.problem_note(ui, &path); if let Some(v) = obj.get(tag).and_then(Value::as_str).and_then(|t| f.kind.variant(t)) { self.fields(ui, v.fields, common, obj, prefix, depth + 1); } @@ -234,7 +235,7 @@ impl Form<'_> { } } } - self.problem_note(ui, &path, f.label); + self.problem_note(ui, &path); } /// A one-row field; returns its new value (`Null` removes it). @@ -504,7 +505,7 @@ impl Form<'_> { *child = schema::switch_variant(f, &[], child, &t); self.touch(&tag_path); } - self.problem_note(ui, &tag_path, row.label); + self.problem_note(ui, &tag_path); if let Some(v) = child.get(tag).and_then(Value::as_str).and_then(|t| f.kind.variant(t)) { self.fields(ui, v.fields, v.fields, child, path, depth + 1); } @@ -830,7 +831,7 @@ mod tests { assert!(draw(&none, true, None).contains("Material: choose a material")); let touched: BTreeSet = ["name".to_string()].into(); assert!(draw(&touched, false, None).contains("Material: choose a material")); - let refused = draw(&none, false, Some(("name", "names no material"))); + let refused = draw(&none, false, Some(("name", "Material: names no material"))); assert!(refused.contains("Material: names no material") && !refused.contains("choose a material"), "{refused}"); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1ee7a3a..e76940f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -164,7 +164,8 @@ pub struct ParamsDialog { /// The parameters as JSON text. pub text: String, pub error: Option, - /// The command refused one parameter: its path and the problem, shown beside its field. + /// The command refused one parameter: its path and the problem phrased for the form, shown + /// beside its field. pub refused: Option<(String, String)>, /// The paths of the fields edited: their problems show; the others wait for OK. pub touched: std::collections::BTreeSet, @@ -179,7 +180,7 @@ pub struct ParamsDialog { pub back: Option>, } -/// A command that failed: its message, and the parameter it refused (path, problem). +/// A command that failed: its message, and the parameter it refused (path, form message). struct Failure { message: String, refused: Option<(String, String)>, @@ -486,22 +487,20 @@ impl ModelApp { /// Run a command with JSON parameters: what every panel, the control channel and scripts /// do. The result or the error goes to the status bar. pub fn run(&mut self, id: &str, params: Value) -> Result { - self.run_form(id, params, None).map_err(|f| f.message) + self.run_form(id, params, false).map_err(|f| f.message) } - /// [`Self::run`] from a form with `fields`: a refused parameter is named by its field's label - /// (the engine's message names its key, which agents need). - fn run_form(&mut self, id: &str, params: Value, fields: Option<&'static [fieldcraft_engine::schema::Field]>) -> Result { + /// [`Self::run`], from a form when `form`: a refused parameter is then named by its field's + /// label (the engine's message names its key, which agents need). + fn run_form(&mut self, id: &str, params: Value, form: bool) -> Result { self.ensure_idle().map_err(|message| Failure { message, refused: None })?; let revision = self.session.revision(); let label = command_label(id); let r = self.session.execute(id, ¶ms).map_err(|e| match e { - EngineError::Param { key, problem } => { - let message = match fields.and_then(|f| fieldcraft_engine::schema::field_at(f, &key)) { - Some(f) => format!("{}: {problem}", f.label), - None => format!("`{key}` {problem}"), - }; - Failure { message, refused: Some((key, problem)) } + EngineError::Param(p) => { + let message = if form { p.for_form() } else { p.for_agents() }; + let shown = p.for_form(); + Failure { refused: Some((p.path, shown)), message } } e => Failure { message: e.to_string(), refused: None }, }); @@ -677,8 +676,7 @@ impl ModelApp { /// Run the open parameter form; it closes when the command succeeds. pub fn run_dialog(&mut self) -> Result { let (command, params) = self.dialog_params()?; - let fields = fieldcraft_engine::find_command(command).and_then(|c| c.form); - match self.run_form(command, params, fields) { + match self.run_form(command, params, true) { Ok(v) => { self.dialog_done(); Ok(v) @@ -1597,7 +1595,7 @@ mod tests { let e = app.run_dialog().unwrap_err(); assert_eq!(e, "Material: must be a name"); let d = app.dialog.as_ref().unwrap(); - assert_eq!(d.refused, Some(("name".to_string(), "must be a name".to_string()))); + assert_eq!(d.refused, Some(("name".to_string(), "Material: must be a name".to_string()))); assert!(app.status.as_ref().is_some_and(|s| s.text == "Plasticity: Material: must be a name")); let raw = app.session.execute("model.material.plasticity", &json!({"name": 7})).unwrap_err(); assert_eq!(raw.to_string(), "`name` must be a name");