From d503688e16f4960cd99f41a33dfb04c42951b9cd Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 7 Nov 2026 20:10:52 -0800 Subject: [PATCH] 3D (Classic): effect.lightingPreset refuses Extrude & Bevel (Classic) Extrude & Bevel (Classic) has no lighting presets; its lights are set in its own options. effect.lightingPreset now refuses a target whose effect at the index is Extrude & Bevel (Classic) with an error that names it, before any object changes. Before, every preset failed on it with an unknown-parameter error for lightColor. The params text and docs/mcp.md state the rule. --- crates/engine/src/cmd/effectcmd.rs | 9 ++++++- .../cmd/effectcmd/lighting_presets_tests.rs | 24 +++++++++++++++++++ docs/mcp.md | 5 ++-- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index a823ee161..3b9b66194 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -62,7 +62,7 @@ pub fn specs() -> Vec { "3D Lighting Preset", [], None, - "{index: int, preset: standard|diffuse|topLeft|right, item?: appearance item index|null (omitted: the active item, else the object's), ids?|id?: [..], target?: object|contents, aligned?: bool (index and item are rows of the first target's stack: each other target edits its equal one, as the Appearance panel does for a mixed selection)} replace an applied 3D effect's light rig and ambient with an original lighting preset; preserve shading, geometry and view; all targets must have a 3D effect at index; one undo step → {ids}", + "{index: int, preset: standard|diffuse|topLeft|right, item?: appearance item index|null (omitted: the active item, else the object's), ids?|id?: [..], target?: object|contents, aligned?: bool (index and item are rows of the first target's stack: each other target edits its equal one, as the Appearance panel does for a mixed selection)} replace an applied 3D effect's light rig and ambient with an original lighting preset; preserve shading, geometry and view; all targets must have a 3D effect at index other than Extrude & Bevel (Classic), which has no lighting presets; one undo step → {ids}", has_doc, lighting_preset ), @@ -441,6 +441,13 @@ fn lighting_preset(s: &mut Session, p: &Value) -> Result { if !effect.is_some_and(|e| effects::is_three_d(&e.id)) { return Err(bad(C, format!("object {} needs a 3D effect at index {index}", id.0))); } + // Extrude & Bevel (Classic) has no lighting presets; its lights are set in its own options. + if effect.is_some_and(|e| e.id == effects::CLASSIC_EXTRUDE) { + return Err(bad( + C, + format!("object {}: Extrude & Bevel (Classic) at index {index} has no lighting presets; set its lights with effect.setParams", id.0), + )); + } } let mut options = p.clone(); let map = options.as_object_mut().ok_or_else(|| bad(C, "parameters must be an object"))?; diff --git a/crates/engine/src/cmd/effectcmd/lighting_presets_tests.rs b/crates/engine/src/cmd/effectcmd/lighting_presets_tests.rs index 82426c5ea..812ce0966 100644 --- a/crates/engine/src/cmd/effectcmd/lighting_presets_tests.rs +++ b/crates/engine/src/cmd/effectcmd/lighting_presets_tests.rs @@ -91,3 +91,27 @@ fn lighting_presets_reject_a_mixed_selection_before_changing_any_effect() { assert_eq!(s.doc().unwrap().history.undo.len(), history); assert!(!s.doc().unwrap().is_dirty()); } + +#[test] +fn lighting_presets_refuse_extrude_and_bevel_classic_and_leave_a_mixed_selection_unchanged() { + let (mut s, first) = setup("threeD.rotate", Value::Null); + let classic = s.execute("shape.rectangle", &json!({"x":250,"y":100,"width":80,"height":80})).unwrap()["id"].as_u64().unwrap(); + s.execute("effect.apply", &json!({"effect":"threeD.classicExtrude","item":null,"params":{"surface":"diffuse","lightRotation":40}})).unwrap(); + for ids in [vec![classic], vec![first.0, classic]] { + for preset in LIGHTING_PRESETS { + s.doc_mut().unwrap().mark_saved(); + let before = s.doc().unwrap().doc.clone(); + let history = s.doc().unwrap().history.undo.len(); + let p = json!({"ids":ids,"index":0,"item":null,"preset":preset.id}); + let err = s.execute("effect.lightingPreset", &p).unwrap_err().to_string(); + assert!(err.contains(&format!("object {classic}: Extrude & Bevel (Classic) at index 0 has no lighting presets")), "{err}"); + assert_eq!(s.doc().unwrap().doc, before); + assert_eq!(s.doc().unwrap().history.undo.len(), history); + assert!(!s.doc().unwrap().is_dirty()); + } + } + // Its lights are set with its own parameters instead. + s.execute("effect.setParams", &json!({"ids":[classic],"index":0,"item":null,"params":{"lightRotation":135,"lightHeight":45}})).unwrap(); + let params = &s.doc().unwrap().doc.node(NodeId(classic)).unwrap().appearance.effects[0].params; + assert_eq!((params["lightRotation"].as_f64(), params["lightHeight"].as_f64()), (Some(135.0), Some(45.0))); +} diff --git a/docs/mcp.md b/docs/mcp.md index 168c377b8..f2d9f5f13 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2137,8 +2137,9 @@ See [Revolve](revolve.md) for the scope and limitations. `effect.lightingPreset {index: int, preset: "standard" | "diffuse" | "topLeft" | "right", item?, ids?, target?}` replaces the lighting rig and ambient of an existing Revolve, Extrude & Bevel, Inflate or Rotate effect. `item: null` addresses object-level effects; an item index addresses a fill/stroke's -effects. All targets must have a 3D effect at `index`, otherwise nothing changes. The operation -is one undo step and preserves Shading, geometry, source paint and the view. +effects. All targets must have a 3D effect at `index`, otherwise nothing changes. Extrude & Bevel +(Classic) has no lighting presets: a target with it at `index` is an error, nothing changes, and +`effect.setParams` sets its lights. The operation is one undo step and preserves Shading, geometry, source paint and the view. The four original rigs use one white front light at 80% intensity. Standard keeps the existing default; Diffuse uses 100% Softness and 35% Ambient Light, Top Left uses Rotation 135 and Height