[#6261] Add modifiers to DamageData (#6834)

* [#6261] Add modifiers to `DamageData`

Adds a new modifiers field for damage that is applied automatically
to the formula. The `DamageData` model now automatically adds these
modifiers to the formula, attaching to the first roll (if one can
be found when using a custom formula).

The `DamageData#scaledFormula` supports passing in additional
modifiers that are also appended. This is to support a future Rule
active effect that adds modifiers to certain rolls at evaluation
time (such as Elemental Adept, which would add `min2` to all damage
rolls from spells that deal a certain damage type).

Closes #6261

* [#6261] Tweak manual formula regex, pass modifiers everywhere

* [#6261] Use simpler damage formula for labels w/o modifiers

* [#6261] Fix issue with joining modifier lists

* [#6261] Fix double background on `<string-tags>`

Author: arbron

lang/en.json

@@ -2161,6 +2161,10 @@
             "label": "Die Denomination",
             "hint": "Denomination of the dice to roll."
           },
+          "modifiers": {
+            "label": "Modifiers",
+            "hint": "Roll modifiers added to the base die."
+          },
           "number": {
             "label": "Die Number",
             "hint": "Number of dice to roll."
@@ -3914,6 +3918,7 @@
 },
 "DND5E.RollExample": "e.g. 1d4",
 "DND5E.RollMode": "Roll Mode",
+"DND5E.RollModifiers": "Modifiers",
 "DND5E.RollSituationalBonus": "Situational Bonus?",
 "DND5E.Rule": {
   "Tooltip": "Tooltip",

less/v2/forms.less

@@ -336,10 +336,13 @@
       &.multi-select {
         position: relative;
 
-        :is(multi-select, string-tags) .tags.input-element-tags {
-          min-height: 16px;
-          padding-left: 40%;
-          &:empty { display: flex; }
+        :is(multi-select, string-tags) {
+          &:has(.input-element-tags:empty) { margin-block-start: 8px; }
+          .tags.input-element-tags {
+            min-height: 16px;
+            padding-left: 40%;
+            &:empty { display: flex; }
+          }
         }
 
         > label {
@@ -538,6 +541,8 @@
   }
 
   :is(document-tags, multi-select, string-tags) {
+    input { --input-background-color: transparent; }
+
     .tags.input-element-tags .tag {
       cursor: var(--cursor-pointer);
       font-size: var(--font-size-10);
@@ -559,7 +564,7 @@
     &.hidden-tags .tags.input-element-tags { display: none; }
   }
 
-  :is(file-picker, document-tags) > button {
+  :is(file-picker, document-tags, string-tags) > button {
     height: var(--form-field-height);
     border: var(--dnd5e-border-gold);
   }
@@ -745,7 +750,7 @@
     &:not(.copy-button, .unbutton, .reveal) { line-height: 28px; }
   }
 
-  :is(document-tags, multi-select) .tags.input-element-tags .tag {
+  :is(document-tags, multi-select, string-tags) .tags.input-element-tags .tag {
     padding: 1px 4px;
     color: var(--color-text-primary);
     height: unset;

module/data/activity/attack-data.mjs

@@ -280,13 +280,9 @@ export default class BaseAttackActivityData extends BaseActivityData {
 
   /* -------------------------------------------- */
 
-  /**
-   * Get the roll parts used to create the damage rolls.
-   * @param {Partial<AttackDamageRollProcessConfiguration>} [config={}]
-   * @returns {AttackDamageRollProcessConfiguration}
-   */
-  getDamageConfig(config={}) {
-    const rollConfig = super.getDamageConfig(config);
+  /** @override */
+  getDamageConfig(config={}, options={}) {
+    const rollConfig = super.getDamageConfig(config, options);
 
     // Handle ammunition
     const ammo = config.ammunition?.system;
@@ -310,14 +306,18 @@ export default class BaseAttackActivityData extends BaseActivityData {
         // If mode is "replace" and base part is present, replace the base part
         if ( ammo.damage.replace & (basePartIndex !== -1) ) {
           damage.base = true;
-          rollConfig.rolls.splice(basePartIndex, 1, this._processDamagePart(damage, config, rollData, basePartIndex));
+          rollConfig.rolls.splice(
+            basePartIndex, 1,
+            this._processDamagePart(damage, config, rollData, basePartIndex, options.formulaOptions)
+          );
         }
 
         // Otherwise stick the ammo damage after base part (or as first part)
         else {
           damage.ammo = true;
           rollConfig.rolls.splice(
-            basePartIndex + 1, 0, this._processDamagePart(damage, rollConfig, rollData, basePartIndex + 1)
+            basePartIndex + 1, 0,
+            this._processDamagePart(damage, rollConfig, rollData, basePartIndex + 1, options.formulaOptions)
           );
         }
       }
@@ -368,8 +368,8 @@ export default class BaseAttackActivityData extends BaseActivityData {
   /* -------------------------------------------- */
 
   /** @inheritDoc */
-  _processDamagePart(damage, rollConfig, rollData, index=0) {
-    if ( !damage.base ) return super._processDamagePart(damage, rollConfig, rollData, index);
+  _processDamagePart(damage, rollConfig, rollData, index=0, options={}) {
+    if ( !damage.base ) return super._processDamagePart(damage, rollConfig, rollData, index, options);
 
     // Swap base damage for versatile if two-handed attack is made on versatile weapon
     if ( this.item.system.isVersatile && (rollConfig.attackMode === "twoHanded") ) {
@@ -381,7 +381,7 @@ export default class BaseAttackActivityData extends BaseActivityData {
       damage = versatile;
     }
 
-    const roll = super._processDamagePart(damage, rollConfig, rollData, index);
+    const roll = super._processDamagePart(damage, rollConfig, rollData, index, options);
     roll.base = true;
 
     if ( this.item.type === "weapon" ) {

module/data/activity/base-activity.mjs

@@ -19,6 +19,7 @@ const {
 /**
  * @import { DamageRollConfiguration, DamageRollProcessConfiguration } from "../../dice/_types.mjs";
  * @import { ActivityRollData } from "../../documents/_types.mjs";
+ * @import { DamageFormulaOptions } from "../shared/_types.mjs";
  * @import { ActivityData } from "./_types.mjs";
  */
 
@@ -636,7 +637,7 @@ export default class BaseActivityData extends foundry.abstract.DataModel {
    * @param {ActivityRollData} rollData  Deterministic roll data from the item.
    */
   prepareDamageLabel(rollData) {
-    const config = this.getDamageConfig({}, { rollData });
+    const config = this.getDamageConfig({}, { formulaOptions: { modifiers: false }, rollData });
     const rolls = aggregateDamageRolls(config.rolls.map(({ base, data, options, parts }) => {
       const formula = parts.join(" + ");
       try {
@@ -728,16 +729,17 @@ export default class BaseActivityData extends foundry.abstract.DataModel {
    * Get the roll parts used to create the damage rolls.
    * @param {Partial<DamageRollProcessConfiguration>} [config={}]  Existing damage configuration to merge into this one.
    * @param {object} [options]                                     Damage configuration options.
+   * @param {DamageFormulaOptions} [options.formulaOptions]        Options to configure the formula.
    * @param {ActivityRollData} [options.rollData]                  Use pre-existing roll data.
    * @returns {DamageRollProcessConfiguration}
    */
-  getDamageConfig(config={}, { rollData }={}) {
+  getDamageConfig(config={}, { formulaOptions, rollData }={}) {
     if ( !this.damage?.parts ) return foundry.utils.mergeObject({ rolls: [] }, config);
 
     const rollConfig = foundry.utils.deepClone(config);
     rollData ??= this.getRollData();
     rollConfig.rolls = this.damage.parts
-      .map((d, index) => this._processDamagePart(d, rollConfig, rollData, index))
+      .map((d, index) => this._processDamagePart(d, rollConfig, rollData, index, formulaOptions))
       .filter(d => d.parts.length)
       .concat(config.rolls ?? []);
 
@@ -752,11 +754,12 @@ export default class BaseActivityData extends foundry.abstract.DataModel {
    * @param {Partial<DamageRollProcessConfiguration>} rollConfig  Roll configuration being built.
    * @param {ActivityRollData} rollData                           Roll data to populate with damage data.
    * @param {number} [index=0]                                    Index of the damage part.
+   * @param {DamageFormulaOptions} [options={}]                   Options to configure the formula.
    * @returns {DamageRollConfiguration}
    * @protected
    */
-  _processDamagePart(damage, rollConfig, rollData, index=0) {
-    const scaledFormula = damage.scaledFormula(rollConfig.scaling ?? rollData.scaling);
+  _processDamagePart(damage, rollConfig, rollData, index=0, options={}) {
+    const scaledFormula = damage.scaledFormula(rollConfig.scaling ?? rollData.scaling, options);
     const parts = scaledFormula ? [scaledFormula] : [];
     const data = { ...rollData };
 

module/data/activity/damage-data.mjs

@@ -63,8 +63,8 @@ export default class BaseDamageActivityData extends BaseActivityData {
   /* -------------------------------------------- */
 
   /** @inheritDoc */
-  getDamageConfig(config={}) {
-    const rollConfig = super.getDamageConfig(config);
+  getDamageConfig(config={}, options={}) {
+    const rollConfig = super.getDamageConfig(config, options);
 
     rollConfig.critical ??= {};
     rollConfig.critical.allow ??= this.damage.critical.allow;

module/data/activity/heal-data.mjs

@@ -46,12 +46,14 @@ export default class BaseHealActivityData extends BaseActivityData {
   /* -------------------------------------------- */
 
   /** @override */
-  getDamageConfig(config={}) {
+  getDamageConfig(config={}, options={}) {
     if ( !this.healing.formula ) return foundry.utils.mergeObject({ rolls: [] }, config);
 
     const rollConfig = foundry.utils.mergeObject({ critical: { allow: false } }, config);
-    const rollData = this.getRollData();
-    rollConfig.rolls = [this._processDamagePart(this.healing, rollConfig, rollData)].concat(config.rolls ?? []);
+    const rollData = options.rollData ?? this.getRollData();
+    rollConfig.rolls = [
+      this._processDamagePart(this.healing, rollConfig, rollData, options.formulaOptions)
+    ].concat(config.rolls ?? []);
 
     return rollConfig;
   }

module/data/activity/save-data.mjs

@@ -138,8 +138,8 @@ export default class BaseSaveActivityData extends BaseActivityData {
   /* -------------------------------------------- */
 
   /** @inheritDoc */
-  getDamageConfig(config={}) {
-    const rollConfig = super.getDamageConfig(config);
+  getDamageConfig(config={}, options={}) {
+    const rollConfig = super.getDamageConfig(config, options);
 
     rollConfig.critical ??= {};
     rollConfig.critical.allow ??= false;

module/data/shared/_types.mjs

@@ -32,12 +32,19 @@
  * @property {object} custom
  * @property {boolean} custom.enabled  Should the custom formula be used?
  * @property {string} custom.formula   Custom damage formula.
+ * @property {Set<string>} modifiers   Modifiers to apply to damage roll.
  * @property {object} scaling
  * @property {string} scaling.mode     How the damage scales in relation with levels.
  * @property {number} scaling.number   Number of dice to add per scaling level.
  * @property {string} scaling.formula  Arbitrary scaling formula which will be multiplied by scaling increase.
  */
 
+/**
+ * @typedef DamageFormulaOptions
+ * @property {Set<string>|false} modifiers  Additional modifiers to apply to the formula, if possible.
+ *                                          A `false` value will remove modifiers provided by damage data.
+ */
+
 /**
  * @typedef DurationData
  * @property {string} value             Scalar value for the activity's duration.

module/data/shared/damage-field.mjs

@@ -3,6 +3,10 @@ import FormulaField from "../fields/formula-field.mjs";
 
 const { BooleanField, EmbeddedDataField, NumberField, SchemaField, SetField, StringField } = foundry.data.fields;
 
+/**
+ * @import { DamageFormulaOptions } from "./_types.mjs";
+ */
+
 /**
  * Field for storing damage data.
  */
@@ -34,6 +38,7 @@ export class DamageData extends foundry.abstract.DataModel {
         enabled: new BooleanField(),
         formula: new FormulaField()
       }),
+      modifiers: new SetField(new StringField()),
       scaling: new SchemaField({
         mode: new StringField(),
         number: new NumberField({ initial: 1, min: 0, integer: true }),
@@ -51,7 +56,7 @@ export class DamageData extends foundry.abstract.DataModel {
    * @type {string}
    */
   get formula() {
-    if ( this.custom.enabled ) return this.custom.formula ?? "";
+    if ( this.custom.enabled ) return this._manualFormula();
     return this._automaticFormula();
   }
 
@@ -60,44 +65,65 @@ export class DamageData extends foundry.abstract.DataModel {
   /* -------------------------------------------- */
 
   /**
-   * Produce the auto-generated formula from the `number`, `denomination`, and `bonus`.
-   * @param {number} [increase=0]  Amount to increase the die count.
+   * Produce the auto-generated formula from the `number`, `denomination`, `modifiers`, and `bonus`.
+   * @param {number} [increase=0]                Amount to increase the die count.
+   * @param {DamageFormulaOptions} [options={}]  Options to configure the formula.
    * @returns {string}
    * @protected
    */
-  _automaticFormula(increase=0) {
+  _automaticFormula(increase=0, { modifiers }={}) {
     let formula;
     const number = (this.number ?? 0) + increase;
-    if ( number && this.denomination ) formula = `${number}d${this.denomination}`;
+    if ( number && this.denomination ) {
+      formula = `${number}d${this.denomination}${modifiers !== false
+        ? Array.from(this.modifiers).concat(...(modifiers ?? [])).join("") : ""}`;
+    }
     if ( this.bonus ) formula = formula ? `${formula} + ${this.bonus}` : this.bonus;
     return formula ?? "";
   }
 
   /* -------------------------------------------- */
 
+  /**
+   * Produce the manual formula from the `custom.formula` and `modifiers` (if possible).
+   * @param {DamageFormulaOptions} [options={}]  Options to configure the formula.
+   * @returns {string}
+   * @protected
+   */
+  _manualFormula({ modifiers }={}) {
+    if ( !this.custom.formula ) return "";
+    if ( modifiers === false ) return this.custom.formula;
+    modifiers = Array.from(this.modifiers).concat(...(modifiers ?? [])).join("");
+    return this.custom.formula.replace(/(?:\d|\))?d(?:\d+\w*|\([^)]+\)\d*\w*)/, `$&${modifiers}`);
+  }
+
+  /* -------------------------------------------- */
+
   /**
    * Scale the damage by a number of steps using its configured scaling configuration.
-   * @param {number|Scaling} increase  Number of steps above base damage to scaling.
+   * @param {number|Scaling} increase            Number of steps above base damage to scaling.
+   * @param {DamageFormulaOptions} [options={}]  Options to configure the formula.
    * @returns {string}
    */
-  scaledFormula(increase) {
+  scaledFormula(increase, options={}) {
     if ( increase instanceof Scaling ) increase = increase.increase;
 
     switch ( this.scaling.mode ) {
       case "whole": break;
       case "half": increase = Math.floor(increase * .5); break;
       default: increase = 0; break;
     }
-    if ( !increase ) return this.formula;
+    if ( !increase ) return this.custom.enabled
+      ? this._manualFormula(options) : this._automaticFormula(0, options);
     let formula;
 
     // If dice count scaling, increase the count on the first die rolled
     const dieIncrease = (this.scaling.number ?? 0) * increase;
     if ( this.custom.enabled ) {
-      formula = this.custom.formula;
+      formula = this._manualFormula(options);
       formula = formula.replace(/^(\d)+d/, (match, number) => `${Number(number) + dieIncrease}d`);
     } else {
-      formula = this._automaticFormula(dieIncrease);
+      formula = this._automaticFormula(dieIncrease, options);
     }
 
     // If custom scaling included, modify to match increase and append for formula

templates/activity/parts/damage-part.hbs

@@ -34,10 +34,14 @@
     {{/unless}}
 </div>
 
+<div class="field-group">
+    {{ formField fields.modifiers name=(concat prefix "modifiers") value=source.modifiers label="DND5E.RollModifiers"
+                 localize=true classes="label-top multi-select" rootId=@root.partId }}
+</div>
+
 <div class="field-group">
     {{ formField fields.types name=(concat prefix "types") value=source.types options=typeOptions hint=false
-                 label="DND5E.Type" localize=true classes="label-top multi-select"
-                 rootId=@root.partId }}
+                 label="DND5E.Type" localize=true classes="label-top multi-select" rootId=@root.partId }}
 </div>
 
 {{#if canScale}}