Improved dependencies-only recalculations and fixed many calculation bugs

app/imports/api/creature/computation/engine/EffectAggregator.js

@@ -1,4 +1,5 @@
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
+import { union } from 'lodash';
 
 export default class EffectAggregator{
   constructor(stat, memo){
@@ -14,7 +15,10 @@ export default class EffectAggregator{
         memo
       });
       this.statBaseValue = result.value;
-      stat.dependencies.push(...dependencies);
+      stat.dependencies = union(
+        stat.dependencies,
+        dependencies,
+      );
       if (context.errors.length){
         this.baseValueErrors = context.errors;
       }

app/imports/api/creature/computation/engine/applyToggles.js

@@ -1,10 +1,15 @@
 import computeToggle from '/imports/api/creature/computation/engine/computeToggle.js';
+import { union } from 'lodash';
 
 export default function applyToggles(prop, memo){
   prop.computationDetails.toggleAncestors.forEach(toggleId => {
     let toggle = memo.togglesById[toggleId];
     computeToggle(toggle, memo);
-    prop.dependencies.push(toggle._id, ...toggle.dependencies);
+    prop.dependencies = union(
+      prop.dependencies,
+      [toggle._id],
+      toggle.dependencies,
+    );
     if (!toggle.toggleResult){
       prop.computationDetails.disabledByToggle = true;
     }

app/imports/api/creature/computation/engine/combineStat.js

@@ -1,6 +1,7 @@
 import computeStat from '/imports/api/creature/computation/engine/computeStat.js';
 import applyToggles from '/imports/api/creature/computation/engine/applyToggles.js';
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
+import { union } from 'lodash';
 
 export default function combineStat(stat, aggregator, memo){
   if (stat.type === 'attribute'){
@@ -45,7 +46,7 @@ function combineAttribute(stat, aggregator, memo){
     });
     stat.spellSlotLevelValue = result.value;
     stat.spellSlotLevelErrors = context.errors;
-    stat.dependencies.push(...dependencies);
+    stat.dependencies = union(stat.dependencies, dependencies);
   }
   stat.currentValue = stat.value - (stat.damage || 0);
   // Ability scores get modifiers
@@ -60,7 +61,11 @@ function combineAttribute(stat, aggregator, memo){
     let conStat = memo.statsByVariableName['constitution'];
     if (conStat && 'modifier' in conStat){
       stat.constitutionMod = conStat.modifier;
-      stat.dependencies.push(conStat._id, ...conStat.dependencies);
+      stat.dependencies = union(
+        stat.dependencies,
+        [conStat._id],
+        conStat.dependencies,
+      );
     }
   }
   // Stats that have no effects can be hidden based on a sheet setting
@@ -77,7 +82,11 @@ function combineSkill(stat, aggregator, memo){
       computeStat(ability, memo);
     }
     stat.abilityMod = ability.modifier;
-    stat.dependencies.push(ability._id, ...ability.dependencies);
+    stat.dependencies = union(
+      stat.dependencies,
+      [ability._id],
+      ability.dependencies,
+    );
   }
   // Combine all the child proficiencies
   stat.proficiency = stat.baseProficiency || 0;
@@ -89,7 +98,11 @@ function combineSkill(stat, aggregator, memo){
       prof.value > stat.proficiency
     ){
       stat.proficiency = prof.value;
-      stat.dependencies.push(prof._id, ...prof.dependencies);
+      stat.dependencies = union(
+        stat.dependencies,
+        [prof._id],
+        prof.dependencies,
+      );
     }
   }
   // Get the character's proficiency bonus to apply
@@ -99,10 +112,18 @@ function combineSkill(stat, aggregator, memo){
   if (typeof profBonus !== 'number' && memo.statsByVariableName['level']){
     let level = memo.statsByVariableName['level'].value;
     profBonus = Math.ceil(level / 4) + 1;
-    if (level._id) stat.dependencies.push(level._id);
-    if (level.dependencies) stat.dependencies.push(...level.dependencies);
+    if (level._id){
+      stat.dependencies = union(stat.dependencies, [level._id]);
+    }
+    if (level.dependencies){
+      stat.dependencies = union(stat.dependencies, level.dependencies);
+    }
   } else {
-    stat.dependencies.push(profBonusStat._id, ...profBonusStat.dependencies);
+    stat.dependencies = union(
+      stat.dependencies,
+      [profBonusStat._id],
+      profBonusStat.dependencies,
+    );
   }
   // Multiply the proficiency bonus by the actual proficiency
   profBonus *= stat.proficiency;

app/imports/api/creature/computation/engine/computeEffect.js

@@ -1,5 +1,6 @@
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
 import applyToggles from '/imports/api/creature/computation/engine/applyToggles.js';
+import { union } from 'lodash';
 
 export default function computeEffect(effect, memo){
   if (effect.computationDetails.computed) return;
@@ -44,7 +45,7 @@ export default function computeEffect(effect, memo){
       memo
     });
     effect.result = result.value;
-    effect.dependencies.push(...dependencies);
+    effect.dependencies = union(effect.dependencies, dependencies);
     if (context.errors.length){
       effect.errors = context.errors;
     }

app/imports/api/creature/computation/engine/computeEndStepProperty.js

@@ -1,5 +1,6 @@
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
 import ConstantNode from '/imports/parser/parseTree/ConstantNode.js';
+import { union } from 'lodash';
 
 export default function computeEndStepProperty(prop, memo){
   switch (prop.type){
@@ -35,7 +36,7 @@ function computeAction(prop, memo){
     dependencies,
   } = evaluateCalculation({ string: prop.uses, prop, memo});
   prop.usesResult = result.value;
-  prop.dependencies.push(...dependencies);
+  prop.dependencies = union(prop.dependencies, dependencies);
   if (context.errors.length){
     prop.usesErrors = context.errors;
   } else {
@@ -57,7 +58,13 @@ function computeAction(prop, memo){
       if (available < attConsumed.quantity){
         prop.insufficientResources = true;
       }
-      if (stat) prop.dependencies.push(stat._id, ...stat.dependencies);
+      if (stat){
+        prop.dependencies = union(
+          prop.dependencies,
+          [stat._id],
+          stat.dependencies
+        );
+      }
     }
   });
   // Items consumed
@@ -76,7 +83,13 @@ function computeAction(prop, memo){
     if (!item || available < itemConsumed.quantity){
       prop.insufficientResources = true;
     }
-    if (item) prop.dependencies.push(item._id, ...item.dependencies);
+    if (item){
+      prop.dependencies = union(
+        prop.dependencies,
+        [item._id],
+        item.dependencies
+      );
+    }
   });
 }
 
@@ -91,7 +104,7 @@ function computePropertyField(prop, memo, fieldName, fn){
   } else {
     prop[`${fieldName}Result`] = result.toString();
   }
-  prop.dependencies.push(...dependencies);
+  prop.dependencies = union(prop.dependencies, dependencies);
   if (context.errors.length){
     prop[`${fieldName}Errors`] = context.errors;
   } else {

app/imports/api/creature/computation/engine/computeInlineCalculations.js

@@ -1,4 +1,5 @@
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
+import { union } from 'lodash';
 
 export default function computeInlineCalculations(prop, memo){
   if (prop.summary){
@@ -28,7 +29,7 @@ function computeInlineCalcsForField(prop, memo, field){
       computation.errors = context.errors;
     }
     inlineComputations.push(computation);
-    prop.dependencies.push(...dependencies);
+    prop.dependencies = union(prop.dependencies, dependencies);
   }
   prop[`${field}Calculations`] = inlineComputations;
 }

app/imports/api/creature/computation/engine/computeLevels.js

@@ -1,4 +1,4 @@
-import { forOwn, has } from 'lodash';
+import { forOwn, has, union } from 'lodash';
 
 export default function computeLevels(memo){
   computeClassLevels(memo);
@@ -8,7 +8,10 @@ export default function computeLevels(memo){
 function computeClassLevels(memo){
   forOwn(memo.classLevelsById, classLevel => {
     //  class levels are mutually dependent
-    classLevel.dependencies.push(Object.keys(memo.classLevelsById));
+    classLevel.dependencies = union(
+      classLevel.dependencies,
+      Object.keys(memo.classLevelsById)
+    );
     let name = classLevel.variableName;
     let stat = memo.statsByVariableName[name];
     if (!stat){
@@ -43,8 +46,18 @@ function computeTotalLevel(memo){
   for (let name in memo.classes){
     let cls = memo.classes[name];
     level += cls.level || 0;
-    if (cls._id) currentLevel.dependencies.push(cls._id);
-    if (cls.dependencies) currentLevel.dependencies.push(...cls.dependencies);
+    if (cls._id){
+      currentLevel.dependencies = union(
+        currentLevel.dependencies,
+        [cls._id]
+      )
+    }
+    if (cls.dependencies){
+      currentLevel.dependencies = union(
+        currentLevel.dependencies,
+        cls.dependencies,
+      )
+    }
   }
   currentLevel.value = level;
 }

app/imports/api/creature/computation/engine/computeStat.js

@@ -2,7 +2,7 @@ import combineStat from '/imports/api/creature/computation/engine/combineStat.js
 import computeEffect from '/imports/api/creature/computation/engine/computeEffect.js';
 import EffectAggregator from '/imports/api/creature/computation/engine/EffectAggregator.js';
 import applyToggles from '/imports/api/creature/computation/engine/applyToggles.js';
-import { each } from 'lodash';
+import { each, union } from 'lodash';
 
 export default function computeStat(stat, memo){
   // If the stat is already computed, skip it
@@ -27,8 +27,16 @@ export default function computeStat(stat, memo){
     let aggregator = new EffectAggregator(stat, memo)
     each(stat.computationDetails.effects, (effect) => {
       computeEffect(effect, memo);
-      if (effect._id) stat.dependencies.push(effect._id);
-      stat.dependencies.push(...effect.dependencies);
+      if (effect._id){
+        stat.dependencies = union(
+          stat.dependencies,
+          [effect._id]
+        );
+      }
+      stat.dependencies = union(
+        stat.dependencies,
+        effect.dependencies
+      )
       if (!effect.computationDetails.disabledByToggle){
         aggregator.addEffect(effect);
       }

app/imports/api/creature/computation/engine/computeToggle.js

@@ -1,4 +1,5 @@
 import evaluateCalculation from '/imports/api/creature/computation/engine/evaluateCalculation.js';
+import { union } from 'lodash';
 
 export default function computeToggle(toggle, memo){
   if (toggle.computationDetails.computed) return;
@@ -32,7 +33,10 @@ export default function computeToggle(toggle, memo){
       dependencies,
     } = evaluateCalculation({string: toggle.condition, prop: toggle, memo});
     toggle.toggleResult = !!result.value;
-    toggle.dependencies.push(...dependencies);
+    toggle.dependencies = union(
+      toggle.dependencies,
+      dependencies,
+    );
     if (context.errors.length){
       toggle.errors = context.errors;
     }

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

@@ -4,6 +4,7 @@ import SymbolNode from '/imports/parser/parseTree/SymbolNode.js';
 import AccessorNode from '/imports/parser/parseTree/AccessorNode.js';
 import ConstantNode from '/imports/parser/parseTree/ConstantNode.js';
 import findAncestorByType from '/imports/api/creature/computation/engine/findAncestorByType.js';
+import { union } from 'lodash';
 
 /* Convert a calculation into a constant output and errors*/
 export default function evaluateCalculation({
@@ -55,7 +56,12 @@ export default function evaluateCalculation({
       if (stat && stat.computationDetails && !stat.computationDetails.computed){
         computeStat(stat, memo);
       }
-      if (stat) dependencies.push(stat._id || node.name, ...stat.dependencies);
+      if (stat){
+        dependencies = union(dependencies, [
+          stat._id || node.name,
+          ...stat.dependencies
+        ]);
+      }
     }
   });
   // Evaluate

app/imports/api/creature/computation/engine/getDependentProperties.js

@@ -1,13 +1,18 @@
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
+import { union } from 'lodash';
 
-export default function getDependentProperties({creatureId, dependencies}){
+export default function getDependentProperties({
+  creatureId,
+  propertyIds,
+  propertiesDependedAponIds,
+ }){
   // find ids of all dependant toggles that have conditions, even if inactive
   let toggleIds = CreatureProperties.find({
     'ancestors.id': creatureId,
     type: 'toggle',
     removed: {$ne: true},
     condition: { $exists: true },
-    dependencies: {$in: dependencies},
+    dependencies: {$in: propertyIds},
   }, {
     fields: {_id: 1},
   }).map(t => t._id);
@@ -15,7 +20,7 @@ export default function getDependentProperties({creatureId, dependencies}){
   let props = CreatureProperties.find({
     'ancestors.id': creatureId,
     removed: {$ne: true},
-    dependencies: {$in: dependencies},
+    dependencies: {$in: propertyIds},
     $or: [
       // All active properties
       {inactive: {$ne: true}},
@@ -25,18 +30,22 @@ export default function getDependentProperties({creatureId, dependencies}){
       // All decendents of the above toggles
       {'ancestors.id': {$in: toggleIds}},
     ]
-  }, {
-    // Filter out fields never used by calculations
-    fields: {
-      icon: 0,
-    },
-    sort: {
-      order: 1,
-    }
-  }).fetch();
-  // Add on all the properties th
-  CreatureProperties.find({_id: {$in: dependencies}}).forEach(prop => {
-    props.push(prop)
+  }, { fields: {_id: 1, dependencies: 1} }).fetch();
+  // Add all the properties that changing props depend on, but haven't yet been
+  // included to make an array of every property we need
+  let allConnectedPropIds = [...propertyIds, ...propertiesDependedAponIds];
+  props.forEach(prop => {
+    allConnectedPropIds = union(
+      allConnectedPropIds,
+      prop.dependencies,
+      [prop._id]);
   });
-  return props;
+  // Add on all the properties and the objects they depend apon
+  return CreatureProperties.find({
+    _id: {$in: allConnectedPropIds}
+  }, {
+    // Ignore fields not used in computations
+    fields: {icon: 0},
+    sort: {order: 1},
+  }).fetch();
 }

app/imports/api/creature/computation/engine/writeAlteredProperties.js

@@ -26,7 +26,7 @@ export default function writeAlteredProperties(memo){
       }
     });
   });
-  bulkWriteProperties(bulkWriteOperations);
+  writePropertiesSequentially(bulkWriteOperations);
 }
 
 function addChangedKeysToOp(op, keys, original, changed) {
@@ -77,11 +77,28 @@ function addUnsetOp(op, key){
   }
 }
 
+// We use this instead of bulkWriteProperties because it functions with latency
+// compensation without needing to roll back changes, which causes multiple
+// expensive redraws of the character sheet
+function writePropertiesSequentially(bulkWriteOps){
+  bulkWriteOps.forEach(op => {
+    let updateOneOrMany = op.updateOne || op.updateMany;
+    CreatureProperties.update(updateOneOrMany.filter, updateOneOrMany.update, {
+      // The bulk code is bypassing validation, so do the same here
+      // selector: {type: op.type} // include this if bypass is off
+      bypassCollection2: true,
+    });
+  });
+}
+
+// This is more efficient on the database, but significantly less efficient
+// in the UI because of incompatibility with latency compensation. If the
+// duplicate redraws can be fixed, this is a strictly better way of processing
+// writes
 function bulkWriteProperties(bulkWriteOps){
   if (!bulkWriteOps.length) return;
-  // Only use bulk writing if there are many writes to do
-  // it makes latency compensation janky, so we avoid it for smaller writes
-  if (Meteor.isServer && bulkWriteOps.length > 16){
+  // bulkWrite is only available on the server
+  if (Meteor.isServer){
     CreatureProperties.rawCollection().bulkWrite(
       bulkWriteOps,
       {ordered : false},
@@ -93,13 +110,6 @@ function bulkWriteProperties(bulkWriteOps){
       }
     );
   } else {
-    bulkWriteOps.forEach(op => {
-      let updateOneOrMany = op.updateOne || op.updateMany;
-      CreatureProperties.update(updateOneOrMany.filter, updateOneOrMany.update, {
-        // The bulk code is bypassing validation, so do the same here
-        // selector: {type: op.type} // include this if bypass is off
-        bypassCollection2: true,
-      });
-    });
+    writePropertiesSequentially(bulkWriteOps);
   }
 }

app/imports/api/creature/computation/methods/recomputeCreature.js

@@ -12,6 +12,7 @@ import recomputeSlotFullness from '/imports/api/creature/denormalise/recomputeSl
 import getRootCreatureAncestor from '/imports/api/creature/creatureProperties/getRootCreatureAncestor.js';
 import getDependentProperties from '/imports/api/creature/computation/engine/getDependentProperties.js';
 import Creatures from '/imports/api/creature/Creatures.js';
+import recomputeInactiveProperties from '/imports/api/creature/denormalise/recomputeInactiveProperties.js';
 
 export const recomputeCreature = new ValidatedMethod({
 
@@ -88,6 +89,7 @@ export function recomputeCreatureByDoc(creature){
   writeCreatureVariables(computationMemo, creatureId);
   recomputeDamageMultipliersById(creatureId);
   recomputeSlotFullness(creatureId);
+  recomputeInactiveProperties(creatureId);
   return computationMemo;
 }
 
@@ -95,17 +97,24 @@ export function recomputePropertyDependencies(property){
   let creature = getRootCreatureAncestor(property);
   recomputeCreatureByDependencies({
     creature,
-    dependencies: [property._id],
+    propertyIds: [property._id],
+    propertiesDependedAponIds: property.dependencies,
   });
 }
 
-export function recomputeCreatureByDependencies({creature, dependencies}){
+export function recomputeCreatureByDependencies({
+  creature,
+  propertyIds,
+  propertiesDependedAponIds
+}){
   let props = getDependentProperties({
     creatureId: creature._id,
-    dependencies,
+    propertyIds,
+    propertiesDependedAponIds,
   });
   let computationMemo = new ComputationMemo(props, creature);
   computeMemo(computationMemo);
   writeAlteredProperties(computationMemo);
+  recomputeInactiveProperties(creature._id);
   return computationMemo;
 }

app/imports/api/creature/creatureProperties/methods/damageProperty.js

@@ -4,7 +4,7 @@ import SimpleSchema from 'simpl-schema';
 import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
 import getRootCreatureAncestor from '/imports/api/creature/creatureProperties/getRootCreatureAncestor.js';
 import { assertEditPermission } from '/imports/api/sharing/sharingPermissions.js';
-import { recomputePropertyDependencies, recomputeCreatureByDoc, recomputeCreature } from '/imports/api/creature/computation/methods/recomputeCreature.js';
+import { recomputePropertyDependencies } from '/imports/api/creature/computation/methods/recomputeCreature.js';
 
 const damageProperty = new ValidatedMethod({
   name: 'creatureProperties.damage',

app/imports/api/creature/creatureProperties/methods/dealDamage.js

@@ -48,7 +48,8 @@ const dealDamage = new ValidatedMethod({
     let totalDamage = Math.floor(amount * multiplier);
     let damageLeft = totalDamage;
     if (damageType === 'healing') damageLeft = -totalDamage;
-    let dependencies = [];
+    let propertyIds = [];
+    let propertiesDependedAponIds = [];
     healthBars.forEach(healthBar => {
       if (damageLeft === 0) return;
       let damageAdded = damagePropertyWork({
@@ -57,10 +58,14 @@ const dealDamage = new ValidatedMethod({
         value: damageLeft,
       });
       damageLeft -= damageAdded;
-      dependencies.push(healthBar.variableName);
-      dependencies.push(...healthBar.dependencies);
+      propertyIds.push(healthBar._id);
+      propertiesDependedAponIds.push(...healthBar.dependencies);
+    });
+    recomputeCreatureByDependencies({
+      creature,
+      propertyIds,
+      propertiesDependedAponIds,
     });
-    recomputeCreatureByDependencies({creature, dependencies});
     return totalDamage;
   },
 });

app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue

@@ -59,13 +59,12 @@
 </template>
 
 <script>
-import CreatureProperties, {
-  damageProperty,
-  pushToProperty,
-  pullFromProperty,
-  softRemoveProperty,
-  restoreProperty,
-} from '/imports/api/creature/creatureProperties/CreatureProperties.js';
+import CreatureProperties from '/imports/api/creature/creatureProperties/CreatureProperties.js';
+import damageProperty from '/imports/api/creature/creatureProperties/methods/damageProperty.js';
+import pushToProperty from '/imports/api/creature/creatureProperties/methods/pushToProperty.js';
+import pullFromProperty from '/imports/api/creature/creatureProperties/methods/pullFromProperty.js';
+import softRemoveProperty from '/imports/api/creature/creatureProperties/methods/softRemoveProperty.js';
+import restoreProperty from '/imports/api/creature/creatureProperties/methods/restoreProperty.js';
 import updateCreatureProperty from '/imports/api/creature/creatureProperties/methods/updateCreatureProperty.js';
 import duplicateProperty from '/imports/api/creature/creatureProperties/methods/duplicateProperty.js';
 import Creatures from '/imports/api/creature/Creatures.js';