OLE2FRAME: arrays refuse before adding copies; clearer refusal (review on #2587)

ARRAYPATH with align and polar arrays that rotate items refused part way, leaving the earlier copies in the drawing. Every copy's matrix is now checked before the first copy is made. The refusal text names the other unsupported transforms. Test: arraypath_refuses_before_adding_any_copy.

crates/engine/src/cmd/array.rs

@@ -247,9 +247,15 @@ fn max_count(s: &Session, others: u64, objs: usize) -> u64 {
 }
 
 fn copy_all(s: &mut Session, hs: &[Handle], mats: impl Iterator<Item = Mat3>) -> Result<usize> {
+    // Every copy is checked before the first is made: a refusal part way would leave the copies
+    // already added in the drawing (a failed command is not rolled back).
+    let mats: Vec<Mat3> = mats.collect();
+    for m in &mats {
+        super::modify::refuse_ole_frames(s, hs, m, true)?;
+    }
     let mut n = 0;
-    for m in mats {
-        n += transform_entities(s, hs, &m, true)?.len();
+    for m in &mats {
+        n += transform_entities(s, hs, m, true)?.len();
     }
     Ok(n)
 }

crates/engine/src/cmd/modify/transform.rs

@@ -37,7 +37,7 @@ pub(crate) fn refuse_ole_frames(s: &Session, hs: &[Handle], m: &Mat3, copy: bool
     };
     if hs.iter().any(ole) {
         return Err(crate::EngineError::Other(
-            "An OLE object can only be moved, copied or scaled uniformly; rotating and mirroring it are not supported.".into(),
+            "An OLE object can only be moved, copied or scaled uniformly; other transforms (rotation, mirroring, non-uniform scale, alignment) are not supported.".into(),
         ));
     }
     Ok(())

crates/engine/src/cmd/modify2/patharray.rs

@@ -35,14 +35,23 @@ pub(crate) fn arraypath(
     };
     crate::cmd::array::check_size(s, n as u64, objs.len())?;
     let Some((p0, t0)) = c.at_length(0.0) else { return Ok(Vec::new()) };
-    let mut out = Vec::new();
+    let mut mats = Vec::new();
     for i in 1..n {
         let Some((p, t)) = c.at_length(step * i as f64) else { continue };
         let mut m = Mat3::translate(p - p0);
         if align {
             m = Mat3::rotate_about(p, t.angle() - t0.angle()).then_before(m);
         }
-        out.extend(crate::cmd::modify::transform_entities(s, &objs, &m, true)?);
+        mats.push(m);
+    }
+    // Every copy is checked before the first is made: a refusal part way would leave the copies
+    // already added in the drawing (a failed command is not rolled back).
+    for m in &mats {
+        crate::cmd::modify::refuse_ole_frames(s, &objs, m, true)?;
+    }
+    let mut out = Vec::new();
+    for m in &mats {
+        out.extend(crate::cmd::modify::transform_entities(s, &objs, m, true)?);
     }
     Ok(out)
 }

crates/engine/tests/ole_frame_transform.rs

@@ -220,3 +220,19 @@ fn mocoro_scales_an_ole_frame() {
     assert_eq!(data_corners(&s)[0], (90.0, -57.0));
     assert_eq!(corners(&s)[0].1, "90");
 }
+
+/// ARRAYPATH with align rotates each copy, which an OLE frame refuses. The refusal comes before
+/// the first copy is added: an error does not roll the drawing back (review on #2587).
+#[test]
+fn arraypath_refuses_before_adding_any_copy() {
+    let (mut s, h) = session();
+    // Straight for 30 units, then a right-angle bend: the first copies only move, a later one turns.
+    let path = s.execute("pline", &json!({ "points": [[0, 0], [30, 0], [30, 30]] })).unwrap();
+    let path = path["handle"].as_str().or_else(|| path["handles"][0].as_str()).unwrap().to_string();
+    let count = s.doc().unwrap().model.len();
+    let before = corners(&s);
+    let e = s.execute("arraypath", &json!({ "handles": [h], "path": path, "count": 7, "align": true })).expect_err("align");
+    assert!(e.to_string().contains("OLE object"), "{e}");
+    assert_eq!(s.doc().unwrap().model.len(), count, "no copy was added");
+    assert_eq!(corners(&s), before);
+}