From b06396c141b2028850a977488cee9734e8a63595 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Oliver=20R=C3=BCmpelein?= <oli_r@fg4f.de>
Date: Fri, 15 Jan 2021 20:46:26 +0100
Subject: [PATCH] Review comments:

 - Error prefix on localization key
 - Different name for Roll dialog title
 - Remove obsolete todos
 - Add some defaults to make args optional
 - Change return types of promises and term generators
---
 src/lang/de.json                  |  4 ++--
 src/lang/en.json                  |  2 +-
 src/module/rolls/check-factory.ts | 21 +++++++++++----------
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/src/lang/de.json b/src/lang/de.json
index cfccf49b..39a139ca 100644
--- a/src/lang/de.json
+++ b/src/lang/de.json
@@ -158,10 +158,10 @@
     "DS4.UnitKilometersAbbr": "km",
     "DS4.UnitCustom": "individuell",
     "DS4.UnitCustomAbbr": " ",
-    "DS4.RollDialogDefaultTitle": "Probenwerte",
+    "DS4.RollDialogDefaultTitle": "Proben-Optionen",
     "DS4.RollDialogOkButton": "Ok",
     "DS4.RollDialogCancelButton": "Abbrechen",
-    "DS4.HtmlTypeError": "Typfehler: Erwartet wurde {exType}, tatsächlich  erhalten wurde {realType}",
+    "DS4.ErrorUnexpectedHtmlType": "Typfehler: Erwartet wurde {exType}, tatsächlich  erhalten wurde {realType}",
     "DS4.RollDialogTargetLabel": "Probenwert",
     "DS4.RollDialogModifierLabel": "SL-Modifikator",
     "DS4.RollDialogCoupLabel": "Immersieg bis",
diff --git a/src/lang/en.json b/src/lang/en.json
index 3c388407..20d06eef 100644
--- a/src/lang/en.json
+++ b/src/lang/en.json
@@ -161,7 +161,7 @@
     "DS4.RollDialogDefaultTitle": "Roll Options",
     "DS4.RollDialogOkButton": "Ok",
     "DS4.RollDialogCancelButton": "Cancel",
-    "DS4.HtmlTypeError": "Type Error: Expected {exType}, got {realType}",
+    "DS4.ErrorUnexpectedHtmlType": "Type Error: Expected {exType}, got {realType}",
     "DS4.RollDialogTargetLabel": "Check Target Number",
     "DS4.RollDialogModifierLabel": "Game Master Modifier",
     "DS4.RollDialogCoupLabel": "Coup to",
diff --git a/src/module/rolls/check-factory.ts b/src/module/rolls/check-factory.ts
index 6ec836fc..066202d6 100644
--- a/src/module/rolls/check-factory.ts
+++ b/src/module/rolls/check-factory.ts
@@ -1,5 +1,3 @@
-// TODO: Rename to something sane.
-
 import { DS4 } from "../config";
 
 /**
@@ -35,7 +33,7 @@ class CheckFactory {
 
     private checkOptions: DS4CheckFactoryOptions;
 
-    async execute(): Promise<void> {
+    async execute(): Promise<ChatMessage | any> {
         const rollCls: typeof Roll = CONFIG.Dice.rolls[0];
 
         const formula = [
@@ -52,15 +50,15 @@ class CheckFactory {
     }
 
     // Term generators
-    createTargetValueTerm(): string {
-        if (this.checkTargetValue != null) {
+    createTargetValueTerm(): string | null {
+        if (this.checkTargetValue !== null) {
             return "v" + (this.checkTargetValue + this.gmModifier);
         } else {
             return null;
         }
     }
 
-    createCritTerm(): string {
+    createCritTerm(): string | null {
         const minCritRequired = this.checkOptions.minCritFailure !== defaultCheckOptions.minCritFailure;
         const maxCritRequired = this.checkOptions.maxCritSuccess !== defaultCheckOptions.maxCritSuccess;
 
@@ -71,7 +69,7 @@ class CheckFactory {
         }
     }
 
-    createSlayingDiceTerm(): string {
+    createSlayingDiceTerm(): string | null {
         return this.checkOptions.useSlayingDice ? "x" : null;
     }
 }
@@ -81,7 +79,10 @@ class CheckFactory {
  * @param targetValue {number} The Check Target Number ("CTN")
  * @param options {Partial<DS4CheckFactoryOptions>} Options changing the behaviour of the roll and message.
  */
-export async function createCheckRoll(targetValue: number, options: Partial<DS4CheckFactoryOptions>): Promise<void> {
+export async function createCheckRoll(
+    targetValue: number,
+    options: Partial<DS4CheckFactoryOptions> = {},
+): Promise<ChatMessage | any> {
     // Ask for additional required data;
     const gmModifierData = await askGmModifier(targetValue, options);
 
@@ -111,7 +112,7 @@ export async function createCheckRoll(targetValue: number, options: Partial<DS4C
  */
 async function askGmModifier(
     targetValue: number,
-    options: Partial<DS4CheckFactoryOptions>,
+    options: Partial<DS4CheckFactoryOptions> = {},
     { template, title }: { template?: string; title?: string } = {},
 ): Promise<IntermediateGmModifierData> {
     // Render model interface and return value
@@ -142,7 +143,7 @@ async function askGmModifier(
                         callback: (html: HTMLElement | JQuery) => {
                             if (!("jquery" in html)) {
                                 throw new Error(
-                                    game.i18n.format("DS4.HtmlTypeError", {
+                                    game.i18n.format("DS4.ErrorUnexpectedHtmlType", {
                                         exType: "JQuery",
                                         realType: "HTMLElement",
                                     }),