From 2c431293e0b26da2490451790141e04ce0c8fbbd Mon Sep 17 00:00:00 2001 From: ThaumRystra Date: Thu, 7 Nov 2024 00:22:46 +0200 Subject: [PATCH] Tested that triggers fire for all properties. Fixed some action engine issues found. --- .../applyAdjustmentProperty.ts | 41 +++-- .../applyProperties/applyBuffProperty.ts | 34 ++-- .../applyBuffRemoverProperty.ts | 4 + .../applyProperties/applyDamageProperty.ts | 13 +- .../applySavingThrowProperty.ts | 1 - .../applyTriggerProperty.test.ts | 162 +++++++++++++----- .../engine/action/applyProperties/index.ts | 3 +- .../api/engine/action/tasks/applyTask.ts | 11 +- app/imports/parser/parseTree/constant.ts | 3 +- 9 files changed, 197 insertions(+), 75 deletions(-) diff --git a/app/imports/api/engine/action/applyProperties/applyAdjustmentProperty.ts b/app/imports/api/engine/action/applyProperties/applyAdjustmentProperty.ts index aee148858..9f706dbd5 100644 --- a/app/imports/api/engine/action/applyProperties/applyAdjustmentProperty.ts +++ b/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); diff --git a/app/imports/api/engine/action/applyProperties/applyBuffProperty.ts b/app/imports/api/engine/action/applyProperties/applyBuffProperty.ts index 241912f63..fca54e4f8 100644 --- a/app/imports/api/engine/action/applyProperties/applyBuffProperty.ts +++ b/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); } /** diff --git a/app/imports/api/engine/action/applyProperties/applyBuffRemoverProperty.ts b/app/imports/api/engine/action/applyProperties/applyBuffRemoverProperty.ts index 791f0b7ad..d4547b33a 100644 --- a/app/imports/api/engine/action/applyProperties/applyBuffRemoverProperty.ts +++ b/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' } diff --git a/app/imports/api/engine/action/applyProperties/applyDamageProperty.ts b/app/imports/api/engine/action/applyProperties/applyDamageProperty.ts index c38c53e54..d5190e441 100644 --- a/app/imports/api/engine/action/applyProperties/applyDamageProperty.ts +++ b/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; diff --git a/app/imports/api/engine/action/applyProperties/applySavingThrowProperty.ts b/app/imports/api/engine/action/applyProperties/applySavingThrowProperty.ts index 956883315..e09186a6a 100644 --- a/app/imports/api/engine/action/applyProperties/applySavingThrowProperty.ts +++ b/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 }, diff --git a/app/imports/api/engine/action/applyProperties/applyTriggerProperty.test.ts b/app/imports/api/engine/action/applyProperties/applyTriggerProperty.test.ts index b92f1c1db..6585e6741 100644 --- a/app/imports/api/engine/action/applyProperties/applyTriggerProperty.test.ts +++ b/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 = {}; + +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 + } }); }); diff --git a/app/imports/api/engine/action/applyProperties/index.ts b/app/imports/api/engine/action/applyProperties/index.ts index 458c5bb68..e4079d3e7 100644 --- a/app/imports/api/engine/action/applyProperties/index.ts +++ b/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; \ No newline at end of file +export default applyPropertyByType; diff --git a/app/imports/api/engine/action/tasks/applyTask.ts b/app/imports/api/engine/action/tasks/applyTask.ts index 494c1bf99..eada60e5a 100644 --- a/app/imports/api/engine/action/tasks/applyTask.ts +++ b/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); } } diff --git a/app/imports/parser/parseTree/constant.ts b/app/imports/parser/parseTree/constant.ts index 390760e7c..36cb78e64 100644 --- a/app/imports/parser/parseTree/constant.ts +++ b/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);