From a06e9a0960197749f37a96d5bd3bd52d3cee7453 Mon Sep 17 00:00:00 2001 From: ThaumRystra <9525416+ThaumRystra@users.noreply.github.com> Date: Sat, 23 Dec 2023 12:31:14 +0200 Subject: [PATCH] Fixed failing tests --- app/imports/api/engine/actions/Actions.ts | 15 +++++++--- .../api/engine/actions/actions.test.ts | 7 +++-- .../shared/recalculateCalculation.js | 11 +++---- .../shared/recalculateInlineCalculations.js | 10 ++++--- app/imports/api/engine/actions/doAction.js | 3 -- .../computation/buildCreatureComputation.js | 7 +---- app/imports/api/engine/loadCreatures.ts | 2 +- app/imports/api/parenting/ChildSchema.ts | 1 + .../api/parenting/parentingFunctions.ts | 2 -- .../tests/propTestBuilder.testFn.js | 29 ++++++++++++------- 10 files changed, 49 insertions(+), 38 deletions(-) diff --git a/app/imports/api/engine/actions/Actions.ts b/app/imports/api/engine/actions/Actions.ts index 182fbda4..73b2ecdb 100644 --- a/app/imports/api/engine/actions/Actions.ts +++ b/app/imports/api/engine/actions/Actions.ts @@ -274,6 +274,7 @@ export async function applyAction(action: Action, userInput?: any[] | Function, action._isSimulation = simulate; action.taskCount = 0; const prop = await getSingleProperty(action.creatureId, action.rootPropId); + if (!prop) throw new Meteor.Error('Not found', 'Root action property could not be found'); await applyTask(action, { prop, targetIds: action.targetIds || [], @@ -311,6 +312,7 @@ async function applyTask(action: Action, task: Task, userInput?): Promise if (prop.triggerIds?.before?.length) { for (const triggerId of prop.triggerIds.before) { const trigger = await getSingleProperty(action.creatureId, triggerId); + if (!trigger) continue; await applyTask(action, { prop: trigger, targetIds: task.targetIds }, userInput); } } @@ -367,6 +369,7 @@ async function applyAfterChildrenTriggers(action: Action, prop, targetIds, userI if (!prop.triggerIds?.afterChildren) return; for (const triggerId of prop.triggerIds.afterChildren) { const trigger = await getSingleProperty(action.creatureId, triggerId); + if (!trigger) continue; await applyTask(action, { prop: trigger, targetIds }, userInput); } } @@ -375,6 +378,7 @@ async function applyAfterTriggers(action: Action, prop, targetIds, userInput) { if (!prop.triggerIds?.after) return; for (const triggerId of prop.triggerIds.after) { const trigger = await getSingleProperty(action.creatureId, triggerId); + if (!trigger) continue; await applyTask(action, { prop: trigger, targetIds }, userInput); } } @@ -437,6 +441,7 @@ async function applyTriggers(action: Action, prop, targetIds: string[], triggerP if (!triggerIds) return; for (const triggerId of triggerIds) { const trigger = await getSingleProperty(action.creatureId, triggerId); + if (!trigger) continue; await applyTask(action, { prop: trigger, targetIds }, userInput); } } @@ -461,6 +466,8 @@ async function applyTaskToEachTarget(action: Action, task: PropTask, targetIds: // Combine all the action results into the scope at present export async function getEffectiveActionScope(action: Action) { const scope = await getVariables(action.creatureId); + delete scope._id; + delete scope._creatureId; // Combine the applied results for (const result of action.results) { // Pop keys that are not longer used by a busy property @@ -524,7 +531,7 @@ const applyPropertyByType = { //Log the name and summary, check that the property has enough resources to fire const content: LogContent = { name: prop.name }; if (prop.summary?.text) { - recalculateInlineCalculations(prop.summary, action); + await recalculateInlineCalculations(prop.summary, action); content.value = prop.summary.value; } if (prop.silent) content.silenced = true; @@ -591,7 +598,7 @@ const applyPropertyByType = { } // Evaluate the amount - recalculateCalculation(prop.amount, action, 'reduce'); + await recalculateCalculation(prop.amount, action, 'reduce'); const value = +prop.amount.value; if (!isFinite(value)) { return; @@ -648,7 +655,7 @@ const applyPropertyByType = { if (!children.length) { return applyAfterTasksSkipChildren(action, prop, targets, userInput); } - recalculateCalculation(prop.condition, action, 'reduce'); + await recalculateCalculation(prop.condition, action, 'reduce'); if (!isFinite(prop.condition?.value)) { result.appendLog({ name: 'Branch Error', @@ -1070,7 +1077,7 @@ async function spendResources(action: Action, prop, targetIds: string[], result: throw 'No ammo was selected'; } const item = getSingleProperty(action.creatureId, itemConsumed.itemId); - if (!item || item.ancestors[0].id !== prop.ancestors[0].id) { + if (!item || item.root.id !== prop.root.id) { throw 'The prop\'s ammo was not found on the creature'; } const quantity = +itemConsumed?.quantity?.value; diff --git a/app/imports/api/engine/actions/actions.test.ts b/app/imports/api/engine/actions/actions.test.ts index b326ab20..3af12baa 100644 --- a/app/imports/api/engine/actions/actions.test.ts +++ b/app/imports/api/engine/actions/actions.test.ts @@ -29,8 +29,9 @@ describe('Interrupt action system', function () { dirty: true, }); await insertActionTestProps(); - loadCreature(creatureId, dummySubscription); + // Compute before load or we might run tests before the computation changes reflect in the cache computeCreature(creatureId); + loadCreature(creatureId, dummySubscription); }); after(function () { unload?.(); @@ -113,7 +114,7 @@ describe('Interrupt action system', function () { function createAction(prop, targetIds?) { const action: Action = { - creatureId: prop.ancestors[0].id, + creatureId: prop.root.id, rootPropId: prop._id, results: [], taskCount: 0, @@ -254,7 +255,7 @@ const propForest = [ ]; function insertActionTestProps() { - const promises = propsFromForest(propForest, [{ id: creatureId, collection: 'creatures' }]).map(prop => { + const promises = propsFromForest(propForest, creatureId).map(prop => { return CreatureProperties.insertAsync(prop); }); return Promise.all(promises); diff --git a/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js b/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js index 4220cd92..d984b57a 100644 --- a/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js +++ b/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateCalculation.js @@ -1,5 +1,5 @@ import logErrors from './logErrors'; -import { toPrimitiveOrString } from '/imports/parser/resolve'; +import { Context, toPrimitiveOrString } from '/imports/parser/resolve'; import { aggregateCalculationEffects, aggregateCalculationProficiencies, @@ -7,15 +7,16 @@ import { } from '/imports/api/engine/computation/computeComputation/computeByType/computeCalculation'; import { getSingleProperty } from '/imports/api/engine/loadCreatures'; import resolve from '/imports/parser/resolve'; +import { getEffectiveActionScope } from '/imports/api/engine/actions/Actions'; // TODO move this whole file to Actions.ts // Redo the work of imports/api/engine/computation/computeComputation/computeByType/computeCalculation.js // But in the action scope -export default function recalculateCalculation(calcObj, action, parseLevel = 'reduce', context, scope) { +export default async function recalculateCalculation(calcObj, action, parseLevel = 'reduce', context, scope) { if (!calcObj?.parseNode) return; calcObj._parseLevel = parseLevel; if (!scope) { - scope = getEffectiveActionScope(action); + scope = await getEffectiveActionScope(action); } // Re-resolve the parse node resolveCalculationNode(calcObj, calcObj.parseNode, scope, context); @@ -41,9 +42,9 @@ export default function recalculateCalculation(calcObj, action, parseLevel = 're // TODO log errors } -export function rollAndReduceCalculation(calcObj, action) { +export async function rollAndReduceCalculation(calcObj, action) { const context = new Context(); - const scope = getEffectiveActionScope(action); + const scope = await getEffectiveActionScope(action); // Compile recalculateCalculation(calcObj, action, 'compile', context, scope); const compiled = calcObj.valueNode; diff --git a/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateInlineCalculations.js b/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateInlineCalculations.js index a7d12852..2722c395 100644 --- a/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateInlineCalculations.js +++ b/app/imports/api/engine/actions/applyPropertyByType/shared/recalculateInlineCalculations.js @@ -1,13 +1,15 @@ import embedInlineCalculations from '/imports/api/engine/computation/utility/embedInlineCalculations'; import recalculateCalculation from './recalculateCalculation' -export default function recalculateInlineCalculations(inlineCalcObj, actionContext) { +export default async function recalculateInlineCalculations(inlineCalcObj, action) { // Skip if there are no calculations if (!inlineCalcObj?.inlineCalculations?.length) return; // Recalculate each calculation with the current scope - inlineCalcObj.inlineCalculations.forEach(calc => { - recalculateCalculation(calc, action); - }); + const promises = []; + for (const calc of inlineCalcObj.inlineCalculations) { + promises.push(recalculateCalculation(calc, action)); + } + await Promise.all(promises); // Embed the new calculated values embedInlineCalculations(inlineCalcObj); } diff --git a/app/imports/api/engine/actions/doAction.js b/app/imports/api/engine/actions/doAction.js index ea4720c0..4a7c5b06 100644 --- a/app/imports/api/engine/actions/doAction.js +++ b/app/imports/api/engine/actions/doAction.js @@ -96,10 +96,7 @@ export async function doActionWork({ // Apply the top level property, it is responsible for applying its children // recursively - console.log('start apply properties') await applyProperty(propertyForest[0], actionContext); - console.log('end apply properties') - // Insert the log actionContext.writeLog(); } diff --git a/app/imports/api/engine/computation/buildCreatureComputation.js b/app/imports/api/engine/computation/buildCreatureComputation.js index 1c48651b..3e5d415b 100644 --- a/app/imports/api/engine/computation/buildCreatureComputation.js +++ b/app/imports/api/engine/computation/buildCreatureComputation.js @@ -87,12 +87,7 @@ export function buildComputationFromProps(properties, creature, variables) { // Get all the properties as a forest, with their nested set properties set const forest = applyNestedSetProperties(properties); - const ops = calculateNestedSetOperations(properties); - if (ops.length > 20) { - console.log(ops.length + ' operations to get fixed nested sets'); - } else { - console.log(JSON.stringify(ops, null, 2)); - } + // Walk the property trees computing things that need to be inherited walkDown(forest, node => { computeInactiveStatus(node); diff --git a/app/imports/api/engine/loadCreatures.ts b/app/imports/api/engine/loadCreatures.ts index 4d41e554..175c4831 100644 --- a/app/imports/api/engine/loadCreatures.ts +++ b/app/imports/api/engine/loadCreatures.ts @@ -234,7 +234,7 @@ class LoadedCreature { self.properties = new Map(); // Observe all creature properties which are needed for computation self.propertyObserver = CreatureProperties.find({ - 'ancestors.id': creatureId, + 'root.id': creatureId, removed: { $ne: true }, }, { sort: { order: 1 }, diff --git a/app/imports/api/parenting/ChildSchema.ts b/app/imports/api/parenting/ChildSchema.ts index 7955496a..6c1d4e56 100644 --- a/app/imports/api/parenting/ChildSchema.ts +++ b/app/imports/api/parenting/ChildSchema.ts @@ -12,6 +12,7 @@ export interface TreeDoc { parentId?: string, left: number, right: number, + removed?: true, } const RefSchema = new SimpleSchema({ diff --git a/app/imports/api/parenting/parentingFunctions.ts b/app/imports/api/parenting/parentingFunctions.ts index 4e5fcab6..67278c91 100644 --- a/app/imports/api/parenting/parentingFunctions.ts +++ b/app/imports/api/parenting/parentingFunctions.ts @@ -369,8 +369,6 @@ export async function moveDocWithinRoot(doc: TreeDoc, collection: Mongo.Collecti throw new Meteor.Error('invalid-move', 'Destination must be outside the doc\'s current location'); } - console.log(`Moving ${doc._id} ${move} spaces, while everything between ${includedRange.left} and ${includedRange.right} is shifted by ${shiftDistance}`); - // If the move isn't meaningfully changing the doc's location, skip if (Math.abs(move) < 1) { return; diff --git a/app/imports/api/properties/tests/propTestBuilder.testFn.js b/app/imports/api/properties/tests/propTestBuilder.testFn.js index 3052394b..59ddfb6f 100644 --- a/app/imports/api/properties/tests/propTestBuilder.testFn.js +++ b/app/imports/api/properties/tests/propTestBuilder.testFn.js @@ -1,3 +1,5 @@ +import { applyNestedSetProperties } from '/imports/api/parenting/parentingFunctions'; + /** * Take a forest of props, which can have sub-props nested in children: [], and return a list of * clean props with correct tree and ancestry data @@ -6,7 +8,9 @@ */ export function propsFromForest( props, - ancestry = [{ id: 'creatureId', collection: 'creatures' }], + creatureId = Random.id(), + parentId = undefined, + recursionDepth = 0 ) { const result = []; props.forEach(prop => { @@ -17,24 +21,29 @@ export function propsFromForest( throw 'Type is required on every property, not found on above doc'; } // Create the clean doc - const doc = { ...prop }; + const doc = { + ...prop, + left: result.length, + root: { id: creatureId, collection: 'creatures' }, + }; + if (parentId) { + doc.parentId = parentId; + } if (!doc._id) { doc._id = Random.id(); } delete doc.children; - doc.order = result.length; - doc.parent = { ...ancestry[ancestry.length - 1] }; - doc.ancestors = [...ancestry]; // Add the doc to the result and ancestry result.push(doc); if (children) { - ancestry.push({ id: doc._id, collection: 'creatureProperties' }); - // Add the children to the result - result.push(...propsFromForest(children, ancestry)); - // Remove the doc from the ancestry after its children are done - ancestry.pop(); + result.push(...propsFromForest(children, creatureId, doc._id, recursionDepth + 1)); } }); + + // Apply the nested set properties on the top level + if (recursionDepth === 0) { + applyNestedSetProperties(result); + } return result; }