Settling on a data structure to balance compatibility

with not being wrong

app/imports/api/engine/actions/applyPropertyByType/applyDamage.js

@@ -3,7 +3,7 @@ import applyChildren from '/imports/api/engine/actions/applyPropertyByType/share
 import { insertCreatureLog } from '/imports/api/creature/log/CreatureLogs.js';
 import resolve, { Context, toString } from '/imports/parser/resolve.js';
 import logErrors from './shared/logErrors.js';
-import applyEffectsToCalculationParseNode from '/imports/api/engine/actions/applyPropertyByType/shared/applyEffectsToCalculationParseNode.js';
+import recalculateCalculation from '/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js'
 import { damagePropertyWork } from '/imports/api/creature/creatureProperties/methods/damageProperty.js';
 import {
   getPropertiesOfType
@@ -37,8 +37,8 @@ export default function applyDamage(node, actionContext) {
   const logName = prop.damageType === 'healing' ? 'Healing' : 'Damage';
 
   // roll the dice only and store that string
-  applyEffectsToCalculationParseNode(prop.amount, actionContext);
+  recalculateCalculation(prop.amount, actionContext, undefined, 'compile');
-  const { result: rolled } = resolve('roll', prop.amount.parseNode, scope, context);
+  const { result: rolled } = resolve('roll', prop.amount.valueNode, scope, context);
   if (rolled.parseType !== 'constant') {
     logValue.push(toString(rolled));
   }
@@ -88,8 +88,8 @@ export default function applyDamage(node, actionContext) {
   let damageOnSave, saveNode, saveRoll;
   if (prop.save) {
     if (prop.save.damageFunction?.calculation) {
-      applyEffectsToCalculationParseNode(prop.save.damageFunction, actionContext);
+      recalculateCalculation(prop.save.damageFunction, actionContext, undefined, 'compile');
-      let { result: saveDamageRolled } = resolve('roll', prop.save.damageFunction.parseNode, scope, context);
+      let { result: saveDamageRolled } = resolve('roll', prop.save.damageFunction.valueNode, scope, context);
       saveRoll = toString(saveDamageRolled);
       let { result: saveDamageResult } = resolve('reduce', saveDamageRolled, scope, context);
       // If we didn't end up with a constant of finite amount, give up

app/imports/api/engine/actions/applyPropertyByType/applyRoll.js

@@ -1,6 +1,6 @@
 import applyChildren from '/imports/api/engine/actions/applyPropertyByType/shared/applyChildren.js';
 import logErrors from './shared/logErrors.js';
-import applyEffectsToCalculationParseNode from '/imports/api/engine/actions/applyPropertyByType/shared/applyEffectsToCalculationParseNode.js';
+import recalculateCalculation from '/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js';
 import resolve, { toString } from '/imports/parser/resolve.js';
 import { applyNodeTriggers } from '/imports/api/engine/actions/applyTriggers.js';
 
@@ -12,8 +12,8 @@ export default function applyRoll(node, actionContext) {
     const logValue = [];
 
     // roll the dice only and store that string
-    applyEffectsToCalculationParseNode(prop.roll, actionContext);
+    recalculateCalculation(prop.roll, actionContext, undefined, 'compile');
-    const { result: rolled, context } = resolve('roll', prop.roll.parseNode, actionContext.scope);
+    const { result: rolled, context } = resolve('roll', prop.roll.valueNode, actionContext.scope);
     if (rolled.parseType !== 'constant') {
       logValue.push(toString(rolled));
     }

app/imports/api/engine/actions/applyPropertyByType/shared/applyEffectsToCalculationParseNode.js

@@ -1,46 +0,0 @@
-import operator from '/imports/parser/parseTree/operator.js';
-import { parse } from '/imports/parser/parser.js';
-import logErrors from './logErrors.js';
-
-export default function applyEffectsToCalculationParseNode(calcObj, actionContext) {
-  calcObj.effects?.forEach(effect => {
-    if (effect.operation !== 'add') return;
-    if (!effect.amount) return;
-    if (effect.amount.value === null) return;
-    let effectParseNode;
-    try {
-      effectParseNode = parse(effect.amount.value.toString());
-      calcObj.parseNode = operator.create({
-        left: calcObj.parseNode,
-        right: effectParseNode,
-        operator: '+',
-        fn: 'add'
-      });
-    } catch (e) {
-      logErrors([e], actionContext)
-    }
-  });
-  // Add the highest proficiency as well
-  let highestProficiency;
-  calcObj.proficiencies?.forEach(proficiency => {
-    if (
-      proficiency.value > highestProficiency
-      || (highestProficiency === undefined && Number.isFinite(proficiency.value))
-    ) {
-      highestProficiency = proficiency.value;
-    }
-  });
-  if (highestProficiency) {
-    try {
-      let profParseNode = parse(highestProficiency.toString());
-      calcObj.parseNode = operator.create({
-        left: calcObj.parseNode,
-        right: profParseNode,
-        operator: '+',
-        fn: 'add'
-      });
-    } catch (e) {
-      logErrors([e], actionContext)
-    }
-  }
-}

app/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js

@@ -1,11 +1,38 @@
-import evaluateCalculation from '/imports/api/engine/computation/utility/evaluateCalculation.js';
-import applyEffectsToCalculationParseNode from '/imports/api/engine/actions/applyPropertyByType/shared/applyEffectsToCalculationParseNode.js';
 import logErrors from './logErrors.js';
+import { toPrimitiveOrString } from '/imports/parser/resolve.js';
+import {
+  aggregateCalculationEffects,
+  aggregateCalculationProficiencies,
+  resolveCalculationNode,
+} from '/imports/api/engine/computation/computeComputation/computeByType/computeCalculation.js';
+import { getSingleProperty } from '/imports/api/engine/loadCreatures';
 
-export default function recalculateCalculation(calc, actionContext, context) {
+// Redo the work of imports/api/engine/computation/computeComputation/computeByType/computeCalculation.js
-  if (!calc?.parseNode) return;
+// But in the action scope
-  calc._parseLevel = 'reduce';
+export default function recalculateCalculation(calcObj, actionContext, context, parseLevel = 'reduce') {
-  applyEffectsToCalculationParseNode(calc, actionContext);
+  if (!calcObj?.parseNode) return;
-  evaluateCalculation(calc, actionContext.scope, context);
+  calcObj._parseLevel = parseLevel;
-  logErrors(calc.errors, actionContext);
+  // Re-resolve the parse node
+  resolveCalculationNode(calcObj, calcObj.parseNode, actionContext.scope, context);
+  // store the unaffected value
+  if (calcObj.effectIds || calcObj.proficiencyIds) {
+    calcObj.unaffected = toPrimitiveOrString(calcObj.valueNode);
+  }
+  // Apply all the effects and proficiencies
+  aggregateCalculationEffects(
+    calcObj,
+    id => getSingleProperty(actionContext.creature._id, id)
+  );
+  aggregateCalculationProficiencies(
+    calcObj,
+    id => getSingleProperty(actionContext.creature._id, id),
+    actionContext.scope['proficiencyBonus']?.value || 0
+  );
+  // Resolve the modified valueNode
+  resolveCalculationNode(calcObj, calcObj.valueNode, actionContext.scope, context);
+
+  // Store the primitive value
+  calcObj.value = toPrimitiveOrString(calcObj.valueNode);
+
+  logErrors(calcObj.errors, actionContext);
 }

app/imports/api/engine/actions/doCheck.js

@@ -7,7 +7,7 @@ import rollDice from '/imports/parser/rollDice.js';
 import numberToSignedString from '/imports/api/utility/numberToSignedString.js';
 import { applyTriggers } from '/imports/api/engine/actions/applyTriggers.js';
 import ActionContext from '/imports/api/engine/actions/ActionContext.js';
-import evaluateCalculation from '/imports/api/engine/computation/utility/evaluateCalculation.js';
+import recalculateCalculation from '/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation';
 
 const doCheck = new ValidatedMethod({
   name: 'creatureProperties.doCheck',
@@ -76,7 +76,7 @@ function rollCheck(prop, actionContext) {
 
   let rollModifierText = numberToSignedString(rollModifier, true);
 
-  const { effectBonus, effectString } = applyUnresolvedEffects(prop, scope)
+  const { effectBonus, effectString } = applyUnresolvedEffects(prop, actionContext)
   rollModifierText += effectString;
   rollModifier += effectBonus;
 
@@ -116,7 +116,8 @@ function rollCheck(prop, actionContext) {
   });
 }
 
-export function applyUnresolvedEffects(prop, scope) {
+// TODO replace this with recalculating and then rolling/reducing the value node
+export function applyUnresolvedEffects(prop, actionContext) {
   let effectBonus = 0;
   let effectString = '';
   if (!prop.effects) {
@@ -125,8 +126,7 @@ export function applyUnresolvedEffects(prop, scope) {
   prop.effects.forEach(effect => {
     if (!effect.amount?.parseNode) return;
     if (effect.operation !== 'add') return;
-    effect.amount._parseLevel = 'reduce';
+    recalculateCalculation(effect.amount, actionContext, context, 'reduce');
-    evaluateCalculation(effect.amount, scope);
     if (typeof effect.amount?.value !== 'number') return;
     effectBonus += effect.amount.value;
     effectString += ` ${effect.amount.value < 0 ? '-' : '+'} [${effect.amount.calculation}] ${Math.abs(effect.amount.value)}`

app/imports/api/engine/computation/buildComputation/linkCalculationDependencies.js

@@ -13,8 +13,8 @@ export default function linkCalculationDependencies(dependencyGraph, prop, { pro
     // Skip empty calculations that aren't targeted by anything
     if (
       !calcObj.calculation
-      && !calcObj.effects
+      && !calcObj.effectIds
-      && !calcObj.proficiencies
+      && !calcObj.proficiencyIds
     ) return;
 
     dependencyGraph.addNode(calcNodeId, calcObj);

app/imports/api/engine/computation/buildComputation/tests/computeInactiveStatus.testFn.js

@@ -2,7 +2,7 @@ import { buildComputationFromProps } from '/imports/api/engine/computation/build
 import { assert } from 'chai';
 import clean from '../../utility/cleanProp.testFn.js';
 
-export default function(){
+export default function () {
   let computation = buildComputationFromProps(testProperties);
   const bySelf = (propId, note) => assertDeactivatedBySelf(computation, propId, note);
   const byAncestor = (propId, note) => assertDeactivatedByAncestor(computation, propId, note);
@@ -24,22 +24,22 @@ export default function(){
 
   // Notes
   active('NoteId', 'Notes should be active');
-  byAncestor('NoteChildId', 'children of notes should always be inactive');
+  active('NoteChildId', 'children of notes should be active');
 }
 
-function assertDeactivatedBySelf(computation, propId, note){
+function assertDeactivatedBySelf(computation, propId, note) {
   const prop = computation.propsById[propId];
   assert.isTrue(prop.deactivatedBySelf, note);
   assert.isTrue(prop.inactive, note + '. The property should be inactive');
 }
 
-function assertDeactivatedByAncestor(computation, propId, note){
+function assertDeactivatedByAncestor(computation, propId, note) {
   const prop = computation.propsById[propId];
   assert.isTrue(prop.deactivatedByAncestor, note);
   assert.isTrue(prop.inactive, 'The property should be inactive');
 }
 
-function assertActive(computation, propId, note){
+function assertActive(computation, propId, note) {
   const prop = computation.propsById[propId];
   assert.isNotTrue(prop.inactive, note);
   assert.isNotTrue(prop.deactivatedBySelf, note);
@@ -51,66 +51,66 @@ var testProperties = [
   clean({
     _id: 'itemUnequippedId',
     type: 'item',
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
   }),
   clean({
     _id: 'itemUnequippedChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'itemUnequippedId'}],
+    ancestors: [{ id: 'charId' }, { id: 'itemUnequippedId' }],
   }),
   clean({
     _id: 'itemEquippedId',
     type: 'item',
     equipped: true,
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
   }),
   clean({
     _id: 'itemEquippedChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'itemEquippedId'}],
+    ancestors: [{ id: 'charId' }, { id: 'itemEquippedId' }],
   }),
   // Spells
   clean({
     _id: 'spellPreparedId',
     type: 'spell',
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
     prepared: true,
   }),
   clean({
     _id: 'spellPreparedChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'spellPreparedId'}],
+    ancestors: [{ id: 'charId' }, { id: 'spellPreparedId' }],
   }),
   clean({
     _id: 'spellAlwaysPreparedId',
     type: 'spell',
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
     alwaysPrepared: true,
   }),
   clean({
     _id: 'spellAlwaysPreparedChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'spellAlwaysPreparedId'}],
+    ancestors: [{ id: 'charId' }, { id: 'spellAlwaysPreparedId' }],
   }),
   clean({
     _id: 'spellUnpreparedId',
     type: 'spell',
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
   }),
   clean({
     _id: 'spellUnpreparedChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'spellUnpreparedId'}],
+    ancestors: [{ id: 'charId' }, { id: 'spellUnpreparedId' }],
   }),
   // Notes
   clean({
     _id: 'NoteId',
     type: 'note',
-    ancestors: [{id: 'charId'}],
+    ancestors: [{ id: 'charId' }],
   }),
   clean({
     _id: 'NoteChildId',
     type: 'folder',
-    ancestors: [{id: 'charId'}, {id: 'NoteId'}],
+    ancestors: [{ id: 'charId' }, { id: 'NoteId' }],
   }),
 ];

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

@@ -1,23 +1,44 @@
-import evaluateCalculation from '../../utility/evaluateCalculation.js';
 import call from '/imports/parser/parseTree/call.js';
 import constant from '/imports/parser/parseTree/constant.js';
 import operator from '/imports/parser/parseTree/operator.js';
 import parenthesis from '/imports/parser/parseTree/parenthesis.js';
-import { toString } from '/imports/parser/resolve.js';
+import resolve, { toPrimitiveOrString } from '/imports/parser/resolve.js';
 
 export default function computeCalculation(computation, node) {
   const calcObj = node.data;
-  evaluateCalculation(calcObj, computation.scope);
+  // resolve the parse node into the initial value
-  if (calcObj.effects || calcObj.proficiencies) {
+  resolveCalculationNode(calcObj, calcObj.parseNode, computation.scope);
-    calcObj.unaffected = calcObj.value;
+  // Store the unaffected value
-    calcObj.displayUnaffected = toString(calcObj.unaffected);
+  if (calcObj.effectIds || calcObj.proficiencyIds) {
+    calcObj.unaffected = toPrimitiveOrString(calcObj.valueNode);
   }
-  aggregateCalculationEffects(node, computation);
+  // link and aggregate the effects and proficiencies
-  aggregateCalculationProficiencies(node, computation);
+  linkCalculationEffects(node, computation);
-  calcObj.displayValue = toString(calcObj.value);
+  aggregateCalculationEffects(calcObj, id => computation.propsById[id]);
+  linkCalculationProficiencies(node, computation)
+  aggregateCalculationProficiencies(calcObj, id => computation.propsById[id], computation.scope['proficiencyBonus']?.value || 0);
+
+  // Resolve the valueNode after effects and proficiencies have been applied to it
+  resolveCalculationNode(calcObj, calcObj.valueNode, computation.scope);
+
+  // Store the value as a primitive
+  calcObj.value = toPrimitiveOrString(calcObj.valueNode);
+
+  // remove the working fields
+  delete calcObj._parseLevel;
+  delete calcObj._localScope;
 }
 
-function aggregateCalculationEffects(node, computation) {
+export function resolveCalculationNode(calculation, parseNode, scope) {
+  const fn = calculation._parseLevel;
+  const calculationScope = { ...calculation._localScope, ...scope };
+  const { result: resultNode, context } = resolve(fn, parseNode, calculationScope);
+  calculation.errors = context.errors;
+  calculation.valueNode = resultNode;
+}
+
+
+function linkCalculationEffects(node, computation) {
   const calcObj = node.data;
   delete calcObj.effectIds;
   computation.dependencyGraph.forEachLinkedNode(
@@ -36,107 +57,110 @@ function aggregateCalculationEffects(node, computation) {
     },
     true // enumerate only outbound links
   );
-  if (calcObj.effectIds) {
+}
-    // dictionary of {[operation]: parseNode}
+
-    const aggregator = {};
+export function aggregateCalculationEffects(calcObj, getEffectFromId) {
-    // Store all effect values
+  // dictionary of {[operation]: parseNode}
-    calcObj.effects.forEach(effect => {
+  const aggregator = {};
-      const op = effect.operation;
+  // Store all effect values
-      switch (op) {
+  calcObj.effectIds?.forEach(effectId => {
-        case undefined:
+    const effect = getEffectFromId(effectId);
-          break;
+    const op = effect.operation;
-        // Conditionals stored as a list of text
+    switch (op) {
-        case 'conditional':
+      case undefined:
-          if (!aggregator[op]) aggregator[op] = [];
+        break;
-          aggregator[op].push(effect.text);
+      // Conditionals stored as a list of text
-          break;
+      case 'conditional':
-        // Adv/Dis and Fails just count instances
+        if (!aggregator[op]) aggregator[op] = [];
-        case 'advantage':
+        aggregator[op].push(effect.text);
-        case 'disadvantage':
+        break;
-        case 'fail':
+      // Adv/Dis and Fails just count instances
-          if (calcObj[op] === undefined) calcObj[op] = 0;
+      case 'advantage':
-          calcObj[op]++;
+      case 'disadvantage':
-          break;
+      case 'fail':
-        // Math functions store value parseNodes
+        if (calcObj[op] === undefined) calcObj[op] = 0;
-        case 'base':
+        calcObj[op]++;
-        case 'add':
+        break;
-        case 'mul':
+      // Math functions store value parseNodes
-        case 'min':
+      case 'base':
-        case 'max':
+      case 'add':
-        case 'set':
+      case 'mul':
-          if (!aggregator[op]) aggregator[op] = [];
+      case 'min':
-          aggregator[op].push(effect.amount.value);
+      case 'max':
-          break;
+      case 'set':
-        // No case for passiveAdd, it doesn't make sense in this context
+        if (!aggregator[op]) aggregator[op] = [];
-      }
+        aggregator[op].push(effect.amount.valueNode);
-    });
+        break;
-    /**
+      // No case for passiveAdd, it doesn't make sense in this context
-     * Aggregate the effects in a parse tree like so
-     * x = ( max(...base, unaffectedValue) + sum(...add) ) * mul(...mul)
-     * min(...min, x)
-     * max(...max, x)
-     * set(last(...set))a
-     */
-    // Set
-    // If we do set, return early, nothing else matters
-    if (aggregator.set) {
-      calcObj.value = aggregator.set[aggregator.set.length - 1];
-      return;
     }
-    // Base value
+  });
-    if (aggregator.base) {
+  /**
-      calcObj.value = call.create({
+   * Aggregate the effects in a parse tree like so
-        functionName: 'max',
+   * x = max(...base, unaffectedValue)
-        args: [calcObj.value, aggregator.base]
+   * x = x + sum(...add)
+   * x = x * mul(...mul)
+   * x = min(...min, x)
+   * x = max(...max, x)
+   * x = set(last(...set))a
+   */
+  // Set
+  // If we do set, return early, nothing else matters
+  if (aggregator.set) {
+    calcObj.valueNode = aggregator.set[aggregator.set.length - 1];
+    return;
+  }
+  // Base value
+  if (aggregator.base) {
+    calcObj.valueNode = call.create({
+      functionName: 'max',
+      args: [calcObj.valueNode, aggregator.base]
+    });
+  }
+  // Add
+  aggregator.add?.forEach(node => {
+    calcObj.valueNode = operator.create({
+      left: calcObj.valueNode,
+      right: node,
+      operator: '+'
+    });
+  });
+  // Multiply
+  if (aggregator.mul) {
+    // Wrap the previous node in brackets if it's another operator
+    if (calcObj.parseType === 'operator') {
+      calcObj.valueNode = parenthesis.create({
+        content: calcObj.valueNode
       });
     }
-    // Add
+    // Append all multiplications
-    aggregator.add?.forEach(node => {
+    aggregator.mul.forEach(node => {
-      calcObj.value = operator.create({
+      calcObj.valueNode = operator.create({
-        left: calcObj.value,
+        left: calcObj.valueNode,
         right: node,
-        operator: '+'
+        operator: '*'
       });
     });
-    // Multiply
+  }
-    if (aggregator.mul) {
+  // Min
-      // Wrap the previous node in brackets if it's another operator
+  if (aggregator.min) {
-      if (calcObj.parseType === 'operator') {
+    calcObj.valueNode = call.create({
-        calcObj.value = parenthesis.create({
+      functionName: 'max',
-          content: calcObj.value
+      args: [calcObj.valueNode, aggregator.min]
-        });
+    });
-      }
+  }
-      // Append all multiplications
+  // Max
-      aggregator.mul.forEach(node => {
+  if (aggregator.max) {
-        calcObj.value = operator.create({
+    calcObj.valueNode = call.create({
-          left: calcObj.value,
+      functionName: 'min',
-          right: node,
+      args: [calcObj.valueNode, aggregator.max]
-          operator: '*'
+    });
-        });
-      });
-    }
-    // Min
-    if (aggregator.min) {
-      calcObj.value = call.create({
-        functionName: 'max',
-        args: [calcObj.value, aggregator.min]
-      });
-    }
-    // Max
-    if (aggregator.max) {
-      calcObj.value = call.create({
-        functionName: 'min',
-        args: [calcObj.value, aggregator.max]
-      });
-    }
   }
 }
 
-function aggregateCalculationProficiencies(node, computation) {
+function linkCalculationProficiencies(node, computation) {
   const calcObj = node.data;
-  delete calcObj.proficiencies;
+  delete calcObj.proficiencyIds;
   delete calcObj.proficiency;
-  let profBonus = computation.scope['proficiencyBonus']?.value || 0;
 
   // Go through all the links and collect them on the calculation
   computation.dependencyGraph.forEachLinkedNode(
@@ -148,53 +172,52 @@ function aggregateCalculationProficiencies(node, computation) {
       if (!linkedNode.data) return;
       // Ignoring inactive props
       if (linkedNode.data.inactive) return;
-      // Compute the proficiency and value
-      let proficiency, value;
-      if (linkedNode.data.type === 'proficiency') {
-        proficiency = linkedNode.data.value || 0;
-        // Multiply the proficiency bonus by the actual proficiency
-        if (proficiency === 0.49) {
-          // Round down proficiency bonus in the special case
-          value = Math.floor(profBonus * 0.5);
-        } else {
-          value = Math.ceil(profBonus * proficiency);
-        }
-      } else if (linkedNode.data.type === 'skill') {
-        value = linkedNode.data.value || 0;
-        proficiency = linkedNode.data.proficiency || 0;
-      }
       // Collate proficiencies
-      calcObj.proficiencies = calcObj.proficiencies || [];
+      calcObj.proficiencyIds = calcObj.proficiencyIds || [];
-      calcObj.proficiencies.push({
+      calcObj.proficiencyIds.push(linkedNode.data._id);
-        _id: linkedNode.data._id,
-        name: linkedNode.data.name,
-        type: linkedNode.data.type,
-        proficiency,
-        value,
-      });
     },
     true // enumerate only outbound links
   );
-
+}
-  // Apply the highest proficiency, marking all others as overridden
+
-  if (calcObj.proficiencies && typeof calcObj.value === 'number') {
+export function aggregateCalculationProficiencies(calcObj, getProficiencyFromId, profBonus) {
-    calcObj.proficiency = 0;
+  if (!calcObj.proficiencyIds) return;
-    calcObj.proficiencyBonus = 0;
+  // Apply the highest proficiency, marking all others as overridden
-    let currentProf;
+  calcObj.proficiency = 0;
-    calcObj.proficiencies.forEach(prof => {
+  calcObj.proficiencyBonus = 0;
-      if (prof.value > calcObj.proficiencyBonus) {
+  let currentProf;
-        if (currentProf) currentProf.overridden = true;
+  calcObj.proficiencyIds.forEach(profId => {
-        calcObj.proficiencyBonus = prof.value;
+    const profProp = getProficiencyFromId(profId)
-        calcObj.proficiency = prof.proficiency;
+    if (!profProp) {
-        currentProf = prof;
+      console.warn('proficiency linked but not found ', profId);
-      } else {
+    }
-        prof.overridden = true;
+    // Compute the proficiency and value
-      }
+    let proficiency, value;
-    });
+    if (profProp.type === 'proficiency') {
-    calcObj.value = operator.create({
+      proficiency = profProp.value || 0;
-      left: calcObj.value,
+      // Multiply the proficiency bonus by the actual proficiency
-      right: constant.create({ value: calcObj.proficiencyBonus }),
+      if (proficiency === 0.49) {
-      operator: '+'
+        // Round down proficiency bonus in the special case
-    });
+        value = Math.floor(profBonus * 0.5);
-  }
+      } else {
+        value = Math.ceil(profBonus * proficiency);
+      }
+    } else if (profProp.type === 'skill') {
+      value = profProp.value || 0;
+      proficiency = profProp.proficiency || 0;
+    }
+    if (value > calcObj.proficiencyBonus) {
+      if (currentProf) currentProf.overridden = true;
+      calcObj.proficiencyBonus = value;
+      calcObj.proficiency = proficiency;
+      currentProf = profProp;
+    } else {
+      profProp.overridden = true;
+    }
+  });
+  calcObj.valueNode = operator.create({
+    left: calcObj.valueNode,
+    right: constant.create({ value: calcObj.proficiencyBonus }),
+    operator: '+'
+  });
 }

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

@@ -1,5 +1,4 @@
 import { has } from 'lodash';
-import evaluateCalculation from '../../utility/evaluateCalculation.js';
 
 export default function computePointBuy(computation, node) {
   const prop = node.data;

app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/aggregate/aggregateDefinition.js

@@ -24,14 +24,7 @@ export default function aggregateDefinition({ node, linkedNode, link }) {
   }
 
   // Aggregate the base value due to the defining properties
-  let propBaseValue = undefined;
+  let propBaseValue = prop.baseValue?.value;
-  const valueNode = prop.baseValue?.value;
-  if (
-    valueNode?.parseType === 'constant'
-    && valueNode?.valueType === 'number'
-  ) {
-    propBaseValue = valueNode.value;
-  }
   // Point buy rows use prop.value instead of prop.baseValue
   if (prop.type === 'pointBuyRow') {
     propBaseValue = prop.value;
@@ -39,23 +32,11 @@ export default function aggregateDefinition({ node, linkedNode, link }) {
 
   if (propBaseValue === undefined) return;
   // Store a summary of the definition as a base value effect
-  node.data.effects = node.data.effects || [];
+  node.data.effectIds = node.data.effectIds || [];
   if (prop.type === 'pointBuyRow') {
-    node.data.effects.push({
+    node.data.effectIds.push(prop.tableId);
-      _id: prop.tableId,
-      name: prop.tableName,
-      operation: 'base',
-      amount: propBaseValue,
-      type: 'pointBuy',
-    });
   } else {
-    node.data.effects.push({
+    node.data.effectIds.push(prop._id);
-      _id: prop._id,
-      name: prop.name,
-      operation: 'base',
-      amount: propBaseValue,
-      type: prop.type,
-    });
   }
   if (node.data.baseValue === undefined || propBaseValue > node.data.baseValue) {
     node.data.baseValue = propBaseValue;

app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/aggregate/aggregateEffect.js

@@ -1,5 +1,3 @@
-import { pick } from 'lodash';
-
 export default function aggregateEffect({ node, linkedNode, link }) {
   if (link.data !== 'effect') return;
   // store the effect aggregator, its presence indicates that the variable is
@@ -19,30 +17,14 @@ export default function aggregateEffect({ node, linkedNode, link }) {
     rollBonus: [],
   };
 
-  // Store a summary of the effect itself
+  // Store a link to the effect
-  node.data.effects = node.data.effects || [];
+  node.data.effectIds = node.data.effectIds || [];
-  // Store either just 
+  node.data.effectIds.push(linkedNode.data._id);
-  node.data.effects.push({
-    _id: linkedNode.data._id,
-    name: linkedNode.data.name,
-    operation: linkedNode.data.operation,
-    amount: linkedNode.data.amount.displayValue,
-    type: linkedNode.data.type,
-    text: linkedNode.data.text,
-    // ancestors: linkedNode.data.ancestors,
-  });
 
   // get a shorter reference to the aggregator document
   const aggregator = node.data.effectAggregator;
   // Get the result of the effect
-  let result = undefined;
+  let result = linkedNode.data.amount?.value;
-  const valueNode = linkedNode.data.amount?.value;
-  if (
-    valueNode?.parseType === 'constant'
-    && valueNode?.valueType === 'number'
-  ) {
-    result = valueNode.value;
-  }
   if (typeof result !== 'number') result = undefined;
 
   // Aggregate the effect based on its operation

app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/computeVariableAsAttribute.js

@@ -49,5 +49,5 @@ export default function computeVariableAsAttribute(computation, node, prop) {
     undefined
 
   // Store effects
-  prop.effects = node.data.effects;
+  prop.effectIds = node.data.effectIds;
 }

app/imports/api/engine/computation/computeComputation/computeByType/computeVariable/computeVariableAsSkill.js

@@ -34,7 +34,7 @@ export default function computeVariableAsSkill(computation, node, prop) {
   const aggregatorBase = aggregator?.base || 0;
 
   // Store effects
-  prop.effects = node.data.effects;
+  prop.effectIds = node.data.effectIds;
 
   // If there is no aggregator, determine if the prop can hide, then exit
   if (!aggregator) {

app/imports/api/engine/computation/computeComputation/tests/computeAttribute.testFn.js

@@ -3,7 +3,7 @@ import { assert } from 'chai';
 import computeCreatureComputation from '../../computeCreatureComputation.js';
 import clean from '../../utility/cleanProp.testFn.js';
 
-export default function(){
+export default function () {
   const computation = buildComputationFromProps(testProperties);
   computeCreatureComputation(computation);
   const prop = id => computation.propsById[id];

app/imports/api/engine/computation/computeComputation/tests/computeProficiencies.testFn.js

@@ -15,8 +15,8 @@ export default function () {
     prop('strengthId').modifier, -1,
     'The proficiency bonus should not change the strength modifier'
   );
-  assert.exists(prop('actionId').attackRoll.proficiencies, 'The proficiency aggregator should be here')
+  assert.exists(prop('actionId').attackRoll.proficiencyIds, 'The proficiency aggregator should be here')
-  assert.exists(prop('actionId').attackRoll.proficiencies[0], 'The proficiency should be here')
+  assert.exists(prop('actionId').attackRoll.proficiencyIds[0], 'The proficiency should be here')
   // attack roll = strength.mod + proficiencyBonus/2 rounded down
   // = -1 + 13/2 = -1 + 6 = 5
   assert.equal(

app/imports/api/engine/computation/utility/evaluateCalculation.js

@@ -1,13 +1,14 @@
-import resolve, { toString } from '/imports/parser/resolve.js';
+import resolve, { toPrimitiveOrString } from '/imports/parser/resolve.js';
-
+console.warn('evaluateCalculation is deprecated use resolveCalculationNode instead')
+// TODO everywhere this is used, replace with more specific code to recalculate fields
 export default function evaluateCalculation(calculation, scope, givenContext) {
   const parseNode = calculation.parseNode;
   const fn = calculation._parseLevel;
   const calculationScope = { ...calculation._localScope, ...scope };
   const { result: resultNode, context } = resolve(fn, parseNode, calculationScope, givenContext);
   calculation.errors = context.errors;
-  calculation.value = resultNode;
+  calculation.valueNode = resultNode;
-  calculation.displayValue = toString(resultNode);
+  calculation.value = toPrimitiveOrString(resultNode);
   // remove the working fields
   delete calculation._parseLevel;
   delete calculation._localScope;

app/imports/api/properties/Attributes.js

@@ -199,14 +199,21 @@ let ComputedOnlyAttributeSchema = createPropertySchema({
     removeBeforeCompute: true,
   },
   // A list of effect ids targeting this attribute
-  effects: {
+  'effectIds': {
     type: Array,
     optional: true,
     removeBeforeCompute: true,
   },
-  'effects.$': {
+  'effectIds.$': {
-    type: Object,
+    type: String,
-    blackbox: true,
+  },
+  'proficiencyIds': {
+    type: Array,
+    optional: true,
+    removeBeforeCompute: true,
+  },
+  'proficiencyIds.$': {
+    type: String,
   },
 });
 

app/imports/api/properties/Skills.js

@@ -134,14 +134,21 @@ let ComputedOnlySkillSchema = createPropertySchema({
     removeBeforeCompute: true,
   },
   // A list of effect ids targeting this skill
-  effects: {
+  'effectIds': {
     type: Array,
     optional: true,
     removeBeforeCompute: true,
   },
-  'effects.$': {
+  'effectIds.$': {
-    type: Object,
+    type: String,
-    blackbox: true,
+  },
+  'proficiencyIds': {
+    type: Array,
+    optional: true,
+    removeBeforeCompute: true,
+  },
+  'proficiencyIds.$': {
+    type: String,
   },
 })
 

app/imports/api/properties/subSchemas/computedField.js

@@ -19,29 +19,23 @@ function fieldToCompute(field) {
 
 function computedOnlyField(field) {
   const schemaObj = {
-    // The parseNode of the compiled value before any effects are applied or rolls made
+    // The value (or calculation string) before any effects/proficiencies are applied or rolls made
     [`${field}.unaffected`]: {
-      type: Object,
+      type: SimpleSchema.oneOf(String, Number),
       optional: true,
       blackbox: true,
     },
-    // toString(.unaffected)
+    // The value (or calculation string) after applying all effects
-    [`${field}.displayUnaffected`]: {
-      type: SimpleSchema.oneOf(String, Number),
-      optional: true,
-      removeBeforeCompute: true,
-    },
-    // The compiled parseNode after applying all effects
     [`${field}.value`]: {
-      type: Object,
+      type: SimpleSchema.oneOf(String, Number),
       optional: true,
       blackbox: true,
     },
-    // The displayed value of the calculation: toString(.value)
+    // The value as a parse node, after applying all effects
-    [`${field}.displayValue`]: {
+    [`${field}.valueNode`]: {
       type: SimpleSchema.oneOf(String, Number),
       optional: true,
-      removeBeforeCompute: true,
+      blackbox: true,
     },
     // A list of effect Ids targeting this calculation
     [`${field}.effectIds`]: {

app/imports/client/ui/creature/character/printedCharacterSheet/components/PrintedAction.vue

@@ -85,21 +85,15 @@
 <script lang="js">
 import { getPropertyName } from '/imports/constants/PROPERTIES.js';
 import numberToSignedString from '/imports/api/utility/numberToSignedString.js';
-import AttributeConsumedView from '/imports/client/ui/properties/components/actions/AttributeConsumedView.vue';
-import ItemConsumedView from '/imports/client/ui/properties/components/actions/ItemConsumedView.vue';
 import PropertyIcon from '/imports/client/ui/properties/shared/PropertyIcon.vue';
 import MarkdownText from '/imports/client/ui/components/MarkdownText.vue';
 import TreeNodeList from '/imports/client/ui/components/tree/TreeNodeList.vue';
 import { nodeArrayToTree } from '/imports/api/parenting/nodesToTree.js';
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
 import { some } from 'lodash';
-import applyEffectsToCalculationParseNode from '/imports/api/engine/actions/applyPropertyByType/shared/applyEffectsToCalculationParseNode.js';
-import resolve, { Context, toString } from '/imports/parser/resolve.js';
 
 export default {
   components: {
-    AttributeConsumedView,
-    ItemConsumedView,
     MarkdownText,
     PropertyIcon,
     TreeNodeList,
@@ -167,10 +161,8 @@ export default {
       if (this.children[0].children[0]?.node?.type !== 'damage') return;
       if (this.children[0].children[0].children?.length !== 0) return;
       const damage = this.children[0].children[0]?.node;
-      applyEffectsToCalculationParseNode(damage.amount);
-      const { result } = resolve('compile', damage.amount.parseNode, {});
       return {
-        damage: toString(result),
+        damage: damage.value,
         suffix: damage.damageType + (damage.damageType !== 'healing' ? ' damage ' : '')
       };
     },

app/imports/parser/parseTree/error.js

@@ -1,16 +1,16 @@
 const error = {
-  create({node, error}) {
+  create({ node, error }) {
     return {
       parseType: 'error',
       node,
       error,
     }
   },
-  compile(node, scope, context){
+  compile(node, scope, context) {
-    return {result: node, context};
+    return { result: node, context };
   },
-  toString(node){
+  toString(node) {
-    return node.error.toString();
+    return `${node.error.type} error: ${node.error.message}`;
   },
 }
 

app/imports/parser/resolve.js

@@ -2,69 +2,76 @@ import nodeTypeIndex from './parseTree/_index.js';
 
 // Takes a parse ndoe and computes it to a set detail level
 // returns {result, context}
-export default function resolve(fn, node, scope, context = new Context()){
+export default function resolve(fn, node, scope, context = new Context()) {
-  if (!node) return {result: undefined, context};
+  if (!node) return { result: undefined, context };
   let type = nodeTypeIndex[node.parseType];
-  if (!type){
+  if (!type) {
     throw new Meteor.Error(`Parse node type: ${node.parseType} not implemented`);
   }
-  if (type.resolve){
+  if (type.resolve) {
     return type.resolve(fn, node, scope, context);
   } else if (type[fn]) {
     return type[fn](node, scope, context);
   } else if (fn === 'reduce' && type.roll) {
     return type.roll(node, scope, context)
-  } else if (type.compile){
+  } else if (type.compile) {
     return type.compile(node, scope, context)
   } else {
     throw new Meteor.Error('Compile not implemented on ' + node.parseType);
   }
 }
 
-export function toString(node){
+export function toString(node) {
   if (!node) return '';
   let type = nodeTypeIndex[node.parseType];
-  if (!type.toString){
+  if (!type.toString) {
     throw new Meteor.Error('toString not implemented on ' + node.parseType);
   }
   return type.toString(node);
 }
 
-export function traverse(node, fn){
+export function toPrimitiveOrString(node) {
+  if (!node) return '';
+  if (node.parseType === 'constant') return node.value;
+  if (node.parseType === 'error') return null;
+  return toString(node);
+}
+
+export function traverse(node, fn) {
   if (!node) return;
   let type = nodeTypeIndex[node.parseType];
-  if (!type){
+  if (!type) {
     console.error(node);
     throw new Meteor.Error('Not valid parse node');
   }
-  if (type.traverse){
+  if (type.traverse) {
     return type.traverse(node, fn);
   }
   return fn(node);
 }
 
-export function map(node, fn){
+export function map(node, fn) {
   if (!node) return;
   let type = nodeTypeIndex[node.parseType];
-  if (!type){
+  if (!type) {
     console.error(node);
     throw new Meteor.Error('Not valid parse node');
   }
-  if (type.map){
+  if (type.map) {
     return type.map(node, fn);
   }
   return fn(node);
 }
 
 export class Context {
-  constructor({errors = [], rolls = [], options = {}} = {}){
+  constructor({ errors = [], rolls = [], options = {} } = {}) {
     this.errors = errors;
     this.rolls = rolls;
     this.options = options;
   }
-  error(e){
+  error(e) {
     if (!e) return;
-    if (typeof e === 'string'){
+    if (typeof e === 'string') {
       this.errors.push({
         type: 'error',
         message: e,
@@ -73,7 +80,7 @@ export class Context {
       this.errors.push(e);
     }
   }
-  roll(r){
+  roll(r) {
     this.rolls.push(r);
   }
 }