Tested that triggers fire for all properties.

Fixed some action engine issues found.

app/imports/api/engine/action/applyProperties/applyAdjustmentProperty.ts

@@ -20,6 +20,11 @@ export default async function applyAdjustmentProperty(
 
   // Get the operation and value and push the damage hooks to the queue
   if (!prop.amount) {
+    result.appendLog({
+      name: 'Error',
+      value: 'Attribute damage does not have an amount set',
+      silenced: prop.silent,
+    }, damageTargetIds);
     return;
   }
 
@@ -27,27 +32,31 @@ export default async function applyAdjustmentProperty(
   await recalculateCalculation(prop.amount, action, 'reduce', userInput);
   const value = +prop.amount.value;
   if (!isFinite(value)) {
-    return;
-  }
-
-  if (!damageTargetIds?.length) {
-    return;
-  }
-
-  if (damageTargetIds.length !== 1) {
-    throw 'At this step, only a single target is supported'
-  }
-  const targetId = damageTargetIds[0];
-  const statId = getVariables(targetId)?.[prop.stat]?._propId;
-  const stat = statId && getSingleProperty(targetId, statId);
-  if (!stat?.type) {
     result.appendLog({
       name: 'Error',
-      value: `Could not apply attribute damage, creature does not have \`${prop.stat}\` set`,
+      value: 'Attribute damage does not have a finite amount set',
       silenced: prop.silent,
     }, damageTargetIds);
     return;
   }
+
+  if (damageTargetIds.length && damageTargetIds.length !== 1) {
+    throw new Meteor.Error('1 target Expected', 'At this step, only a single target is supported');
+  }
+  const targetId = damageTargetIds[0];
+  let stat;
+  if (targetId) {
+    const statId = getVariables(targetId)?.[prop.stat]?._propId;
+    stat = statId && getSingleProperty(targetId, statId);
+    if (!stat?.type) {
+      result.appendLog({
+        name: 'Error',
+        value: `Could not apply attribute damage, creature does not have \`${prop.stat}\` set`,
+        silenced: prop.silent,
+      }, damageTargetIds);
+      return;
+    }
+  }
   await applyTask(action, {
     targetIds: damageTargetIds,
     subtaskFn: 'damageProp',
@@ -55,7 +64,7 @@ export default async function applyAdjustmentProperty(
       title: getPropertyTitle(prop),
       operation: prop.operation,
       value,
-      targetProp: stat,
+      targetProp: stat ?? { _id: 'dummyStat', name: prop.stat, type: 'attribute' },
     },
   }, userInput);
   return applyDefaultAfterPropTasks(action, prop, damageTargetIds, userInput);

app/imports/api/engine/action/applyProperties/applyBuffProperty.ts

@@ -26,6 +26,13 @@ export default async function applyBuffProperty(
   const prop = EJSON.clone(task.prop);
   const targetIds = prop.target === 'self' ? [action.creatureId] : task.targetIds;
 
+  // Log the buff and return if there are no targets
+  if (!targetIds.length) {
+    logBuff(prop, targetIds, action, userInput, result);
+    await applyAfterTasksSkipChildren(action, prop, targetIds, userInput);
+    return;
+  }
+
   // Get the buff and its descendants
   const propList = [
     EJSON.clone(prop),
@@ -55,16 +62,7 @@ export default async function applyBuffProperty(
     });
 
     //Log the buff
-    let logValue = prop.description?.value
-    if (prop.description?.text) {
-      recalculateInlineCalculations(prop.description, action, 'reduce', userInput);
-      logValue = prop.description?.value;
-    }
-    result.appendLog({
-      name: getPropertyTitle(prop),
-      value: logValue,
-      silenced: prop.silent,
-    }, [target]);
+    logBuff(prop, targetIds, action, userInput, result);
 
     // remove all the computed fields
     targetPropList = cleanProps(targetPropList);
@@ -75,7 +73,21 @@ export default async function applyBuffProperty(
     // Add the mutation to the results
     result.mutations.push(mutation);
   });
-  applyAfterTasksSkipChildren(action, prop, targetIds, userInput);
+  await applyAfterTasksSkipChildren(action, prop, targetIds, userInput);
+}
+
+async function logBuff(prop, targetIds, action, userInput, result) {
+  //Log the buff
+  let logValue = prop.description?.value
+  if (prop.description?.text) {
+    recalculateInlineCalculations(prop.description, action, 'reduce', userInput);
+    logValue = prop.description?.value;
+  }
+  result.appendLog({
+    name: getPropertyTitle(prop),
+    ...logValue && { value: logValue },
+    silenced: prop.silent,
+  }, targetIds);
 }
 
 /**

app/imports/api/engine/action/applyProperties/applyBuffRemoverProperty.ts

@@ -27,6 +27,10 @@ export default async function applyBuffRemoverProperty(
     return applyTaskToEachTarget(action, task, targetIds, userInput);
   }
 
+  if (!targetIds.length) {
+    return applyDefaultAfterPropTasks(action, prop, task.targetIds, userInput);
+  }
+
   if (targetIds.length !== 1) {
     throw 'At this step, only a single target is supported'
   }

app/imports/api/engine/action/applyProperties/applyDamageProperty.ts

@@ -23,11 +23,14 @@ export default async function applyDamageProperty(
   const prop = task.prop;
   const scope = getEffectiveActionScope(action);
 
-  // Skip if there is no parse node to work with
-  if (!prop.amount?.parseNode) return;
-
   // Choose target
   const damageTargets = prop.target === 'self' ? [action.creatureId] : task.targetIds;
+
+  // Skip if there is no parse node to work with
+  if (!prop.amount?.valueNode) {
+    return applyDefaultAfterPropTasks(action, prop, damageTargets, inputProvider);
+  }
+
   // Determine if the hit is critical
   const criticalHit = await getConstantValueFromScope('~criticalHit', scope)
     && prop.damageType !== 'healing'; // Can't critically heal
@@ -91,9 +94,9 @@ export default async function applyDamageProperty(
   }
 
   // Memoise the damage suffix for the log
-  const suffix = (criticalHit ? ' critical ' : ' ') +
+  const suffix = (criticalHit ? 'critical ' : '') +
     prop.damageType +
-    (prop.damageType !== 'healing' ? ' damage ' : '');
+    (prop.damageType !== 'healing' ? ' damage' : '');
 
   // If there is a save, calculate the save damage
   let damageOnSave, saveProp, saveRoll;

app/imports/api/engine/action/applyProperties/applySavingThrowProperty.ts

@@ -46,7 +46,6 @@ export default async function applySavingThrowProperty(
   // If there are no save targets, apply all children as if the save both
   // succeeded and failed
   if (!targetId) {
-    console.warn('no target, returning early');
     result.pushScope = {
       ['~saveFailed']: { value: true },
       ['~saveSucceeded']: { value: true },

app/imports/api/engine/action/applyProperties/applyTriggerProperty.test.ts

@@ -9,53 +9,91 @@ import {
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties';
 
 const [
-  creatureId, targetCreatureId, targetCreature2Id, actionWithTriggerId, triggerBeforeActionId,
-  triggerAfterActionId, triggerAfterActionChildrenId
+  creatureId, targetCreatureId, targetCreature2Id,
 ] = getRandomIds(100);
 
+const idMap: Record<string, string> = {};
+
+const propsWithTriggers = ([
+  { type: 'action' },
+  {
+    type: 'adjustment',
+    target: 'target',
+    stat: 'strength',
+    operation: 'increment',
+    amount: { calculation: '2' }
+  },
+  {
+    type: 'branch',
+    branchType: 'if',
+    condition: { calculation: 'true' },
+  },
+  { type: 'buff' },
+  { type: 'buffRemover' },
+  {
+    type: 'damage',
+    amount: { calculation: '1d13 + 3' },
+  },
+  { type: 'note' },
+  {
+    type: 'roll',
+    roll: { calculation: '1d20' },
+  },
+  {
+    type: 'savingThrow',
+    stat: 'strengthSave',
+    dc: { calculation: '10' },
+  },
+  { type: 'spell' },
+] as any[]).map(prop => {
+  idMap[prop.type] = Random.id();
+  idMap[prop.type + 'Before'] = Random.id();
+  idMap[prop.type + 'After'] = Random.id();
+  idMap[prop.type + 'AfterChildren'] = Random.id();
+  prop._id = idMap[prop.type];
+  prop.name = prop.type;
+  return prop;
+});
+
 const actionTestCreature = {
   _id: creatureId,
-  props: [
-    // Action with triggers
+  props: propsWithTriggers.map(prop => [
+    // Props with triggers
     {
-      _id: actionWithTriggerId,
-      type: 'action',
       tags: ['trigger tag'],
       children: [
         {
           type: 'note',
-          name: 'Action Child'
+          name: 'Note Child'
         }
       ],
-    },
-    {
-      _id: triggerBeforeActionId,
+      ...prop,
+    }, {
+      _id: idMap[prop.type + 'Before'],
       type: 'trigger',
       targetTags: ['trigger tag'],
-      name: 'Before Action Trigger',
+      name: `Before ${prop.type} Trigger`,
       event: 'doActionProperty',
-      actionPropertyType: 'action',
+      actionPropertyType: prop.type,
       timing: 'before',
-    },
-    {
-      _id: triggerAfterActionId,
+    }, {
+      _id: idMap[prop.type + 'After'],
       type: 'trigger',
       targetTags: ['trigger tag'],
-      name: 'After Action Trigger',
+      name: `After ${prop.type} Trigger`,
       event: 'doActionProperty',
-      actionPropertyType: 'action',
+      actionPropertyType: prop.type,
       timing: 'after',
-    },
-    {
-      _id: triggerAfterActionChildrenId,
+    }, {
+      _id: idMap[prop.type + 'AfterChildren'],
       type: 'trigger',
       targetTags: ['trigger tag'],
-      name: 'After Action Children Trigger',
+      name: `After ${prop.type} Children Trigger`,
       event: 'doActionProperty',
-      actionPropertyType: 'action',
+      actionPropertyType: prop.type,
       timing: 'afterChildren',
     },
-  ],
+  ]).flat(),
 }
 
 const actionTargetCreature = {
@@ -93,27 +131,73 @@ describe('Triggers', function () {
     await createTestCreature(actionTargetCreature2);
   });
 
-  it('should run triggers on actions', async function () {
-    const actionProp = CreatureProperties.findOne(actionWithTriggerId);
-    assert.deepEqual(actionProp.triggerIds, {
-      before: [triggerBeforeActionId],
-      after: [triggerAfterActionId],
-      afterChildren: [triggerAfterActionChildrenId],
-    }, 'Prop\'s triggerIds should be set');
-    const action = await runActionById(actionWithTriggerId);
-    assert.exists(action);
-    assert.deepEqual(allLogContent(action), [
+  it('should run triggers on all props', async function () {
+    const expectedLogs: any[] = [
       {
-        name: 'Before Action Trigger',
+        type: 'action',
+        content: [{ name: 'action' }],
       }, {
-        name: 'Action',
+        type: 'adjustment',
+        content: [{ inline: true, name: 'Attribute damage', value: 'strength 2' }],
       }, {
-        name: 'After Action Trigger',
+        type: 'branch',
+        content: [],
       }, {
-        name: 'Action Child',
+        type: 'buff',
+        content: [{ name: 'buff' }],
+        noChildren: true,
       }, {
-        name: 'After Action Children Trigger',
+        type: 'buffRemover',
+        content: [{ name: 'buffRemover' }],
+      }, {
+        type: 'damage',
+        content: [{
+          inline: true, name: 'Damage', value: '1d13 [7] + 3\n**10** slashing damage'
+        }],
+      }, {
+        type: 'note',
+        content: [{ name: 'note' }],
+      }, {
+        type: 'roll',
+        content: [{
+          name: 'roll', inline: true, value: '1d20 [10]\n**10**'
+        }],
+      }, {
+        type: 'savingThrow',
+        content: [{ name: 'savingThrow', inline: true, value: 'DC **10**' }],
+      }, {
+        type: 'spell',
+        content: [{ name: 'spell' }],
       },
-    ]);
+    ];
+    for (const log of expectedLogs) try {
+      const type = log.type;
+      const actionProp = CreatureProperties.findOne(idMap[type]);
+      assert.deepEqual(actionProp.triggerIds, {
+        before: [idMap[type + 'Before']],
+        after: [idMap[type + 'After']],
+        afterChildren: [idMap[type + 'AfterChildren']],
+      }, 'Prop\'s triggerIds should be set');
+      const action = await runActionById(idMap[type]);
+      assert.exists(action);
+      assert.deepEqual(allLogContent(action), [
+        {
+          name: `Before ${type} Trigger`,
+        },
+        ...log.content,
+        {
+          name: `After ${type} Trigger`,
+        },
+        ...log.noChildren ? [] : [{
+          name: 'Note Child',
+        }],
+        {
+          name: `After ${type} Children Trigger`,
+        },
+      ]);
+    } catch (e) {
+      console.error(`failed when running ${log.type}`);
+      throw e
+    }
   });
 });

app/imports/api/engine/action/applyProperties/index.ts

@@ -31,9 +31,10 @@ const applyPropertyByType: {
   note,
   roll,
   savingThrow,
+  spell: action,
   propertySlot: folder,
   toggle,
   trigger,
 }
 
-export default applyPropertyByType;
+export default applyPropertyByType;

app/imports/api/engine/action/tasks/applyTask.ts

@@ -9,6 +9,7 @@ import InputProvider from '/imports/api/engine/action/functions/userInput/InputP
 import applyCheckTask from '/imports/api/engine/action/tasks/applyCheckTask';
 import applyResetTask from '/imports/api/engine/action/tasks/applyResetTask';
 import applyCastSpellTask from '/imports/api/engine/action/tasks/applyCastSpellTask';
+import { getPropertyName } from '/imports/constants/PROPERTIES';
 
 // DamagePropTask promises a number of actual damage done
 export default async function applyTask(
@@ -79,6 +80,14 @@ export default async function applyTask(
     action.results.push(result);
 
     // Apply the property
-    return applyProperties[prop.type]?.(task, action, result, inputProvider);
+    if (!applyProperties[prop.type]) {
+      result.appendLog({
+        name: 'Warning',
+        value: `Could not apply ${getPropertyName(prop.type)}, only certain properties can be run as part of an action`,
+        silenced: false,
+      }, task.targetIds);
+      return;
+    }
+    return applyProperties[prop.type](task, action, result, inputProvider);
   }
 }

app/imports/parser/parseTree/constant.ts

@@ -42,7 +42,8 @@ const constant: ConstantFactory = {
 }
 
 export function isFiniteNode(node: ParseNode): node is FiniteNumberConstantNode {
-  return node.parseType === 'constant'
+  return node
+    && node.parseType === 'constant'
     && node.valueType === 'number'
     && typeof node.value === 'number'
     && isFinite(node.value);