From a1083cbb1694fe63d339fe249f268066e19c7c29 Mon Sep 17 00:00:00 2001 From: Mitchel Pigsley Date: Mon, 2 Nov 2026 09:43:47 -0700 Subject: [PATCH 1/4] Spec: a params effect may bound a parameter the run already has (#252) An entry with only min and/or max bounds the action's own parameter (or one another effect adds) instead of declaring it. Every bound must hold; out-of-range names that parameter, with boundBy. describeAction lists them as addedBounds. A bound on a name nothing declares is invalid-added-param (reason undeclared). Signed-off-by: Mitchel Pigsley --- .changeset/added-param-bounds.md | 13 ++++ apps/website/guides/rolling-dice.md | 2 + packages/engine/src/core/actions.ts | 57 ++++++++++++++--- packages/engine/src/core/describe.ts | 14 ++++- packages/engine/src/core/load.ts | 24 +++++++- packages/engine/src/core/pack-types.ts | 8 +++ packages/engine/src/core/upgrade.ts | 3 +- packages/engine/test/added-params.test.ts | 61 +++++++++++++++++++ packages/spec/data/error-codes.json | 4 +- packages/spec/docs/dice-and-actions.md | 20 ++++-- packages/spec/docs/effects.md | 4 +- packages/spec/docs/engine-actions.md | 5 +- packages/spec/docs/engine-interface.md | 6 +- packages/spec/i18n/en.yaml | 1 + packages/spec/schemas/effect.schema.json | 48 ++++++++++----- .../engine/describe-action.schema.json | 21 +++++++ packages/spec/src/types.ts | 7 +++ .../added-params-bound-own-describe/case.yaml | 7 +++ .../character.yaml | 7 +++ .../expected.yaml | 24 ++++++++ .../actions/added-params-bound-own/case.yaml | 8 +++ .../added-params-bound-own/character.yaml | 7 +++ .../added-params-bound-own/expected.yaml | 2 + .../added-params-bound-undeclared/case.yaml | 25 ++++++++ .../expected.yaml | 2 + .../content/features/grace.yaml | 12 ++++ .../packs/test-action-params/i18n/en.yaml | 3 + scripts/validate/checks.ts | 7 ++- 28 files changed, 361 insertions(+), 41 deletions(-) create mode 100644 .changeset/added-param-bounds.md create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own-describe/case.yaml create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own-describe/character.yaml create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own-describe/expected.yaml create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own/case.yaml create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own/character.yaml create mode 100644 packages/spec/tests/cases/actions/added-params-bound-own/expected.yaml create mode 100644 packages/spec/tests/cases/loads/added-params-bound-undeclared/case.yaml create mode 100644 packages/spec/tests/cases/loads/added-params-bound-undeclared/expected.yaml create mode 100644 packages/spec/tests/packs/test-action-params/content/features/grace.yaml diff --git a/.changeset/added-param-bounds.md b/.changeset/added-param-bounds.md new file mode 100644 index 000000000..297e40fe7 --- /dev/null +++ b/.changeset/added-param-bounds.md @@ -0,0 +1,13 @@ +--- +'@opencharacter/spec': minor +'@opencharacter/engine': minor +--- + +A `params` effect may add bounds to a parameter the run already has: an entry with only `min` +and/or `max` (no `type`) bounds the action's own parameter, or one another effect adds, instead +of declaring it. Every bound must hold; the `out-of-range` error names that parameter, with the +failing bound's `min` and `max` and `boundBy`, the content that added it. describeAction lists +them under the parameter's `addedBounds`. Two bounds on one name don't conflict; a bound on a +name no action declares and no effect adds is `invalid-added-param` (reason `undeclared`). So +Healing Grace's enhancements bound the ability's own `spend`, and an overspend names `spend` +(#252). diff --git a/apps/website/guides/rolling-dice.md b/apps/website/guides/rolling-dice.md index 719ee572b..f981682b8 100644 --- a/apps/website/guides/rolling-dice.md +++ b/apps/website/guides/rolling-dice.md @@ -70,6 +70,8 @@ const described = describeAction(ruleset, character, { - **A parameter the item's `use` fixes** has `fixed: true` and its `value`: don't ask for it. - **A parameter content adds** (a `params` effect: an ability's damage type chosen on each use, a feature's opt-in) comes after the action's own, with `addedBy`. Ask for it like the others. + Bounds content adds to a parameter are in its `addedBounds` (each with `addedBy`): every one + must hold, and an `out-of-range` error for one carries `boundBy`, the content that added it. - **Label each** with `text.param(definedBy, name)` (an added one with `text.addedParam(addedBy, name)`), or `text.lookup` with its own `label` key. - **`participants`** lists the action's roles with `min` and `max` diff --git a/packages/engine/src/core/actions.ts b/packages/engine/src/core/actions.ts index d726fb6ac..828e9e8f6 100644 --- a/packages/engine/src/core/actions.ts +++ b/packages/engine/src/core/actions.ts @@ -31,6 +31,7 @@ import type { } from './pack-types.ts'; import { innerSteps, + isAddedBound, optionQuery, pendingSource, stackPolicy, @@ -949,6 +950,10 @@ function params( ): Record { const out: Record = {}; const own = paramAction(frame); + const added = + frame.depth === 0 + ? addedParams(ctx.sheet, frame.def, frame.pack, frame.declared, frame.roots, frame) + : { params: [], bounds: [] }; const all: ParamSpec[] = [ ...Object.entries(frame.def.params ?? {}).map(([name, decl]): ParamSpec => ({ name, @@ -957,10 +962,7 @@ function params( roots: frame.roots, at: own, })), - ...(frame.depth === 0 - ? addedParams(ctx.sheet, frame.def, frame.pack, frame.declared, frame.roots, frame) - : [] - ).map((p): ParamSpec => ({ + ...added.params.map((p): ParamSpec => ({ name: p.name, decl: p.decl, pack: p.pack, @@ -968,8 +970,10 @@ function params( at: { ...own, addedBy: p.addedBy }, })), ]; - // Bounds read `params`, so they are checked once every parameter has its value. + // Bounds read `params`, so they are checked once every parameter has its value: each + // parameter's own, then those `params` effects add to it. const ranges: (() => void)[] = []; + const typedOf = new Map }>(); for (const { name, decl, pack, roots, at } of all) { let value: unknown = raw[name]; if (fixed[name] !== undefined) { @@ -1030,8 +1034,19 @@ function params( return v.kind === 'int' || v.kind === 'decimal' ? v.value : undefined; }), ); + typedOf.set(name, { value: typed, at }); out[name] = rootValue(typed); } + for (const bound of added.bounds) { + const param = typedOf.get(bound.name); + if (!param) continue; + ranges.push(() => + paramRange(bound.name, { ...param.at, boundBy: bound.addedBy }, bound, param.value, (b) => { + const v = ctx.sheet.evaluate(b, bound.pack, { ...bound.roots, params: out }); + return v.kind === 'int' || v.kind === 'decimal' ? v.value : undefined; + }), + ); + } for (const range of ranges) range(); return out; } @@ -1048,11 +1063,21 @@ export interface AddedParam { roots: Record; } +/** Bounds a `params` effect adds to a parameter the run already has. */ +export interface AddedBound { + name: string; + min?: Formula; + max?: Formula; + pack: string; + addedBy: string; + roots: Record; +} + /** * The parameters `params` effects add to an action as listed with these roots: each instance * whose target tags match the listing's tags, whose `from: self` (if any) names its entry and * whose `when` holds, in instance order. A name the action declares, or an earlier instance - * added, is left out. + * added, is left out. An entry with only bounds goes to `bounds`, for the parameter it names. */ export function addedParams( sheet: Sheet, @@ -1061,8 +1086,9 @@ export function addedParams( declared: string, roots: Record, owner: RunOwner, -): AddedParam[] { +): { params: AddedParam[]; bounds: AddedBound[] } { const out: AddedParam[] = []; + const bounds: AddedBound[] = []; let tags: string[] | undefined; for (const instance of sheet.instances) { const effect = instance.effect; @@ -1089,12 +1115,23 @@ export function addedParams( } if (!holds) continue; for (const [name, decl] of Object.entries(effect.value as Record)) { - if (Object.hasOwn(def.params ?? {}, name) || out.some((p) => p.name === name)) continue; const by = addedBy(sheet.ruleset, instance); + if (isAddedBound(decl)) { + bounds.push({ + name, + ...(decl.min !== undefined ? { min: decl.min } : {}), + ...(decl.max !== undefined ? { max: decl.max } : {}), + pack: instance.pack, + addedBy: by, + roots: own, + }); + continue; + } + if (Object.hasOwn(def.params ?? {}, name) || out.some((p) => p.name === name)) continue; out.push({ name, decl, pack: instance.pack, addedBy: by, roots: own }); } } - return out; + return { params: out, bounds }; } /** The content an added option or parameter is named by: a type's own effects by the type. */ @@ -1162,7 +1199,7 @@ export function contentParamTakes( function paramRange( name: string, at: Record, - decl: ValueDecl, + decl: { min?: Formula; max?: Formula }, value: f.Value, boundOf: (bound: Formula) => number | undefined, ): void { diff --git a/packages/engine/src/core/describe.ts b/packages/engine/src/core/describe.ts index b02554463..6c34c7be9 100644 --- a/packages/engine/src/core/describe.ts +++ b/packages/engine/src/core/describe.ts @@ -61,6 +61,7 @@ export function describeAction( }; // The action's own, then those `params` effects add, their bounds seeing the effect's roots. + const added = addedParams(sheet, def, pack, declared, roots, { entry, source, activation }); const all: { name: string; decl: ValueDecl; @@ -69,7 +70,7 @@ export function describeAction( by?: string; }[] = [ ...Object.entries(def.params ?? {}).map(([name, decl]) => ({ name, decl, pack, roots })), - ...addedParams(sheet, def, pack, declared, roots, { entry, source, activation }).map((p) => ({ + ...added.params.map((p) => ({ name: p.name, decl: p.decl, pack: p.pack, @@ -95,6 +96,17 @@ export function describeAction( const max = number(decl.max, declPack, seen); if (min !== undefined) out.min = min; if (max !== undefined) out.max = max; + const bounds = added.bounds.filter((b) => b.name === name); + if (bounds.length > 0) + out.addedBounds = bounds.map((b) => { + const bmin = number(b.min, b.pack, b.roots); + const bmax = number(b.max, b.pack, b.roots); + return { + addedBy: b.addedBy, + ...(bmin !== undefined ? { min: bmin } : {}), + ...(bmax !== undefined ? { max: bmax } : {}), + }; + }); if (decl.values) out.values = [...decl.values]; const options = paramOptions(ruleset, sheet, decl, declPack, roots); if (options) out.options = options; diff --git a/packages/engine/src/core/load.ts b/packages/engine/src/core/load.ts index 348d8f5cb..7f583757f 100644 --- a/packages/engine/src/core/load.ts +++ b/packages/engine/src/core/load.ts @@ -36,7 +36,7 @@ import { thenToSelf, type EnumAddress, } from './pack-types.ts'; -import { bandSet, innerSteps, stepArms } from './pack-types.ts'; +import { bandSet, innerSteps, isAddedBound, stepArms } from './pack-types.ts'; import type { PackRelease } from './release.ts'; import { expandFamilies } from './families.ts'; import { @@ -168,6 +168,8 @@ function checkAddedParams( err: (code: string, data: Record) => void, ): Added[] { const added: Added[] = []; + // Entries with only `min` and `max`: bounds on a parameter the run has, not declarations. + const bounds: { source: string; name: string }[] = []; const walkChoices = ( choices: readonly Choice[] | undefined, source: string, @@ -193,7 +195,16 @@ function checkAddedParams( if (e.op !== 'params' || typeof e.target !== 'object' || !('actions' in e.target)) continue; const self = e.target.from === 'self' ? item : undefined; for (const [name, decl] of Object.entries((e.value ?? {}) as Record)) - added.push({ source, pack, tags: e.target.actions, name, decl, ...(self ? { self } : {}) }); + if (isAddedBound(decl)) bounds.push({ source, name }); + else + added.push({ + source, + pack, + tags: e.target.actions, + name, + decl, + ...(self ? { self } : {}), + }); } }; for (const type of Object.values(ruleset.contentTypes)) { @@ -209,7 +220,7 @@ function checkAddedParams( for (const e of ruleset.rules.effects) walk([e], `${e.pack}:rules`, e.pack); for (const choice of Object.values(ruleset.rules.choices)) walkChoices([choice], `${choice.pack}:rules`, choice.pack); - if (added.length === 0) return added; + if (added.length === 0 && bounds.length === 0) return added; const actions: { key: string; pack: string; def: ActionDef }[] = [ ...Object.entries(ruleset.rules.actions).map(([key, def]) => ({ key, pack: def.pack, def })), @@ -226,6 +237,13 @@ function checkAddedParams( ), ), ]; + for (const { source, name } of bounds) + if ( + !actions.some((a) => Object.hasOwn(a.def.params ?? {}, name)) && + !added.some((a) => a.name === name) + ) + err('invalid-added-param', { param: name, source, reason: 'undeclared' }); + if (added.length === 0) return added; const literal = actions.map((a) => ({ ...a, tags: writtenTags(a.def.tags) })); // Whether an effect may reach an action: a target tag meets one of its literal tags, over packs // that meet on that target tag. diff --git a/packages/engine/src/core/pack-types.ts b/packages/engine/src/core/pack-types.ts index aac487593..bae42f622 100644 --- a/packages/engine/src/core/pack-types.ts +++ b/packages/engine/src/core/pack-types.ts @@ -845,6 +845,14 @@ export type { UpgradeScript as UpgradeDoc } from '@opencharacter/spec'; export type { ChoicePick as Pick, ChoiceTree, Entry } from '@opencharacter/spec'; export type { CharacterFile as CharacterDoc } from '@opencharacter/spec'; +/** + * A `params` effect's entry with no `type`, only `min` and `max`: bounds on a parameter the run + * already has, not a declaration. + */ +export function isAddedBound(decl: ValueDecl): boolean { + return (decl as { type?: string }).type === undefined; +} + export function appliesQueries(item: { appliesTo?: Query | Query[] | undefined }): Query[] { const a = item.appliesTo; return a === undefined ? [] : Array.isArray(a) ? a : [a]; diff --git a/packages/engine/src/core/upgrade.ts b/packages/engine/src/core/upgrade.ts index 5a85d0e78..ce9e6f2ed 100644 --- a/packages/engine/src/core/upgrade.ts +++ b/packages/engine/src/core/upgrade.ts @@ -22,7 +22,7 @@ import type { Pick, ValueDecl, } from './pack-types.ts'; -import { appliesQueries } from './pack-types.ts'; +import { appliesQueries, isAddedBound } from './pack-types.ts'; import type { PackRelease } from './release.ts'; import type { Ruleset } from './ruleset.ts'; import { homebrewFormulas, withHomebrew } from './homebrew.ts'; @@ -1475,6 +1475,7 @@ function actionDecls(release: PackRelease): ActionDecls { for (const effect of effects ?? []) { if (effect.op !== 'params' || typeof effect.value !== 'object') continue; for (const [name, decl] of Object.entries(effect.value as Record)) { + if (isAddedBound(decl)) continue; const id = `${key}.params.${name}`; const found = out.params.get(id); if (found) found.decls.push(decl); diff --git a/packages/engine/test/added-params.test.ts b/packages/engine/test/added-params.test.ts index e92eb7d7d..f429ef7c9 100644 --- a/packages/engine/test/added-params.test.ts +++ b/packages/engine/test/added-params.test.ts @@ -237,3 +237,64 @@ describe('parameter bounds that read params', () => { expect(described.action.params.find((p) => p.name === 'recoveries')).not.toHaveProperty('max'); }); }); + +/** A character with these features of pack `p`. */ +const holding = (ids: string[]) => ({ + specVersion: '0.3.0', + id: 'hero', + packs: [{ id: 'p', version: '1.0.0' }], + features: ids.map((id) => ({ id, item: `p:features.${id}` })), +}); + +describe('bounds a params effect adds to a parameter the run has', () => { + // Healing Grace: the enhancements bound the ability's own spend, so an overspend names it. + const grace = adds('ability', { + stand: { type: 'boolean', default: false }, + recoveries: { type: 'integer', default: 0, min: 0 }, + surges: { min: 'params.recoveries + (params.stand ? 1 : 0)' }, + }); + const capped = adds('ability', { surges: { max: 2 } }); + const loaded = loadRuleset([pack({ grace: [grace], capped: [capped] })]); + if (!loaded.ok) throw new Error(JSON.stringify(loaded.errors)); + const run = (ids: string[], params: Record) => { + const result = startAction(loaded.ruleset, holding(ids), { + action: { id: 'p:use-ability' }, + params, + }); + return 'errors' in result && result.errors ? result.errors.map((e) => e.data) : 'ok'; + }; + + it('load: a bound is no declaration, and two bounds on one name do not conflict', () => { + expect(reasons(pack({ grace: [grace], capped: [capped] }))).toEqual([]); + }); + + it('load: a bound on a name no action declares and no effect adds is undeclared', () => { + expect(reasons(pack({ a: [adds('ability', { spend: { min: 1 } })] }))).toEqual([ + ['invalid-added-param', 'undeclared'], + ]); + }); + + it('every bound must hold; the error names the parameter and who added the bound', () => { + expect(run(['grace'], { surges: 2, recoveries: 1, stand: true })).toBe('ok'); + expect(run(['grace'], { surges: 1, recoveries: 1, stand: true })).toEqual([ + { target: 'surges', min: 2, action: 'p:use-ability', boundBy: 'p:features.grace' }, + ]); + expect(run(['grace', 'capped'], { surges: 3, recoveries: 3 })).toEqual([ + { target: 'surges', max: 2, action: 'p:use-ability', boundBy: 'p:features.capped' }, + ]); + // Without the feature, no bound. + expect(run([], { surges: 3 })).toBe('ok'); + }); + + it('describeAction lists them under the parameter', () => { + const described = describeAction(loaded.ruleset, holding(['grace', 'capped']), { + id: 'p:use-ability', + }); + if (!('action' in described)) throw new Error(JSON.stringify(described)); + expect(described.action.params.map((p) => p.name)).toEqual(['surges', 'stand', 'recoveries']); + expect(described.action.params[0]?.addedBounds).toEqual([ + { addedBy: 'p:features.grace' }, + { addedBy: 'p:features.capped', max: 2 }, + ]); + }); +}); diff --git a/packages/spec/data/error-codes.json b/packages/spec/data/error-codes.json index d5c6c5a2d..021f7e8f8 100644 --- a/packages/spec/data/error-codes.json +++ b/packages/spec/data/error-codes.json @@ -36,7 +36,7 @@ { "id": "out-of-range", "kind": "runtime", - "description": "A value is outside the `min` and `max` of its field, state key or action parameter (for a quantity, below 0). Data: `target` (the field ID, state key, parameter name or `quantity`), and `min`, `max` or both: only the bounds the target declares. For an action parameter, also `action` (the qualified action ID; `#` for an item's inline action), and `entry` and `source` when the action was started on them, so text names the parameter by its label (`actions..params..name`); for a parameter a `params` effect adds, also `addedBy`, the content whose text names it (`.params..name`)." + "description": "A value is outside the `min` and `max` of its field, state key or action parameter (for a quantity, below 0). Data: `target` (the field ID, state key, parameter name or `quantity`), and `min`, `max` or both: only the bounds the target declares. For an action parameter, also `action` (the qualified action ID; `#` for an item's inline action), and `entry` and `source` when the action was started on them, so text names the parameter by its label (`actions..params..name`); for a parameter a `params` effect adds, also `addedBy`, the content whose text names it (`.params..name`); for a bound a `params` effect adds to a parameter, `min` and `max` are that bound's and `boundBy` is the content that added it." }, { "id": "not-owned", @@ -271,7 +271,7 @@ { "id": "invalid-added-param", "kind": "load", - "description": "A `params` effect's parameter can't be added: it is neither `optional` nor has a `default` (reason `required`), an action its target tags reach by its literal tags declares a parameter of that name (reason `declared`, data `action`), or another `params` effect adds that name with another declaration (reason `conflict`, data `other`): their target tags meet (one is the other or below it) over packs that meet on that tag, or one action's literal tags meet a target tag of each; two `from: self` effects of different content items conflict only when one item names both (itself, its `grants`, its choices' `items` and inline options) and has an action, its own or its type's, that both reach. An action counts by the pack that wrote it and only when the effect reaches it over packs that meet (effects.md, Which packs a tag target reaches). Data: `param`, `source` (the effect's content, `:types.` or `:rules`), `reason`." + "description": "A `params` effect's parameter can't be added: it is neither `optional` nor has a `default` (reason `required`), an action its target tags reach by its literal tags declares a parameter of that name (reason `declared`, data `action`), or another `params` effect adds that name with another declaration (reason `conflict`, data `other`), or an entry that only adds bounds (`min`, `max`) names a parameter no action declares and no `params` effect adds (reason `undeclared`): their target tags meet (one is the other or below it) over packs that meet on that tag, or one action's literal tags meet a target tag of each; two `from: self` effects of different content items conflict only when one item names both (itself, its `grants`, its choices' `items` and inline options) and has an action, its own or its type's, that both reach. An action counts by the pack that wrote it and only when the effect reaches it over packs that meet (effects.md, Which packs a tag target reaches). Data: `param`, `source` (the effect's content, `:types.` or `:rules`), `reason`." }, { "id": "circular-extends", diff --git a/packages/spec/docs/dice-and-actions.md b/packages/spec/docs/dice-and-actions.md index 969dd176d..14f3937b1 100644 --- a/packages/spec/docs/dice-and-actions.md +++ b/packages/spec/docs/dice-and-actions.md @@ -127,16 +127,28 @@ chooses its damage type on each use, and a feature adds an opt-in to a rules act - Bounds, an action's own and added ones alike, are checked once every parameter has its value (the app's, a `use`'s fixed one, or the default), so one parameter bounds another. A bound reads values, never another bound, so bounds that read each other can't loop. An optional parameter - left out is absent: read it behind `has()` (`has(params.cap) ? params.cap : 10`). Healing Grace spends - one piety per enhancement: + left out is absent: read it behind `has()` (`has(params.cap) ? params.cap : 10`). +- **Bounds on a parameter the run has.** An entry naming a parameter the run already has (the + action's own, or one another `params` effect adds) with only `min` and/or `max` (no `type`, + `default` or `optional`) adds those bounds to it instead of declaring it. They see the + effect's roots and `params`, as an added parameter's do, and are checked with the parameter's + own: every bound must hold. The `out-of-range` error names that parameter, with `min` and + `max` those of the failing bound and `boundBy` the content that added it; describeAction lists + them under the parameter's `addedBounds`, each with its `addedBy`. Two effects bounding one + parameter don't conflict. An entry with a `type` stays a declaration (`invalid-added-param` + when the action declares the name); one naming a parameter no action declares and no `params` + effect adds is `invalid-added-param` (reason `undeclared`). A bound on a parameter a run + doesn't have (an action its tags reach through computed tags that lacks it) is ignored. + Healing Grace spends one piety per enhancement, so the enhancements bound the ability's own + `spend`, and an overspend names `spend`: ```yaml - target: { actions: [ability], from: self } op: params value: - graceRecoveries: - { type: integer, default: 0, min: 0, max: 'params.spend - (params.graceStand ? 1 : 0)' } + graceRecoveries: { type: integer, default: 0, min: 0 } graceStand: { type: boolean, default: false } + spend: { min: 'params.graceRecoveries + (params.graceStand ? 1 : 0)' } ``` - A `use` may fix an added parameter in its `params`, as its own. diff --git a/packages/spec/docs/effects.md b/packages/spec/docs/effects.md index 571702953..a56f1f4d7 100644 --- a/packages/spec/docs/effects.md +++ b/packages/spec/docs/effects.md @@ -113,7 +113,9 @@ effects: actions and effects of packs that meet ([Which packs a tag target reaches](#which-packs-a-tag-target-reaches)). The action's own formulas only see the parameters it declares; content reads an added one as `params.` in its roll, option and `changes` - effects. Text: `.params..name` (and `.values..name`), the content + effects. An entry naming a parameter the run already has, with only `min` and/or `max`, adds + bounds to it instead: every bound must hold, and the `out-of-range` error names that parameter + with `boundBy` ([dice-and-actions.md](dice-and-actions.md#actions)). Text: `.params..name` (and `.values..name`), the content key the item's, `types.` for a content type's effect, `rules` for the rules'. Content items, templates, content types and the rules' effects add parameters, as for options. See [dice-and-actions.md](dice-and-actions.md#actions). Removing or retyping an added parameter diff --git a/packages/spec/docs/engine-actions.md b/packages/spec/docs/engine-actions.md index 5e9339eae..7ba66965b 100644 --- a/packages/spec/docs/engine-actions.md +++ b/packages/spec/docs/engine-actions.md @@ -57,7 +57,10 @@ shape, an optional **dice function**. the action's `item` and `self`, `situational`, the character's fields and `params`, every parameter as the action's formulas will read it, but no `steps` or `flags` (`amount: { type: integer, min: 1, max: effort.free }`, - `recoveries: { max: params.spend }`). An optional parameter left out is absent, so a bound + `recoveries: { max: params.spend }`). Bounds a `params` effect adds to a parameter (an entry + with only `min` and `max`) are checked with its own, after them, each in its effect's pack + and roots; the error for one carries that bound's `min` and `max` and `boundBy`, the content + that added it. An optional parameter left out is absent, so a bound reads it behind `has()` (`max: 'has(params.cap) ? params.cap : 10'`). Every parameter's missing, type and option errors come first, as each takes its value; then the bounds are checked in order, the first out of range failing the action. A bound reads values, never diff --git a/packages/spec/docs/engine-interface.md b/packages/spec/docs/engine-interface.md index 2463d4fc7..f757e83ff 100644 --- a/packages/spec/docs/engine-interface.md +++ b/packages/spec/docs/engine-interface.md @@ -222,14 +222,16 @@ the definition, wherever it lives: a rules action, one inline on an item or on i - `tags` are the action's tags for this listing, computed ones evaluated. - `params` lists the parameters in declaration order, then those `params` effects add ([effects.md](effects.md#targets)), each `{ name, type, of?, required, default?, fixed?, - value?, min?, max?, values?, options?, unique?, label?, addedBy? }`. An added one has + value?, min?, max?, addedBounds?, values?, options?, unique?, label?, addedBy? }`. An added one has `addedBy`, the content whose text names it (``, `:types.` or `:rules`; its key `.params..name`). `required` is true when the parameter is neither `optional`, defaulted nor fixed. A parameter a `use` fixes has `fixed: true` and its `value` as startAction evaluates it; the app's is ignored. `min` and `max` are evaluated as startAction evaluates them, with every situational choice at its default; one that fails, or that reads `params` (it depends on the values the app is yet to - give), is left out. + give), is left out. `addedBounds` lists the bounds `params` effects add to the parameter + ([dice-and-actions.md](dice-and-actions.md#actions)), each `{ addedBy, min?, max? }` + evaluated likewise. - `values` is an enum's values, those dependent packs add included. `options` lists every ID an `id`, `set` or `list` parameter of a content type, `field` or `tag` takes, its `where` applied: content in ruleset order (hidden items left out), fields in ruleset order, tags diff --git a/packages/spec/i18n/en.yaml b/packages/spec/i18n/en.yaml index e44216187..e38b46552 100644 --- a/packages/spec/i18n/en.yaml +++ b/packages/spec/i18n/en.yaml @@ -86,6 +86,7 @@ errors.invalid-added-param: 'The parameter "{param}" that {source} adds to actio errors.invalid-added-param.required: 'The parameter "{param}" that {source} adds to actions must be optional or have a default.' errors.invalid-added-param.declared: 'The parameter "{param}" that {source} adds is already a parameter of {action}.' errors.invalid-added-param.conflict: '{source} and {other} both add the parameter "{param}", declared differently.' +errors.invalid-added-param.undeclared: '{source} adds bounds to the parameter "{param}", which no action declares.' errors.circular-extends: 'These items extend each other in a loop.' errors.safety-limit-exceeded: 'A safety limit was reached ({limit}).' # `safety-limit-exceeded` by its `limit` (data/safety-limits.json). diff --git a/packages/spec/schemas/effect.schema.json b/packages/spec/schemas/effect.schema.json index 2a284e3ca..5654f5289 100644 --- a/packages/spec/schemas/effect.schema.json +++ b/packages/spec/schemas/effect.schema.json @@ -129,33 +129,51 @@ "$ref": "#/$defs/actionTarget" }, "value": { - "description": "The parameters added, declared as an action's own `params`: name to declaration.", + "description": "The parameters added, declared as an action's own `params`: name to declaration; or, for a name the run already has, only `min` and `max` bounds added to it.", "type": "object", "propertyNames": { "$ref": "common.schema.json#/$defs/name" }, "additionalProperties": { - "description": "Optional or defaulted: only the run the app starts has it, and it may leave it out.", - "allOf": [ - { - "$ref": "content-type.schema.json#/$defs/valueDecl" - }, + "anyOf": [ { - "anyOf": [ + "description": "A parameter added: optional or defaulted, since only the run the app starts has it, and the app may leave it out.", + "allOf": [ { - "type": "object", - "required": ["default"] + "$ref": "content-type.schema.json#/$defs/valueDecl" }, { - "type": "object", - "required": ["optional"], - "properties": { - "optional": { - "const": true + "anyOf": [ + { + "type": "object", + "required": ["default"] + }, + { + "type": "object", + "required": ["optional"], + "properties": { + "optional": { + "const": true + } + } } - } + ] } ] + }, + { + "description": "Bounds added to a parameter the run already has (the action's own or one another `params` effect adds): only `min` and/or `max`, read as a parameter's. They are checked with its own and every one must hold; the `out-of-range` error names the parameter, with `boundBy` the content that added the failing bound.", + "type": "object", + "properties": { + "min": { + "$ref": "formula.schema.json" + }, + "max": { + "$ref": "formula.schema.json" + } + }, + "additionalProperties": false, + "minProperties": 1 } ] }, diff --git a/packages/spec/schemas/engine/describe-action.schema.json b/packages/spec/schemas/engine/describe-action.schema.json index 03d62576a..723e86be4 100644 --- a/packages/spec/schemas/engine/describe-action.schema.json +++ b/packages/spec/schemas/engine/describe-action.schema.json @@ -79,6 +79,27 @@ "description": "The `max` bound, evaluated likewise.", "type": "number" }, + "addedBounds": { + "description": "Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`.", + "type": "array", + "items": { + "type": "object", + "required": ["addedBy"], + "properties": { + "addedBy": { + "description": "The content that added it, as a parameter's `addedBy`.", + "type": "string" + }, + "min": { + "type": "number" + }, + "max": { + "type": "number" + } + }, + "additionalProperties": false + } + }, "values": { "description": "An enum's values, with those dependent packs add (`extendEnums`).", "type": "array", diff --git a/packages/spec/src/types.ts b/packages/spec/src/types.ts index ef6979965..d60df85e6 100644 --- a/packages/spec/src/types.ts +++ b/packages/spec/src/types.ts @@ -2443,6 +2443,13 @@ export type DescribedParam = { min?: number; /** The `max` bound, evaluated likewise. */ max?: number; + /** Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`. */ + addedBounds?: { + /** The content that added it, as a parameter's `addedBy`. */ + addedBy: string; + min?: number; + max?: number; + }[]; /** An enum's values, with those dependent packs add (`extendEnums`). */ values?: string[]; /** id/set/list of a content type, `field` or `tag`: every ID startAction takes, its `where` applied. Content in ruleset order (hidden items left out), fields in ruleset order, tags sorted. */ diff --git a/packages/spec/tests/cases/actions/added-params-bound-own-describe/case.yaml b/packages/spec/tests/cases/actions/added-params-bound-own-describe/case.yaml new file mode 100644 index 000000000..f9485b444 --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own-describe/case.yaml @@ -0,0 +1,7 @@ +description: "describeAction lists the bounds a `params` effect adds to the action's own parameter under it, with `addedBy`; a bound that reads `params` is left out" +specVersion: 0.3.0 +call: describeAction +packs: + - path: ../../../packs/test-action-params +input: + action: { id: test-action-params:use-ability, entry: spike } diff --git a/packages/spec/tests/cases/actions/added-params-bound-own-describe/character.yaml b/packages/spec/tests/cases/actions/added-params-bound-own-describe/character.yaml new file mode 100644 index 000000000..316424b1b --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own-describe/character.yaml @@ -0,0 +1,7 @@ +specVersion: 0.3.0 +id: hero +packs: + - { id: test-action-params, version: 0.1.0 } +features: + - { id: spike, item: test-action-params:abilities.mind-spike } + - { id: grace, item: test-action-params:features.grace } diff --git a/packages/spec/tests/cases/actions/added-params-bound-own-describe/expected.yaml b/packages/spec/tests/cases/actions/added-params-bound-own-describe/expected.yaml new file mode 100644 index 000000000..1a07346b7 --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own-describe/expected.yaml @@ -0,0 +1,24 @@ +action: + definedBy: test-action-params:use-ability + tags: [ability, usage.maneuver, kw.psionic] + results: [used] + params: + - name: surges + type: integer + required: false + default: 0 + min: 0 + addedBounds: + - { addedBy: test-action-params:features.grace, max: 4 } + - name: graceStand + type: boolean + required: false + default: false + addedBy: test-action-params:features.grace + - name: graceEnd + type: boolean + required: false + default: false + addedBy: test-action-params:features.grace + participants: + - { role: target, min: 0, max: 1 } diff --git a/packages/spec/tests/cases/actions/added-params-bound-own/case.yaml b/packages/spec/tests/cases/actions/added-params-bound-own/case.yaml new file mode 100644 index 000000000..6a6b93664 --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own/case.yaml @@ -0,0 +1,8 @@ +description: "A `params` effect's entry with only bounds bounds the shared action's own parameter: two enhancements with one surge fail on `surges`, not on an added parameter" +specVersion: 0.3.0 +call: startAction +packs: + - path: ../../../packs/test-action-params +input: + action: { id: 'test-action-params:use-ability', entry: spike } + params: { surges: 1, graceStand: true, graceEnd: true } diff --git a/packages/spec/tests/cases/actions/added-params-bound-own/character.yaml b/packages/spec/tests/cases/actions/added-params-bound-own/character.yaml new file mode 100644 index 000000000..316424b1b --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own/character.yaml @@ -0,0 +1,7 @@ +specVersion: 0.3.0 +id: hero +packs: + - { id: test-action-params, version: 0.1.0 } +features: + - { id: spike, item: test-action-params:abilities.mind-spike } + - { id: grace, item: test-action-params:features.grace } diff --git a/packages/spec/tests/cases/actions/added-params-bound-own/expected.yaml b/packages/spec/tests/cases/actions/added-params-bound-own/expected.yaml new file mode 100644 index 000000000..5d79fae01 --- /dev/null +++ b/packages/spec/tests/cases/actions/added-params-bound-own/expected.yaml @@ -0,0 +1,2 @@ +# Two enhancements need two surges: `surges` is out of range, `boundBy` the feature. +errors: [out-of-range] diff --git a/packages/spec/tests/cases/loads/added-params-bound-undeclared/case.yaml b/packages/spec/tests/cases/loads/added-params-bound-undeclared/case.yaml new file mode 100644 index 000000000..b17fa0f0c --- /dev/null +++ b/packages/spec/tests/cases/loads/added-params-bound-undeclared/case.yaml @@ -0,0 +1,25 @@ +description: "A `params` effect's entry with only bounds names a parameter no action declares and no `params` effect adds: `invalid-added-param` (reason `undeclared`)" +specVersion: 0.3.0 +call: loadRuleset +packs: + - inline: + manifest: + id: test-pack + version: 1.0.0 + specVersion: 0.3.0 + license: CC0-1.0 + defaultLanguage: en + rules: + tags: [attack] + actions: + - id: strike + tags: [attack] + params: + spend: { type: integer, default: 0 } + steps: [{ id: done, kind: outcome }] + effects: + - target: { actions: [attack] } + op: params + value: + spent: { min: 1 } + content: [] diff --git a/packages/spec/tests/cases/loads/added-params-bound-undeclared/expected.yaml b/packages/spec/tests/cases/loads/added-params-bound-undeclared/expected.yaml new file mode 100644 index 000000000..7567bb9a9 --- /dev/null +++ b/packages/spec/tests/cases/loads/added-params-bound-undeclared/expected.yaml @@ -0,0 +1,2 @@ +errors: + - { code: invalid-added-param } diff --git a/packages/spec/tests/packs/test-action-params/content/features/grace.yaml b/packages/spec/tests/packs/test-action-params/content/features/grace.yaml new file mode 100644 index 000000000..34a644448 --- /dev/null +++ b/packages/spec/tests/packs/test-action-params/content/features/grace.yaml @@ -0,0 +1,12 @@ +# Healing Grace: each enhancement costs one surge, so the enhancements bound the shared action's +# own `surges` (an entry with only bounds), and the feature's limit caps it. +id: features.grace +type: feature +fields: { limit: 4 } +effects: + - target: { actions: [ability] } + op: params + value: + graceStand: { type: boolean, default: false } + graceEnd: { type: boolean, default: false } + surges: { min: '(params.graceStand ? 1 : 0) + (params.graceEnd ? 1 : 0)', max: item.limit } diff --git a/packages/spec/tests/packs/test-action-params/i18n/en.yaml b/packages/spec/tests/packs/test-action-params/i18n/en.yaml index ff597dfea..e0449664b 100644 --- a/packages/spec/tests/packs/test-action-params/i18n/en.yaml +++ b/packages/spec/tests/packs/test-action-params/i18n/en.yaml @@ -43,3 +43,6 @@ features.appeal.name: 'Appeal to the Muses' features.appeal.params.appeal.name: 'Appeal' features.appeal.params.verses.name: 'Verses' conditions.dazed.name: 'Dazed' +features.grace.name: 'Healing Grace' +features.grace.params.graceStand.name: 'Stand' +features.grace.params.graceEnd.name: 'End an effect' diff --git a/scripts/validate/checks.ts b/scripts/validate/checks.ts index fd04f2988..9c785dc30 100644 --- a/scripts/validate/checks.ts +++ b/scripts/validate/checks.ts @@ -34,6 +34,7 @@ import { appliesQueries, enumAddress, innerSteps, + isAddedBound, stepArms, thenRule, thenToSelf, @@ -1908,6 +1909,8 @@ export class PackChecker { ); else for (const [name, decl] of Object.entries(effect.value as Record)) { + // An entry with only bounds adds to a parameter named elsewhere. + if (isAddedBound(decl)) continue; const key = `${this.contentOwner}.params.${name}`; this.requiredKeys.add(decl.label ?? `${key}.name`); this.optionalKeys.add(`${key}.description`); @@ -2078,7 +2081,9 @@ export class PackChecker { const names = new Set(); const add = (effects: readonly Effect[] | undefined) => { for (const e of effects ?? []) - if (e.op === 'params') for (const name of Object.keys(e.value ?? {})) names.add(name); + if (e.op === 'params') + for (const [name, decl] of Object.entries((e.value ?? {}) as Record)) + if (!isAddedBound(decl)) names.add(name); }; for (const pid of this.scope) { const rules = this.ruleset.rules.get(pid); From b8323b9f87075c6626c2b09a3b6ce602f188983e Mon Sep 17 00:00:00 2001 From: Mitchel Pigsley Date: Mon, 2 Nov 2026 10:00:19 -0700 Subject: [PATCH 2/4] Spec: derived parameters, and computed values in action-time effects (#251) A parameter may declare a formula instead of being given: worked out as the action starts, after the given ones and before bounds, in declaration order, reading only earlier derived ones (a later one is unknown-name at load). Giving one is the new error invalid-param; describeAction marks it derived. Effects that act as an action runs (rolls, choice and changes steps, listed actions, incoming changes) read their content's computed values as self., as its actions do. Field effects still can't: computed values are worked out after every field. Signed-off-by: Mitchel Pigsley --- .../derived-params-computed-in-effects.md | 18 ++ apps/website/glossary.md | 3 +- apps/website/guides/rolling-dice.md | 2 + packages/engine/src/check/formulas.ts | 55 ++++++ packages/engine/src/core/actions.ts | 65 +++++-- packages/engine/src/core/changes.ts | 2 +- packages/engine/src/core/describe.ts | 7 +- packages/engine/src/core/load.ts | 12 +- packages/engine/src/core/pack-types.ts | 2 + packages/engine/src/core/sheet.ts | 41 +++++ packages/engine/src/core/upgrade.ts | 13 +- packages/engine/src/formula/check.ts | 2 +- .../engine/test/computed-in-effects.test.ts | 126 ++++++++++++++ packages/engine/test/derived-params.test.ts | 160 ++++++++++++++++++ packages/spec/data/error-codes.json | 7 +- packages/spec/docs/content.md | 19 ++- packages/spec/docs/dice-and-actions.md | 40 ++++- packages/spec/docs/effects.md | 4 +- packages/spec/docs/engine-actions.md | 9 +- packages/spec/docs/engine-interface.md | 5 +- packages/spec/i18n/en.yaml | 3 +- packages/spec/schemas/action.schema.json | 2 +- .../spec/schemas/content-type.schema.json | 31 +++- packages/spec/schemas/effect.schema.json | 6 +- .../engine/describe-action.schema.json | 4 + packages/spec/src/types.ts | 9 +- .../actions/derived-param-describe/case.yaml | 7 + .../derived-param-describe/character.yaml | 7 + .../derived-param-describe/expected.yaml | 18 ++ .../actions/derived-param-given/case.yaml | 8 + .../derived-param-given/character.yaml | 7 + .../actions/derived-param-given/expected.yaml | 1 + .../cases/actions/derived-param-run/case.yaml | 11 ++ .../actions/derived-param-run/character.yaml | 7 + .../actions/derived-param-run/expected.yaml | 12 ++ .../loads/computed-in-field-effect/case.yaml | 26 +++ .../computed-in-field-effect/expected.yaml | 2 + .../loads/derived-param-reads-later/case.yaml | 20 +++ .../derived-param-reads-later/expected.yaml | 2 + .../content/features/overload.yaml | 19 +++ .../packs/test-action-params/i18n/en.yaml | 2 + scripts/validate/action-params.test.ts | 24 +++ scripts/validate/checks.ts | 37 +++- 43 files changed, 809 insertions(+), 48 deletions(-) create mode 100644 .changeset/derived-params-computed-in-effects.md create mode 100644 packages/engine/test/computed-in-effects.test.ts create mode 100644 packages/engine/test/derived-params.test.ts create mode 100644 packages/spec/tests/cases/actions/derived-param-describe/case.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-describe/character.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-describe/expected.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-given/case.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-given/character.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-given/expected.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-run/case.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-run/character.yaml create mode 100644 packages/spec/tests/cases/actions/derived-param-run/expected.yaml create mode 100644 packages/spec/tests/cases/loads/computed-in-field-effect/case.yaml create mode 100644 packages/spec/tests/cases/loads/computed-in-field-effect/expected.yaml create mode 100644 packages/spec/tests/cases/loads/derived-param-reads-later/case.yaml create mode 100644 packages/spec/tests/cases/loads/derived-param-reads-later/expected.yaml create mode 100644 packages/spec/tests/packs/test-action-params/content/features/overload.yaml diff --git a/.changeset/derived-params-computed-in-effects.md b/.changeset/derived-params-computed-in-effects.md new file mode 100644 index 000000000..c8a6264e5 --- /dev/null +++ b/.changeset/derived-params-computed-in-effects.md @@ -0,0 +1,18 @@ +--- +'@opencharacter/spec': minor +'@opencharacter/engine': minor +--- + +Derived parameters: an action parameter (its own, or one a `params` effect adds) may declare a +`formula` instead of being given. startAction works it out once every given parameter has its +value and before any bound, in declaration order, reading the given parameters and the derived +ones before it (a later one is `unknown-name` at load, so they can't loop), and the run's +`participants`, now checked before the parameters (`invalid-participants` comes first); everything reads it +as `params.`. A value for it from the app, a `use`, an `action` step or a handoff is the new +error `invalid-param` (reason `derived`). describeAction lists it with `derived: true`. + +Effects that act as an action runs (on rolls, choice and changes steps, listed actions and +incoming changes) read their content's computed values as `self.`, as its actions do. A +field effect still can't (`unknown-name` at load): computed values are worked out after every +field, so one feeding a field could loop. So Cunning Strike's cost is worked out once, and its +DC is a computed value its strike effects read (#251). diff --git a/apps/website/glossary.md b/apps/website/glossary.md index f0bcdaeca..c2eb63f70 100644 --- a/apps/website/glossary.md +++ b/apps/website/glossary.md @@ -23,7 +23,7 @@ One-line definitions of OCF terms, consistent with the [specification](../../pac - **Choice path:** the address of a choice: an optional entry, then alternating choice IDs and picked options. - **Clock action:** a rules action the app runs at a moment of its clock on every character: `clock: turn-boundary` at each turn change, with the IDs of the characters whose turn ended and started. - **Competing options:** several active `set` effects on one field, or the effects of a `playerChooses` stacking group (one group across every field for a key spanning fields, an option per source); the engine reports them and the recommended one, and the app picks. -- **Computed value:** a value worked out from the character for each copy of an item (a FitD cohort's quality), or for picked or granted content (a Draw Steel ability's damage), declared by its content type, item or templates, never saved; computeSheet returns it in `copies` or `activationValues`, its actions and follow-ups read it as `self.`, and its text as `{self_}`. +- **Computed value:** a value worked out from the character for each copy of an item (a FitD cohort's quality), or for picked or granted content (a Draw Steel ability's damage), declared by its content type, item or templates, never saved; computeSheet returns it in `copies` or `activationValues`, its actions, follow-ups and the effects that act as they run (on rolls, steps, listed actions and incoming changes) read it as `self.`, and its text as `{self_}`. - **Condition:** a formula that must evaluate to true or false (`when`, `showWhen`, `where`, prerequisites). - **Constraint:** a game rule in the rules data, checked against the current character, whose failure is a rules issue. - **Content order:** the order the engine lists content in: packs in load order, each pack's content in its release's order (file path order), homebrew last; query matches and choice options follow it, so a dependency's content comes first. @@ -36,6 +36,7 @@ One-line definitions of OCF terms, consistent with the [specification](../../pac - **Default language:** the pack's language whose translation file must be complete and ships with the mechanics. - **Derived field:** a field computed from a formula and effects; never saved. - **Derived offer:** a follow-up (`while`) offered exactly as long as its condition, an imbalance on the sheet, holds: a level-up the experience has earned, or a linked character to make for an empty link slot (`create`). computeSheet derives it; it is never stored or dismissed. +- **Derived parameter:** an action parameter that declares a `formula` instead of being given: worked out as the action starts from the given parameters (and earlier derived ones), read as `params.`; describeAction marks it `derived`. - **Dice function:** the swappable function the engine calls for die faces; app-supplied or the built-in random one. - **Dice modifier:** one of the fixed set of notation modifiers (keep, drop, explode, reroll, count successes, …). - **Display hint:** optional presentation data (groups, priority, format, unit, `showWhen`, hidden) that never affects calculations. A display group holds fields, actions or content. diff --git a/apps/website/guides/rolling-dice.md b/apps/website/guides/rolling-dice.md index f981682b8..6d7f81245 100644 --- a/apps/website/guides/rolling-dice.md +++ b/apps/website/guides/rolling-dice.md @@ -68,6 +68,8 @@ const described = describeAction(ruleset, character, { included), and for a field, content or tag parameter the `options` startAction takes, its `where` applied. Offer those as the picker. - **A parameter the item's `use` fixes** has `fixed: true` and its `value`: don't ask for it. +- **A derived parameter** has `derived: true`: startAction works it out from the others (a + Cunning Strike's cost in Sneak Attack dice). Don't ask for it; giving one is `invalid-param`. - **A parameter content adds** (a `params` effect: an ability's damage type chosen on each use, a feature's opt-in) comes after the action's own, with `addedBy`. Ask for it like the others. Bounds content adds to a parameter are in its `addedBounds` (each with `addedBy`): every one diff --git a/packages/engine/src/check/formulas.ts b/packages/engine/src/check/formulas.ts index 6235d48e7..dac01bd09 100644 --- a/packages/engine/src/check/formulas.ts +++ b/packages/engine/src/check/formulas.ts @@ -514,7 +514,9 @@ class FormulaChecker { this.formula(aura.size, at, `${where} size`, NUMBER); this.typed({ applier: viewType(aura.fields ?? {}) }, () => this.withRoots(['applier'], () => { + this.inAura = true; for (const effect of aura.effects ?? []) this.effect(effect, at, where); + this.inAura = false; for (const immunity of aura.immuneTo ?? []) this.immunity(immunity, at, where); }), ); @@ -734,8 +736,32 @@ class FormulaChecker { }; } + /** Inside an aura: its effects' `self` is the aura's entry, which has no computed values. */ + private inAura = false; + + /** + * An effect that acts as an action runs (on rolls, choice and changes steps, listed actions and + * incoming changes) reads its content's computed values as `self.`: they are worked out + * after every field, which such an effect never feeds. A field effect can't (`unknown-name`), + * nor can an applied one or an aura's, whose `self` is an applied entry. + */ private effect(effect: Effect, at: L, where: string): void { const kind = targetKind(effect.target); + const self = this.types.self; + if (kind === 'field' || effect.scope === 'applied' || this.inAura || self === undefined) + this.effectOf(effect, at, where, kind); + else + this.typed({ self: revealComputed(self) as FType }, () => + this.effectOf(effect, at, where, kind), + ); + } + + private effectOf( + effect: Effect, + at: L, + where: string, + kind: ReturnType, + ): void { if (kind === 'roll') { this.typed(this.rollTypes(), () => this.withRoots(ROLL_ROOTS, () => this.effectBody(effect, at, where, kind)), @@ -852,6 +878,12 @@ class FormulaChecker { this.formula(decl.max, at, `${place} max`, NUMBER); }), ); + // A derived one sees the run's participants too. + this.typed({ params: 'dyn', participants: 'dyn' }, () => + this.withRoots(['params', 'participants'], () => + this.formula(decl.formula, at, `${place} formula`, [fieldFormulaType(decl.type)]), + ), + ); if (decl.where === undefined || decl.type !== 'id' || decl.of === undefined) continue; if (decl.of === 'field') this.query({ type: 'field', where: decl.where }, at, place); else if (decl.of !== 'tag' && decl.of !== 'string') @@ -1105,6 +1137,29 @@ class FormulaChecker { }), ), ); + // A derived parameter sees what a bound sees and `participants`, but only the derived + // parameters before it: a later one, or its own, is `unknown-name`, so they can't loop. + const derived = Object.entries(action.params ?? {}) + .filter(([, decl]) => decl.formula !== undefined) + .map(([name]) => name); + for (const [index, name] of derived.entries()) { + const decl = action.params?.[name] as ValueDecl; + const later = new Set(derived.slice(index)); + const all = (types.params as { object: Record }).object; + const params: FType = { + object: Object.fromEntries(Object.entries(all).filter(([n]) => !later.has(n))), + }; + const seen = { situational: types.situational, participants: types.participants }; + this.typed({ ...own, ...(seen as Record), params }, () => + this.at('content', () => + this.onlyRoots(['situational', 'participants', 'params'], () => + this.formula(decl.formula, at, `${key} param ${name} formula`, [ + fieldFormulaType(decl.type), + ]), + ), + ), + ); + } this.typed(types, () => this.actionBody(action, at, key, each)); } diff --git a/packages/engine/src/core/actions.ts b/packages/engine/src/core/actions.ts index 828e9e8f6..f4adb54ec 100644 --- a/packages/engine/src/core/actions.ts +++ b/packages/engine/src/core/actions.ts @@ -787,11 +787,12 @@ function startFrame( added: nested.added, situational: given, }; - frame.roots.params = params(ctx, frame, rawParams, fixed, fixedPack); + // Participants first: a derived parameter reads them. const roles = def.participants ?? {}; const passed = depth === 0 ? (ctx.state.participants ?? {}) : (nested.participants ?? {}); frame.roots.participants = participantsRoot(roles, passed); for (const list of Object.values(passed)) for (const view of list) frame.views.set(view.id, view); + frame.roots.params = params(ctx, frame, rawParams, fixed, fixedPack); return frame; } @@ -974,7 +975,16 @@ function params( // parameter's own, then those `params` effects add to it. const ranges: (() => void)[] = []; const typedOf = new Map }>(); - for (const { name, decl, pack, roots, at } of all) { + const derived: ParamSpec[] = []; + for (const spec of all) { + const { name, decl, pack, roots, at } = spec; + // A derived parameter is worked out once every given one has its value. + if (decl.formula !== undefined) { + if (raw[name] !== undefined || fixed[name] !== undefined) + throw new ActionError('invalid-param', { param: name, reason: 'derived', ...at }); + derived.push(spec); + continue; + } let value: unknown = raw[name]; if (fixed[name] !== undefined) { const evaluated = ctx.sheet.evaluate(fixed[name] as Formula, fixedPack, frame.roots); @@ -1037,6 +1047,29 @@ function params( typedOf.set(name, { value: typed, at }); out[name] = rootValue(typed); } + // In declaration order, each reading the given parameters and the derived ones before it. + for (const { name, decl, pack, roots, at } of derived) { + const value = ctx.sheet.evaluate(decl.formula as Formula, pack, { + ...roots, + participants: frame.roots.participants, + params: out, + }); + const typed = typedValue(decl, f.toData(value)); + if (typeof typed === 'string') + throw new ActionError('type-mismatch', { + message: `the derived parameter "${name}" must be ${decl.type}, not ${value.kind}`, + param: name, + ...at, + }); + ranges.push(() => + paramRange(name, at, decl, typed, (bound) => { + const v = ctx.sheet.evaluate(bound, pack, { ...roots, params: out }); + return v.kind === 'int' || v.kind === 'decimal' ? v.value : undefined; + }), + ); + typedOf.set(name, { value: typed, at }); + out[name] = rootValue(typed); + } for (const bound of added.bounds) { const param = typedOf.get(bound.name); if (!param) continue; @@ -1101,7 +1134,7 @@ export function addedParams( ...(roots.item !== undefined ? { item: roots.item } : {}), ...(roots.self !== undefined ? { self: roots.self } : {}), }; - const own = { ...instance.roots, action }; + const own = { ...sheet.effectRoots(instance), action }; let holds = false; try { holds = sheet.gateHolds(instance, own); @@ -1782,6 +1815,11 @@ function handoff(ctx: Ctx, frame: Frame, step: Step, path: string): NestedResult if (decl.type === 'id' && decl.of === 'field' && name in given) given[name] = fieldIdOf(given[name]); const value = given[name]; + // A derived parameter is worked out when the participant's run starts. + if (decl.formula !== undefined) { + if (value === undefined) continue; + throw new ActionError('invalid-param', { param: name, reason: 'derived', action: qid }); + } if (value === undefined) { if (decl.optional || decl.default !== undefined) continue; throw new ActionError('missing-param', { param: name, action: qid }); @@ -1962,7 +2000,7 @@ function rollCandidates(ctx: Ctx, frame: Frame, stepTags: string[]): RollCandida instance, effect: instance.effect, pack: instance.pack, - roots: instance.roots, + roots: sheet.effectRoots(instance), }); } const situational = frame.roots.situational as Record; @@ -2662,7 +2700,7 @@ function proposeThens( if (rule.to !== 'self') proposeChanges( ctx, - optionFrame(frame, i), + optionFrame(ctx.sheet, frame, i), step, path, rule.to, @@ -3225,7 +3263,7 @@ function actionRoot( } /** The action's frame with `item`, `self` and pack of the content carrying the effect. */ -function optionFrame(frame: Frame, instance: EffectInstance): Frame { +function optionFrame(sheet: Sheet, frame: Frame, instance: EffectInstance): Frame { const roots = { ...frame.roots }; delete roots.item; delete roots.self; @@ -3233,7 +3271,7 @@ function optionFrame(frame: Frame, instance: EffectInstance): Frame { return { ...rest, pack: instance.pack, - roots: { ...roots, ...instance.roots }, + roots: { ...roots, ...sheet.effectRoots(instance) }, ...(instance.source.entry !== undefined ? { entry: instance.source.entry } : {}), adder: instance.source, owner: { activation: instance.own }, @@ -3295,7 +3333,10 @@ function addedOptions( const value = effect.value as OptionValue; const id = `${instance.source.id}#${value.id}`; if (out.some((c) => c.id === id)) continue; - const roots = context(optionFrame(frame, instance), { picks: counts.get(id) ?? 0, action }); + const roots = context(optionFrame(ctx.sheet, frame, instance), { + picks: counts.get(id) ?? 0, + action, + }); if (!ctx.sheet.gateHolds(instance, roots)) continue; if (effect.when !== undefined) { const v = ctx.sheet.evaluate(effect.when, instance.pack, roots); @@ -3559,7 +3600,7 @@ function choiceStep(ctx: Ctx, given: Frame, step: Step, path: string): void { for (const [name, formula] of Object.entries(candidate.option?.flags ?? {})) frame.flags[name] = rootValue(evaluate(ctx, frame, formula, { picked })); if (candidate.added) { - const added = optionFrame(frame, candidate.added.instance); + const added = optionFrame(ctx.sheet, frame, candidate.added.instance); for (const [name, formula] of Object.entries(candidate.added.value.flags ?? {})) frame.flags[name] = rootValue(evaluate(ctx, added, formula, { picked, action })); } @@ -3577,7 +3618,7 @@ function choiceStep(ctx: Ctx, given: Frame, step: Step, path: string): void { if (candidate.added) proposeChanges( ctx, - optionFrame(frame, candidate.added.instance), + optionFrame(ctx.sheet, frame, candidate.added.instance), step, path, candidate.added.value.to ?? 'self', @@ -4013,7 +4054,7 @@ function addedRuns(ctx: Ctx, frame: Frame, step: Step, path: string): void { if (!addsToSteps(instance, frame, target.from)) continue; const value = effect.value as StepAction; const id = `${instance.source.id}#${value.id}`; - const added = optionFrame(frame, instance); + const added = optionFrame(ctx.sheet, frame, instance); const runs: { frame: Frame; path: string }[] = []; const role = value.each; if (frame.each) { @@ -4087,7 +4128,7 @@ function addedChanges(ctx: Ctx, frame: Frame, step: Step): AddedChanges[] { if (effect.op !== 'changes' || typeof target !== 'object' || !('steps' in target)) continue; if (!tagsReach(ctx.ruleset, instance.pack, frame.declared, target.steps, tags)) continue; if (!addsToSteps(instance, frame, target.from)) continue; - const added = optionFrame(frame, instance); + const added = optionFrame(ctx.sheet, frame, instance); const roots = context(added, { action }); if (!ctx.sheet.gateHolds(instance, roots)) continue; if (effect.when !== undefined) { diff --git a/packages/engine/src/core/changes.ts b/packages/engine/src/core/changes.ts index cde2a5acb..597379837 100644 --- a/packages/engine/src/core/changes.ts +++ b/packages/engine/src/core/changes.ts @@ -1606,7 +1606,7 @@ function route( .map((i): EffectInstance => ({ ...i, roots: { - ...i.roots, + ...sheet.effectRoots(i), incoming: incomingRoot, situational: situationalRoot(ruleset, i.pack, given), batch, diff --git a/packages/engine/src/core/describe.ts b/packages/engine/src/core/describe.ts index 6c34c7be9..b3d72f6fb 100644 --- a/packages/engine/src/core/describe.ts +++ b/packages/engine/src/core/describe.ts @@ -82,8 +82,13 @@ export function describeAction( const out: DescribedParam = { name, type: decl.type as DescribedParam['type'], - required: !decl.optional && decl.default === undefined && fixed[name] === undefined, + required: + !decl.optional && + decl.default === undefined && + decl.formula === undefined && + fixed[name] === undefined, }; + if (decl.formula !== undefined) out.derived = true; if (by !== undefined) out.addedBy = by; if (decl.of !== undefined) out.of = decl.of; if (decl.default !== undefined) out.default = decl.default; diff --git a/packages/engine/src/core/load.ts b/packages/engine/src/core/load.ts index 7f583757f..9be653a02 100644 --- a/packages/engine/src/core/load.ts +++ b/packages/engine/src/core/load.ts @@ -321,7 +321,7 @@ function checkAddedParams( ); for (const [i, p] of added.entries()) { const data = { param: p.name, source: p.source }; - if (!p.decl.optional && p.decl.default === undefined) + if (!p.decl.optional && p.decl.default === undefined && p.decl.formula === undefined) err('invalid-added-param', { ...data, reason: 'required' }); for (const { key, pack, def, tags } of literal) { if (!Object.hasOwn(def.params ?? {}, p.name)) continue; @@ -965,7 +965,8 @@ export function loadPacks(packs: PackRelease[], options: LoadOptions = {}): Load for (const [name, decl] of Object.entries(def.params ?? {})) { if ((name === 'ended' || name === 'started') && decl.type !== 'string') bad({ param: name }); - else if (decl.default === undefined && !decl.optional) bad({ param: name }); + else if (decl.default === undefined && !decl.optional && decl.formula === undefined) + bad({ param: name }); } for (const [role, decl] of Object.entries(def.participants ?? {})) if ((decl.min ?? 0) > 0) bad({ role }); @@ -1165,7 +1166,12 @@ export function loadPacks(packs: PackRelease[], options: LoadOptions = {}): Load const action = q.actionId(start.action, pid); const def = ruleset.rules.actions[action]; for (const [name, decl] of Object.entries(def?.params ?? {})) { - if (decl.optional || decl.default !== undefined || start.params?.[name] !== undefined) + if ( + decl.optional || + decl.default !== undefined || + decl.formula !== undefined || + start.params?.[name] !== undefined + ) continue; err('missing-param', { param: name, creation: start.id, pack: pid }); } diff --git a/packages/engine/src/core/pack-types.ts b/packages/engine/src/core/pack-types.ts index bae42f622..f3a12de6a 100644 --- a/packages/engine/src/core/pack-types.ts +++ b/packages/engine/src/core/pack-types.ts @@ -49,6 +49,8 @@ export interface ValueDecl { min?: Formula; max?: Formula; label?: string; + /** Action parameters: a derived one, worked out as the action starts, never given. */ + formula?: Formula; } export interface ComputedDecl { diff --git a/packages/engine/src/core/sheet.ts b/packages/engine/src/core/sheet.ts index 4ba29489d..65d13d00a 100644 --- a/packages/engine/src/core/sheet.ts +++ b/packages/engine/src/core/sheet.ts @@ -186,6 +186,8 @@ export interface Activation { gates?: readonly Gate[] | undefined; /** Its effects' shared condition: its item's `effectsWhen`, else its type's (an inline option's owner's). */ effectsWhen?: EffectsWhen | undefined; + /** An inline option's: the content activation it was picked under, whose computed values it reads. */ + holder?: Activation | undefined; } /** A grant's `available`, with the grant's pack and roots. */ @@ -309,6 +311,11 @@ export interface EffectInstance { own?: Activation; /** Its content's `effectsWhen`, ANDed into its `when` with the same roots. */ gate?: EffectsWhen; + /** + * The holder activation whose computed values its `self` reads when it acts as an action runs + * (`effectRoots`); absent for the rules', applied content's and auras' effects. + */ + holder?: Activation; } interface FieldResult { @@ -2063,6 +2070,7 @@ export class Sheet { Object.keys(stateDecls(this.ruleset, item, [])).length > 0 ? activation : undefined; + const holder = activation.scope === 'holder' ? this.computedHolder(activation) : undefined; if (type) { (type.effects ?? []).forEach((effect, index) => { if (effect.id !== undefined && item?.suppress?.includes(effect.id)) return; @@ -2077,6 +2085,7 @@ export class Sheet { activation.applied, owner, gate, + holder, ); }); } @@ -2094,6 +2103,7 @@ export class Sheet { activation.applied, owner, gate, + holder, ); }); if (activation.scope === 'applied') { @@ -2301,6 +2311,7 @@ export class Sheet { undefined, undefined, owner.effectsWhen, + owner.scope === 'holder' ? this.computedHolder(owner) : undefined, ); }); const nested: Pick['choices'] = pick.choices ?? {}; @@ -2316,6 +2327,7 @@ export class Sheet { { ...owner, kind: 'inline', + holder: this.computedHolder(owner), waived: choice.waivePrerequisites === true, item: undefined as unknown as LoadedItem, inline, @@ -2394,11 +2406,13 @@ export class Sheet { applied?: string, owner?: Activation, gate?: EffectsWhen, + holder?: Activation, ): void { const own = { ...(applied === undefined ? {} : { applied }), ...(owner === undefined ? {} : { own: owner }), ...(gate === undefined ? {} : { gate }), + ...(holder === undefined ? {} : { holder }), }; const target = effect.target; if (typeof target === 'object' && 'formula' in target) { @@ -2420,6 +2434,7 @@ export class Sheet { applied, owner, gate, + holder, ); return; } @@ -4131,6 +4146,32 @@ export class Sheet { return (this.activations[0] as Activation).roots; } + /** + * The activation whose computed values an activation's effects read: an inline option's content + * (the copy or content it was picked under), else itself. + */ + private computedHolder(a: Activation): Activation { + return a.kind === 'inline' ? (a.holder ?? a) : a; + } + + /** + * An effect's roots as it acts while an action runs (on rolls, choice and changes steps, listed + * actions, incoming changes): `self` also holds its holder's computed values, as its actions + * read them. A field effect keeps `roots`, since computed values are worked out after fields. + */ + effectRoots(i: EffectInstance): Record { + if (!i.holder) return i.roots; + const values = this.holderComputed(i.holder); + if (Object.keys(values).length === 0) return i.roots; + return { ...i.roots, self: { ...(i.roots.self as object), ...values } }; + } + + /** A holder activation's copy's computed values, then its own as picked or granted content. */ + private holderComputed(a: Activation): Record { + const copy = a.roots.self && a.entry ? (this.computedOf(a.entry.id) ?? {}) : {}; + return { ...computedRoots(copy), ...computedRoots(this.contentComputedOf(a)) }; + } + /** * A holder activation's roots, with `self` also holding its copy's computed values, then, for * picked or granted content, its own. diff --git a/packages/engine/src/core/upgrade.ts b/packages/engine/src/core/upgrade.ts index ce9e6f2ed..b9980315e 100644 --- a/packages/engine/src/core/upgrade.ts +++ b/packages/engine/src/core/upgrade.ts @@ -1191,7 +1191,15 @@ export function checkRelease( const now = nextDecls.params.get(id)?.decls; if (!now) interfaceChanges.push({ kind: 'param-removed', id }); else if ( - decls.some((was) => now.every((d) => narrowed(was, d) || (!required(was) && required(d)))) + // A given parameter that becomes derived no longer takes the app's value. + decls.some((was) => + now.every( + (d) => + narrowed(was, d) || + (!required(was) && required(d)) || + (was.formula === undefined && d.formula !== undefined), + ), + ) ) interfaceChanges.push({ kind: 'param-type-changed', id }); } @@ -1437,7 +1445,8 @@ function narrowed( ); } -const required = (decl: ValueDecl): boolean => !decl.optional && decl.default === undefined; +const required = (decl: ValueDecl): boolean => + !decl.optional && decl.default === undefined && decl.formula === undefined; /** A release's actions and action parameters, by text-key address. */ interface ActionDecls { diff --git a/packages/engine/src/formula/check.ts b/packages/engine/src/formula/check.ts index 282d75b2e..899583a78 100644 --- a/packages/engine/src/formula/check.ts +++ b/packages/engine/src/formula/check.ts @@ -311,7 +311,7 @@ function select(type: FType, key: string, path: string): FType { if (type.hidden?.[key] !== undefined) return fail( 'unknown-name', - `"${path}" is a computed value, which only the copy's own actions and follow-ups, and its later computed values, read`, + `"${path}" is a computed value, worked out after every field: only the copy's own actions and follow-ups, its effects that act as an action runs (on rolls, choice and changes steps, listed actions and incoming changes) and its later computed values read it, never a field effect, field formula, constraint or activation condition`, { name: path }, ); if (type.open) return 'dyn'; diff --git a/packages/engine/test/computed-in-effects.test.ts b/packages/engine/test/computed-in-effects.test.ts new file mode 100644 index 000000000..82ba04289 --- /dev/null +++ b/packages/engine/test/computed-in-effects.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from 'vitest'; +import { diceFrom, loadRuleset, startAction } from '../src/index.ts'; +import type { CharacterDoc, PackRelease } from '../src/index.ts'; + +/** A feature computing `bonus` (the tier), with these effects; `strike` rolls and hits. */ +const pack = (effects: unknown[]): PackRelease => + ({ + manifest: { + id: 'p', + version: '1.0.0', + specVersion: '0.3.0', + license: 'CC0-1.0', + defaultLanguage: 'en', + }, + rules: { + tags: ['attack'], + fields: { + tier: { type: 'integer', input: true, default: 2 }, + n: { type: 'integer', input: true, default: 0 }, + }, + contentTypes: { feature: { computed: { bonus: { type: 'integer', formula: 'tier' } } } }, + actions: [ + { + id: 'strike', + tags: ['attack'], + params: { spend: { type: 'integer', default: 0 } }, + steps: [ + { id: 'roll', kind: 'roll', tags: ['attack'], parts: [{ dice: '1d20' }] }, + { id: 'hit', kind: 'changes', tags: ['attack'] }, + { id: 'done', kind: 'outcome', data: { total: 'steps.roll.total' } }, + ], + }, + ], + }, + content: [{ id: 'features.f', type: 'feature', effects }], + }) as unknown as PackRelease; + +const character: CharacterDoc = { + specVersion: '0.3.0', + id: 'hero', + packs: [{ id: 'p', version: '1.0.0' }], + features: [{ id: 'f', item: 'p:features.f' }], +}; + +const load = (effects: unknown[]) => loadRuleset([pack(effects)]); + +const run = (effects: unknown[], params: Record = {}) => { + const loaded = load(effects); + if (!loaded.ok) throw new Error(JSON.stringify(loaded.errors)); + return startAction( + loaded.ruleset, + character, + { action: { id: 'p:strike' }, params }, + diceFrom([10]), + ); +}; + +const causes = (effects: unknown[]) => { + const loaded = load(effects); + return loaded.ok ? [] : loaded.errors.map((e) => (e.data as { cause?: string }).cause); +}; + +describe("effects reading their content's computed values", () => { + it('roll effects add them', () => { + const result = run([{ target: { rolls: ['attack'] }, op: 'add', value: 'self.bonus' }]); + expect(result).toMatchObject({ outcome: { data: { total: 12 } } }); + }); + + it('changes effects propose with them, and their when reads them', () => { + const result = run([ + { + target: { steps: ['attack'] }, + op: 'changes', + when: 'self.bonus > 1', + value: { changes: [{ type: 'adjust', field: 'n', by: 'self.bonus' }] }, + }, + ]); + expect(result).toMatchObject({ + proposedChanges: [{ change: { type: 'adjust', field: 'p:n', by: 2 } }], + }); + }); + + it('params effects bound with them', () => { + const effects = [ + { target: { actions: ['attack'] }, op: 'params', value: { spend: { max: 'self.bonus' } } }, + ]; + expect(run(effects, { spend: 2 })).toMatchObject({ status: 'finished' }); + expect(run(effects, { spend: 3 })).toMatchObject({ errors: [{ code: 'out-of-range' }] }); + }); + + it("granted content's effects read its own, as its actions do", () => { + const base = pack([]); + const release = { + ...base, + rules: { ...base.rules, contentTypes: { ...base.rules.contentTypes, class: {} } }, + content: [ + { id: 'classes.c', type: 'class', grants: [{ item: 'features.g' }] }, + { + id: 'features.g', + type: 'feature', + computed: { extra: { type: 'integer', formula: 'self.bonus + 1' } }, + effects: [{ target: { rolls: ['attack'] }, op: 'add', value: 'self.extra' }], + }, + ], + } as unknown as PackRelease; + const loaded = loadRuleset([release]); + if (!loaded.ok) throw new Error(JSON.stringify(loaded.errors)); + const result = startAction( + loaded.ruleset, + { ...character, features: [{ id: 'c', item: 'p:classes.c' }] }, + { action: { id: 'p:strike' } }, + diceFrom([10]), + ); + expect(result).toMatchObject({ outcome: { data: { total: 13 } } }); + }); + + it('field effects, and applied ones, cannot: unknown-name at load', () => { + expect(causes([{ target: 'n', op: 'add', value: 'self.bonus' }])).toEqual(['unknown-name']); + expect(causes([{ target: 'n', op: 'add', value: '1', when: 'self.bonus > 0' }])).toEqual([ + 'unknown-name', + ]); + expect( + causes([{ target: { rolls: ['attack'] }, op: 'add', value: 'self.bonus', scope: 'applied' }]), + ).toEqual(['unknown-name']); + }); +}); diff --git a/packages/engine/test/derived-params.test.ts b/packages/engine/test/derived-params.test.ts new file mode 100644 index 000000000..226890c00 --- /dev/null +++ b/packages/engine/test/derived-params.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +import { describeAction, loadRuleset, startAction, type PackRelease } from '../src/index.ts'; + +/** + * A pack whose `strike` (tagged `attack`) takes `spend` and derives `cost` from it, adjusting `n` + * by `cost`; a feature's effects may add more. + */ +const pack = (params: Record, effects: unknown[] = []): PackRelease => + ({ + manifest: { + id: 'p', + version: '1.0.0', + specVersion: '0.3.0', + license: 'CC0-1.0', + defaultLanguage: 'en', + }, + rules: { + tags: ['attack'], + fields: { n: { type: 'integer', input: true, default: 0 } }, + contentTypes: { feature: {} }, + actions: [ + { + id: 'strike', + tags: ['attack'], + params, + participants: { target: { max: 2 } }, + steps: [ + { + id: 'hit', + kind: 'changes', + tags: ['attack'], + changes: [{ type: 'adjust', field: 'n', by: 'params.cost' }], + }, + ], + }, + { + id: 'twice', + steps: [{ id: 'again', kind: 'action', use: 'strike', params: { spend: 1, cost: 5 } }], + }, + ], + }, + content: [{ id: 'features.f', type: 'feature', effects }], + }) as unknown as PackRelease; + +const character = { + specVersion: '0.3.0', + id: 'hero', + packs: [{ id: 'p', version: '1.0.0' }], + features: [{ id: 'f', item: 'p:features.f' }], +}; + +const own = { + spend: { type: 'integer', default: 0 }, + cost: { type: 'integer', formula: 'params.spend * 2' }, +}; + +const load = (release: PackRelease) => { + const loaded = loadRuleset([release]); + if (!loaded.ok) throw new Error(JSON.stringify(loaded.errors)); + return loaded.ruleset; +}; + +const run = ( + release: PackRelease, + params: Record, + id = 'p:strike', + participants: Record }[]> = {}, +) => { + const result = startAction(load(release), character, { action: { id }, params, participants }); + if ('errors' in result && result.errors) + return result.errors.map((e) => [e.code, (e.data as { param?: string }).param]); + return 'proposedChanges' in result + ? result.proposedChanges.map((c) => (c.change as { by?: number }).by) + : result; +}; + +describe('derived parameters', () => { + it('are worked out from the given ones, and steps read them', () => { + expect(run(pack(own), { spend: 3 })).toEqual([6]); + expect(run(pack(own), {})).toEqual([0]); + }); + + it('read the derived ones before them, and bounds see them', () => { + const params = { + ...own, + total: { type: 'integer', formula: 'params.cost + 1', max: 5 }, + spend: { type: 'integer', default: 0, max: 'params.total' }, + }; + expect(run(pack(params), { spend: 2 })).toEqual([4]); + // total 7: over its own max. + expect(run(pack(params), { spend: 3 })).toEqual([['out-of-range', undefined]]); + }); + + it('are never given: invalid-param, from the app or an action step', () => { + expect(run(pack(own), { spend: 1, cost: 9 })).toEqual([['invalid-param', 'cost']]); + expect(run(pack(own), {}, 'p:twice')).toEqual([['invalid-param', 'cost']]); + }); + + it('give their declared type', () => { + const params = { ...own, cost: { type: 'integer', formula: 'params.spend / 2' } }; + // A decimal formula for an integer parameter is caught at load. + const loaded = loadRuleset([pack(params)]); + expect(loaded.ok).toBe(false); + }); + + it('can read only those declared before them: a later one is unknown at load', () => { + const loaded = loadRuleset([ + pack({ + spend: { type: 'integer', default: 0 }, + cost: { type: 'integer', formula: 'params.total' }, + total: { type: 'integer', formula: 'params.cost' }, + }), + ]); + expect(loaded.ok).toBe(false); + if (!loaded.ok) + expect(loaded.errors.map((e) => (e.data as { cause?: string }).cause)).toEqual([ + 'unknown-name', + ]); + }); + + it('may be added by a params effect, read by its changes', () => { + const release = pack({ ...own }, [ + { + target: { actions: ['attack'] }, + op: 'params', + value: { + extra: { type: 'boolean', default: false }, + bonus: { type: 'integer', formula: 'params.extra ? params.cost : 0' }, + }, + }, + { + target: { steps: ['attack'] }, + op: 'changes', + value: { changes: [{ type: 'adjust', field: 'n', by: 'params.bonus' }] }, + }, + ]); + expect(run(release, { spend: 2, extra: true })).toEqual([4, 4]); + expect(run(release, { spend: 2 })).toEqual([4, 0]); + }); + + it("read the run's participants", () => { + const params = { cost: { type: 'integer', formula: 'size(participants.target) * 3' } }; + const two = { + target: [ + { id: 'a', values: {} }, + { id: 'b', values: {} }, + ], + }; + expect(run(pack(params), {}, 'p:strike', two)).toEqual([6]); + }); + + it('describeAction marks them derived, never required', () => { + const described = describeAction(load(pack(own)), character, { id: 'p:strike' }); + if (!('action' in described)) throw new Error(JSON.stringify(described)); + expect(described.action.params).toEqual([ + { name: 'spend', type: 'integer', required: false, default: 0 }, + { name: 'cost', type: 'integer', required: false, derived: true }, + ]); + }); +}); diff --git a/packages/spec/data/error-codes.json b/packages/spec/data/error-codes.json index 021f7e8f8..91787e979 100644 --- a/packages/spec/data/error-codes.json +++ b/packages/spec/data/error-codes.json @@ -271,7 +271,7 @@ { "id": "invalid-added-param", "kind": "load", - "description": "A `params` effect's parameter can't be added: it is neither `optional` nor has a `default` (reason `required`), an action its target tags reach by its literal tags declares a parameter of that name (reason `declared`, data `action`), or another `params` effect adds that name with another declaration (reason `conflict`, data `other`), or an entry that only adds bounds (`min`, `max`) names a parameter no action declares and no `params` effect adds (reason `undeclared`): their target tags meet (one is the other or below it) over packs that meet on that tag, or one action's literal tags meet a target tag of each; two `from: self` effects of different content items conflict only when one item names both (itself, its `grants`, its choices' `items` and inline options) and has an action, its own or its type's, that both reach. An action counts by the pack that wrote it and only when the effect reaches it over packs that meet (effects.md, Which packs a tag target reaches). Data: `param`, `source` (the effect's content, `:types.` or `:rules`), `reason`." + "description": "A `params` effect's parameter can't be added: it is neither `optional`, defaulted nor derived (reason `required`), an action its target tags reach by its literal tags declares a parameter of that name (reason `declared`, data `action`), or another `params` effect adds that name with another declaration (reason `conflict`, data `other`), or an entry that only adds bounds (`min`, `max`) names a parameter no action declares and no `params` effect adds (reason `undeclared`): their target tags meet (one is the other or below it) over packs that meet on that tag, or one action's literal tags meet a target tag of each; two `from: self` effects of different content items conflict only when one item names both (itself, its `grants`, its choices' `items` and inline options) and has an action, its own or its type's, that both reach. An action counts by the pack that wrote it and only when the effect reaches it over packs that meet (effects.md, Which packs a tag target reaches). Data: `param`, `source` (the effect's content, `:types.` or `:rules`), `reason`." }, { "id": "circular-extends", @@ -303,6 +303,11 @@ "kind": "action", "description": "A required action parameter is missing. Data: `param`, and `action`, `entry` and `source` as for `out-of-range`." }, + { + "id": "invalid-param", + "kind": "action", + "description": "A value was given for an action parameter that takes none: a derived parameter (its declaration's `formula`), given by the app, a `use`, an `action` step or a handoff (reason `derived`). Data: `param`, `reason`, and `action`, `entry`, `source` and `addedBy` as for `out-of-range`." + }, { "id": "invalid-participants", "kind": "action", diff --git a/packages/spec/docs/content.md b/packages/spec/docs/content.md index 7cc5e918e..fde0d17c3 100644 --- a/packages/spec/docs/content.md +++ b/packages/spec/docs/content.md @@ -825,10 +825,21 @@ contentTypes: values: a computed value reads those declared **before** it, in declaration order (the type's, then the item's and each template's, a replacement keeping the replaced name's place). Later names, and its own, are `unknown-name` there. Picked or granted content's actions read - the owning copy's computed values too, its own over them by name. Field formulas, effects - (roll effects included), constraints, activation conditions, state formulas and a choice - step's candidate copies can't read them: there they are `unknown-name` at load, though `self` - is open. Since they are computed after everything they could feed, and each reads only + the owning copy's computed values too, its own over them by name. +- **Effects that act as an action runs** read them too: the content's (and a type's or + template's) effects on rolls, choice steps, changes steps (`changes` and `action`), listed + actions (`params`, its bounds and derived parameters included) and incoming changes, in their + `when`, value and `then`, and its choices' and inline options' effects of those kinds. Their + `self` is what the content's actions see: the copy's computed values, then, for picked or + granted content (an inline option reads the content it was picked under), its own. These run + on the computed sheet, after every computed value, and never feed a field, so they can't + loop. A field effect (a field, a set of fields or a formula target), with its `when`, can't: + computed values read every field, so one feeding a field could loop. Nor can an applied + effect or an aura's, whose `self` is an applied entry. There, and in field formulas, + constraints, activation conditions (`effectsWhen` included), state formulas and a choice + step's candidate copies, `self.` is `unknown-name` at load, though `self` is open. A + Cunning Strike's DC, a computed value, is then read by the strike's own `action` effect. +- Since computed values are worked out after everything they could feed, and each reads only earlier ones, they can't loop. An earlier value that failed is absent (`has(self.)` is false), so a later value reading it fails too, with its own `formula-error`. The copy aggregates (`countCopies`, `sumCopies`, `maxCopies`, `minCopies`) don't see them. diff --git a/packages/spec/docs/dice-and-actions.md b/packages/spec/docs/dice-and-actions.md index 14f3937b1..b16eac030 100644 --- a/packages/spec/docs/dice-and-actions.md +++ b/packages/spec/docs/dice-and-actions.md @@ -98,6 +98,40 @@ contentTypes: - { use: saving-throw, scope: applied, available: "self.duration == 'save'" } ``` +**Derived parameters.** A parameter (an action's own, or one a `params` effect adds) may +declare a `formula` instead of being given: a value the run works out from its other +parameters, named once and read as `params.` everywhere `params` is (steps, effects and +bounds). Its type is `integer`, `decimal`, `boolean` or `string`, and it has no `default` and +isn't `optional`. + +- **When:** as the action starts, once every given parameter has its value (the app's, a + `use`'s fixed one, or the default) and before any bound is checked, in declaration order (the + action's own, then those `params` effects add). It sees the action's (or the effect's) `item` + and `self`, `situational`, the character's fields, the run's `participants` (which a bound + doesn't see: Cunning Strike's Trip costs nothing against a Huge target), and `params` with the + given parameters and the derived ones declared **before** it. A later derived parameter, and + its own, is `unknown-name` at load for an action's own (through a `params` effect, absent when + it runs), so derived parameters can't loop. Its bounds, and every other bound, see its value. +- **Never given:** a value for it from the app, a `use`, an `action` step or a handoff is + `invalid-param` (reason `derived`); the build reports a `use`'s, a creation start's, a field + roll's and an `action` effect's. describeAction lists it with `derived: true` and + `required: false`. Its label (`.params..name`) is optional: it is for apps + that show it. +- A value of another type is `type-mismatch` as the action starts. + +Cunning Strike's cost in Sneak Attack dice is worked out once from the effect chosen on the +attack, and the rules' Sneak Attack effect and the strike's own effects read it: + +```yaml +- target: { actions: [attack-hit] } + op: params + value: + cunningStrike: { type: enum, values: [poison, trip, withdraw, daze], optional: true } + cunningStrikeCost: + type: integer + formula: "!has(params.cunningStrike) ? 0 : params.cunningStrike == 'daze' ? 2 : 1" +``` + **The action's own tags** may hold `{ formula }` entries too, evaluated for each listing with its `item`, `self` and the character's fields (no `params`, `steps` or `flags`); a formula that fails leaves only the written tags. computeSheet's listing, display groups, describeAction and `params` @@ -117,7 +151,7 @@ own with `addedBy`. The action's own formulas see only the parameters it declare reads its own as `params.` in its roll, option and `changes` effects. So an ability chooses its damage type on each use, and a feature adds an opt-in to a rules action. -- An added parameter is `optional` or has a `default`, and only the run the app starts +- An added parameter is `optional`, has a `default` or is derived, and only the run the app starts (startAction, a follow-up taken up, a handoff's run on its own character) has it: an `action` step's nested run doesn't. A required one is the load error `invalid-added-param`. - Its `min` and `max` see the effect's roots, as its `when` (`item` and `self` the content's, @@ -522,11 +556,11 @@ needs the app to say when turns change. A rules action with `clock: turn-boundar moment: at each turn change, the app runs it on **every** character in the encounter where it is listed, with `params.ended` the `id` of the character whose turn ended and `params.started` the one whose turn starts (each left out when there is none, at the start or end of a round). It must -run with nothing else the app makes up and never pause: every parameter has a default or is +run with nothing else the app makes up and never pause: every parameter has a default, is derived or is `optional` (`ended` and `started` are strings), no role has a `min`, it isn't `listed: false`, neither it nor a rules action its `action` steps run has a choice, allocate or handoff step, and only the rules declare one (`invalid-clock-action` at load). A parameter content adds with a -`params` effect is optional or defaulted already (`invalid-added-param` otherwise), so it keeps the +`params` effect is optional, defaulted or derived already (`invalid-added-param` otherwise), so it keeps the action runnable. Its rolls roll with the app's dice function under the default policy. computeSheet lists it with `clock: turn-boundary`. Content joins in through the action's tagged `changes` steps, as for any shared action: diff --git a/packages/spec/docs/effects.md b/packages/spec/docs/effects.md index a56f1f4d7..aeec76969 100644 --- a/packages/spec/docs/effects.md +++ b/packages/spec/docs/effects.md @@ -90,7 +90,7 @@ effects: scope: applied ``` -- **Rolls as they happen:** `target: { rolls: [attack, damage], from: self }`. The effect applies to any roll carrying one of the tags, in an action written by a pack that meets its own over that tag ([Which packs a tag target reaches](#which-packs-a-tag-target-reaches)). `from: self` limits it to rolls made by the item carrying the effect (a +1 weapon): the action's copy, and for an action run for content granted or picked under the copy (named by `source`) that content, so abilities a class grants that share their type's action each reach only their own rolls (a template is part of its copy, so its own action matches by the copy, as the copy's item's does; a nested `action` step keeps its `source`); `from: any` (the default) covers every matching roll the character makes (Bless). Roll effects add items to the itemized roll or modify its dice, and `atLeast` and `atMost` bound its total once it is rolled (see below). With `tally: `, the effect adds to one of the roll's tallies instead (see below). With `band: value`, an `atLeast` or `atMost` bounds the value of the roll's band set instead (see below). With `parts: []`, it reaches the roll's own parts carrying a matching tag instead of the whole roll: a `set` replaces them, a `tag` adds tags to them, and a `stepDie` or `setFaces` roll modifier changes only their dice (see [Replacing a roll's part](#replacing-a-rolls-part) and [Tagging a roll's part](#tagging-a-rolls-part)). Its formulas see `item` and `self` as the content carrying the effect, the roll as `roll`, the action's own item and copy as `action.item` and `action.self`, and the running action's `params` (those `params` effects add included), `situational`, `steps`, `flags`, `participants` and, in an `each` step, `participant`, so Bracers of Archery test their own copy while a bow attacks. +- **Rolls as they happen:** `target: { rolls: [attack, damage], from: self }`. The effect applies to any roll carrying one of the tags, in an action written by a pack that meets its own over that tag ([Which packs a tag target reaches](#which-packs-a-tag-target-reaches)). `from: self` limits it to rolls made by the item carrying the effect (a +1 weapon): the action's copy, and for an action run for content granted or picked under the copy (named by `source`) that content, so abilities a class grants that share their type's action each reach only their own rolls (a template is part of its copy, so its own action matches by the copy, as the copy's item's does; a nested `action` step keeps its `source`); `from: any` (the default) covers every matching roll the character makes (Bless). Roll effects add items to the itemized roll or modify its dice, and `atLeast` and `atMost` bound its total once it is rolled (see below). With `tally: `, the effect adds to one of the roll's tallies instead (see below). With `band: value`, an `atLeast` or `atMost` bounds the value of the roll's band set instead (see below). With `parts: []`, it reaches the roll's own parts carrying a matching tag instead of the whole roll: a `set` replaces them, a `tag` adds tags to them, and a `stepDie` or `setFaces` roll modifier changes only their dice (see [Replacing a roll's part](#replacing-a-rolls-part) and [Tagging a roll's part](#tagging-a-rolls-part)). Its formulas see `item` and `self` as the content carrying the effect, the roll as `roll`, the action's own item and copy as `action.item` and `action.self`, and the running action's `params` (those `params` effects add included), `situational`, `steps`, `flags`, `participants` and, in an `each` step, `participant`, so Bracers of Archery test their own copy while a bow attacks. `self` also holds the content's computed values, as for every effect that acts as an action runs (on rolls, choice and changes steps, listed actions and incoming changes), never a field effect ([content.md](content.md#computed-values)). - **Incoming changes:** `target: { incoming: { fields: [hp.current], tags: [damage.fire] } }`. The effect modifies the amount of an `adjust` change aimed at this character before it is applied, so apps don't apply resistances themselves. Both filters are optional; omitting both matches every incoming adjustment. After these effects, the change is routed through absorbing fields and overflow ([rules-and-fields.md](rules-and-fields.md#absorbing-and-overflow)). How a multiplied incoming amount rounds is a game setting (`settings.roundIncoming`). The effect's `when`, `value` and `then` see the change as `incoming` (see below), and the receiving player's [situational choices](rules-and-fields.md#choices-on-incoming-changes) on the change as `situational`. A situational choice's own incoming-change effects apply only to a change that selects it. - **Choice steps as actions run:** `target: { choices: [push-cost], from: self }`. The effect adds an option to every choice step of the character's actions that carries a matching tag; `from: self` limits it to the actions of the item carrying the effect, as for rolls. The only op is `option`. - **Changes steps as actions run:** `target: { steps: [strike.hit], from: self }`. The effect adds changes to every `changes` step of the character's actions that carries a matching tag, after the step's own, each time it runs (per participant in an `each` step); `from: self` limits it to the actions of the item carrying the effect, as for rolls. The only op is `changes`, whose value is `{ changes?, to?, flags?, order? }` (one of `changes` and `flags` at least): `flags` are set as the step runs, before its own changes (per participant in an `each` step; only flags the action sets elsewhere, checked at load, as an added option's), and `changes` are proposed after the step's own, or with `order: before` before them, `to` the value's `to`, else the step's. Its `when`, flags and changes see what an added option's do, with the action's `participants` (and `participant` in an `each` step) and no `picks`. Content items, templates, content types (for each active item of the type) and the rules' effects add changes, as for options (a build error on a situational choice's or a choice's). See [engine-actions.md](engine-actions.md#other-steps). @@ -105,7 +105,7 @@ effects: and the character's fields), each is a parameter of the action after its own, checked by startAction like them (on the run the app starts only, never a nested one) and listed by describeAction with `addedBy`. Its `min` and `max` also see the action's `params`, checked - once every parameter has its value. Each is `optional` or has a `default`; a name an action its + once every parameter has its value. Each is `optional`, has a `default` or is derived (`formula`, [dice-and-actions.md](dice-and-actions.md#actions)); a name an action its literal tags reach declares, or another effect's other declaration of the name (not between `from: self` effects of different content items, unless one item holding both has an action both reach), is the load error diff --git a/packages/spec/docs/engine-actions.md b/packages/spec/docs/engine-actions.md index 7ba66965b..f6565ddf1 100644 --- a/packages/spec/docs/engine-actions.md +++ b/packages/spec/docs/engine-actions.md @@ -44,8 +44,13 @@ shape, an optional **dice function**. `max` see the effect's roots (as its `when`) and `params`. A `use` that fixes a parameter, an added one included, wins over the app's value; its formula is evaluated in the pack that writes the use, with the listing's `item` and `self`. Missing required parameters are `missing-param`; wrong types are - `wrong-type`. Every parameter error (`missing-param`, `wrong-type`, `out-of-range`, - `invalid-option`), here or at a handoff, also carries `action` (the qualified ID of the action + `wrong-type`. A derived parameter (`formula`, [dice-and-actions.md](dice-and-actions.md#actions)) + takes no value (`invalid-param` if given); once every given parameter has its value, each + derived one is evaluated in declaration order with what its bounds see, `participants` and `params` so far, + and then the bounds are checked. So a derived parameter reads them, the participants + ([step 4](#starting-an-action)) are checked before the parameters: `invalid-participants` + comes first. Every parameter error (`missing-param`, `wrong-type`, + `out-of-range`, `invalid-option`, `invalid-param`), here or at a handoff, also carries `action` (the qualified ID of the action the parameter belongs to; `#` for an item's inline action), for an added parameter `addedBy` (as describeAction gives it), and, for an action started on them, `entry` and `source`, so text names the parameter by its label diff --git a/packages/spec/docs/engine-interface.md b/packages/spec/docs/engine-interface.md index f757e83ff..20a4f8a05 100644 --- a/packages/spec/docs/engine-interface.md +++ b/packages/spec/docs/engine-interface.md @@ -222,10 +222,11 @@ the definition, wherever it lives: a rules action, one inline on an item or on i - `tags` are the action's tags for this listing, computed ones evaluated. - `params` lists the parameters in declaration order, then those `params` effects add ([effects.md](effects.md#targets)), each `{ name, type, of?, required, default?, fixed?, - value?, min?, max?, addedBounds?, values?, options?, unique?, label?, addedBy? }`. An added one has + derived?, value?, min?, max?, addedBounds?, values?, options?, unique?, label?, addedBy? }`. An added one has `addedBy`, the content whose text names it (``, `:types.` or `:rules`; its key `.params..name`). `required` is - true when the parameter is neither `optional`, defaulted nor fixed. A parameter a `use` fixes + true when the parameter is neither `optional`, defaulted, derived nor fixed. A derived one + (its declaration's `formula`) has `derived: true`: startAction works it out, so don't ask for it. A parameter a `use` fixes has `fixed: true` and its `value` as startAction evaluates it; the app's is ignored. `min` and `max` are evaluated as startAction evaluates them, with every situational choice at its default; one that fails, or that reads `params` (it depends on the values the app is yet to diff --git a/packages/spec/i18n/en.yaml b/packages/spec/i18n/en.yaml index e38b46552..b9afc6283 100644 --- a/packages/spec/i18n/en.yaml +++ b/packages/spec/i18n/en.yaml @@ -83,7 +83,7 @@ errors.template-field-mismatch: 'Two packs declare the template field "{field}" errors.unknown-role: 'An action step names a role the action does not declare, or hands off outside a step over a role.' errors.invalid-added-param: 'The parameter "{param}" that {source} adds to actions cannot be added.' # `invalid-added-param` by its `reason`. -errors.invalid-added-param.required: 'The parameter "{param}" that {source} adds to actions must be optional or have a default.' +errors.invalid-added-param.required: 'The parameter "{param}" that {source} adds to actions must be optional, have a default or be derived.' errors.invalid-added-param.declared: 'The parameter "{param}" that {source} adds is already a parameter of {action}.' errors.invalid-added-param.conflict: '{source} and {other} both add the parameter "{param}", declared differently.' errors.invalid-added-param.undeclared: '{source} adds bounds to the parameter "{param}", which no action declares.' @@ -108,6 +108,7 @@ errors.unknown-action: 'That action is not available.' errors.situational-unavailable: 'That situational choice is not available to this character.' errors.unknown-situational: 'That situational choice does not exist.' errors.missing-param: 'The action needs "{param}".' +errors.invalid-param: '"{param}" is worked out by the rules, so it cannot be given.' errors.invalid-participants: 'The action cannot take those characters as "{role}".' errors.answer-mismatch: 'That answer does not fit what the action is waiting for.' errors.undeclared-result: 'The action finished with "{result}", which it does not declare.' diff --git a/packages/spec/schemas/action.schema.json b/packages/spec/schemas/action.schema.json index 079866ad4..6199f4520 100644 --- a/packages/spec/schemas/action.schema.json +++ b/packages/spec/schemas/action.schema.json @@ -18,7 +18,7 @@ "$ref": "formula.schema.json#/$defs/condition" }, "clock": { - "description": "A moment the app runs this rules action at, on every character it applies to: `turn-boundary`, at each turn change, on every character in the encounter, with `params.ended` the `id` of the character whose turn ended and `params.started` the one whose turn starts (each left out when there is none). It runs with nothing the app must make up and never pauses: every parameter has a default or is `optional` (`ended` and `started` are strings), no role has a minimum, it is listed, and neither it nor a rules action it runs has a choice, allocate or handoff step; rolls roll with the app's dice function under the default policy. Content's actions can't have one (`invalid-clock-action` at load). computeSheet lists it with its `clock`. Content joins in through its tagged `changes` steps: resetting what lasts a turn, ending what the creature whose turn starts applied until then (`self.appliedBy == params.started`).", + "description": "A moment the app runs this rules action at, on every character it applies to: `turn-boundary`, at each turn change, on every character in the encounter, with `params.ended` the `id` of the character whose turn ended and `params.started` the one whose turn starts (each left out when there is none). It runs with nothing the app must make up and never pauses: every parameter has a default, is derived (`formula`) or is `optional` (`ended` and `started` are strings), no role has a minimum, it is listed, and neither it nor a rules action it runs has a choice, allocate or handoff step; rolls roll with the app's dice function under the default policy. Content's actions can't have one (`invalid-clock-action` at load). computeSheet lists it with its `clock`. Content joins in through its tagged `changes` steps: resetting what lasts a turn, ending what the creature whose turn starts applied until then (`self.appliedBy == params.started`).", "enum": ["turn-boundary"] }, "listed": { diff --git a/packages/spec/schemas/content-type.schema.json b/packages/spec/schemas/content-type.schema.json index 38192fb63..5c3da003a 100644 --- a/packages/spec/schemas/content-type.schema.json +++ b/packages/spec/schemas/content-type.schema.json @@ -33,7 +33,7 @@ } }, "computed": { - "description": "Computed values on each copy: per-copy values with a formula, worked out after every field and never saved. computeSheet returns them per copy (`copies`); the copy's own actions (and the actions it reaches through `use`) and its follow-ups read them as `self.`, as do the copy's later computed values. Nothing else can read them. A name must not reuse a state key or a core property of the copy.", + "description": "Computed values on each copy: per-copy values with a formula, worked out after every field and never saved. computeSheet returns them per copy (`copies`); the copy's own actions (and the actions it reaches through `use`) and its follow-ups read them as `self.`, as do its effects that act as an action runs (on rolls, choice and changes steps, listed actions and incoming changes) and the copy's later computed values. Nothing else can read them: a field effect, field formula, constraint or activation condition reading one is `unknown-name` at load. A name must not reuse a state key or a core property of the copy.", "type": "object", "propertyNames": { "$ref": "common.schema.json#/$defs/name" @@ -160,6 +160,10 @@ "description": "Parameters: may be omitted.", "type": "boolean" }, + "formula": { + "description": "Parameters of type `integer`, `decimal`, `boolean` or `string`: a derived parameter, worked out by the engine, never given (a value from the app, a `use` or an action step is `invalid-param`). Evaluated as the action starts, once every given parameter has its value and before any bound is checked, in declaration order (the action's own, then those `params` effects add), seeing what a bound sees and the run's `participants`: `params` holds the given parameters and the derived ones before it. Read as `params.` everywhere `params` is, bounds included. No `default` or `optional`.", + "$ref": "formula.schema.json" + }, "perCopy": { "description": "Item fields: each copy may save its own value, in its entry's `fields` (`addItem`, `setField`): merged into the copy's `item` after its templates, so a custom stunt's skill is the player's. Elsewhere it is an error.", "type": "boolean" @@ -228,6 +232,31 @@ "type": "object" } }, + { + "description": "A derived parameter is a scalar, never given: no default, nor optional.", + "if": { + "required": ["formula"], + "type": "object" + }, + "then": { + "properties": { + "type": { + "enum": ["integer", "decimal", "boolean", "string"] + } + }, + "not": { + "anyOf": [ + { + "required": ["default"] + }, + { + "required": ["optional"] + } + ] + }, + "type": "object" + } + }, { "description": "Integer members are for a list.", "if": { diff --git a/packages/spec/schemas/effect.schema.json b/packages/spec/schemas/effect.schema.json index 5654f5289..ba7ae57b2 100644 --- a/packages/spec/schemas/effect.schema.json +++ b/packages/spec/schemas/effect.schema.json @@ -137,7 +137,7 @@ "additionalProperties": { "anyOf": [ { - "description": "A parameter added: optional or defaulted, since only the run the app starts has it, and the app may leave it out.", + "description": "A parameter added: optional, defaulted or derived (`formula`), since only the run the app starts has it, and the app may leave it out.", "allOf": [ { "$ref": "content-type.schema.json#/$defs/valueDecl" @@ -156,6 +156,10 @@ "const": true } } + }, + { + "type": "object", + "required": ["formula"] } ] } diff --git a/packages/spec/schemas/engine/describe-action.schema.json b/packages/spec/schemas/engine/describe-action.schema.json index 723e86be4..59dc66dba 100644 --- a/packages/spec/schemas/engine/describe-action.schema.json +++ b/packages/spec/schemas/engine/describe-action.schema.json @@ -64,6 +64,10 @@ "default": { "$ref": "../common.schema.json#/$defs/literal" }, + "derived": { + "description": "A derived parameter (its declaration's `formula`): startAction works it out, so the app never gives it (`invalid-param` if it does). Its value depends on the others, so none is listed.", + "const": true + }, "fixed": { "description": "The item's `use` fixes it: startAction takes `value`, never the app's.", "const": true diff --git a/packages/spec/src/types.ts b/packages/spec/src/types.ts index d60df85e6..283887168 100644 --- a/packages/spec/src/types.ts +++ b/packages/spec/src/types.ts @@ -8,7 +8,7 @@ export type Action = { tags?: TagList; /** When the action is offered, e.g. `self.equipped`. */ available?: Condition; - /** A moment the app runs this rules action at, on every character it applies to: `turn-boundary`, at each turn change, on every character in the encounter, with `params.ended` the `id` of the character whose turn ended and `params.started` the one whose turn starts (each left out when there is none). It runs with nothing the app must make up and never pauses: every parameter has a default or is `optional` (`ended` and `started` are strings), no role has a minimum, it is listed, and neither it nor a rules action it runs has a choice, allocate or handoff step; rolls roll with the app's dice function under the default policy. Content's actions can't have one (`invalid-clock-action` at load). computeSheet lists it with its `clock`. Content joins in through its tagged `changes` steps: resetting what lasts a turn, ending what the creature whose turn starts applied until then (`self.appliedBy == params.started`). */ + /** A moment the app runs this rules action at, on every character it applies to: `turn-boundary`, at each turn change, on every character in the encounter, with `params.ended` the `id` of the character whose turn ended and `params.started` the one whose turn starts (each left out when there is none). It runs with nothing the app must make up and never pauses: every parameter has a default, is derived (`formula`) or is `optional` (`ended` and `started` are strings), no role has a minimum, it is listed, and neither it nor a rules action it runs has a choice, allocate or handoff step; rolls roll with the app's dice function under the default policy. Content's actions can't have one (`invalid-clock-action` at load). computeSheet lists it with its `clock`. Content joins in through its tagged `changes` steps: resetting what lasts a turn, ending what the creature whose turn starts applied until then (`self.appliedBy == params.started`). */ clock?: 'turn-boundary'; /** `false`: computeSheet doesn't list the action and apps don't show it, but an `action` step with `source` or `entry` still runs it (while `available` holds), and so does `startAction`. For an action only other actions run: a class's part of a shared rules action. Default `true`. */ listed?: boolean; @@ -1267,7 +1267,7 @@ export type ContentType = { nameFrom?: Name; /** Per-copy state on the character's copy (charges, shots, level). Visible in formulas as `self.`. `appliedBy` is reserved: an applied entry's `self.appliedBy` is its applier. */ state?: Record; - /** Computed values on each copy: per-copy values with a formula, worked out after every field and never saved. computeSheet returns them per copy (`copies`); the copy's own actions (and the actions it reaches through `use`) and its follow-ups read them as `self.`, as do the copy's later computed values. Nothing else can read them. A name must not reuse a state key or a core property of the copy. */ + /** Computed values on each copy: per-copy values with a formula, worked out after every field and never saved. computeSheet returns them per copy (`copies`); the copy's own actions (and the actions it reaches through `use`) and its follow-ups read them as `self.`, as do its effects that act as an action runs (on rolls, choice and changes steps, listed actions and incoming changes) and the copy's later computed values. Nothing else can read them: a field effect, field formula, constraint or activation condition reading one is `unknown-name` at load. A name must not reuse a state key or a core property of the copy. */ computed?: Record; /** Copies live in the character's `inventory` (true) or `features` (false, default). */ carried?: boolean; @@ -1314,6 +1314,8 @@ export type ValueDecl = { required?: boolean; /** Parameters: may be omitted. */ optional?: boolean; + /** Parameters of type `integer`, `decimal`, `boolean` or `string`: a derived parameter, worked out by the engine, never given (a value from the app, a `use` or an action step is `invalid-param`). Evaluated as the action starts, once every given parameter has its value and before any bound is checked, in declaration order (the action's own, then those `params` effects add), seeing what a bound sees and the run's `participants`: `params` holds the given parameters and the derived ones before it. Read as `params.` everywhere `params` is, bounds included. No `default` or `optional`. */ + formula?: Formula; /** Item fields: each copy may save its own value, in its entry's `fields` (`addItem`, `setField`): merged into the copy's `item` after its templates, so a custom stunt's skill is the player's. Elsewhere it is an error. */ perCopy?: boolean; /** Parameters: the label key. Default: `.params..name`; an optional one-line hint is `.params..description`. */ @@ -2435,6 +2437,8 @@ export type DescribedParam = { /** Neither `optional`, defaulted nor fixed: startAction needs a value (`missing-param`). */ required: boolean; default?: Literal; + /** A derived parameter (its declaration's `formula`): startAction works it out, so the app never gives it (`invalid-param` if it does). Its value depends on the others, so none is listed. */ + derived?: true; /** The item's `use` fixes it: startAction takes `value`, never the app's. */ fixed?: true; /** A fixed parameter's value, evaluated as startAction does; dice as canonical text. */ @@ -3413,6 +3417,7 @@ export type ErrorCode = | 'situational-unavailable' | 'unknown-situational' | 'missing-param' + | 'invalid-param' | 'invalid-participants' | 'undeclared-result' | 'answer-mismatch' diff --git a/packages/spec/tests/cases/actions/derived-param-describe/case.yaml b/packages/spec/tests/cases/actions/derived-param-describe/case.yaml new file mode 100644 index 000000000..5c25feff5 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-describe/case.yaml @@ -0,0 +1,7 @@ +description: 'describeAction lists a derived parameter with `derived: true`, never required' +specVersion: 0.3.0 +call: describeAction +packs: + - path: ../../../packs/test-action-params +input: + action: { id: test-action-params:use-ability, entry: spike } diff --git a/packages/spec/tests/cases/actions/derived-param-describe/character.yaml b/packages/spec/tests/cases/actions/derived-param-describe/character.yaml new file mode 100644 index 000000000..5f23df5e2 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-describe/character.yaml @@ -0,0 +1,7 @@ +specVersion: 0.3.0 +id: hero +packs: + - { id: test-action-params, version: 0.1.0 } +features: + - { id: spike, item: test-action-params:abilities.mind-spike } + - { id: overload, item: test-action-params:features.overload } diff --git a/packages/spec/tests/cases/actions/derived-param-describe/expected.yaml b/packages/spec/tests/cases/actions/derived-param-describe/expected.yaml new file mode 100644 index 000000000..e95deff0b --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-describe/expected.yaml @@ -0,0 +1,18 @@ +action: + definedBy: test-action-params:use-ability + tags: [ability, usage.maneuver, kw.psionic] + results: [used] + params: + - { name: surges, type: integer, required: false, default: 0, min: 0 } + - name: overload + type: boolean + required: false + default: false + addedBy: test-action-params:features.overload + - name: overloadCost + type: integer + required: false + derived: true + addedBy: test-action-params:features.overload + participants: + - { role: target, min: 0, max: 1 } diff --git a/packages/spec/tests/cases/actions/derived-param-given/case.yaml b/packages/spec/tests/cases/actions/derived-param-given/case.yaml new file mode 100644 index 000000000..29c3d60d9 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-given/case.yaml @@ -0,0 +1,8 @@ +description: 'The app gives a derived parameter: `invalid-param`, since the engine works it out' +specVersion: 0.3.0 +call: startAction +packs: + - path: ../../../packs/test-action-params +input: + action: { id: 'test-action-params:use-ability', entry: spike } + params: { overload: true, overloadCost: 0 } diff --git a/packages/spec/tests/cases/actions/derived-param-given/character.yaml b/packages/spec/tests/cases/actions/derived-param-given/character.yaml new file mode 100644 index 000000000..5f23df5e2 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-given/character.yaml @@ -0,0 +1,7 @@ +specVersion: 0.3.0 +id: hero +packs: + - { id: test-action-params, version: 0.1.0 } +features: + - { id: spike, item: test-action-params:abilities.mind-spike } + - { id: overload, item: test-action-params:features.overload } diff --git a/packages/spec/tests/cases/actions/derived-param-given/expected.yaml b/packages/spec/tests/cases/actions/derived-param-given/expected.yaml new file mode 100644 index 000000000..cd26476c7 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-given/expected.yaml @@ -0,0 +1 @@ +errors: [invalid-param] diff --git a/packages/spec/tests/cases/actions/derived-param-run/case.yaml b/packages/spec/tests/cases/actions/derived-param-run/case.yaml new file mode 100644 index 000000000..ea2820a2a --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-run/case.yaml @@ -0,0 +1,11 @@ +description: "A derived parameter a `params` effect adds is worked out from the run's given parameters (the action's own `surges` and the added `overload`) and the feature's computed `drain` (`self.drain`, which effects that act as an action runs read), and read by the feature's changes" +specVersion: 0.3.0 +call: startAction +packs: + - path: ../../../packs/test-action-params +input: + action: { id: 'test-action-params:use-ability', entry: spike } + params: { surges: 2, overload: true } + participants: + target: + - { id: orc, values: {} } diff --git a/packages/spec/tests/cases/actions/derived-param-run/character.yaml b/packages/spec/tests/cases/actions/derived-param-run/character.yaml new file mode 100644 index 000000000..5f23df5e2 --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-run/character.yaml @@ -0,0 +1,7 @@ +specVersion: 0.3.0 +id: hero +packs: + - { id: test-action-params, version: 0.1.0 } +features: + - { id: spike, item: test-action-params:abilities.mind-spike } + - { id: overload, item: test-action-params:features.overload } diff --git a/packages/spec/tests/cases/actions/derived-param-run/expected.yaml b/packages/spec/tests/cases/actions/derived-param-run/expected.yaml new file mode 100644 index 000000000..bc2a8a52b --- /dev/null +++ b/packages/spec/tests/cases/actions/derived-param-run/expected.yaml @@ -0,0 +1,12 @@ +# Mind Spike deals 2; overloading with 2 surges costs 2 + drain (drama 0, plus 1): 3 drama. +status: finished +outcome: { result: used } +proposedChanges: + - to: target + character: orc + step: hit + change: { type: adjust, field: test-action-params:stamina, by: -2, tags: [damage], from: hero } + - to: self + step: hit + change: { type: adjust, field: test-action-params:drama, by: -3 } +log: [] diff --git a/packages/spec/tests/cases/loads/computed-in-field-effect/case.yaml b/packages/spec/tests/cases/loads/computed-in-field-effect/case.yaml new file mode 100644 index 000000000..cefedd6c2 --- /dev/null +++ b/packages/spec/tests/cases/loads/computed-in-field-effect/case.yaml @@ -0,0 +1,26 @@ +description: "A copy's computed values are worked out after every field: its roll effect reads one, but its field effect can't (`unknown-name`), since a computed value feeding a field could loop" +specVersion: 0.3.0 +call: loadRuleset +packs: + - inline: + manifest: + id: test-pack + version: 1.0.0 + specVersion: 0.3.0 + license: CC0-1.0 + defaultLanguage: en + rules: + tags: [attack] + fields: + tier: { type: integer, input: true, default: 1 } + ac: { type: integer, formula: 10 } + contentTypes: + gear: + computed: + bonus: { type: integer, formula: tier + ac } + content: + - id: gear.blade + type: gear + effects: + - { target: { rolls: [attack] }, op: add, value: self.bonus } + - { target: ac, op: add, value: self.bonus } diff --git a/packages/spec/tests/cases/loads/computed-in-field-effect/expected.yaml b/packages/spec/tests/cases/loads/computed-in-field-effect/expected.yaml new file mode 100644 index 000000000..53d3e51ec --- /dev/null +++ b/packages/spec/tests/cases/loads/computed-in-field-effect/expected.yaml @@ -0,0 +1,2 @@ +errors: + - { code: formula-error, cause: unknown-name, path: /packs/0/content/0 } diff --git a/packages/spec/tests/cases/loads/derived-param-reads-later/case.yaml b/packages/spec/tests/cases/loads/derived-param-reads-later/case.yaml new file mode 100644 index 000000000..29f51a70c --- /dev/null +++ b/packages/spec/tests/cases/loads/derived-param-reads-later/case.yaml @@ -0,0 +1,20 @@ +description: "A derived parameter reads only the derived parameters declared before it: two that read each other are a `formula-error` (`unknown-name`) at load, so they can't loop" +specVersion: 0.3.0 +call: loadRuleset +packs: + - inline: + manifest: + id: test-pack + version: 1.0.0 + specVersion: 0.3.0 + license: CC0-1.0 + defaultLanguage: en + rules: + actions: + - id: strike + params: + spend: { type: integer, default: 0 } + cost: { type: integer, formula: params.spend + params.total } + total: { type: integer, formula: params.cost + 1 } + steps: [{ id: done, kind: outcome }] + content: [] diff --git a/packages/spec/tests/cases/loads/derived-param-reads-later/expected.yaml b/packages/spec/tests/cases/loads/derived-param-reads-later/expected.yaml new file mode 100644 index 000000000..b303e87f7 --- /dev/null +++ b/packages/spec/tests/cases/loads/derived-param-reads-later/expected.yaml @@ -0,0 +1,2 @@ +errors: + - { code: formula-error, cause: unknown-name, path: /packs/0/rules } diff --git a/packages/spec/tests/packs/test-action-params/content/features/overload.yaml b/packages/spec/tests/packs/test-action-params/content/features/overload.yaml new file mode 100644 index 000000000..72318f80c --- /dev/null +++ b/packages/spec/tests/packs/test-action-params/content/features/overload.yaml @@ -0,0 +1,19 @@ +# An opt-in on any ability whose cost is derived from the run's other parameters and the +# feature's own computed `drain`: worked out once (`overloadCost`, never given) and read by the +# feature's own changes. +id: features.overload +type: feature +computed: + drain: { type: integer, formula: drama + 1 } +effects: + - target: { actions: [ability] } + op: params + value: + overload: { type: boolean, default: false } + overloadCost: { type: integer, formula: 'params.overload ? params.surges + self.drain : 0' } + - target: { steps: [hit] } + op: changes + when: params.overloadCost > 0 + value: + to: self + changes: [{ type: adjust, field: drama, by: -params.overloadCost }] diff --git a/packages/spec/tests/packs/test-action-params/i18n/en.yaml b/packages/spec/tests/packs/test-action-params/i18n/en.yaml index e0449664b..9cc618781 100644 --- a/packages/spec/tests/packs/test-action-params/i18n/en.yaml +++ b/packages/spec/tests/packs/test-action-params/i18n/en.yaml @@ -46,3 +46,5 @@ conditions.dazed.name: 'Dazed' features.grace.name: 'Healing Grace' features.grace.params.graceStand.name: 'Stand' features.grace.params.graceEnd.name: 'End an effect' +features.overload.name: 'Overload' +features.overload.params.overload.name: 'Overload' diff --git a/scripts/validate/action-params.test.ts b/scripts/validate/action-params.test.ts index c2fa2aba9..604028c76 100644 --- a/scripts/validate/action-params.test.ts +++ b/scripts/validate/action-params.test.ts @@ -116,6 +116,30 @@ describe('pnpm validate on params effects, computed action tags and a use with a expect(added(own.errors)).toEqual([]); }); + it('rejects a derived parameter given by a use, and a formula off an action parameter', () => { + const { errors } = validate({ + 'rules/extra.yaml': [ + 'actions:', + ' - id: spend', + ' params:', + ' n: { type: integer, default: 0 }', + ' cost: { type: integer, formula: params.n * 2 }', + ' steps: [{ id: done, kind: outcome, result: "\'done\'" }]', + ' results: [done]', + 'contentTypes:', + ' charm:', + ' state:', + ' uses: { type: integer, formula: 1 }', + ' actions:', + ' - { use: spend, params: { cost: 1 } }', + ].join('\n'), + }); + expect(errors.filter((m) => m.includes('derived'))).toEqual([ + expect.stringContaining('formula (a derived value) is for action parameters'), + expect.stringContaining('parameter "cost" is derived'), + ]); + }); + it('warns about a recommend naming #done on a step that never offers it', () => { expect(warned(validate({ 'rules/extra.yaml': step('') }))).toHaveLength(1); expect(warned(validate({ 'rules/extra.yaml': step('repeat: { max: 1 }') }))).toEqual([]); diff --git a/scripts/validate/checks.ts b/scripts/validate/checks.ts index 9c785dc30..3167e529b 100644 --- a/scripts/validate/checks.ts +++ b/scripts/validate/checks.ts @@ -301,7 +301,7 @@ export class PackChecker { `${where}: field IDs cannot start with the context root "${id.split('.')[0]}"`, ); } - this.memberKind(def, file, where, { canOpen: true }); + this.memberKind(def, file, where, { canOpen: true, formula: true }); if (def.members && def.default !== undefined) for (const tag of [def.default].flat()) this.memberTag(String(tag), def, file, `${where} default`); @@ -589,8 +589,14 @@ export class PackChecker { for (const name of Object.keys(def.params ?? {})) if (!target.def.params?.[name]) this.report.error(file, `${where}: action "${def.action}" has no parameter "${name}"`); + else this.derivedGiven(target.def, name, file, where); for (const [name, decl] of Object.entries(target.def.params ?? {})) - if (!decl.optional && decl.default === undefined && def.params?.[name] === undefined) + if ( + !decl.optional && + decl.default === undefined && + decl.formula === undefined && + def.params?.[name] === undefined + ) this.report.error(file, `${where}: action "${def.action}" needs parameter "${name}"`); if (def.random) this.randomRun(target.def, target.qid, file, where, new Set()); // A creation action's rolls are a wizard's cards: each needs its own name, or every card @@ -1269,6 +1275,7 @@ export class PackChecker { file, `${where}: creation start "${create.start}"'s action has no parameter "${name}"`, ); + else if (action) this.derivedGiven(action.def, name, file, where); } /** @@ -1471,8 +1478,10 @@ export class PackChecker { decl: ValueDecl, file: string, where: string, - { canOpen = false, itemField = false, takesWhere = false } = {}, + { canOpen = false, itemField = false, takesWhere = false, formula = false } = {}, ): void { + if (decl.formula !== undefined && !formula) + this.report.error(file, `${where}: formula (a derived value) is for action parameters`); const refs = ['id', 'set', 'list'].includes(decl.type) && decl.of !== undefined && @@ -1630,6 +1639,7 @@ export class PackChecker { this.report.error(file, `${where}: no parameter "${name}"`); continue; } + this.derivedGiven(target.def, name, file, where); const bad = (expected: string) => this.report.error( file, @@ -1897,6 +1907,7 @@ export class PackChecker { for (const name of Object.keys(value.params ?? {})) if (!used.def.params?.[name]) this.report.error(file, `${label}: action "${value.use}" has no parameter "${name}"`); + else this.derivedGiven(used.def, name, file, label); for (const change of value.changes ?? []) this.proposedChange(change, file, label, false, new Set(), value.to); } @@ -1912,7 +1923,9 @@ export class PackChecker { // An entry with only bounds adds to a parameter named elsewhere. if (isAddedBound(decl)) continue; const key = `${this.contentOwner}.params.${name}`; - this.requiredKeys.add(decl.label ?? `${key}.name`); + (decl.formula === undefined ? this.requiredKeys : this.optionalKeys).add( + decl.label ?? `${key}.name`, + ); this.optionalKeys.add(`${key}.description`); if (decl.type === 'enum') for (const value of decl.values ?? []) @@ -2075,6 +2088,12 @@ export class PackChecker { private addedNames: Set | undefined; + /** A derived parameter is worked out by the engine: giving it is `invalid-param` at run time. */ + private derivedGiven(def: ActionDef, name: string, file: string, where: string): void { + if (def.params?.[name]?.formula !== undefined) + this.report.error(file, `${where}: parameter "${name}" is derived (formula), never given`); + } + /** Every parameter name a `params` effect in scope adds: a `use` may fix one. */ private addedParamNames(): Set { if (this.addedNames) return this.addedNames; @@ -2105,6 +2124,7 @@ export class PackChecker { for (const name of Object.keys(action.params ?? {})) { if (!target.def.params?.[name] && !this.addedParamNames().has(name)) this.report.error(file, `action "${action.use}" has no parameter "${name}"`); + else this.derivedGiven(target.def, name, file, `${keyPrefix} use ${action.use}`); } return; } @@ -2128,7 +2148,7 @@ export class PackChecker { for (const [name, decl] of Object.entries(action.params ?? {})) { if ((name === 'ended' || name === 'started') && decl.type !== 'string') this.report.error(file, `${key}: a clock action's ${name} is a string (a character ID)`); - else if (decl.default === undefined && !decl.optional) + else if (decl.default === undefined && !decl.optional && decl.formula === undefined) this.report.error(file, `${key}: a clock action's parameter ${name} needs a default`); } for (const [role, def] of Object.entries(action.participants ?? {})) @@ -2225,8 +2245,11 @@ export class PackChecker { this.tagList(action.tags, file, key); for (const [name, decl] of Object.entries(action.params ?? {})) { // A content type's action is named per item, so its parameters stay closed. - this.memberKind(decl, file, key, { canOpen: !key.startsWith('types.') }); - this.requiredKeys.add(decl.label ?? `${key}.params.${name}.name`); + this.memberKind(decl, file, key, { canOpen: !key.startsWith('types.'), formula: true }); + // A derived parameter is never asked for: its name is for apps that show it. + (decl.formula === undefined ? this.requiredKeys : this.optionalKeys).add( + decl.label ?? `${key}.params.${name}.name`, + ); this.optionalKeys.add(`${key}.params.${name}.description`); if (decl.type === 'enum') for (const value of decl.values ?? []) From b4c4b9917d2bb6673e41a847bb2d34d2ff003760 Mon Sep 17 00:00:00 2001 From: Mitchel Pigsley Date: Mon, 2 Nov 2026 10:40:39 -0700 Subject: [PATCH 3/4] Added bounds: an entry needs min or max; document an addedBounds entry with only addedBy (#252) Signed-off-by: Mitchel Pigsley --- packages/engine/src/core/pack-types.ts | 5 ++++- packages/spec/docs/engine-interface.md | 2 +- packages/spec/schemas/engine/describe-action.schema.json | 2 +- packages/spec/src/types.ts | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/engine/src/core/pack-types.ts b/packages/engine/src/core/pack-types.ts index f3a12de6a..9bd47ae5c 100644 --- a/packages/engine/src/core/pack-types.ts +++ b/packages/engine/src/core/pack-types.ts @@ -852,7 +852,10 @@ export type { CharacterFile as CharacterDoc } from '@opencharacter/spec'; * already has, not a declaration. */ export function isAddedBound(decl: ValueDecl): boolean { - return (decl as { type?: string }).type === undefined; + return ( + (decl as { type?: string }).type === undefined && + (decl.min !== undefined || decl.max !== undefined) + ); } export function appliesQueries(item: { appliesTo?: Query | Query[] | undefined }): Query[] { diff --git a/packages/spec/docs/engine-interface.md b/packages/spec/docs/engine-interface.md index 20a4f8a05..223799d28 100644 --- a/packages/spec/docs/engine-interface.md +++ b/packages/spec/docs/engine-interface.md @@ -232,7 +232,7 @@ the definition, wherever it lives: a rules action, one inline on an item or on i default; one that fails, or that reads `params` (it depends on the values the app is yet to give), is left out. `addedBounds` lists the bounds `params` effects add to the parameter ([dice-and-actions.md](dice-and-actions.md#actions)), each `{ addedBy, min?, max? }` - evaluated likewise. + evaluated likewise (an entry whose bounds all read `params` has only `addedBy`). - `values` is an enum's values, those dependent packs add included. `options` lists every ID an `id`, `set` or `list` parameter of a content type, `field` or `tag` takes, its `where` applied: content in ruleset order (hidden items left out), fields in ruleset order, tags diff --git a/packages/spec/schemas/engine/describe-action.schema.json b/packages/spec/schemas/engine/describe-action.schema.json index 59dc66dba..ead7e7b9d 100644 --- a/packages/spec/schemas/engine/describe-action.schema.json +++ b/packages/spec/schemas/engine/describe-action.schema.json @@ -84,7 +84,7 @@ "type": "number" }, "addedBounds": { - "description": "Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`.", + "description": "Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`, so an entry whose bounds all read `params` has only `addedBy`: the parameter is bounded there, by values the app is yet to give.", "type": "array", "items": { "type": "object", diff --git a/packages/spec/src/types.ts b/packages/spec/src/types.ts index 283887168..c23089ccb 100644 --- a/packages/spec/src/types.ts +++ b/packages/spec/src/types.ts @@ -2447,7 +2447,7 @@ export type DescribedParam = { min?: number; /** The `max` bound, evaluated likewise. */ max?: number; - /** Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`. */ + /** Bounds `params` effects add to it (an entry with only `min` and `max`), in instance order, each with the content that added it. startAction checks every one with its own; a bound that reads `params` is left out, as for `min` and `max`, so an entry whose bounds all read `params` has only `addedBy`: the parameter is bounded there, by values the app is yet to give. */ addedBounds?: { /** The content that added it, as a parameter's `addedBy`. */ addedBy: string; From e181acca2f45c474ed298f8101628ad263e4d6a2 Mon Sep 17 00:00:00 2001 From: Mitchel Pigsley Date: Mon, 2 Nov 2026 10:40:39 -0700 Subject: [PATCH 4/4] Derived parameters: the build reports an action step giving one, and one effect's later one (#251) Also: participants' errors come before any parameter's (documented), and the checker restores the aura flag it sets. Signed-off-by: Mitchel Pigsley --- .../derived-params-computed-in-effects.md | 6 ++-- packages/engine/src/check/formulas.ts | 29 +++++++++++++++++-- packages/engine/test/derived-params.test.ts | 21 ++++++++++++++ packages/spec/docs/dice-and-actions.md | 6 ++-- packages/spec/docs/engine-actions.md | 4 +-- scripts/validate/action-params.test.ts | 8 ++++- scripts/validate/checks.ts | 6 ++++ 7 files changed, 70 insertions(+), 10 deletions(-) diff --git a/.changeset/derived-params-computed-in-effects.md b/.changeset/derived-params-computed-in-effects.md index c8a6264e5..8c89f66e5 100644 --- a/.changeset/derived-params-computed-in-effects.md +++ b/.changeset/derived-params-computed-in-effects.md @@ -7,9 +7,11 @@ Derived parameters: an action parameter (its own, or one a `params` effect adds) `formula` instead of being given. startAction works it out once every given parameter has its value and before any bound, in declaration order, reading the given parameters and the derived ones before it (a later one is `unknown-name` at load, so they can't loop), and the run's -`participants`, now checked before the parameters (`invalid-participants` comes first); everything reads it +`participants`, now checked before the parameters (their errors come first). Everything reads it as `params.`. A value for it from the app, a `use`, an `action` step or a handoff is the new -error `invalid-param` (reason `derived`). describeAction lists it with `derived: true`. +error `invalid-param` (reason `derived`); the build reports a `use`'s, a creation start's, a +field roll's, an `action` step's and an `action` effect's. describeAction lists it with +`derived: true`. Effects that act as an action runs (on rolls, choice and changes steps, listed actions and incoming changes) read their content's computed values as `self.`, as its actions do. A diff --git a/packages/engine/src/check/formulas.ts b/packages/engine/src/check/formulas.ts index dac01bd09..d07d35ba4 100644 --- a/packages/engine/src/check/formulas.ts +++ b/packages/engine/src/check/formulas.ts @@ -514,9 +514,13 @@ class FormulaChecker { this.formula(aura.size, at, `${where} size`, NUMBER); this.typed({ applier: viewType(aura.fields ?? {}) }, () => this.withRoots(['applier'], () => { + const outer = this.inAura; this.inAura = true; - for (const effect of aura.effects ?? []) this.effect(effect, at, where); - this.inAura = false; + try { + for (const effect of aura.effects ?? []) this.effect(effect, at, where); + } finally { + this.inAura = outer; + } for (const immunity of aura.immuneTo ?? []) this.immunity(immunity, at, where); }), ); @@ -884,6 +888,27 @@ class FormulaChecker { this.formula(decl.formula, at, `${place} formula`, [fieldFormulaType(decl.type)]), ), ); + // Of this effect's derived ones, it reads only those before it, so they can't loop. + const tree = decl.formula === undefined ? undefined : formulaTree(decl.formula); + if (tree?.ok) { + const entries = Object.entries(effect.value as Record); + const index = entries.findIndex(([n]) => n === name); + const later = new Set( + entries + .slice(index) + .filter(([, d]) => d.formula !== undefined) + .map(([n]) => `params.${n}`), + ); + for (const ref of references(tree.value, new Set())) + if (later.has(ref) || [...later].some((l) => ref.startsWith(`${l}.`))) + this.problems.push({ + at, + where: `${place} formula`, + formula: decl.formula as Formula, + code: 'unknown-name', + message: `"${ref}" is a derived parameter declared at or after "${name}": a derived parameter reads only those before it`, + }); + } if (decl.where === undefined || decl.type !== 'id' || decl.of === undefined) continue; if (decl.of === 'field') this.query({ type: 'field', where: decl.where }, at, place); else if (decl.of !== 'tag' && decl.of !== 'string') diff --git a/packages/engine/test/derived-params.test.ts b/packages/engine/test/derived-params.test.ts index 226890c00..01633cc61 100644 --- a/packages/engine/test/derived-params.test.ts +++ b/packages/engine/test/derived-params.test.ts @@ -74,6 +74,10 @@ const run = ( : result; }; +/** Loads the pack with a feature whose `params` effect adds these. */ +const added = (value: Record) => + loadRuleset([pack(own, [{ target: { actions: ['attack'] }, op: 'params', value }])]); + describe('derived parameters', () => { it('are worked out from the given ones, and steps read them', () => { expect(run(pack(own), { spend: 3 })).toEqual([6]); @@ -138,6 +142,23 @@ describe('derived parameters', () => { expect(run(release, { spend: 2 })).toEqual([4, 0]); }); + it("in one params effect, read only that effect's earlier ones", () => { + const later = added({ + a: { type: 'integer', formula: 'params.b' }, + b: { type: 'integer', formula: '1' }, + }); + expect(later.ok).toBe(false); + if (!later.ok) + expect(later.errors.map((e) => (e.data as { cause?: string }).cause)).toEqual([ + 'unknown-name', + ]); + const earlier = added({ + b: { type: 'integer', formula: '1' }, + a: { type: 'integer', formula: 'params.b' }, + }); + expect(earlier.ok).toBe(true); + }); + it("read the run's participants", () => { const params = { cost: { type: 'integer', formula: 'size(participants.target) * 3' } }; const two = { diff --git a/packages/spec/docs/dice-and-actions.md b/packages/spec/docs/dice-and-actions.md index b16eac030..81d40d280 100644 --- a/packages/spec/docs/dice-and-actions.md +++ b/packages/spec/docs/dice-and-actions.md @@ -110,11 +110,11 @@ isn't `optional`. and `self`, `situational`, the character's fields, the run's `participants` (which a bound doesn't see: Cunning Strike's Trip costs nothing against a Huge target), and `params` with the given parameters and the derived ones declared **before** it. A later derived parameter, and - its own, is `unknown-name` at load for an action's own (through a `params` effect, absent when - it runs), so derived parameters can't loop. Its bounds, and every other bound, see its value. + its own, is `unknown-name` at load (among an action's own, or one effect's; another effect's + is absent when it runs), so derived parameters can't loop. Its bounds, and every other bound, see its value. - **Never given:** a value for it from the app, a `use`, an `action` step or a handoff is `invalid-param` (reason `derived`); the build reports a `use`'s, a creation start's, a field - roll's and an `action` effect's. describeAction lists it with `derived: true` and + roll's, an `action` step's and an `action` effect's. describeAction lists it with `derived: true` and `required: false`. Its label (`.params..name`) is optional: it is for apps that show it. - A value of another type is `type-mismatch` as the action starts. diff --git a/packages/spec/docs/engine-actions.md b/packages/spec/docs/engine-actions.md index f6565ddf1..cce94a228 100644 --- a/packages/spec/docs/engine-actions.md +++ b/packages/spec/docs/engine-actions.md @@ -48,8 +48,8 @@ shape, an optional **dice function**. takes no value (`invalid-param` if given); once every given parameter has its value, each derived one is evaluated in declaration order with what its bounds see, `participants` and `params` so far, and then the bounds are checked. So a derived parameter reads them, the participants - ([step 4](#starting-an-action)) are checked before the parameters: `invalid-participants` - comes first. Every parameter error (`missing-param`, `wrong-type`, + ([step 4](#starting-an-action)) are checked before the parameters: their errors + (`invalid-participants`, `duplicate-id`, `invalid-link-value`) come before any parameter's. Every parameter error (`missing-param`, `wrong-type`, `out-of-range`, `invalid-option`, `invalid-param`), here or at a handoff, also carries `action` (the qualified ID of the action the parameter belongs to; `#` for an item's inline action), for an added parameter `addedBy` (as describeAction gives it), and, for an diff --git a/scripts/validate/action-params.test.ts b/scripts/validate/action-params.test.ts index 604028c76..3e1795811 100644 --- a/scripts/validate/action-params.test.ts +++ b/scripts/validate/action-params.test.ts @@ -116,7 +116,7 @@ describe('pnpm validate on params effects, computed action tags and a use with a expect(added(own.errors)).toEqual([]); }); - it('rejects a derived parameter given by a use, and a formula off an action parameter', () => { + it('rejects a derived parameter given by a use or an action step, and a formula off a parameter', () => { const { errors } = validate({ 'rules/extra.yaml': [ 'actions:', @@ -126,6 +126,8 @@ describe('pnpm validate on params effects, computed action tags and a use with a ' cost: { type: integer, formula: params.n * 2 }', ' steps: [{ id: done, kind: outcome, result: "\'done\'" }]', ' results: [done]', + ' - id: twice', + ' steps: [{ id: again, kind: action, use: spend, params: { cost: 1 } }]', 'contentTypes:', ' charm:', ' state:', @@ -137,6 +139,10 @@ describe('pnpm validate on params effects, computed action tags and a use with a expect(errors.filter((m) => m.includes('derived'))).toEqual([ expect.stringContaining('formula (a derived value) is for action parameters'), expect.stringContaining('parameter "cost" is derived'), + expect.stringContaining('parameter "cost" is derived'), + ]); + expect(errors.filter((m) => m.includes('actions.twice') && m.includes('derived'))).toEqual([ + expect.stringContaining('parameter "cost" is derived'), ]); }); diff --git a/scripts/validate/checks.ts b/scripts/validate/checks.ts index 3167e529b..bff9577b0 100644 --- a/scripts/validate/checks.ts +++ b/scripts/validate/checks.ts @@ -2323,6 +2323,12 @@ export class PackChecker { ) { this.report.error(file, `${where}: unknown action "${step.use}"`); } + if (step.kind === 'action' && step.use) { + const used = this.ruleset.action(step.use, this.scope); + if (used) + for (const name of Object.keys(step.params ?? {})) + this.derivedGiven(used.def, name, file, where); + } // Roles: `each` and a role in `to` must be declared (`target` needn't be), and a handoff // needs an `each`. const declared = (role: string) => Object.hasOwn(action.participants ?? {}, role);