Merge pull request #1686 from YsDirector/pr/d1-sysvar

Apply sysvar batches atomically

# Conflicts:
#	src/app/control/sheets.rs

src/app/control/p1_tests.rs

@@ -131,6 +131,52 @@ fn sysvar_round_trips_and_refuses_unknowns() {
     assert_eq!(r["code"], "unknown_sysvar");
 }
 
+#[test]
+fn sysvar_batches_validate_atomically_and_undo_as_one_step() {
+    let mut app = OpenCADStudio::new_for_test();
+    app.automation_op(r#"{"op":"new"}"#);
+    let state = app.automation_op(r#"{"protocol":1,"op":"state"}"#);
+    let doc = state["document_id"].as_u64().unwrap();
+    mutate(
+        &mut app,
+        r#"{"protocol":1,"op":"entities_create","request_id":"sv-atomic-layer","document_id":{doc},"entities":[{"type":"Point","location":[0,0],"layer":"Walls"}]}"#,
+    );
+    let i = app.active_tab;
+    // Isolate this operation's dirty/history assertions from the layer setup.
+    app.tabs[i].dirty = false;
+    let history_before = app.tabs[i].history.undo_stack.len();
+
+    let r = app.automation_op(&format!(
+        r#"{{"protocol":1,"op":"sysvar","request_id":"sv-atomic-ok","document_id":{doc},"set":{{"ltscale":2.5,"clayer":"Walls"}}}}"#
+    ));
+    assert_eq!(r["ok"], true, "{}", r["error"]);
+    assert_eq!(app.tabs[i].scene.document.header.current_layer_name, "Walls");
+    assert_eq!(app.tabs[i].scene.document.header.linetype_scale, 2.5);
+    assert_eq!(app.tabs[i].active_layer, "Walls");
+    assert_eq!(app.tabs[i].layers.current_layer, "Walls");
+    assert_eq!(app.ribbon.active_layer, "Walls");
+    assert!(app.tabs[i].dirty);
+    assert_eq!(app.tabs[i].history.undo_stack.len(), history_before + 1);
+
+    // A bad later value must not leak the valid earlier value or an undo entry.
+    let r = app.automation_op(&format!(
+        r#"{{"protocol":1,"op":"sysvar","request_id":"sv-atomic-bad","document_id":{doc},"set":{{"ltscale":9.0,"clayer":"NOPE"}}}}"#
+    ));
+    assert_eq!(r["ok"], false);
+    assert_eq!(r["code"], "invalid_sysvar_value");
+    assert_eq!(app.tabs[i].scene.document.header.linetype_scale, 2.5);
+    assert_eq!(app.tabs[i].scene.document.header.current_layer_name, "Walls");
+    assert_eq!(app.tabs[i].history.undo_stack.len(), history_before + 1);
+
+    // One undo restores both header variables and all current-layer mirrors.
+    app.automation_op(r#"{"op":"undo"}"#);
+    assert_eq!(app.tabs[i].scene.document.header.linetype_scale, 1.0);
+    assert_eq!(app.tabs[i].scene.document.header.current_layer_name, "0");
+    assert_eq!(app.tabs[i].active_layer, "0");
+    assert_eq!(app.tabs[i].layers.current_layer, "0");
+    assert_eq!(app.ribbon.active_layer, "0");
+}
+
 #[test]
 fn layout_create_and_page_setup_configure_sheets() {
     let mut app = OpenCADStudio::new_for_test();

src/app/control/sheets.rs

@@ -50,15 +50,38 @@ impl OpenCADStudio {
     /// "set":{"MIRRTEXT":0}}` writes (one undo step, validated per name).
     pub(super) fn control_sysvar(&mut self, req: &Value) -> Result<Task<Message>, Value> {
         if let Some(set) = req["set"].as_object() {
-            self.push_undo_snapshot(self.active_tab, "SYSVAR");
+            let i = self.active_tab;
+            // Validate the complete batch before opening history or changing the
+            // document. Apart from making the operation atomic, this keeps a
+            // later bad variable from leaving an earlier variable half-applied.
+            {
+                let document = &self.tabs[i].scene.document;
+                for (name, value) in set {
+                    validate_sysvar(document, name, value)?;
+                }
+            }
+
+            self.push_undo_snapshot(i, "SYSVAR");
             let mut applied = serde_json::Map::new();
             for (name, value) in set {
-                set_sysvar(&mut self.tabs[self.active_tab].scene.document, name, value).map_err(
-                    |e| {
-                        // Nothing committed yet — the snapshot is empty.
-                        e
-                    },
-                )?;
+                if name.eq_ignore_ascii_case("clayer") {
+                    let layer = value
+                        .as_str()
+                        .expect("validate_sysvar checked CLAYER as a string");
+                    self.set_current_layer_name(i, layer).map_err(|error| {
+                        failure(
+                            "invalid_sysvar_value",
+                            format!("Layer {layer:?} could not be made current: {error}"),
+                        )
+                    })?;
+                } else {
+                    set_sysvar(&mut self.tabs[i].scene.document, name, value)?;
+                }
+                // System variables are persisted drawing state, not merely a
+                // UI preference. CLAYER is already marked dirty by the unified
+                // layer path; keeping this assignment for every variable makes
+                // mixed batches follow the same contract.
+                self.tabs[i].dirty = true;
                 applied.insert(name.to_ascii_lowercase(), value.clone());
             }
             self.set_control_result(json!({ "set": applied }));
@@ -587,7 +610,79 @@ fn read_sysvar(scene: &crate::scene::Scene, name: &str) -> Option<Value> {
     Some(value)
 }
 
+fn validate_sysvar(document: &codec::CadDocument, name: &str, value: &Value) -> Result<(), Value> {
+    let failure = |message: &str| crate::app::control::failure("invalid_sysvar_value", message);
+    match name.to_ascii_lowercase().as_str() {
+        "ltscale" => {
+            positive(value, "ltscale", failure)?;
+        }
+        "pdmode" => {
+            integer(value, "pdmode", failure)?;
+        }
+        "pdsize" => {
+            value
+                .as_f64()
+                .filter(|v| v.is_finite())
+                .ok_or_else(|| failure("pdsize needs a finite number"))?;
+        }
+        "celtscale" => {
+            positive(value, "celtscale", failure)?;
+        }
+        "textsize" => {
+            positive(value, "textsize", failure)?;
+        }
+        "filletrad" => {
+            value
+                .as_f64()
+                .filter(|v| v.is_finite() && *v >= 0.0)
+                .ok_or_else(|| failure("filletrad needs a finite number >= 0"))?;
+        }
+        "mirrtext" => {
+            // Keep the historical coercion used by set_sysvar: omitted or
+            // non-integer values resolve to the enabled state.
+        }
+        "insunits" => {
+            integer(value, "insunits", failure)?;
+        }
+        "osmode" => {
+            value
+                .as_i64()
+                .and_then(|v| i32::try_from(v).ok())
+                .ok_or_else(|| failure("osmode needs an integer"))?;
+        }
+        "clayer" => {
+            let name = value
+                .as_str()
+                .filter(|s| !s.is_empty())
+                .ok_or_else(|| failure("clayer needs a layer name"))?;
+            if document.layers.get(name).is_none() {
+                return Err(failure(&format!("Layer '{name}' does not exist")));
+            }
+        }
+        "ctextstyle" => {
+            let name = value
+                .as_str()
+                .filter(|s| !s.is_empty())
+                .ok_or_else(|| failure("ctextstyle needs a text style name"))?;
+            if document.text_styles.get(name).is_none() {
+                return Err(failure(&format!("Text style '{name}' does not exist")));
+            }
+        }
+        other => {
+            return Err(crate::app::control::failure(
+                "unknown_sysvar",
+                format!(
+                    "System variable '{other}' is read-only or unknown. Writable: {}",
+                    "celtscale, clayer, ctextstyle, filletrad, insunits, ltscale, mirrtext, osmode, pdmode, pdsize, textsize"
+                ),
+            ))
+        }
+    }
+    Ok(())
+}
+
 fn set_sysvar(document: &mut codec::CadDocument, name: &str, value: &Value) -> Result<(), Value> {
+    validate_sysvar(document, name, value)?;
     let failure = |message: &str| crate::app::control::failure("invalid_sysvar_value", message);
     let header = &mut document.header;
     match name.to_ascii_lowercase().as_str() {