From 280f30dab58b45604ba70f3a1ec7e2c8ba0e1a4a Mon Sep 17 00:00:00 2001 From: Stefan Zermatten Date: Thu, 4 Feb 2021 16:16:51 +0200 Subject: [PATCH] Improved dependencies-only recalculations and fixed many calculation bugs --- .../computation/engine/EffectAggregator.js | 6 ++- .../computation/engine/applyToggles.js | 7 +++- .../computation/engine/combineStat.js | 35 ++++++++++++---- .../computation/engine/computeEffect.js | 3 +- .../engine/computeEndStepProperty.js | 21 ++++++++-- .../engine/computeInlineCalculations.js | 3 +- .../computation/engine/computeLevels.js | 21 ++++++++-- .../computation/engine/computeStat.js | 14 +++++-- .../computation/engine/computeToggle.js | 6 ++- .../computation/engine/evaluateCalculation.js | 8 +++- .../engine/getDependentProperties.js | 41 +++++++++++-------- .../engine/writeAlteredProperties.js | 34 +++++++++------ .../computation/methods/recomputeCreature.js | 15 +++++-- .../methods/damageProperty.js | 2 +- .../creatureProperties/methods/dealDamage.js | 13 ++++-- .../CreaturePropertyDialog.vue | 13 +++--- 16 files changed, 175 insertions(+), 67 deletions(-) diff --git a/app/imports/api/creature/computation/engine/EffectAggregator.js b/app/imports/api/creature/computation/engine/EffectAggregator.js index 61535e9b..ecd0a881 100644 --- a/app/imports/api/creature/computation/engine/EffectAggregator.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/applyToggles.js b/app/imports/api/creature/computation/engine/applyToggles.js index 7d7c1a4e..926531a7 100644 --- a/app/imports/api/creature/computation/engine/applyToggles.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/combineStat.js b/app/imports/api/creature/computation/engine/combineStat.js index 6fd251c4..da417234 100644 --- a/app/imports/api/creature/computation/engine/combineStat.js +++ b/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; diff --git a/app/imports/api/creature/computation/engine/computeEffect.js b/app/imports/api/creature/computation/engine/computeEffect.js index 28985057..75d880b4 100644 --- a/app/imports/api/creature/computation/engine/computeEffect.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/computeEndStepProperty.js b/app/imports/api/creature/computation/engine/computeEndStepProperty.js index b322845f..8d6b90d7 100644 --- a/app/imports/api/creature/computation/engine/computeEndStepProperty.js +++ b/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 { diff --git a/app/imports/api/creature/computation/engine/computeInlineCalculations.js b/app/imports/api/creature/computation/engine/computeInlineCalculations.js index 22ea0e4f..c5979af5 100644 --- a/app/imports/api/creature/computation/engine/computeInlineCalculations.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/computeLevels.js b/app/imports/api/creature/computation/engine/computeLevels.js index 41aad1f2..f07f4adf 100644 --- a/app/imports/api/creature/computation/engine/computeLevels.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/computeStat.js b/app/imports/api/creature/computation/engine/computeStat.js index ebab8789..b981a311 100644 --- a/app/imports/api/creature/computation/engine/computeStat.js +++ b/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); } diff --git a/app/imports/api/creature/computation/engine/computeToggle.js b/app/imports/api/creature/computation/engine/computeToggle.js index b3f5de2d..672f87ea 100644 --- a/app/imports/api/creature/computation/engine/computeToggle.js +++ b/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; } diff --git a/app/imports/api/creature/computation/engine/evaluateCalculation.js b/app/imports/api/creature/computation/engine/evaluateCalculation.js index c1f5b472..fb49431c 100644 --- a/app/imports/api/creature/computation/engine/evaluateCalculation.js +++ b/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 diff --git a/app/imports/api/creature/computation/engine/getDependentProperties.js b/app/imports/api/creature/computation/engine/getDependentProperties.js index 516d7d90..d87a70ed 100644 --- a/app/imports/api/creature/computation/engine/getDependentProperties.js +++ b/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(); } diff --git a/app/imports/api/creature/computation/engine/writeAlteredProperties.js b/app/imports/api/creature/computation/engine/writeAlteredProperties.js index 7c49d74b..5ffef21e 100644 --- a/app/imports/api/creature/computation/engine/writeAlteredProperties.js +++ b/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); } } diff --git a/app/imports/api/creature/computation/methods/recomputeCreature.js b/app/imports/api/creature/computation/methods/recomputeCreature.js index cb938a96..099e5578 100644 --- a/app/imports/api/creature/computation/methods/recomputeCreature.js +++ b/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; } diff --git a/app/imports/api/creature/creatureProperties/methods/damageProperty.js b/app/imports/api/creature/creatureProperties/methods/damageProperty.js index d9930841..2efcfc2f 100644 --- a/app/imports/api/creature/creatureProperties/methods/damageProperty.js +++ b/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', diff --git a/app/imports/api/creature/creatureProperties/methods/dealDamage.js b/app/imports/api/creature/creatureProperties/methods/dealDamage.js index a8f0d13f..270221b9 100644 --- a/app/imports/api/creature/creatureProperties/methods/dealDamage.js +++ b/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; }, }); diff --git a/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue b/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue index 110aabb3..9dc17409 100644 --- a/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue +++ b/app/imports/ui/creature/creatureProperties/CreaturePropertyDialog.vue @@ -59,13 +59,12 @@