Merge pull request dmdorman#1424 from phBalance/phBalance/more-resour…

…ce-use-refactor-sharing

more resource use refactor sharing (_onStartTurn)

module/actor/actor-sheet.mjs

@@ -69,7 +69,7 @@ export class HeroSystemActorSheet extends ActorSheet {
             data.isGM = game.user.isGM;
 
             // enrichedData
-            for (let field of [
+            for (const field of [
                 "BIOGRAPHY",
                 "BACKGROUND",
                 "PERSONALITY",
@@ -1003,9 +1003,9 @@ export class HeroSystemActorSheet extends ActorSheet {
             resourcesUsedDescriptionRenderedRoll,
         } = await userInteractiveVerifyOptionallyPromptThenSpendResources(item, strengthUsed);
         if (resourceError) {
-            return ui.notifications.error(resourceError);
+            return ui.notifications.error(`${item.name} ${resourceError}`);
         } else if (resourceWarning) {
-            return ui.notifications.warn(resourceWarning);
+            return ui.notifications.warn(`${item.name} ${resourceWarning}`);
         }
 
         // NOTE: Characteristic rolls can't have +1 to their roll.

module/combat.mjs

@@ -1,6 +1,7 @@
 import { HEROSYS } from "./herosystem6e.mjs";
 import { clamp } from "./utility/compatibility.mjs";
 import { whisperUserTargetsForActor, expireEffects } from "./utility/util.mjs";
+import { userInteractiveVerifyOptionallyPromptThenSpendResources } from "./item/item-attack.mjs";
 
 export class HeroSystem6eCombat extends Combat {
     constructor(data, context) {
@@ -473,54 +474,74 @@ export class HeroSystem6eCombat extends Combat {
         // Use actor.canAct to block actions
         // Remove STUNNED effect _onEndTurn
 
-        // Spend END for all active powers
+        // Spend resources for all active powers
         let content = "";
-        let spentEnd = 0;
 
-        for (const powerUsingEnd of combatant.actor.items.filter(
+        /**
+         * @type {HeroSystemItemResourcesToUse}
+         */
+        const spentResources = {
+            totalEnd: 0,
+            end: 0,
+            reserveEnd: 0,
+            charges: 0,
+        };
+
+        for (const powerUsingResourcesToContinue of combatant.actor.items.filter(
             (item) =>
-                item.system.active === true &&
-                parseInt(item.system?.end || 0) > 0 &&
-                (item.system.subType || item.type) !== "attack",
+                item.system.active === true && // Is the power active?
+                item.baseInfo.duration !== "instant" && // Is the power non instant
+                ((parseInt(item.system.end || 0) > 0 && // Does the power use END?
+                    !item.system.MODIFIER?.find((o) => o.XMLID === "COSTSEND" && o.OPTION === "ACTIVATE")) || // Does the power use END continuously?
+                    item.system.charges), // Does the power use charges?
         )) {
-            const costEndOnlyToActivate = powerUsingEnd.system.MODIFIER?.find(
-                (o) => o.XMLID === "COSTSEND" && o.OPTION === "ACTIVATE",
-            );
-            if (!costEndOnlyToActivate) {
-                const end = parseInt(powerUsingEnd.system.end);
-                const value = parseInt(this.combatant.actor.system.characteristics.end.value);
-                if (value - spentEnd >= end) {
-                    spentEnd += end;
-                    if (end >= 0) {
-                        content += `<li>${powerUsingEnd.name} (${end})</li>`;
-                    }
-                } else {
-                    content += `<li>${powerUsingEnd.name} (insufficient END; power turned off)</li>`;
-                    await powerUsingEnd.toggle();
-                }
+            const {
+                error,
+                warning,
+                resourcesUsedDescription,
+                resourcesUsedDescriptionRenderedRoll,
+                resourcesRequired,
+            } = await userInteractiveVerifyOptionallyPromptThenSpendResources(powerUsingResourcesToContinue, {});
+            if (error || warning) {
+                content += `<li>(${powerUsingResourcesToContinue.name} ${error || warning}: power turned off)</li>`;
+                await powerUsingResourcesToContinue.toggle();
+            } else {
+                content += resourcesUsedDescription
+                    ? `<li>${powerUsingResourcesToContinue.name} spent ${resourcesUsedDescription}${resourcesUsedDescriptionRenderedRoll}</li>`
+                    : "";
+
+                spentResources.totalEnd += resourcesRequired.totalEnd;
+                spentResources.end += resourcesRequired.end;
+                spentResources.reserveEnd += resourcesRequired.reserveEnd;
+                spentResources.charges += resourcesRequired.charges;
             }
         }
 
+        // TODO: This should be END per turn calculated on the first phase of action for the actor.
         const encumbered = combatant.actor.effects.find((effect) => effect.flags.encumbrance);
         if (encumbered) {
             const endCostPerTurn = Math.abs(parseInt(encumbered.flags?.dcvDex)) - 1;
             if (endCostPerTurn > 0) {
-                spentEnd += endCostPerTurn;
+                spentResources.totalEnd += endCostPerTurn;
+                spentResources.end += endCostPerTurn;
+
                 content += `<li>${encumbered.name} (${endCostPerTurn})</li>`;
+
+                // TODO: There should be a better way of integrating this with userInteractiveVerifyOptionallyPromptThenSpendResources
+                // TODO: This is wrong as it does not use STUN when there is no END
+                const value = parseInt(this.combatant.actor.system.characteristics.end.value);
+                const newEnd = value - endCostPerTurn;
+
+                await this.combatant.actor.update({
+                    "system.characteristics.end.value": newEnd,
+                });
             }
         }
 
-        if (content != "" && spentEnd > 0) {
-            let segment = this.combatant.flags.segment;
-            let value = parseInt(this.combatant.actor.system.characteristics.end.value);
-            let newEnd = value;
-            newEnd -= spentEnd;
-
-            await this.combatant.actor.update({
-                "system.characteristics.end.value": newEnd,
-            });
+        if (content !== "" && (spentResources.totalEnd > 0 || spentResources.charges > 0)) {
+            const segment = this.combatant.flags.segment;
 
-            content = `Spent ${spentEnd} END (${value} to ${newEnd}) on turn ${this.round} segment ${segment}:<ul>${content}</ul>`;
+            content = `Spent ${spentResources.end} END, ${spentResources.reserveEnd} reserve END, and ${spentResources.charges} charge${spentResources.charges > 1 ? "s" : ""} on turn ${this.round} segment ${segment}:<ul>${content}</ul>`;
 
             const token = combatant.token;
             const speaker = ChatMessage.getSpeaker({

module/item/item-attack.mjs

@@ -391,9 +391,9 @@ export async function AttackToHit(item, options) {
         ...{ noResourceUse: false },
     });
     if (resourceError) {
-        return ui.notifications.error(resourceError);
+        return ui.notifications.error(`${item.name} ${resourceError}`);
     } else if (resourceWarning) {
-        return ui.notifications.warn(resourceWarning);
+        return ui.notifications.warn(`${item.name} ${resourceWarning}`);
     }
 
     // STR 0 character must succeed with
@@ -3321,8 +3321,9 @@ async function _calcKnockback(body, item, options, knockbackMultiplier) {
  * @param {HeroSystem6eItem} item
  * @param {Object} options
  * @param {boolean} options.noResourceUse - true to not consume resources but still indicate how many would have been consumed
+ * @param {boolean} options.forceStunUsage - true to force STUN to be used if there is insufficient END
  *
- * @returns Object discriminated union based on error or warning being falsy/truthy
+ * @returns Object - discriminated union based on error or warning being falsy/truthy
  */
 export async function userInteractiveVerifyOptionallyPromptThenSpendResources(item, options) {
     const useResources = !options.noResourceUse;
@@ -3344,37 +3345,38 @@ export async function userInteractiveVerifyOptionallyPromptThenSpendResources(it
             if (enduranceReserve) {
                 if (resourcesRequired.end > reserveEnd && useResources) {
                     return {
-                        error: `${item.name} needs ${resourcesRequired.end} END but ${enduranceReserve.name} only has ${reserveEnd} END.`,
+                        error: `needs ${resourcesRequired.end} END but ${enduranceReserve.name} only has ${reserveEnd} END`,
                     };
                 }
             } else {
                 return {
-                    error: `${item.name} needs an endurance reserve to spend END but none found.`,
+                    error: `needs an endurance reserve to spend END but none found`,
                 };
             }
         } else {
             if (resourcesRequired.end > actorEndurance && useResources) {
-                // Auotmation or other actor without STUN
+                // Automation or other actor without STUN
                 const hasSTUN = getCharacteristicInfoArrayForActor(actor).find((o) => o.key === "STUN");
                 if (!hasSTUN) {
                     return {
-                        warning: `${item.name} needs ${resourcesRequired.end} END but ${actor.name} only has ${actorEndurance} END. This actor cannot use STUN for END.`,
+                        error: `${item.name} needs ${resourcesRequired.end} END but ${actor.name} only has ${actorEndurance} END. This actor cannot use STUN for END`,
                     };
                 }
 
                 // Is the actor willing to use STUN to make up for the lack of END?
                 const potentialStunCost = calculateRequiredStunDiceForLackOfEnd(actor, resourcesRequired.end);
 
-                const confirmed = await Dialog.confirm({
-                    title: "USING STUN FOR ENDURANCE",
-                    content: `<p><b>${item.name}</b> requires ${resourcesRequired.end} END. 
-                <b>${actor.name}</b> has ${actorEndurance} END. 
-                Do you want to take ${potentialStunCost.stunDice}d6 STUN damage to make up for the lack of END?</p>`,
-                });
-                if (!confirmed) {
-                    return {
-                        warning: `${item.name} needs ${resourcesRequired.end} END but ${actor.name} only has ${actorEndurance} END. The player is not spending STUN to make up the difference.`,
-                    };
+                if (!options.forceStunUsage) {
+                    const confirmed = await Dialog.confirm({
+                        title: "USING STUN FOR ENDURANCE",
+                        content: `<p><b>${item.name}</b> requires ${resourcesRequired.end} END. <b>${actor.name}</b> has ${actorEndurance} END. 
+                                Do you want to take ${potentialStunCost.stunDice}d6 STUN damage to make up for the lack of END?</p>`,
+                    });
+                    if (!confirmed) {
+                        return {
+                            warning: `needs ${resourcesRequired.end} END but ${actor.name} only has ${actorEndurance} END. The player is not spending STUN to make up the difference`,
+                        };
+                    }
                 }
 
                 ({ damage: actualStunDamage, roller: actualStunRoller } = await rollStunForEnd(
@@ -3390,9 +3392,9 @@ export async function userInteractiveVerifyOptionallyPromptThenSpendResources(it
     if (resourcesRequired.charges > 0) {
         if (resourcesRequired.charges > startingCharges && useResources) {
             return {
-                error: `${item.name} does not have ${resourcesRequired.charges} charge${
+                error: `does not have ${resourcesRequired.charges} charge${
                     resourcesRequired.charges > 1 ? "s" : ""
-                } remaining.`,
+                } remaining`,
             };
         }
     }
@@ -3438,21 +3440,33 @@ export async function userInteractiveVerifyOptionallyPromptThenSpendResources(it
     };
 }
 
+/**
+ * @typedef {Object} HeroSystemItemResourcesToUse
+ * @property {number} totalEnd - Total endurance consumed. This is the sum of actor endurance and reserve endurance
+ * @property {number} end - Total endurance consumed from actor's characteristic.
+ * @property {number} reserveEnd - Total endurance consumed from the item's associated endurance reserve.
+ *
+ * @property {number} charges - Total charges consumed from the item.
+ *
+ */
 /**
  * Calculate the total expendable cost to use this item
  *
  * @param {HeroSystem6eItem} item
  * @param {Object} options
  *
- * @returns Object
+ * @returns HeroSystemItemResourcesToUse
  */
 function calculateRequiredResourcesToUse(item, options) {
     const chargesRequired = calculateRequiredCharges(item, options.boostableChargesToUse || 0);
     const endRequired = calculateRequiredEnd(item, parseInt(options.effectiveStr) || 0);
 
     return {
-        charges: chargesRequired,
+        totalEnd: endRequired, // TODO: Needs to be implemented
         end: endRequired,
+        reserveEnd: 0, // TODO: Needs to be implemented
+
+        charges: chargesRequired,
     };
 }
 

module/item/item.mjs

@@ -479,9 +479,9 @@ export class HeroSystem6eItem extends Item {
                 noResourceUse: overrideCanAct,
             });
             if (resourceError) {
-                return ui.notifications.error(resourceError);
+                return ui.notifications.error(`${item.name} ${resourceError}`);
             } else if (resourceWarning) {
-                return ui.notifications.warn(resourceWarning);
+                return ui.notifications.warn(`${item.name} ${resourceWarning}`);
             }
 
             const success = await RequiresASkillRollCheck(this, event);
@@ -1508,9 +1508,10 @@ export class HeroSystem6eItem extends Item {
                 boostable: !!(CHARGES.ADDER || []).find((o) => o.XMLID === "BOOSTABLE"),
                 fuel: !!(CHARGES.ADDER || []).find((o) => o.XMLID === "FUEL"),
             };
-            if (this.system.charges?.value === undefined || this.system.charges?.value === null) {
-                console.log("Invalid charges. Resetting to max", this);
-                this.system.charges.value ??= this.system.charges.max;
+
+            // The first time through, on creation, there will be no value (number of charges) defined.
+            if (this.system.charges?.value == null) {
+                this.system.charges.value = this.system.charges.max;
                 changed = true;
             }
         } else {

module/item/skill.mjs

@@ -96,7 +96,7 @@ async function skillRoll(item, actor, target) {
     if (resourceError || resourceWarning) {
         const chatData = {
             user: game.user._id,
-            content: resourceError || resourceWarning,
+            content: `${item.name} ${resourceError || resourceWarning}`,
             speaker: speaker,
         };
 

module/testing/testing-dice.mjs

@@ -1,8 +1,6 @@
 import { HeroRoller } from "../utility/dice.mjs";
 
-// v11/v12 compatibility shim
-// TODO: Cleanup eslint file with these terms
-const Die = CONFIG.Dice.terms.d;
+const Die = foundry.dice.terms.Die;
 
 function FixedDieRoll(fixedRollResult) {
     return class extends Die {
@@ -11,7 +9,7 @@ function FixedDieRoll(fixedRollResult) {
         }
 
         /**
-         * Roll for this Die, but always roll rollResult (i.e. it's not random)
+         * Roll for this die, but always roll rollResult (i.e. it's not random)
          */
         _evaluate() {
             for (let i = 0; i < this.number; ++i) {
@@ -31,7 +29,7 @@ function DynamicDieRoll(generateRollResult) {
         }
 
         /**
-         * Roll for this Die based on the provided function
+         * Roll for this die based on the provided function
          */
         _evaluate() {
             for (let i = 0; i < this.number; ++i) {
@@ -49,7 +47,7 @@ class RollMock extends Roll {
 
     static fromTerms(terms, options) {
         const newTerms = terms.map((term) => {
-            // Replace all Die with a Die class that will always return 1 when rolling
+            // Replace all Die with a DieClass that will always return an expected behaviour when rolling
             if (term instanceof Die) {
                 return new this.DieClass({
                     number: term.number,

module/utility/dice.mjs

@@ -56,11 +56,9 @@ export const DICE_SO_NICE_CUSTOM_SETS = Object.freeze({
     },
 });
 
-// v11/v12 compatibility shim.
-// TODO: Cleanup eslint file with these terms
-const Die = CONFIG.Dice.terms.d;
-const NumericTerm = CONFIG.Dice.termTypes.NumericTerm;
-const OperatorTerm = CONFIG.Dice.termTypes.OperatorTerm;
+const Die = foundry.dice.terms.Die;
+const NumericTerm = foundry.dice.terms.NumericTerm;
+const OperatorTerm = foundry.dice.terms.OperatorTerm;
 
 // foundry.dice.terms.RollTerm is the v12 way of finding the class
 const RollTermClass = foundry.dice?.terms.RollTerm ? foundry.dice.terms.RollTerm : RollTerm;