Fixed: tagless triggers should target everything not nothing.

also fixed: check triggers now correctly fire on skill used for checks

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

@@ -4,7 +4,8 @@ import {
   createTestCreature,
   getRandomIds,
   removeAllCreaturesAndProps,
-  runActionById
+  runActionById,
+  TestCreature
 } from '/imports/api/engine/action/functions/actionEngineTest.testFn';
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties';
 
@@ -55,7 +56,7 @@ const propsWithTriggers = ([
   return prop;
 });
 
-const actionTestCreature = {
+const actionTestCreature: TestCreature = {
   _id: creatureId,
   props: propsWithTriggers.map(prop => [
     // Props with triggers
@@ -96,7 +97,7 @@ const actionTestCreature = {
   ]).flat(),
 }
 
-const actionTargetCreature = {
+const actionTargetCreature: TestCreature = {
   _id: targetCreatureId,
   props: [
     {
@@ -108,7 +109,7 @@ const actionTargetCreature = {
   ]
 }
 
-const actionTargetCreature2 = {
+const actionTargetCreature2: TestCreature = {
   _id: targetCreature2Id,
   props: [
     {
@@ -173,7 +174,7 @@ describe('Triggers', function () {
     for (const log of expectedLogs) try {
       const type = log.type;
       const actionProp = CreatureProperties.findOne(idMap[type]);
-      assert.deepEqual(actionProp.triggerIds, {
+      assert.deepEqual(actionProp?.triggerIds, {
         before: [idMap[type + 'Before']],
         after: [idMap[type + 'After']],
         afterChildren: [idMap[type + 'AfterChildren']],
@@ -200,4 +201,44 @@ describe('Triggers', function () {
       throw e
     }
   });
+
+  it('Targets all properties when no tags are specified', async function () {
+    const [creatureId, actionId, triggerId] = getRandomIds(3);
+    const creature: TestCreature = {
+      _id: creatureId,
+      props: [
+        {
+          _id: actionId,
+          type: 'action',
+          name: 'Action',
+          children: [{
+            type: 'note',
+            summary: { text: 'note summary {1 + 2}' }
+          }]
+        }, {
+          _id: triggerId,
+          type: 'trigger',
+          targetTags: [],
+          name: 'Before Trigger',
+          event: 'doActionProperty',
+          actionPropertyType: 'action',
+          timing: 'before',
+        }
+      ],
+    };
+    await createTestCreature(creature);
+    const action = await runActionById(actionId, [creature._id]);
+    const actionProp = CreatureProperties.findOne(actionId);
+    assert.exists(actionProp);
+    assert.deepEqual(actionProp?.triggerIds, {
+      before: [triggerId],
+    });
+    assert.deepEqual(allLogContent(action), [{
+      name: 'Before Trigger',
+    }, {
+      name: 'Action',
+    }, {
+      value: 'note summary 3',
+    }]);
+  });
 });

app/imports/api/engine/computation/computeComputation/computeByType/computeTrigger.js

@@ -1,48 +0,0 @@
-import { get, set } from 'lodash';
-import { getEffectTagTargets } from '/imports/api/engine/computation/buildComputation/linkTypeDependencies';
-
-export default function computeTrigger(computation, node) {
-  const prop = node.data;
-
-  // Triggers that aren't active aren't linked to properties
-  if (prop.inactive) return;
-
-  // Link triggers to all the properties that would fire them when applied
-  const tagTargets = getEffectTagTargets(prop, computation);
-  for (const targetId of tagTargets) {
-    const targetProp = computation.propsById[targetId];
-    switch (prop.event) {
-      case 'doActionProperty':
-        // Only apply if the trigger matches this property type
-        if (targetProp.type === prop.actionPropertyType) {
-          setTrigger(prop, targetProp, 'triggerIds');
-        }
-        // Or on an item used as ammo
-        else if (prop.actionPropertyType === 'ammo' && targetProp.type === 'item') {
-          setTrigger(prop, targetProp, 'ammoTriggerIds');
-        }
-        break;
-      case 'damageProperty':
-        // Only apply to attributes
-        if (targetProp.type === 'attribute') {
-          setTrigger(prop, targetProp, 'damageTriggerIds');
-        }
-        break;
-      case 'check':
-        // Only apply to attributes and skills
-        if (targetProp.type === 'attribute' || targetProp.type === 'skill') {
-          setTrigger(prop, targetProp, 'checkTriggerIds');
-        }
-        break;
-    }
-  }
-}
-
-function setTrigger(prop, targetProp, field = 'triggerIds') {
-  let triggerIdArray = get(targetProp, `${field}.${prop.timing}`);
-  if (!triggerIdArray) {
-    triggerIdArray = [];
-    set(targetProp, `${field}.${prop.timing}`, triggerIdArray);
-  }
-  triggerIdArray.push(prop._id);
-}

app/imports/api/engine/computation/computeComputation/computeByType/computeTrigger.ts

@@ -0,0 +1,48 @@
+import { getEffectTagTargets } from '/imports/api/engine/computation/buildComputation/linkTypeDependencies';
+import CreatureComputation from '/imports/api/engine/computation/CreatureComputation';
+import { CreaturePropertyTypes } from '/imports/api/creature/creatureProperties/CreatureProperties';
+
+export default function computeTrigger(computation: CreatureComputation, node: { data: CreaturePropertyTypes['trigger'] }) {
+  const prop = node.data;
+
+  // Triggers that aren't active aren't linked to properties
+  if (prop.inactive) return;
+
+  // Link triggers to all the properties that would fire them when applied
+  let tagTargets: string[] = getEffectTagTargets(prop, computation);
+  // If we have no tags or extra tags, target everything
+  if (!prop.targetTags?.length && !prop.extraTags?.length) {
+    tagTargets = computation.props.map(targetProp => targetProp._id).filter(id => id !== prop._id);
+  }
+  for (const targetId of tagTargets) {
+    const targetProp = computation.propsById[targetId];
+    switch (prop.event) {
+      case 'doActionProperty':
+        // Only apply if the trigger matches this property type
+        if (targetProp.type === prop.actionPropertyType) {
+          targetProp.triggerIds ??= {};
+          (targetProp.triggerIds[prop.timing] ??= []).push(prop._id);
+        }
+        // Or on an item used as ammo
+        else if (prop.actionPropertyType === 'ammo' && targetProp.type === 'item') {
+          targetProp.ammoTriggerIds ??= {};
+          (targetProp.ammoTriggerIds[prop.timing] ??= []).push(prop._id);
+        }
+        break;
+      case 'damageProperty':
+        // Only apply to attributes
+        if (targetProp.type === 'attribute') {
+          targetProp.damageTriggerIds ??= {};
+          (targetProp.damageTriggerIds[prop.timing] ??= []).push(prop._id);
+        }
+        break;
+      case 'check':
+        // Only apply to attributes and skills
+        if (targetProp.type === 'attribute' || targetProp.type === 'skill') {
+          targetProp.checkTriggerIds ??= {};
+          (targetProp.checkTriggerIds[prop.timing] ??= []).push(prop._id);
+        }
+        break;
+    }
+  }
+}

app/imports/api/properties/Skills.ts

@@ -169,6 +169,37 @@ const ComputedOnlySkillSchema = createPropertySchema({
     type: Number,
     optional: true,
   },
+
+  // Triggers that fire when this property is used to make a check
+  'checkTriggerIds': {
+    type: Object,
+    optional: true,
+    removeBeforeCompute: true,
+  },
+  'checkTriggerIds.before': {
+    type: Array,
+    optional: true,
+  },
+  'checkTriggerIds.before.$': {
+    type: String,
+    max: 32,
+  },
+  'checkTriggerIds.after': {
+    type: Array,
+    optional: true,
+  },
+  'checkTriggerIds.after.$': {
+    type: String,
+    max: 32,
+  },
+  'checkTriggerIds.afterChildren': {
+    type: Array,
+    optional: true,
+  },
+  'checkTriggerIds.afterChildren.$': {
+    type: String,
+    max: 32,
+  },
 })
 
 const ComputedSkillSchema = TypedSimpleSchema.from({})