From 115d6113de695c17935913e7cb95e6717f724ef5 Mon Sep 17 00:00:00 2001
From: Johannes Loher <johannes.loher@fg4f.de>
Date: Wed, 17 Mar 2021 20:00:36 +0100
Subject: [PATCH] Small check factory cleanup and use foundry form formating
 for roll dialog

---
 src/lang/de.json                    | 10 ++---
 src/lang/en.json                    | 10 ++---
 src/module/item/item.ts             |  8 ++--
 src/module/rolls/check-factory.ts   | 55 ++++++++++++++--------------
 src/templates/roll/roll-options.hbs | 57 ++++++++++++++++++++---------
 5 files changed, 81 insertions(+), 59 deletions(-)

diff --git a/src/lang/de.json b/src/lang/de.json
index 4bc08249..7b7ecad3 100644
--- a/src/lang/de.json
+++ b/src/lang/de.json
@@ -224,11 +224,11 @@
     "DS4.ErrorActorDoesNotHaveItem": "Der Aktor '{actor}' hat kein Item mit der ID '{id}'.",
     "DS4.ErrorUnexpectedError": "Es gab einen unerwarteten Fehler im Dungeonslayers 4 System. Für mehr Details schauen Sie bitte in die Konsole (F12).",
     "DS4.ErrorItemDoesNotHaveEffect": "Das Item '{item}' hat keinen Effekt mit der ID '{id}'.",
-    "DS4.RollDialogTargetLabel": "Probenwert",
-    "DS4.RollDialogModifierLabel": "SL-Modifikator",
-    "DS4.RollDialogCoupLabel": "Immersieg bis",
-    "DS4.RollDialogFumbleLabel": "Patzer ab",
-    "DS4.RollDialogVisibilityLabel": "Sichtbarkeit",
+    "DS4.RollDialogCheckTargetNumberLabel": "Probenwert",
+    "DS4.RollDialogGMModifierLabel": "SL-Modifikator",
+    "DS4.RollDialogMaximumCoupResultLabel": "Immersieg bis",
+    "DS4.RollDialogMinimumFumbleResultLabel": "Patzer ab",
+    "DS4.RollDialogRollModeLabel": "Sichtbarkeit",
     "DS4.TooltipBaseValue": "Basiswert",
     "DS4.TooltipModifier": "Modifikator",
     "DS4.TooltipEffects": "Effekte"
diff --git a/src/lang/en.json b/src/lang/en.json
index 2454030b..c1718b7c 100644
--- a/src/lang/en.json
+++ b/src/lang/en.json
@@ -224,11 +224,11 @@
     "DS4.ErrorActorDoesNotHaveItem": "The actor '{actor}' does not have any item with the id '{id}'.",
     "DS4.ErrorUnexpectedError": "There was an unexpected error in the Dungeonslayers 4 system. For more details, please take a look at the console (F12).",
     "DS4.ErrorItemDoesNotHaveEffect": "The item '{item}' does not have any effect with the id '{id}'.",
-    "DS4.RollDialogTargetLabel": "Check Target Number",
-    "DS4.RollDialogModifierLabel": "Game Master Modifier",
-    "DS4.RollDialogCoupLabel": "Coup to",
-    "DS4.RollDialogFumbleLabel": "Fumble from",
-    "DS4.RollDialogVisibilityLabel": "Visibility",
+    "DS4.RollDialogCheckTargetNumberLabel": "Check Target Number",
+    "DS4.RollDialogGMModifierLabel": "Game Master Modifier",
+    "DS4.RollDialogMaximumCoupResultLabel": "Coup to",
+    "DS4.RollDialogMinimumFumbleResultLabel": "Fumble from",
+    "DS4.RollDialogRollModeLabel": "Visibility",
     "DS4.TooltipBaseValue": "Base Value",
     "DS4.TooltipModifier": "Modifier",
     "DS4.TooltipEffects": "Effects"
diff --git a/src/module/item/item.ts b/src/module/item/item.ts
index 1374f9fd..0501fd1d 100644
--- a/src/module/item/item.ts
+++ b/src/module/item/item.ts
@@ -93,8 +93,8 @@ export class DS4Item extends Item<DS4ItemData> {
         const checkTargetValue = (ownerDataData.combatValues[combatValue].total as number) + weaponBonus;
         await createCheckRoll(checkTargetValue, {
             rollMode: game.settings.get("core", "rollMode"),
-            maxCritSuccess: ownerDataData.rolling.maximumCoupResult,
-            minCritFailure: ownerDataData.rolling.minimumFumbleResult,
+            maximumCoupResult: ownerDataData.rolling.maximumCoupResult,
+            minimumFumbleResult: ownerDataData.rolling.minimumFumbleResult,
         });
     }
 
@@ -135,8 +135,8 @@ export class DS4Item extends Item<DS4ItemData> {
 
         await createCheckRoll(checkTargetValue, {
             rollMode: game.settings.get("core", "rollMode"),
-            maxCritSuccess: ownerDataData.rolling.maximumCoupResult,
-            minCritFailure: ownerDataData.rolling.minimumFumbleResult,
+            maximumCoupResult: ownerDataData.rolling.maximumCoupResult,
+            minimumFumbleResult: ownerDataData.rolling.minimumFumbleResult,
         });
     }
 
diff --git a/src/module/rolls/check-factory.ts b/src/module/rolls/check-factory.ts
index 76f1a348..fa26b2ae 100644
--- a/src/module/rolls/check-factory.ts
+++ b/src/module/rolls/check-factory.ts
@@ -1,11 +1,9 @@
-import { DS4 } from "../config";
-
 /**
  * Provides default values for all arguments the `CheckFactory` expects.
  */
 class DefaultCheckOptions implements DS4CheckFactoryOptions {
-    readonly maxCritSuccess = 1;
-    readonly minCritFailure = 20;
+    readonly maximumCoupResult = 1;
+    readonly minimumFumbleResult = 20;
     readonly useSlayingDice = false;
     readonly rollMode: Const.DiceRollMode = "roll";
 
@@ -52,11 +50,13 @@ class CheckFactory {
     }
 
     createCritTerm(): string | null {
-        const minCritRequired = this.checkOptions.minCritFailure !== defaultCheckOptions.minCritFailure;
-        const maxCritRequired = this.checkOptions.maxCritSuccess !== defaultCheckOptions.maxCritSuccess;
+        const minCritRequired = this.checkOptions.minimumFumbleResult !== defaultCheckOptions.minimumFumbleResult;
+        const maxCritRequired = this.checkOptions.maximumCoupResult !== defaultCheckOptions.maximumCoupResult;
 
         if (minCritRequired || maxCritRequired) {
-            return "c" + (this.checkOptions.maxCritSuccess ?? "") + ":" + (this.checkOptions.minCritFailure ?? "");
+            return (
+                "c" + (this.checkOptions.maximumCoupResult ?? "") + ":" + (this.checkOptions.minimumFumbleResult ?? "")
+            );
         } else {
             return null;
         }
@@ -75,11 +75,11 @@ export async function createCheckRoll(
     // Ask for additional required data;
     const gmModifierData = await askGmModifier(targetValue, options);
 
-    const newTargetValue = gmModifierData.checkTargetValue ?? targetValue;
+    const newTargetValue = gmModifierData.checkTargetNumber ?? targetValue;
     const gmModifier = gmModifierData.gmModifier ?? 0;
     const newOptions: Partial<DS4CheckFactoryOptions> = {
-        maxCritSuccess: gmModifierData.maxCritSuccess ?? options.maxCritSuccess ?? undefined,
-        minCritFailure: gmModifierData.minCritFailure ?? options.minCritFailure ?? undefined,
+        maximumCoupResult: gmModifierData.maximumCoupResult ?? options.maximumCoupResult ?? undefined,
+        minimumFumbleResult: gmModifierData.minimumFumbleResult ?? options.minimumFumbleResult ?? undefined,
         useSlayingDice: gmModifierData.useSlayingDice ?? options.useSlayingDice ?? undefined,
         rollMode: gmModifierData.rollMode ?? options.rollMode ?? undefined,
     };
@@ -102,7 +102,7 @@ export async function createCheckRoll(
  * @returns The data given by the user.
  */
 async function askGmModifier(
-    targetValue: number,
+    checkTargetNumber: number,
     options: Partial<DS4CheckFactoryOptions> = {},
     { template, title }: { template?: string; title?: string } = {},
 ): Promise<Partial<IntermediateGmModifierData>> {
@@ -110,11 +110,10 @@ async function askGmModifier(
     const usedTemplate = template ?? "systems/ds4/templates/roll/roll-options.hbs";
     const usedTitle = title ?? game.i18n.localize("DS4.RollDialogDefaultTitle");
     const templateData = {
-        cssClass: "roll-option",
         title: usedTitle,
-        checkTargetValue: targetValue,
-        maxCritSuccess: options.maxCritSuccess ?? defaultCheckOptions.maxCritSuccess,
-        minCritFailure: options.minCritFailure ?? defaultCheckOptions.minCritFailure,
+        checkTargetNumber: checkTargetNumber,
+        maximumCoupResult: options.maximumCoupResult ?? defaultCheckOptions.maximumCoupResult,
+        minimumFumbleResult: options.minimumFumbleResult ?? defaultCheckOptions.minimumFumbleResult,
         rollMode: options.rollMode,
         rollModes: CONFIG.Dice.rollModes,
     };
@@ -168,11 +167,11 @@ async function askGmModifier(
  */
 function parseDialogFormData(formData: HTMLFormElement): Partial<IntermediateGmModifierData> {
     return {
-        checkTargetValue: parseInt(formData["ctv"]?.value),
-        gmModifier: parseInt(formData["gmmod"]?.value),
-        maxCritSuccess: parseInt(formData["maxcoup"]?.value),
-        minCritFailure: parseInt(formData["minfumble"]?.value),
-        rollMode: formData["visibility"]?.value,
+        checkTargetNumber: parseInt(formData["check-target-number"]?.value),
+        gmModifier: parseInt(formData["gm-modifier"]?.value),
+        maximumCoupResult: parseInt(formData["maximum-coup-result"]?.value),
+        minimumFumbleResult: parseInt(formData["minimum-fumble-result"]?.value),
+        rollMode: formData["roll-mode"]?.value,
     };
 }
 
@@ -189,10 +188,10 @@ interface GmModifierData {
  *
  * @deprecated
  * Quite a lot of this information is requested due to a lack of automation:
- *  - maxCritSuccess
- *  - minCritFailure
+ *  - maximumCoupResult
+ *  - minimumFumbleResult
  *  - useSlayingDice
- *  - checkTargetValue
+ *  - checkTargetNumber
  *
  * They will and should be removed once effects and data retrieval is in place.
  * If a "raw" roll dialog is necessary, create another pre-processing Dialog
@@ -200,10 +199,10 @@ interface GmModifierData {
  * This interface should then be replaced with the `GmModifierData`.
  */
 interface IntermediateGmModifierData extends GmModifierData {
-    checkTargetValue: number;
+    checkTargetNumber: number;
     gmModifier: number;
-    maxCritSuccess: number;
-    minCritFailure: number;
+    maximumCoupResult: number;
+    minimumFumbleResult: number;
     // TODO: In final version from system settings
     useSlayingDice: boolean;
     rollMode: Const.DiceRollMode;
@@ -213,8 +212,8 @@ interface IntermediateGmModifierData extends GmModifierData {
  * The minimum behavioral options that need to be passed to the factory.
  */
 export interface DS4CheckFactoryOptions {
-    maxCritSuccess: number;
-    minCritFailure: number;
+    maximumCoupResult: number;
+    minimumFumbleResult: number;
     useSlayingDice: boolean;
     rollMode: Const.DiceRollMode;
 }
diff --git a/src/templates/roll/roll-options.hbs b/src/templates/roll/roll-options.hbs
index 217c4cf3..76c840aa 100644
--- a/src/templates/roll/roll-options.hbs
+++ b/src/templates/roll/roll-options.hbs
@@ -1,18 +1,41 @@
-<form class="{{cssClass}} grid">
-    <label for="ctv">{{localize "DS4.RollDialogTargetLabel"}}</label>
-    <input id="ctv" data-type="Number" type="number" name="ctv" value="{{checkTargetValue}}" />
-    <label for="gmmod">{{localize "DS4.RollDialogModifierLabel"}}</label>
-    <input id="gmmod" data-type="Number" type="number" name="gmmod" value="0" autofocus />
-    <label for="maxcoup">{{localize "DS4.RollDialogCoupLabel"}}</label>
-    <input id="maxcoup" data-type="Number" type="number" name="maxcoup" value="{{maxCritSuccess}}" />
-    <label for="minfumble">{{localize "DS4.RollDialogFumbleLabel"}}</label>
-    <input id="minfumble" data-type="Number" type="number" name="minfumble" value="{{minCritFailure}}" />
-    <label for="visibility">{{localize "DS4.RollDialogVisibilityLabel"}}</label>
-    <select id="visibility" data-type="String">
-        {{#select rollMode}}
-        {{#each rollModes as |rollModeValue rollModeKey|}}
-        <option value="{{rollModeKey}}">{{localize rollModeValue}}</option>
-        {{/each}}
-        {{/select}}
-    </select>
+{{!--
+!-- Render a roll options dialog. It uses the default form classes of Foundry VTT.
+!-- @param checkTargetNumber: The preselected check target number.
+!-- @param maximumCoupResult: The preselected maximum coup result.
+!-- @param minimumFumbleResult: The preselected minimum fumble result.
+!-- @param rollMode: The preselected roll mode (= chat roll-mode).
+!-- @param rollModes: A map of all roll modes and their i18n keys.
+--}}
+<form class="ds4-roll-options">
+    <div class="form-group">
+        <label for="check-target-number">{{localize "DS4.RollDialogCheckTargetNumberLabel"}}</label>
+        <input id="check-target-number" data-type="Number" type="number" name="check-target-number"
+            value="{{checkTargetNumber}}" />
+    </div>
+    <div class="form-group">
+        <label for="gm-modifier">{{localize "DS4.RollDialogGMModifierLabel"}}</label>
+        <input id="gm-modifier" data-type="Number" type="number" name="gm-modifier" value="0" />
+    </div>
+    <div class="form-group">
+        <label for="maximum-coup-result">{{localize "DS4.RollDialogMaximumCoupResultLabel"}}</label>
+        <input id="maximum-coup-result" data-type="Number" type="number" name="maximum-coup-result"
+            value="{{maximumCoupResult}}" />
+    </div>
+    <div class="form-group">
+        <label for="minimum-fumble-result">{{localize "DS4.RollDialogMinimumFumbleResultLabel"}}</label>
+        <input id="minimum-fumble-result" data-type="Number" type="number" name="minimum-fumble-result"
+            value="{{minimumFumbleResult}}" />
+    </div>
+    <div class="form-group">
+        <label for="roll-mode">{{localize "DS4.RollDialogRollModeLabel"}}</label>
+        <div class="form-fields">
+            <select id="roll-mode" name="roll-mode" data-type="String">
+                {{#select rollMode}}
+                {{#each rollModes as |rollModeValue rollModeKey|}}
+                <option value="{{rollModeKey}}">{{localize rollModeValue}}</option>
+                {{/each}}
+                {{/select}}
+            </select>
+        </div>
+    </div>
 </form>