Computations now occupy their own nodes on the dependency graph

This mitigates most issues with properties having self-loops, 
particularly in cases like Strength where the value `strength` is used 
in the description of Strength

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

@@ -2,32 +2,41 @@ import walkDown from '/imports/api/engine/computation/utility/walkdown.js';
 
 export default function computeInactiveStatus(node){
   const prop = node.node;
-  if (isActive(prop)) return;
-  // Unequipped items, notes, spells, and actions disable their children,
-  // but are not disabled themselves
-  if (
-    prop.type !== 'item' &&
-    prop.type !== 'note' &&
-    prop.type !== 'action' &&
-    prop.type !== 'spell'
-  ){
+  if (!isActive(prop)){
+    // Mark prop inactive due to self
     prop.inactive = true;
     prop.deactivatedBySelf = true;
   }
-  // Mark children as inactive due to ancestor
-  walkDown(node.children, child => {
-    child.node.inactive = true;
-    child.node.deactivatedByAncestor = true;
-  });
+  if(!childrenActive(prop)){
+    // Mark children as inactive due to ancestor
+    walkDown(node.children, child => {
+      child.node.inactive = true;
+      child.node.deactivatedByAncestor = true;
+    });
+  }
 }
 
 function isActive(prop){
   if (prop.disabled) return false;
   switch (prop.type){
-    case 'item': return !!prop.equipped;
-    case 'spell': return false;
-    case 'note': return false;
-    case 'action': return false;
+    // Unprepared spells are inactive
+    case 'spell': return !!prop.prepared || !!prop.alwaysPrepared;
+    default: return true;
+  }
+}
+
+function childrenActive(prop){
+  // Children of disabled properties are always inactive
+  if (prop.disabled) return false;
+  switch (prop.type){
+    // Only equipped items have active children
+    case 'item': return !!prop.equipped;
+    // The children of actions are always inactive
+    case 'action': return false;
+    case 'spell': return false;
+    // The children of notes are always inactive
+    case 'note': return false;
+    // Other children are active
     default: return true;
   }
 }

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

@@ -11,6 +11,7 @@ export default function computeToggleDependencies(node, dependencyGraph){
   ) return;
   walkDown(node.children, child => {
     child.node._computationDetails.toggleAncestors.push(prop);
+    // The child nodes depend on the toggle condition compuation
     dependencyGraph.addLink(child.node._id, prop._id, 'toggle');
   });
 }

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

@@ -4,9 +4,13 @@ import { traverse } from '/imports/parser/resolve.js';
 export default function linkCalculationDependencies(dependencyGraph, prop, {propsById}){
   prop._computationDetails.calculations.forEach(calcObj => {
     // Store resolved ancestors
-    let memo = {
+    const memo = {
       // ancestors: {} //this gets added if there are resolved ancestors
     };
+    // Add this calculation to the dependency graph
+    const calcNodeId = `${prop._id}.${calcObj._key}`;
+    dependencyGraph.addNode(calcNodeId, calcObj);
+
     // Traverse the parsed calculation looking for variable names
     traverse(calcObj.parseNode, node => {
       // Skip nodes that aren't symbols or accessors
@@ -17,10 +21,15 @@ export default function linkCalculationDependencies(dependencyGraph, prop, {prop
           node.name.slice(1), memo, prop, propsById
         );
         if (!ancestorProp) return;
-        dependencyGraph.addLink(prop._id, ancestorProp._id, calcObj);
+        // Link the ancestor prop as a direct dependency
+        dependencyGraph.addLink(
+          calcNodeId, ancestorProp._id, 'ancestorReference'
+        );
       } else {
         // Link variable name references as variable dependencies
-        dependencyGraph.addLink(prop._id, node.name, calcObj);
+        dependencyGraph.addLink(
+          calcNodeId, node.name, 'variableReference'
+        );
       }
     });
     // Store the resolved ancestors in this calculation's local scope

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

@@ -1,21 +1,111 @@
+import { get } from 'lodash';
+
 const linkDependenciesByType = {
-  action: linkResources,
+  action: linkAction,
+  adjustment: linkAdjustment,
   attribute: linkAttribute,
+  branch: linkBranch,
+  buff: linkBuff,
   class: linkVariableName,
   classLevel: linkClassLevel,
   constant: linkVariableName,
+  damage: linkDamage,
   damageMultiplier: linkDamageMultiplier,
-  proficiency: linkStats,
-  effect: linkStats,
+  effect: linkEffects,
+  proficiency: linkProficiencies,
+  roll: linkRoll,
+  slot: linkSlot,
   skill: linkSkill,
-  spell: linkResources,
-  toggle: linkVariableName,
+  spell: linkAction,
+  spellList: linkSpellList,
+  savingThrow: linkSavingThrow,
+  toggle: linkToggle,
 }
 
 export default function linkTypeDependencies(dependencyGraph, prop, computation){
   linkDependenciesByType[prop.type]?.(dependencyGraph, prop, computation);
 }
 
+function dependOnCalc({dependencyGraph, prop, key}){
+  let calc = get(prop, key);
+  if (!calc) return;
+  if (calc.type !== '_calculation'){
+    console.log(calc);
+    throw `Expected calculation got ${calc.type}`
+  }
+  dependencyGraph.addLink(prop._id, `${prop._id}.${key}`, 'calculation');
+}
+
+function linkAction(dependencyGraph, prop, {propsById}){
+  // The action depends on its attack roll and uses calculations
+  dependOnCalc({dependencyGraph, prop, key: 'attackRoll'});
+  dependOnCalc({dependencyGraph, prop, key: 'uses'});
+
+  // Link the resources the action uses
+  if (!prop.resources) return;
+  // Link items consumed
+  prop.resources.itemsConsumed.forEach((itemConsumed, index) => {
+    if (!itemConsumed.itemId) return;
+    const item = propsById[itemConsumed.itemId];
+    if (!item || item.inactive){
+      // Unlink if the item doesn't exist or is inactive
+      itemConsumed.itemId = undefined;
+      return;
+    }
+    // none of these dependencies are computed, we can use them immediately
+    itemConsumed.available = item.quantity;
+    itemConsumed.itemName = item.name;
+    itemConsumed.itemIcon = item.icon;
+    itemConsumed.itemColor = item.color;
+    dependencyGraph.addLink(prop._id, item._id, 'inventory');
+    // Link the property to its resource quantity calculation
+
+    dependOnCalc({
+      dependencyGraph,
+      prop,
+      key: `${prop._id}.resources.itemsConsumed.${index}.quantity`,
+    });
+  });
+  // Link attributes consumed
+  prop.resources.attributesConsumed.forEach((attConsumed, index) => {
+    if (!attConsumed.variableName) return;
+    dependencyGraph.addLink(prop._id, attConsumed.variableName, 'resource');
+    // Link the property to its resource quantity calculation
+    dependOnCalc({
+      dependencyGraph,
+      prop,
+      key: `${prop._id}.resources.attributesConsumed.${index}.quantity`,
+    });
+  });
+}
+
+function linkAdjustment(dependencyGraph, prop){
+  // Adjustment depends on its amount
+  dependOnCalc({dependencyGraph, prop, key: 'amount'});
+}
+
+function linkAttribute(dependencyGraph, prop){
+  linkVariableName(dependencyGraph, prop);
+  // Depends on spellSlotLevel
+  dependOnCalc({dependencyGraph, prop, key: 'spellSlotLevel'});
+
+  // Depends on base value
+  dependOnCalc({dependencyGraph, prop, key: 'baseValue'});
+
+  // hit dice depend on constitution
+  if (prop.attributeType === 'hitDice'){
+    dependencyGraph.addLink(prop._id, 'constitution', 'hitDiceConMod');
+  }
+}
+
+function linkBranch(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'condition'});
+}
+
+function linkBuff(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'duration'});
+}
+
 function linkClassLevel(dependencyGraph, prop){
   // The variableName of the prop depends on the prop
   if (prop.variableName && prop.level){
@@ -28,6 +118,24 @@ function linkClassLevel(dependencyGraph, prop){
   }
 }
 
+function linkDamage(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'amount'});
+}
+
+function linkEffects(dependencyGraph, prop){
+  // The effect depends on its amount calculation
+  dependOnCalc({dependencyGraph, prop, key: 'amount'});
+  // The stats depend on the effect
+  prop.stats.forEach(statName => {
+    if (!statName) return;
+    dependencyGraph.addLink(statName, prop._id, 'effect');
+  });
+}
+
+function linkRoll(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'roll'});
+}
+
 function linkVariableName(dependencyGraph, prop){
   // The variableName of the prop depends on the prop
   if (prop.variableName){
@@ -35,37 +143,6 @@ function linkVariableName(dependencyGraph, prop){
   }
 }
 
-function linkResources(dependencyGraph, prop, {propsById}){
-  if (!prop.resources) return;
-  prop.resources.itemsConsumed.forEach(itemConsumed => {
-    if (!itemConsumed.itemId) return;
-    const item = propsById[itemConsumed.itemId];
-    if (!item || item.inactive){
-      // Unlink if the item doesn't exist or is inactive
-      itemConsumed.itemId = undefined;
-      return;
-    }
-    // none of these dependencies are computed, we can use them immediately
-    itemConsumed.available = item.quantity;
-    itemConsumed.itemName = item.name;
-    itemConsumed.itemIcon = item.icon;
-    itemConsumed.itemColor = item.color;
-    dependencyGraph.addLink(prop._id, item._id, 'inventory');
-  });
-  prop.resources.attributesConsumed.forEach(attConsumed => {
-    if (!attConsumed.variableName) return;
-    dependencyGraph.addLink(prop._id, attConsumed.variableName, 'resource');
-  });
-}
-
-function linkAttribute(dependencyGraph, prop){
-  linkVariableName(dependencyGraph, prop);
-  // hit dice depend on constitution
-  if (prop.attributeType === 'hitDice'){
-    dependencyGraph.addLink(prop._id, 'constitution', 'hitDiceConMod');
-  }
-}
-
 function linkDamageMultiplier(dependencyGraph, prop){
   prop.damageTypes.forEach(damageType => {
     // Remove all non-letter characters from the damage name
@@ -74,14 +151,18 @@ function linkDamageMultiplier(dependencyGraph, prop){
   });
 }
 
-function linkStats(dependencyGraph, prop){
-  // The stats a prop references depend on that prop
-  prop.stats.forEach(variableName => {
-    if (!variableName) return;
-    dependencyGraph.addLink(variableName, prop._id, prop.type);
+function linkProficiencies(dependencyGraph, prop){
+  // The stats depend on the proficiency
+  prop.stats.forEach(statName => {
+    if (!statName) return;
+    dependencyGraph.addLink(statName, prop._id, prop.type);
   });
 }
 
+function linkSavingThrow(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'dc'});
+}
+
 function linkSkill(dependencyGraph, prop){
   linkVariableName(dependencyGraph, prop);
   // The prop depends on the variable references as the ability
@@ -91,3 +172,19 @@ function linkSkill(dependencyGraph, prop){
   // Skills depend on the creature's proficiencyBonus
   dependencyGraph.addLink(prop._id, 'proficiencyBonus', 'skillProficiencyBonus');
 }
+
+function linkSlot(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'quantityExpected'});
+  dependOnCalc({dependencyGraph, prop, key: 'slotCondition'});
+}
+
+function linkSpellList(dependencyGraph, prop){
+  dependOnCalc({dependencyGraph, prop, key: 'maxPrepared'});
+  dependOnCalc({dependencyGraph, prop, key: 'attackRollBonus'});
+  dependOnCalc({dependencyGraph, prop, key: 'dc'});
+}
+
+function linkToggle(dependencyGraph, prop){
+  linkVariableName(dependencyGraph, prop);
+  dependOnCalc({dependencyGraph, prop, key: 'condition'});
+}

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

@@ -26,12 +26,17 @@ function discoverInlineCalculationFields(prop, schemas){
         unset(prop, calcKey);
         return;
       }
+      // Has the text, if it matches the existing hash, stop
       const inlineCalcHash = cyrb53(inlineCalcObj.text);
       if (inlineCalcHash === inlineCalcObj.hash){
         return;
       }
       inlineCalcObj.hash = inlineCalcHash;
       inlineCalcObj.inlineCalculations = [];
+      // Set the value to the uncomputed string for use in calculations
+      // It will be re set including the embedded calculation at the end of
+      // the computation
+      inlineCalcObj.value = string;
       let matches = string.matchAll(INLINE_CALCULATION_REGEX);
       for (let match of matches){
         let calculation = match[1];
@@ -63,6 +68,10 @@ function parseAllCalculationFields(prop, schemas){
       prop._computationDetails.calculations.push(calcObj);
       // Store the level to compute down to later
       calcObj._parseLevel = parseLevel;
+      // Store the key
+      calcObj._key = key;
+      // Set a type
+      calcObj.type = '_calculation';
       // Parse the calculation
       parseCalculation(calcObj);
     });

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

@@ -11,16 +11,16 @@ export default function(){
   // Items
   active('itemUnequippedId', 'Unequipped items should be active');
   byAncestor('itemUnequippedChildId', 'Children of unequipped items should be inactive');
-  active('itemEquippedId');
-  active('itemEquippedChildId');
+  active('itemEquippedId', 'Equipped items should be active');
+  active('itemEquippedChildId', 'Children of equipped items should be active');
 
   // Spells
-  active('spellPreparedId');
-  active('spellPreparedChildId');
-  active('spellAlwaysPreparedId');
-  active('spellAlwaysPreparedChildId');
-  bySelf('spellUnpreparedId');
-  byAncestor('spellUnpreparedChildId');
+  active('spellPreparedId', 'Prepared spells should be active');
+  byAncestor('spellPreparedChildId', 'Children of prepared spells should be deactivatedByAncestor');
+  active('spellAlwaysPreparedId', 'Always prepared spells should be active');
+  byAncestor('spellAlwaysPreparedChildId', 'Children of always prepared spells should be deactivatedByAncestor');
+  bySelf('spellUnpreparedId', 'Unprepared spells should be deactivatedBySelf');
+  byAncestor('spellUnpreparedChildId', 'Children of unprepared spells should be deactivatedByAncestor');
 
   // Notes
   active('NoteId', 'Notes should be active');
@@ -30,7 +30,7 @@ export default function(){
 function assertDeactivatedBySelf(computation, propId, note){
   const prop = computation.propsById[propId];
   assert.isTrue(prop.deactivatedBySelf, note);
-  assert.isTrue(prop.inactive, 'The property should be inactive');
+  assert.isTrue(prop.inactive, note + '. The property should be inactive');
 }
 
 function assertDeactivatedByAncestor(computation, propId, note){
@@ -42,8 +42,8 @@ function assertDeactivatedByAncestor(computation, propId, note){
 function assertActive(computation, propId, note){
   const prop = computation.propsById[propId];
   assert.isNotTrue(prop.inactive, note);
-  assert.isNotTrue(prop.deactivatedBySelf);
-  assert.isNotTrue(prop.deactivatedBySelf);
+  assert.isNotTrue(prop.deactivatedBySelf, note);
+  assert.isNotTrue(prop.deactivatedBySelf, note);
 }
 
 var testProperties = [

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

@@ -6,21 +6,20 @@ export default function(){
   const computation = buildComputationFromProps(testProperties);
   const hasLink = computation.dependencyGraph.hasLink;
   const prop = (id) => computation.propsById[id];
-
   assert.isTrue(
-    !!hasLink('childId', 'spellListId'),
+    !!hasLink('childId.description.inlineCalculations[0]', 'spellListId'),
     'Ancestor references of parent in inline calculations should create dependency'
   );
   assert.isTrue(
-    !!hasLink('grandchildId', 'spellListId'),
+    !!hasLink('grandchildId.dc', 'spellListId'),
     'References to higher ancestor should create dependency'
   );
   assert.isTrue(
-    !!hasLink('grandchildId', 'strength'),
+    !!hasLink('grandchildId.dc', 'strength'),
     'Variable references create dependencies'
   );
   assert.isTrue(
-    !!hasLink('grandchildId', 'wisdom'),
+    !!hasLink('grandchildId.dc', 'wisdom'),
     'Variable references create dependencies even if the attributes don\'t exist'
   );
   assert.equal(

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

@@ -4,9 +4,11 @@ import attribute from './computeByType/computeAttribute.js';
 import skill from './computeByType/computeSkill.js';
 import slot from './computeByType/computeSlot.js';
 import container from './computeByType/computeContainer.js';
+import _calculation from './computeByType/computeCalculation.js';
 
 export default Object.freeze({
   _variable,
+  _calculation,
   action,
   attribute,
   container,

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

@@ -0,0 +1,6 @@
+import evaluateCalculation from '../../utility/evaluateCalculation.js';
+
+export default function computeCalculation(computation, node){
+  const calcObj = node.data;
+  evaluateCalculation(calcObj, computation.scope);
+}

app/imports/api/engine/computation/computeComputation/computeCalculations.js

@@ -1,13 +0,0 @@
-import embedInlineCalculations from '../utility/embedInlineCalculations.js';
-import evaluateCalculation from '../utility/evaluateCalculation.js';
-
-export default function computeCalculations(computation, node){
-  if (!node.data) return;
-  // evaluate all the calculations
-  node.data._computationDetails?.calculations?.forEach(calcObj => {
-    evaluateCalculation(calcObj, computation.scope)
-  });
-  node.data._computationDetails?.inlineCalculations?.forEach(inlineCalcObj => {
-    embedInlineCalculations(inlineCalcObj);
-  });
-}

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

@@ -8,13 +8,13 @@ export default function(){
   computeCreatureComputation(computation);
   const prop = id => computation.propsById[id];
   const scope = variableName => computation.scope[variableName];
-  assert.equal(prop('emptyId').value, 0);
-  assert.equal(prop('noVariableNameId').value, 8);
-  assert.equal(prop('strengthId').value, 12);
-  assert.equal(prop('strengthId').modifier, 1);
-  assert.equal(scope('strength').modifier, 1);
-  assert.equal(prop('referencesDexId').value, 4);
-  assert.equal(prop('hitDiceId').constitutionMod, 5);
+  assert.equal(prop('emptyId').value, 0, 'calculates empty props to zero');
+  assert.equal(prop('noVariableNameId').value, 8, 'Calculates props without a variable name');
+  assert.equal(prop('strengthId').value, 12, 'applies base values');
+  assert.equal(prop('strengthId').modifier, 1, 'calculates modifiers for basic properties');
+  assert.equal(scope('strength').modifier, 1, 'Access properties via variables');
+  assert.equal(prop('referencesDexId').value, 4, 'Access variable properties in calculations');
+  assert.equal(prop('hitDiceId').constitutionMod, 5, 'Hit dice get constitution modifier');
   assert.equal(prop('overriddenDexId').overridden, true, 'override properties with the same variable name');
   assert.equal(
     prop('parseErrorId').baseValue.value, null,

app/imports/api/engine/computation/computeCreatureComputation.js

@@ -1,6 +1,6 @@
-import computeCalculations from '/imports/api/engine/computation/computeComputation/computeCalculations.js';
 import computeToggles from '/imports/api/engine/computation/computeComputation/computeToggles.js';
 import computeByType from '/imports/api/engine/computation/computeComputation/computeByType.js';
+import embedInlineCalculations from './utility/embedInlineCalculations.js';
 
 export default function computeCreatureComputation(computation){
   const stack = [];
@@ -22,7 +22,7 @@ export default function computeCreatureComputation(computation){
       // Mark the object as visited and remove from stack
       top._visited = true;
       stack.pop();
-      // Compute the top object of the stack
+    // Compute the top object of the stack
       compute(computation, top);
     } else {
       top._visitedChildren = true;
@@ -30,13 +30,14 @@ export default function computeCreatureComputation(computation){
       pushDependenciesToStack(top.id, graph, stack);
     }
   }
+
+  // Finish the props after the dependency graph has been traversed
+  computation.props.forEach(finalizeProp);
 }
 
 function compute(computation, node){
   // Determine the prop's active status by its toggles
   computeToggles(computation, node);
-  computeCalculations(computation, node);
-  if (node.data) delete node.data._computationDetails;
   // Compute the property by type
   computeByType[node.data?.type || '_variable']?.(computation, node);
 }
@@ -46,3 +47,12 @@ function pushDependenciesToStack(nodeId, graph, stack){
     stack.push(linkedNode);
   }, true);
 }
+
+function finalizeProp(prop){
+  // Embed the inline calculations
+  prop._computationDetails?.inlineCalculations?.forEach(inlineCalcObj => {
+    embedInlineCalculations(inlineCalcObj);
+  });
+  // Clean up the computation details
+  delete prop._computationDetails;
+}

app/imports/api/library/LibraryNodes.js

@@ -32,6 +32,15 @@ let LibraryNodeSchema = new SimpleSchema({
 		type: String,
     max: STORAGE_LIMITS.tagLength,
 	},
+  libraryTags: {
+		type: Array,
+		defaultValue: [],
+    maxCount: STORAGE_LIMITS.tagCount,
+	},
+	'libraryTags.$': {
+		type: String,
+    max: STORAGE_LIMITS.tagLength,
+	},
   icon: {
     type: storedIconsSchema,
     optional: true,

app/imports/api/properties/Attributes.js

@@ -92,7 +92,6 @@ let ComputedOnlyAttributeSchema = createPropertySchema({
 	// The computed value of the attribute
   total: {
     type: SimpleSchema.oneOf(Number, String, Boolean),
-		defaultValue: 0,
     optional: true,
     removeBeforeCompute: true,
   },
@@ -112,7 +111,7 @@ let ComputedOnlyAttributeSchema = createPropertySchema({
   // Attributes with proficiency grant it to all skills based on the attribute
   proficiency: {
 		type: Number,
-    allowedValues: [0.49, 0.5, 1, 2],
+    allowedValues: [0, 0.49, 0.5, 1, 2],
 		optional: true,
     removeBeforeCompute: true,
 	},

app/imports/api/properties/Buffs.js

@@ -41,7 +41,6 @@ let ComputedOnlyBuffSchema = createPropertySchema({
 		type: Number,
 		optional: true,
 		min: 0,
-    removeBeforeCompute: true,
 	},
 	appliedBy: {
 		type: Object,
@@ -59,7 +58,7 @@ let ComputedOnlyBuffSchema = createPropertySchema({
 		type: String,
     max: STORAGE_LIMITS.collectionName,
 	},
-})
+});
 
 const ComputedBuffSchema = new SimpleSchema()
   .extend(BuffSchema)

app/imports/api/properties/SpellLists.js

@@ -35,15 +35,15 @@ const ComputedOnlySpellListSchema = new SimpleSchema({
     optional: true,
   },
   maxPrepared: {
-    type: 'fieldToCompute',
+    type: 'computedOnlyField',
     optional: true,
   },
   attackRollBonus: {
-    type: 'fieldToCompute',
+    type: 'computedOnlyField',
     optional: true,
   },
   dc: {
-    type: 'fieldToCompute',
+    type: 'computedOnlyField',
     optional: true,
   },
 });

app/imports/migrations/server/2.0-beta.33-dbv1.js

@@ -185,11 +185,12 @@ function getInlineComputationTransforms(key){
 }
 
 function calculationUp(val){
-  if (!val) return val;
+  if (!val || !val.replace) return val;
   return val.replace('.value', '.total').replace('.currentValue', '.value');
 }
 
 function calculationDown(val){
+  if (!val || !val.replace) return val;
   return val.replace('.value', '.currentValue').replace('.total', '.value');
 }