Motion analysis review round 2: a number names a study by id first, the dialog analyses the study it made on the scratch copy, duration 0.001 s or more, volumes by the innermost member

crates/engine/src/cmd/motion.rs

@@ -476,12 +476,15 @@ fn check(s: &mut Session, _p: &Value) -> Result<Value> {
 // ---- Motion studies ----
 
 fn study_index(s: &Session, p: &Value, cmd: &str) -> Result<usize> {
-    let key = match p.get("study") {
-        Some(Value::Number(n)) => n.to_string(),
-        Some(Value::String(x)) => x.clone(),
+    // A number is an id first, text a name first: a study may be named like another's id.
+    let (key, id_first) = match p.get("study") {
+        Some(Value::Number(n)) => (n.to_string(), true),
+        Some(Value::String(x)) => (x.clone(), false),
         _ => return Err(bad(cmd, "`study` must be an id or a name")),
     };
-    s.doc.assembly.studies.iter().position(|m| m.id.to_string() == key || m.name == key).ok_or_else(|| bad(cmd, format!("no motion study `{key}`")))
+    let studies = &s.doc.assembly.studies;
+    let (by_id, by_name) = (studies.iter().position(|m| m.id.to_string() == key), studies.iter().position(|m| m.name == key));
+    if id_first { by_id.or(by_name) } else { by_name.or(by_id) }.ok_or_else(|| bad(cmd, format!("no motion study `{key}`")))
 }
 
 fn study(s: &mut Session, p: &Value) -> Result<Value> {

crates/engine/src/cmd/motion_analysis.rs

@@ -19,7 +19,7 @@ use crate::{EngineError, Result, Session};
 
 pub const PLOT_DOC: &str = "study: id|name; joints?: [id|name] (default: every joint that moves); occurrence? and point? (a point \
      of a part to follow, as motion.trace takes them); samples? (3…1000, default the study's steps + 1); duration?: seconds the \
-     whole study takes (velocities and accelerations then per second, else per step) → step: [the study step of each sample], \
+     whole study takes, 0.001 or more (velocities and accelerations then per second, else per step) → step: [the study step of each sample], \
      time?: [s], joints: [{joint, name, index, dof, unit, value, velocity, acceleration, velocity_unit, acceleration_unit}] \
      (degrees for angles, mm for distances; angles unwrapped), point?: {occurrence, position: [[x,y,z]…], velocity, \
      acceleration (vectors), speed, acceleration_norm, unit, velocity_unit, acceleration_unit}, samples (taken), stopped_at, \
@@ -173,7 +173,8 @@ pub(super) fn plot(s: &mut Session, p: &Value) -> Result<Value> {
     let duration = match p.get("duration") {
         None | Some(Value::Null) => None,
         Some(v) => {
-            Some(v.as_f64().filter(|d| d.is_finite() && *d > 0.0 && *d <= 1e9).ok_or_else(|| bad(cmd, "`duration` must be seconds, more than 0"))?)
+            // At least a millisecond: shorter sample spacings square to nothing.
+            Some(v.as_f64().filter(|d| (1e-3..=1e9).contains(d)).ok_or_else(|| bad(cmd, "`duration` must be seconds, 0.001 or more"))?)
         }
     };
     let series = plotted(s, p, cmd)?;
@@ -261,10 +262,11 @@ fn shared_volume(s: &Session, doc: &Document, a: u64, b: u64, booleans: &mut usi
     // The bodies under each (and not under the other), with their world frames and boxes.
     let mut sides: [Vec<(Mat, Aabb3, &solidcraft_doc::ModelBody)>; 2] = Default::default();
     for (comp, path, m) in doc.placements() {
-        let side = match (path.contains(&a), path.contains(&b)) {
-            (true, false) => 0,
-            (false, true) => 1,
-            _ => continue,
+        // A body belongs to the innermost of the two on its path, as contact sets pair them.
+        let side = match path.iter().rev().find(|o| **o == a || **o == b) {
+            Some(o) if *o == a => 0,
+            Some(_) => 1,
+            None => continue,
         };
         for body in st.bodies.iter().filter(|x| doc.model_body_component(x) == comp && !x.body.is_mesh()) {
             let frame = doc.model_body_offset(body).map(|o| mat_mul(&m, o)).unwrap_or(m);

crates/engine/src/cmd/motion_analysis_tests.rs

@@ -153,6 +153,18 @@ fn interference_names_the_step_two_parts_start_to_collide() {
     assert!(r["clear"] == true && r["collisions"] == json!([]), "{r}");
 }
 
+/// A number names a study by its id first, text by its name first: a study may be named like
+/// another's id.
+#[test]
+fn studies_by_id_or_name() {
+    let mut s = rail();
+    let keys = json!([{"joint": "Rail", "points": [[0, 10], [15, -5]]}]);
+    let first = run(&mut s, "motion.study", json!({"name": "Late", "steps": 15, "keys": keys}))["study"].as_u64().unwrap_or(0);
+    run(&mut s, "motion.study", json!({"name": first.to_string(), "steps": 15, "keys": keys}));
+    assert_eq!(run(&mut s, "motion.plot", json!({"study": first, "samples": 3}))["study"], "Late");
+    assert_eq!(run(&mut s, "motion.plot", json!({"study": first.to_string(), "samples": 3}))["study"], first.to_string());
+}
+
 /// Hostile parameters are refused, never a panic.
 #[test]
 fn motion_analysis_refuses_what_it_cannot_do() {
@@ -165,6 +177,7 @@ fn motion_analysis_refuses_what_it_cannot_do() {
         ("motion.plot", json!({"study": "Push", "samples": "many"})),
         ("motion.plot", json!({"study": "Push", "duration": 0})),
         ("motion.plot", json!({"study": "Push", "duration": -3})),
+        ("motion.plot", json!({"study": "Push", "duration": 1e-200})),
         ("motion.plot", json!({"study": "Push", "joints": ["Nope"]})),
         ("motion.plot", json!({"study": "Push", "joints": vec![json!("Rail"); 300]})),
         ("motion.plot", json!({"study": "Push", "point": [1, 2]})),

crates/ui-egui/src/dialogs_motion.rs

@@ -264,8 +264,9 @@ pub fn study_params(s: &Session, name: &str, study: Option<&str>, steps: &str, t
         return Err("add a key for a joint first".into());
     }
     let mut p = json!({"name": name, "steps": steps, "keys": keys});
+    // The id as a number: a study may be named like another's id.
     if let Some(st) = study {
-        p["study"] = json!(st);
+        p["study"] = st.parse::<u64>().map_or_else(|_| json!(st), |id| json!(id));
     }
     Ok(p)
 }
@@ -376,9 +377,8 @@ fn moved(before: &Session, after: &Session) -> BTreeMap<u64, Mat> {
 /// The study's pose at `step`, computed on a scratch copy.
 fn study_pose(app: &SolveApp, params: &Value, step: u32) -> Result<BTreeMap<u64, Mat>, String> {
     let mut sc = app.session.scratch();
-    let name = params["study"].as_str().or(params["name"].as_str()).unwrap_or_default().to_string();
-    sc.execute("motion.study", params).map_err(|e| e.to_string())?;
-    sc.execute("motion.play", &json!({"study": name, "step": step})).map_err(|e| e.to_string())?;
+    let id = scratch_study(&mut sc, params)?;
+    sc.execute("motion.play", &json!({"study": id, "step": step})).map_err(|e| e.to_string())?;
     Ok(moved(&app.session, &sc))
 }
 
@@ -410,6 +410,12 @@ fn export_animation(app: &mut SolveApp, params: Value, name: &str) {
     });
 }
 
+/// Makes (or edits) the study on a scratch copy, as OK would: its id there (a name may be
+/// another study's too).
+pub fn scratch_study(sc: &mut Session, params: &Value) -> Result<Value, String> {
+    Ok(sc.execute("motion.study", params).map_err(|e| e.to_string())?["study"].clone())
+}
+
 /// The explosion at `scale`, computed on a scratch copy.
 fn explode_pose(app: &SolveApp, scale: f64) -> Result<BTreeMap<u64, Mat>, String> {
     let mut sc = app.session.scratch();

crates/ui-egui/src/dialogs_motion_plot.rs

@@ -79,7 +79,7 @@ pub fn plot_params(s: &Session, what: Plotted, study: &Value, duration: &str, in
     }
     let d = duration.trim();
     if !d.is_empty() {
-        let seconds = d.parse::<f64>().ok().filter(|x| x.is_finite() && *x > 0.0).ok_or("the duration must be a number of seconds, more than 0")?;
+        let seconds = d.parse::<f64>().ok().filter(|x| (1e-3..=1e9).contains(x)).ok_or("the duration must be a number of seconds, 0.001 or more")?;
         p["duration"] = json!(seconds);
     }
     Ok(p)
@@ -276,8 +276,10 @@ fn collision_rows(
             && let Some((st, mut p)) = check
         {
             let mut sc = app.session.scratch();
-            p["study"] = st.get("study").or_else(|| st.get("name")).cloned().unwrap_or(Value::Null);
-            let r = sc.execute("motion.study", &st).and_then(|_| sc.execute("motion.interference", &p)).map_err(|e| e.to_string());
+            let r = super::scratch_study(&mut sc, &st).and_then(|id| {
+                p["study"] = id;
+                sc.execute("motion.interference", &p).map_err(|e| e.to_string())
+            });
             a.collisions = Some((key, r));
         }
     });
@@ -369,9 +371,9 @@ fn plot_rows(
         let params = study.clone().and_then(|st| plot_params(s, what, &st, &a.duration, inputs).map(|p| (st, p)));
         let key = key_of(&params);
         if a.plot.as_ref().is_none_or(|(k, _)| *k != key) {
-            let r = params.and_then(|(st, p)| {
+            let r = params.and_then(|(st, mut p)| {
                 let mut sc = s.scratch();
-                sc.execute("motion.study", &st).map_err(|e| e.to_string())?;
+                p["study"] = super::scratch_study(&mut sc, &st)?;
                 sc.execute("motion.plot", &p).map_err(|e| e.to_string())
             });
             a.plot = Some((key, r));
@@ -380,11 +382,11 @@ fn plot_rows(
         match a.plot.as_ref().map(|(_, r)| r) {
             Some(Ok(v)) => {
                 ui.vertical(|ui| {
-                    if let Some((xs, ys, unit)) = line(v, what, a.quantity) {
-                        if let Some(to) = chart(ui, &xs, &ys, &unit, total, *step, &bands, t) {
+                    if let Some((xs, ys, unit)) = line(v, what, a.quantity)
+                        && let Some(to) = chart(ui, &xs, &ys, &unit, total, *step, &bands, t)
+                    {
                         (*step, *playing) = (to, false);
                     }
-                    }
                     if let Some(why) = stop_text(v) {
                         ui.add(egui::Label::new(RichText::new(why).color(t.warning)).wrap());
                     }