Roll crits and fumbles exactly once so the dice roll shown matches the table result. (#113)

* Roll crits and fumbles exactly once so the dice roll shown matches the
table result.
Ensure crits and fumbles are displayed correctly when using standard
cards with no tables setup.
Add localised string for 'Fumble'.
Closes #109.

* Fix formatting and tests.

Author: mooped

lang/en.json

@@ -248,6 +248,7 @@
   "DCC.FlagsTitle": "Configure Special Traits",
   "DCC.ForgeDocument": "Forge Document",
   "DCC.Formula": "Formula",
+  "DCC.Fumble": "Fumble",
   "DCC.FumbleDie": "Fumble Die",
   "DCC.Generic": "Generic",
   "DCC.GrantedAbilities": "Granted Abilities",

module/__tests__/actor.test.js

@@ -111,7 +111,7 @@ test('roll weapon attack', () => {
     user: 1,
     speaker: { alias: 'test character', _id: 1 },
     type: 'emote',
-    content: 'AttackRollEmote,weaponName:longsword,rollHTML:<a class="inline-roll inline-result" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,damageRollHTML:<a class="inline-roll inline-result damage-applyable" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" data-damage="undefined" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,crit:,fumble:[object Object]',
+    content: 'AttackRollEmote,weaponName:longsword,rollHTML:<a class="inline-roll inline-result" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%22dcc%22%3A%7B%22upperThreshold%22%3A20%7D%7D%7D%5D%7D" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,damageRollHTML:<a class="inline-roll inline-result damage-applyable" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" data-damage="undefined" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,crit:,fumble:[object Object]',
     sound: 'diceSound'
   })
 
@@ -134,7 +134,7 @@ test('roll weapon attack', () => {
     user: 1,
     speaker: { alias: 'test character', _id: 1 },
     type: 'emote',
-    content: 'AttackRollEmote,weaponName:longsword,rollHTML:<a class="inline-roll inline-result" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,damageRollHTML:<a class="inline-roll inline-result damage-applyable" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" data-damage="undefined" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,crit:,fumble:[object Object]',
+    content: 'AttackRollEmote,weaponName:longsword,rollHTML:<a class="inline-roll inline-result" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%22dcc%22%3A%7B%22upperThreshold%22%3A20%7D%7D%7D%5D%7D" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,damageRollHTML:<a class="inline-roll inline-result damage-applyable" data-roll="%7B%22dice%22%3A%5B%7B%22results%22%3A%5B10%5D%2C%22options%22%3A%7B%7D%7D%5D%7D" data-damage="undefined" title="undefined"><i class="fas fa-dice-d20"></i> undefined</a>,crit:,fumble:[object Object]',
     sound: 'diceSound'
   })
 

module/actor-sheets-dcc.js

@@ -1,3 +1,5 @@
+/* global game */
+
 /**
  * DCC specific character sheet overrides
  */

module/actor.js

@@ -424,6 +424,9 @@ class DCCActor extends Actor {
     const roll = new Roll(formula, { ab: attackBonus, critical: critRange })
     roll.roll()
     const d20RollResult = roll.dice[0].total
+    roll.dice[0].options.dcc = {
+      upperThreshold: critRange
+    }
 
     if (displayStandardCards) {
       roll.toMessage({
@@ -484,12 +487,8 @@ class DCCActor extends Actor {
     // Display standard cards in chat?
     const displayStandardCards = game.settings.get('dcc', 'useStandardDiceRoller')
 
-    // Roll the crit
-    const roll = new Roll(`${this.data.data.attributes.critical.die} + ${this.data.data.abilities.lck.mod}`)
-    roll.roll()
-    const rollData = escape(JSON.stringify(roll))
-    const rollTotal = roll.total
-    const rollHTML = `<a class="inline-roll inline-result" data-roll="${rollData}" data-damage="${rollTotal}" title="${Roll.cleanFormula(roll.terms || roll.formula)}"><i class="fas fa-dice-d20"></i> ${rollTotal}</a>`
+    // Roll object for the crit die
+    let roll = new Roll(`${this.data.data.attributes.critical.die} + ${this.data.data.abilities.lck.mod}`)
 
     // Lookup the crit table if available
     let critResult = null
@@ -507,13 +506,31 @@ class DCCActor extends Actor {
       }
     }
 
+    // Either roll the die or grab the roll from the table lookup
+    if (!critResult) {
+      roll.roll()
+    } else {
+      roll = critResult.roll
+    }
+
     if (!displayStandardCards) {
+      // Create the roll emote
+      const rollData = escape(JSON.stringify(roll))
+      const rollTotal = roll.total
+      const rollHTML = `<a class="inline-roll inline-result" data-roll="${rollData}" data-damage="${rollTotal}" title="${Roll.cleanFormula(roll.terms || roll.formula)}"><i class="fas fa-dice-d20"></i> ${rollTotal}</a>`
+
       // Display crit result or just a notification of the crit
       if (critResult) {
         return ` <br/><br/><span style='color:#ff0000; font-weight: bolder'>${game.i18n.localize('DCC.CriticalHit')}!</span> ${rollHTML}<br/>${critResult.results[0].text}`
       } else {
         return ` <br/><br/><span style='color:#ff0000; font-weight: bolder'>${game.i18n.localize('DCC.CriticalHit')}!</span> ${rollHTML}`
       }
+    } else if (!critResult) {
+      // Display the raw crit roll
+      await roll.toMessage({
+        speaker: ChatMessage.getSpeaker({ actor: this }),
+        flavor: `${game.i18n.localize('DCC.CriticalHit')}!`
+      })
     }
   }
 
@@ -531,12 +548,8 @@ class DCCActor extends Actor {
       fumbleDie = '1d4'
     }
 
-    // Roll the fumble
-    const roll = new Roll(`${fumbleDie} - ${this.data.data.abilities.lck.mod}`)
-    roll.roll()
-    const rollData = escape(JSON.stringify(roll))
-    const rollTotal = roll.total
-    const rollHTML = `<a class="inline-roll inline-result" data-roll="${rollData}" data-damage="${rollTotal}" title="${Roll.cleanFormula(roll.terms || roll.formula)}"><i class="fas fa-dice-d20"></i> ${rollTotal}</a>`
+    // Roll object for the fumble die
+    let roll = new Roll(`${fumbleDie} - ${this.data.data.abilities.lck.mod}`)
 
     // Lookup the fumble table if available
     let fumbleResult = null
@@ -557,13 +570,31 @@ class DCCActor extends Actor {
       }
     }
 
+    // Either roll the die or grab the roll from the table lookup
+    if (!fumbleResult) {
+      roll.roll()
+    } else {
+      roll = fumbleResult.roll
+    }
+
     if (!displayStandardCards) {
+      // Create the roll emote
+      const rollData = escape(JSON.stringify(roll))
+      const rollTotal = roll.total
+      const rollHTML = `<a class="inline-roll inline-result" data-roll="${rollData}" data-damage="${rollTotal}" title="${Roll.cleanFormula(roll.terms || roll.formula)}"><i class="fas fa-dice-d20"></i> ${rollTotal}</a>`
+
       // Display fumble result or just a notification of the fumble
       if (fumbleResult) {
-        return ` <br/><br/><span style='color:red; font-weight: bolder'>Fumble!</span> ${rollHTML}<br/>${fumbleResult.results[0].text}`
+        return ` <br/><br/><span style='color:red; font-weight: bolder'>${game.i18n.localize('DCC.Fumble')}!</span> ${rollHTML}<br/>${fumbleResult.results[0].text}`
       } else {
-        return ` <br/><br/><span style='color:red; font-weight: bolder'>Fumble!</span> ${rollHTML}`
+        return ` <br/><br/><span style='color:red; font-weight: bolder'>${game.i18n.localize('DCC.Fumble')}!</span> ${rollHTML}`
       }
+    } else if (!fumbleResult) {
+      // Display the raw fumble roll
+      await roll.toMessage({
+        speaker: ChatMessage.getSpeaker({ actor: this }),
+        flavor: `${game.i18n.localize('DCC.Fumble')}!`
+      })
     }
   }
 

module/item.js

@@ -17,9 +17,9 @@ class DCCItem extends Item {
 
       // Set default inherit crit range for legacy items
       if (this.data.data.config.inheritCritRange === undefined) {
-        this.data.data.config.inheritCritRange == true
+        this.data.data.config.inheritCritRange = true
         this.update({
-          "data.config.inheritCritRange": true
+          'data.config.inheritCritRange': true
         })
       }
 
@@ -30,11 +30,11 @@ class DCCItem extends Item {
         // If not inheriting crit range make sure there is a value (for legacy items)
         if (this.data.data.critRange === null || this.data.data.critRange === undefined) {
           this.update({
-            "data.critRange": 20
+            'data.critRange': 20
           })
         }
       }
-        
+
       // Spells can inherit the owner's spell check
       if (this.data.data.config.inheritSpellCheck) {
         this.data.data.spellCheck.value = this.actor.data.data.class.spellCheck

module/migrations.js

@@ -110,7 +110,7 @@ export const migrateActorData = function (actor) {
     // Add useDisapprovalRange to cleric skills
     'data.skills.divineAid.useDisapprovalRange': true,
     'data.skills.turnUnholy.useDisapprovalRange': true,
-    'data.skills.layOnHands.useDisapprovalRange': true,
+    'data.skills.layOnHands.useDisapprovalRange': true
   }
 
   // Migrate Owned Items