diff --git a/crates/engine/src/cmd/expr_tools.rs b/crates/engine/src/cmd/expr_tools.rs index bd62a8e46..77c6a066d 100644 --- a/crates/engine/src/cmd/expr_tools.rs +++ b/crates/engine/src/cmd/expr_tools.rs @@ -73,15 +73,7 @@ pub fn errors_in(ctx: &EvalCtx, check: Option std::result::Result<() if !keep_going() { return None; } - let msg = if e.enabled { - match &ctx.expr { - Some(h) => h.eval(ctx, l, p, &p.value_at(l.layer_time(ctx.time))).err(), - None => None, - } - } else { - check.and_then(|check| check(&e.text).err()) - }; - if let Some(message) = msg { + if let Some(message) = property_error(ctx, check, l, p) { out.push(error_of(ctx.comp_id, i, l, p, message, !e.enabled)); } } @@ -89,6 +81,20 @@ pub fn errors_in(ctx: &EvalCtx, check: Option std::result::Result<() keep_going().then_some(out) } +/// Why property `p` of layer `l` fails at `ctx.time` (comp time), as `expr.errors` reports it and +/// scripts read it as `Property.expressionError`: the evaluation error (an exception or a +/// non-finite result) of an enabled expression, or the syntax error that disabled one. `None` +/// without an expression or when it evaluates. Evaluates this one expression; renders nothing. +/// Without an evaluator an enabled expression gets the syntax check, so a broken one (from a +/// project file) still reports why. +pub fn property_error(ctx: &EvalCtx, check: Option std::result::Result<(), String>>, l: &Layer, p: &Property) -> Option { + let e = p.expr.as_ref().filter(|e| !e.text.trim().is_empty())?; + match ctx.expr { + Some(h) if e.enabled => h.eval(ctx, l, p, &p.value_at(l.layer_time(ctx.time))).err(), + _ => check.and_then(|check| check(&e.text).err()), + } +} + fn error_of(cid: ItemId, i: usize, l: &Layer, p: &Property, message: String, disabled: bool) -> ExprError { ExprError { comp: cid.0, diff --git a/crates/script/src/model.rs b/crates/script/src/model.rs index d0785487f..c7307e642 100644 --- a/crates/script/src/model.rs +++ b/crates/script/src/model.rs @@ -33,6 +33,7 @@ fn layer_of<'a>(s: &'a Session, a: &J) -> Result<(ItemId, &'a Comp, &'a Layer), Ok((cid, c, l)) } +/// Answers a script model query of the given `kind` against the session, dispatching to the matching reader, and errors on an unknown kind. pub fn query(s: &mut Session, kind: &str, a: &J) -> R { match kind { "project" => Ok(project(s)), @@ -52,13 +53,7 @@ pub fn query(s: &mut Session, kind: &str, a: &J) -> R { let i = u(a, "index").unwrap_or(0) as usize; Ok(effectcraft_engine::command_specs().get(i.wrapping_sub(1)).map(|c| json!(c.id)).unwrap_or(J::Null)) } - "exprError" => { - let code = a.get("code").and_then(J::as_str).unwrap_or(""); - Ok(match s.expr_check { - Some(check) => json!(check(code).err().unwrap_or_default()), - None => json!(""), - }) - } + "exprError" => expr_error(s, a), "labels" => Ok(json!(Label::ALL.iter().map(|l| l.name()).collect::>())), "effects" => Ok(json!( effectcraft_engine::effects::all() @@ -549,6 +544,21 @@ fn value(s: &Session, a: &J) -> R { Ok(js_value(c, &v)) } +/// `Property.expressionError`: why the property's expression fails at the comp's current time +/// (where After Effects evaluates it; the prelude sends no `time`, a direct query may give one +/// in comp seconds). The message is the one `expr.errors` gives: an evaluation error (an +/// exception or a non-finite result) or the syntax error that disabled it; `""` when it +/// evaluates (#3219). +fn expr_error(s: &Session, a: &J) -> R { + let (cid, c, l) = layer_of(s, a)?; + let uid = u(a, "uid").ok_or("missing uid")?; + let p = l.props.find(uid).ok_or("the property no longer exists (it was deleted)")?; + let t = f(a, "time").map(Tick::from_seconds_f64).unwrap_or_else(|| s.time_of(cid)); + // With the footage source, like `value`: `sampleImage` must not fail for want of it (#1000). + let ctx = EvalCtx { footage: Some(s.footage.as_ref()), expr: s.expr.as_deref(), ..EvalCtx::new(&s.project, cid, c, t) }; + Ok(json!(effectcraft_engine::commands::expr_tools::property_error(&ctx, s.expr_check, l, p).unwrap_or_default())) +} + fn interp(i: Interp) -> &'static str { match i { Interp::Linear => "LINEAR", diff --git a/crates/script/src/prelude.js b/crates/script/src/prelude.js index ce6cd44f4..f33da7091 100644 --- a/crates/script/src/prelude.js +++ b/crates/script/src/prelude.js @@ -868,8 +868,8 @@ Property.prototype.property = function () { return null; }; __call("prop.setExpression", this.__ref({ enabled: !!v })); }); def("expressionError", function () { - var e = this.expression; - return e ? __get("exprError", { code: e }) : ""; + // Evaluated at the comp's current time, so a runtime error or a non-finite result shows too. + return this.expression ? __get("exprError", { comp: this.__layer.__comp, layer: this.__layer.__id, uid: this.__uid }) : ""; }); def("canSetExpression", function () { return !!this.__info().canSetExpression; }); def("canVaryOverTime", function () { return !!this.__info().canVaryOverTime; }); diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 25933d165..52b338e31 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -1306,6 +1306,41 @@ fn sample_image_reads_footage_pixels_in_scripts() { assert_eq!(out.result, json!([0, 1, 0, 1, ""]), "green footage sampled, not transparent black"); } +/// `expressionError` reports what evaluating the expression at the comp's current time does, as +/// After Effects: a non-finite result and an exception, not only a syntax error (#3219). +#[test] +fn expression_error_reports_evaluation_errors() { + let mut s = session(); + let out = ok( + &mut s, + r#" + var c = app.project.items.addComp("E", 320, 240, 1, 5, 30); + var p = c.layers.addSolid([1, 1, 1], "S", 320, 240, 1).transform.opacity; + p.expression = "Math.sqrt(-1)"; + var nan = p.expressionError; + p.expression = 'throw new Error("boom")'; + var thrown = p.expressionError; + p.expression = "value"; + var fine = p.expressionError; + p.expression = "thisLayer.nope("; + var syntax = p.expressionError; + p.expression = "time > 1 ? Math.sqrt(-1) : value"; + c.time = 0; + var early = p.expressionError; + c.time = 2; + var late = p.expressionError; + [nan, thrown, fine, syntax.length > 0, early, late]; + "#, + ); + let v = out.result; + assert!(v[0].as_str().is_some_and(|e| e.contains("not a finite number")), "{v}"); + assert!(v[1].as_str().is_some_and(|e| e.contains("boom")), "{v}"); + assert_eq!(v[2], json!(""), "{v}"); + assert_eq!(v[3], json!(true), "a syntax error is still reported: {v}"); + assert_eq!(v[4], json!(""), "evaluated at the current time (0), where it is fine: {v}"); + assert!(v[5].as_str().is_some_and(|e| e.contains("not a finite number")), "evaluated at the current time (2): {v}"); +} + #[test] fn binary_file_io_preserves_all_byte_values() { let dir = std::env::temp_dir().join(format!("ec-script-binary-{}", std::process::id()));