From be413af02691c5ce7b3d5ab9474ab1e33021c03c Mon Sep 17 00:00:00 2001
From: SBaksa <sabaksa11@gmail.com>
Date: Tue, 3 Nov 2026 16:37:51 +1100
Subject: [PATCH] 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             | 10 ++++++++--
 crates/engine/src/cmd/modify/transform.rs  |  2 +-
 crates/engine/src/cmd/modify2/patharray.rs | 13 +++++++++++--
 crates/engine/tests/ole_frame_transform.rs | 16 ++++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/crates/engine/src/cmd/array.rs b/crates/engine/src/cmd/array.rs
index da96ccfed..5fc16b310 100644
--- a/crates/engine/src/cmd/array.rs
+++ b/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)
 }
diff --git a/crates/engine/src/cmd/modify/transform.rs b/crates/engine/src/cmd/modify/transform.rs
index d1e6a463e..2fad5ee7b 100644
--- a/crates/engine/src/cmd/modify/transform.rs
+++ b/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(())
diff --git a/crates/engine/src/cmd/modify2/patharray.rs b/crates/engine/src/cmd/modify2/patharray.rs
index 3f9445249..aff058ab5 100644
--- a/crates/engine/src/cmd/modify2/patharray.rs
+++ b/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)
 }
diff --git a/crates/engine/tests/ole_frame_transform.rs b/crates/engine/tests/ole_frame_transform.rs
index 2578c4e18..3569d39e5 100644
--- a/crates/engine/tests/ole_frame_transform.rs
+++ b/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);
+}