Add docs, type clarifications.

spec/support/ds4rolls/utils.spec.ts

@@ -3,22 +3,22 @@ import { isDiceSwapNecessary } from "../../../src/module/rolls/roll-utils";
 
 describe("Utility function testing if dice swap is necessary", () => {
     it("Should not swap if all dice are crit successes.", () => {
-        expect(isDiceSwapNecessary([1, 1, 1], [], 9)).toBeFalse();
+        expect(isDiceSwapNecessary([[1, 1, 1], []], 9)).toBeFalse();
     });
 
     it("Should not swap if no die is crit success.", () => {
-        expect(isDiceSwapNecessary([], [2, 2, 2], 9)).toBeFalse();
+        expect(isDiceSwapNecessary([[], [2, 2, 2]], 9)).toBeFalse();
     });
 
     it("Should not swap if all dice are already in use", () => {
-        expect(isDiceSwapNecessary([1], [9, 8], 10)).toBeFalse();
+        expect(isDiceSwapNecessary([[1], [9, 8]], 10)).toBeFalse();
     });
 
     it("Should not swap if result does not get any better", () => {
-        expect(isDiceSwapNecessary([1], [8], 4)).toBeFalse();
+        expect(isDiceSwapNecessary([[1], [8]], 4)).toBeFalse();
     });
 
     it("Should swap if result does get better", () => {
-        expect(isDiceSwapNecessary([1], [19], 18)).toBeTrue();
+        expect(isDiceSwapNecessary([[1], [19]], 18)).toBeTrue();
     });
 });

src/module/rolls/roll-executor.ts

@@ -1,17 +1,36 @@
 import { DefaultRollOptions, RollOptions, RollResult, RollResultStatus } from "./roll-data";
 import { DS4RollProvider, RollProvider } from "./roll-provider";
-import { calculateRollResult, isDiceSwapNecessary, isSlayingDiceRepetition } from "./roll-utils";
+import { calculateRollResult, isDiceSwapNecessary, isSlayingDiceRepetition, separateCriticalHits } from "./roll-utils";
 
+/**
+ * Performs a roll against a check target number, e.g. for usage in battle, but not for herbs.
+ * @param {number} checkTargetValue the final CTN, including all static modifiers.
+ * @param {Partial<RollOptions>} rollOptions optional, final option override that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
+ */
 export function ds4roll(checkTargetValue: number, rollOptions: Partial<RollOptions> = {}): RollResult {
-    // TODO: Add CTN modifiers from options.
+    if (checkTargetValue <= 20) {
-    const finalTargetValue = checkTargetValue;
+        return rollCheckSingleDie(checkTargetValue, rollOptions);
-    if (finalTargetValue <= 20) {
-        return rollCheckSingleDie(finalTargetValue, rollOptions);
     } else {
-        return rollCheckMultipleDice(finalTargetValue, rollOptions);
+        return rollCheckMultipleDice(checkTargetValue, rollOptions);
     }
 }
 
+/**
+ * Performs a roll against a single die (CTN less than or equal 20).
+ *
+ * @internal
+ * This is not intended for direct usage. Use
+ * {@link ds4roll | the function that is not bound to an amount of Dice} instead.
+ *
+ * @remarks
+ * The `provider` is only exposed for testing.
+ *
+ * @param {number} checkTargetValue - The target value to check against.
+ * @param {RollOptions} rollOptions - Options that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
+ * @param {RollProvider} provider - Service providing the various, real dice throws.
+ *
+ * @returns {RollResult} An object containing detailed information on the roll result.
+ */
 export function rollCheckSingleDie(
     checkTargetValue: number,
     rollOptions: Partial<RollOptions>,
@@ -34,23 +53,22 @@ export function rollCheckSingleDie(
     }
 }
 
-function separateCriticalHits(dice: Array<number>, usedOptions: RollOptions): [Array<number>, Array<number>] {
+/**
-    const partitionCallback = (prev: [Array<number>, Array<number>], cur: number) => {
+ * Performs a roll against a multitude of die (CTN greater than 20).
-        if (cur <= usedOptions.maxCritSucc) {
+ *
-            prev[0].push(cur);
+ * @internal
-        } else {
+ * This is not intended for direct usage. Use
-            prev[1].push(cur);
+ * {@link ds4roll | the function that is not bound to an amount of Dice} instead.
-        }
+ *
-        return prev;
+ * @remarks
-    };
+ * The `provider` is only exposed for testing.
-
+ *
-    const [critSuccesses, otherRolls] = dice
+ * @param {number} checkTargetValue- - The target value to check against.
-        .reduce(partitionCallback, [[], []])
+ * @param {RollOptions} rollOptions - Options that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
-        .map((a) => a.sort((r1, r2) => r2 - r1));
+ * @param {RollProvider} provider - Service providing the various, real dice throws.
-
+ *
-    return [critSuccesses, otherRolls];
+ * @returns {RollResult} An object containing detailed information on the roll result.
-}
+ */
-
 export function rollCheckMultipleDice(
     targetValue: number,
     rollOptions: Partial<RollOptions>,
@@ -72,7 +90,7 @@ export function rollCheckMultipleDice(
 
     const [critSuccesses, otherRolls] = separateCriticalHits(dice, usedOptions);
 
-    const swapLastWithCrit: boolean = isDiceSwapNecessary(critSuccesses, otherRolls, remainderTargetValue);
+    const swapLastWithCrit: boolean = isDiceSwapNecessary([critSuccesses, otherRolls], remainderTargetValue);
 
     let sortedRollResults: Array<number>;
 

src/module/rolls/roll-provider.ts

@@ -1,4 +1,10 @@
-export class DS4RollProvider {
+/**
+ * Runtime-implementation of the {@link RollProvider}.
+ *
+ * @remarks
+ * Do not use for tests, it will inevitably fail because the `Roll` class is only provided from declarations, not as implementation!
+ */
+export class DS4RollProvider implements RollProvider {
     getNextRoll(): number {
         return new Roll("1d20").roll().total;
     }
@@ -10,7 +16,10 @@ export class DS4RollProvider {
     }
 }
 
+/**
+ * Provides methods to fetch one or multiple rolls.
+ */
 export interface RollProvider {
     getNextRoll(): number;
-    getNextRolls(number: number): Array<number>;
+    getNextRolls(amount: number): Array<number>;
 }

src/module/rolls/roll-utils.ts

@@ -1,8 +1,67 @@
 import { RollOptions } from "./roll-data";
 
+/**
+ * Separates critical hits ("Coups") from throws, that get counted with their regular value.
+ *
+ * @internal
+ *
+ * @private_remarks
+ * This uses an internal implementation of a `partition` method. Don't let typescript fool you, it will tell you that a partition method is available for Arrays, but that one's imported globally from foundry's declarations and not available during the test stage!
+ *
+ * @param {Array<number>} dice - The dice values.
+ * @param {RollOptions} usedOptions - Options that affect the check's behaviour.
+ * @returns {[Array<number>, Array<number>]} A tuple containing two arrays of dice values, the first one containing all critical hits, the second one containing all others. Both arrays are sorted descendingby value.
+ */
+export function separateCriticalHits(dice: Array<number>, usedOptions: RollOptions): CritsAndNonCrits {
+    const [critSuccesses, otherRolls] = partition(dice, (v: number) => {
+        return v <= usedOptions.maxCritSucc;
+    }).map((a) => a.sort((r1, r2) => r2 - r1));
+
+    return [critSuccesses, otherRolls];
+}
+/**
+ * Helper type to properly bind combinations of critical and non critical dice.
+ * @internal
+ */
+type CritsAndNonCrits = [Array<number>, Array<number>];
+
+/**
+ * Partition an array into two, following a predicate.
+ * @param {Array<T>} input The Array to split.
+ * @param {(T) => boolean} predicate The predicate by which to split.
+ * @returns A tuple of two arrays, the first one containing all elements from `input` that matched the predicate, the second one containing those that don't.
+ */
+// TODO: Move to generic utils method?
+function partition<T>(input: Array<T>, predicate: (v: T) => boolean) {
+    return input.reduce(
+        (p: [Array<T>, Array<T>], cur: T) => {
+            if (predicate(cur)) {
+                p[0].push(cur);
+            } else {
+                p[1].push(cur);
+            }
+            return p;
+        },
+        [[], []],
+    );
+}
+
+/**
+ * Calculates if a critical success should be moved to the last position in order to maximize the check's result.
+ *
+ * @example
+ * With regular dice rolling rules and a check target number of 31, the two dice 1 and 19 can get to a check result of 30.
+ * This method would be called as follows:
+ * ```
+ * isDiceSwapNecessary([[1], [19]], 11)
+ * ```
+ *
+ * @param {[Array<number>, Array<number>]} critsAndNonCrits the dice values thrown. It is assumed that both critical successes and other rolls are sorted descending.
+ * @param {number} remainingTargetValue the target value for the last dice, that is the only one that can be less than 20.
+ * @returns {boolean} Bool indicating whether a critical success has to be used as the last dice.
+ */
 export function isDiceSwapNecessary(
-    critSuccesses: Array<number>,
+    [critSuccesses, otherRolls]: CritsAndNonCrits,
-    otherRolls: Array<number>,
     remainingTargetValue: number,
 ): boolean {
     if (critSuccesses.length == 0 || otherRolls.length == 0) {
@@ -17,10 +76,28 @@ export function isDiceSwapNecessary(
     return lastDice + remainingTargetValue > 20;
 }
 
+/**
+ * Checks if the options indicate that the current check is emerging from a crit success on a roll with slaying dice.
+ *
+ * @internal
+ *
+ * @param {RollOptions} opts the roll options to check against
+ */
 export function isSlayingDiceRepetition(opts: RollOptions): boolean {
     return opts.useSlayingDice && opts.slayingDiceRepetition;
 }
 
+/**
+ * Calculate the check value of an array of dice, assuming the dice should be used in order of occurence.
+ *
+ * @internal
+ *
+ * @param assignedRollResults The dice values in the order of usage.
+ * @param remainderTargetValue Target value for the last dice (the only one differing from `20`).
+ * @param rollOptions Config object containing options that change the way dice results are handled.
+ *
+ * @returns {number} The total check value.
+ */
 export function calculateRollResult(
     assignedRollResults: Array<number>,
     remainderTargetValue: number,
@@ -43,6 +120,16 @@ export function calculateRollResult(
 }
 
 // TODO: Move to generic utils method?
+/**
+ * Zips two Arrays to an array of pairs of elements with corresponding indices. Excessive elements are dropped.
+ * @param {Array<T>} a1 First array to zip.
+ * @param {Array<U>} a2 Second array to zip.
+ *
+ * @typeParam T - Type of elements contained in `a1`.
+ * @typeParam U - Type of elements contained in `a2`.
+ *
+ * @returns {Array<[T,U]>} The array of pairs that had the same index in their source array.
+ */
 function zip<T, U>(a1: Array<T>, a2: Array<U>): Array<[T, U]> {
     if (a1.length <= a2.length) {
         return a1.map((e1, i) => [e1, a2[i]]);