Implement swapping edge case, restructure sources.

spec/support/ds4rolls/executor.spec.ts

@@ -1,13 +1,8 @@
-import {
+import { rollCheckMultipleDice, rollCheckSingleDie } from "../../../src/module/rolls/roll-executor";
-    rollCheckMultipleDice,
+import { RollProvider } from "../../../src/module/rolls/roll-provider";
-    rollCheckSingleDie,
-    RollOptions,
-    RollResult,
-    RollResultStatus,
-} from "../../src/module/rolls/roll-executor";
-import { RollProvider } from "../../src/module/rolls/roll-provider";
 
 import "jasmine";
+import { RollResult, RollResultStatus } from "../../../src/module/rolls/roll-data";
 
 function mockSingleThrow(value: number): RollProvider {
     const rollProvider = jasmine.createSpyObj("rollProvider", ["getNextRoll"]);
@@ -67,7 +62,7 @@ describe("DS4 Rolls with one die and slaying dice, first throw.", () => {
     it("Should do a crit success on `1`", () => {
         const rollProvider = mockSingleThrow(1);
 
-        expect(rollCheckSingleDie(4, { useSlayingDice: true } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { useSlayingDice: true }, rollProvider)).toEqual(
             new RollResult(4, RollResultStatus.CRITICAL_SUCCESS, [1]),
         );
     });
@@ -75,7 +70,7 @@ describe("DS4 Rolls with one die and slaying dice, first throw.", () => {
     it("Should do a crit fail on `20`", () => {
         const rollProvider = mockSingleThrow(20);
 
-        expect(rollCheckSingleDie(4, { useSlayingDice: true } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { useSlayingDice: true }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [20]),
         );
     });
@@ -85,25 +80,25 @@ describe("DS4 Rolls with one die and slaying dice, followup throw.", () => {
     it("Should do a crit success on `1`", () => {
         const rollProvider = mockSingleThrow(1);
 
-        expect(
+        expect(rollCheckSingleDie(4, { useSlayingDice: true, slayingDiceRepetition: true }, rollProvider)).toEqual(
-            rollCheckSingleDie(4, { useSlayingDice: true, slayingDiceRepetition: true } as RollOptions, rollProvider),
+            new RollResult(4, RollResultStatus.CRITICAL_SUCCESS, [1]),
-        ).toEqual(new RollResult(4, RollResultStatus.CRITICAL_SUCCESS, [1]));
+        );
     });
 
     it("Should do a regular fail on `20`", () => {
         const rollProvider = mockSingleThrow(20);
 
-        expect(
+        expect(rollCheckSingleDie(4, { useSlayingDice: true, slayingDiceRepetition: true }, rollProvider)).toEqual(
-            rollCheckSingleDie(4, { useSlayingDice: true, slayingDiceRepetition: true } as RollOptions, rollProvider),
+            new RollResult(0, RollResultStatus.FAILURE, [20]),
-        ).toEqual(new RollResult(0, RollResultStatus.FAILURE, [20]));
+        );
     });
 
     it("Should do a regular success on `20` with a test value of 20", () => {
         const rollProvider = mockSingleThrow(20);
 
-        expect(
+        expect(rollCheckSingleDie(20, { useSlayingDice: true, slayingDiceRepetition: true }, rollProvider)).toEqual(
-            rollCheckSingleDie(20, { useSlayingDice: true, slayingDiceRepetition: true } as RollOptions, rollProvider),
+            new RollResult(20, RollResultStatus.SUCCESS, [20]),
-        ).toEqual(new RollResult(20, RollResultStatus.SUCCESS, [20]));
+        );
     });
 });
 
@@ -111,7 +106,7 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a crit success on `1`.", () => {
         const rollProvider = mockSingleThrow(1);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(4, RollResultStatus.CRITICAL_SUCCESS, [1]),
         );
     });
@@ -119,7 +114,7 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a crit success on `maxCritSucc`.", () => {
         const rollProvider = mockSingleThrow(2);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(4, RollResultStatus.CRITICAL_SUCCESS, [2]),
         );
     });
@@ -127,7 +122,7 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a success on lower edge case `3`.", () => {
         const rollProvider = mockSingleThrow(3);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(3, RollResultStatus.SUCCESS, [3]),
         );
     });
@@ -135,7 +130,7 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a success on upper edge case `18`.", () => {
         const rollProvider = mockSingleThrow(18);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.FAILURE, [18]),
         );
     });
@@ -143,7 +138,7 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a crit fail on `minCritFail`.", () => {
         const rollProvider = mockSingleThrow(19);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [19]),
         );
     });
@@ -151,13 +146,13 @@ describe("DS4 Rolls with one die and crit roll modifications.", () => {
     it("Should do a crit fail on `20`", () => {
         const rollProvider = mockSingleThrow(20);
 
-        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckSingleDie(4, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [20]),
         );
     });
 });
 
-describe("DS4 Rools with multiple dice and no modifiers.", () => {
+describe("DS4 Rolls with multiple dice and no modifiers.", () => {
     it("Should do a crit fail on `20` for first roll.", () => {
         const rollProvider = mockMultipleThrows([20, 15, 6]);
 
@@ -186,7 +181,7 @@ describe("DS4 Rools with multiple dice and no modifiers.", () => {
         const rollProvider = mockMultipleThrows([15, 15, 1]);
 
         expect(rollCheckMultipleDice(48, {}, rollProvider)).toEqual(
-            new RollResult(35, RollResultStatus.SUCCESS, [1, 15, 15]),
+            new RollResult(38, RollResultStatus.SUCCESS, [15, 15, 1]),
         );
     });
 
@@ -194,7 +189,7 @@ describe("DS4 Rools with multiple dice and no modifiers.", () => {
         const rollProvider = mockMultipleThrows([15, 1, 20]);
 
         expect(rollCheckMultipleDice(48, {}, rollProvider)).toEqual(
-            new RollResult(40, RollResultStatus.SUCCESS, [1, 20, 15]),
+            new RollResult(43, RollResultStatus.SUCCESS, [20, 15, 1]),
         );
     });
 
@@ -213,13 +208,37 @@ describe("DS4 Rools with multiple dice and no modifiers.", () => {
             new RollResult(35, RollResultStatus.SUCCESS, [20, 15, 8]),
         );
     });
+
+    it("Should maximize on 'lowest dice higher than last test and crit success thrown'-Edge case, no change required.", () => {
+        const rollProvider = mockMultipleThrows([15, 1, 8]);
+
+        expect(rollCheckMultipleDice(46, {}, rollProvider)).toEqual(
+            new RollResult(35, RollResultStatus.SUCCESS, [1, 15, 8]),
+        );
     });
 
-describe("DS4 Rools with multiple dice and min/max modifiers.", () => {
+    it("Should maximize on 2-dice 'lowest dice higher than last test and crit success thrown'-Edge case, no change required.", () => {
+        const rollProvider = mockMultipleThrows([1, 8]);
+
+        expect(rollCheckMultipleDice(24, {}, rollProvider)).toEqual(
+            new RollResult(20, RollResultStatus.SUCCESS, [1, 8]),
+        );
+    });
+
+    it("Should maximize on 2-dice 'lowest dice higher than last test and crit success thrown'-Edge case,  change required.", () => {
+        const rollProvider = mockMultipleThrows([1, 19]);
+
+        expect(rollCheckMultipleDice(38, {}, rollProvider)).toEqual(
+            new RollResult(37, RollResultStatus.SUCCESS, [19, 1]),
+        );
+    });
+});
+
+describe("DS4 Rolls with multiple dice and min/max modifiers.", () => {
     it("Should do a crit fail on `19` for first roll.", () => {
         const rollProvider = mockMultipleThrows([19, 15, 6]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [19, 15, 6]),
         );
     });
@@ -227,7 +246,7 @@ describe("DS4 Rools with multiple dice and min/max modifiers.", () => {
     it("Should succeed with all rolls crit successes (1 and 2).", () => {
         const rollProvider = mockMultipleThrows([2, 1, 2]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(48, RollResultStatus.SUCCESS, [2, 2, 1]),
         );
     });
@@ -235,7 +254,7 @@ describe("DS4 Rools with multiple dice and min/max modifiers.", () => {
     it("Should succeed with the last roll not being suficient.", () => {
         const rollProvider = mockMultipleThrows([15, 15, 15]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(30, RollResultStatus.SUCCESS, [15, 15, 15]),
         );
     });
@@ -243,44 +262,65 @@ describe("DS4 Rools with multiple dice and min/max modifiers.", () => {
     it("Should succeed with the last roll a crit success `2`.", () => {
         const rollProvider = mockMultipleThrows([15, 15, 2]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
-            new RollResult(35, RollResultStatus.SUCCESS, [2, 15, 15]),
+            new RollResult(38, RollResultStatus.SUCCESS, [15, 15, 2]),
         );
     });
 
     it("Should succeed with the last roll being `20` and one crit success '2'.", () => {
         const rollProvider = mockMultipleThrows([15, 2, 20]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
-            new RollResult(40, RollResultStatus.SUCCESS, [2, 20, 15]),
+            new RollResult(43, RollResultStatus.SUCCESS, [20, 15, 2]),
         );
     });
 
     it("Should succeed with the last roll being `19` and one crit success '2'.", () => {
         const rollProvider = mockMultipleThrows([15, 2, 19]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2, minCritFail: 19 }, rollProvider)).toEqual(
-            new RollResult(39, RollResultStatus.SUCCESS, [2, 19, 15]),
+            new RollResult(42, RollResultStatus.SUCCESS, [19, 15, 2]),
         );
     });
 });
 
-describe("DS4 Rools with multiple dice and fail modifiers.", () => {
+describe("DS4 Rolls with multiple dice and fail modifiers.", () => {
     it("Should do a crit fail on `19` for first roll.", () => {
         const rollProvider = mockMultipleThrows([19, 15, 6]);
 
-        expect(rollCheckMultipleDice(48, { minCritFail: 19 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { minCritFail: 19 }, rollProvider)).toEqual(
             new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [19, 15, 6]),
         );
     });
 });
 
-describe("DS4 Rools with multiple dice and success modifiers.", () => {
+describe("DS4 Rolls with multiple dice and success modifiers.", () => {
     it("Should succeed with all rolls crit successes (1 and 2).", () => {
         const rollProvider = mockMultipleThrows([2, 1, 2]);
 
-        expect(rollCheckMultipleDice(48, { maxCritSucc: 2 } as RollOptions, rollProvider)).toEqual(
+        expect(rollCheckMultipleDice(48, { maxCritSucc: 2 }, rollProvider)).toEqual(
             new RollResult(48, RollResultStatus.SUCCESS, [2, 2, 1]),
         );
     });
 });
+
+describe("DS4 Rolls with multiple and slaying dice, first throw", () => {
+    it("Should fail with the first roll being a `20`", () => {
+        const rollProvider = mockMultipleThrows([20, 2, 19]);
+
+        expect(rollCheckMultipleDice(48, {}, rollProvider)).toEqual(
+            new RollResult(0, RollResultStatus.CRITICAL_FAILURE, [20, 2, 19]),
+        );
+    });
+});
+
+// TODO: Implement & reactivate
+xdescribe("DS4 Rolls with multiple and slaying dice, recurrent throw", () => {
+    it("Should regularly succeed with the first roll being a `20`", () => {
+        const rollProvider = mockMultipleThrows([20, 2, 19]);
+
+        expect(rollCheckMultipleDice(48, {}, rollProvider)).toEqual(
+            new RollResult(40, RollResultStatus.CRITICAL_FAILURE, [20, 2, 19]),
+        );
+    });
+});

spec/support/ds4rolls/utils.spec.ts

@@ -0,0 +1,24 @@
+import "jasmine";
+import { isDiceSwapNecessary } from "../../../src/module/rolls/roll-utils";
+
+describe("Utility function testing if dice swap is necessery", () => {
+    it("Should not swap if all dice are crit successes.", () => {
+        expect(isDiceSwapNecessary([1, 1, 1], [], 9)).toBeFalse();
+    });
+
+    it("Should not swap if no die is crit success.", () => {
+        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();
+    });
+
+    it("Should not swap if result does not get any better", () => {
+        expect(isDiceSwapNecessary([1], [8], 4)).toBeFalse();
+    });
+
+    it("Should swap if result does get better", () => {
+        expect(isDiceSwapNecessary([1], [19], 18)).toBeTrue();
+    });
+});

src/module/rolls/roll-data.ts

@@ -0,0 +1,28 @@
+export interface RollOptions {
+    maxCritSucc: number;
+    minCritFail: number;
+    useSlayingDice: boolean;
+    slayingDiceRepetition: boolean;
+}
+
+export class DefaultRollOptions implements RollOptions {
+    public maxCritSucc = 1;
+    public minCritFail = 20;
+    public useSlayingDice = false;
+    public slayingDiceRepetition = false;
+
+    mergeWith(other: Partial<RollOptions>): RollOptions {
+        return { ...this, ...other } as RollOptions;
+    }
+}
+
+export class RollResult {
+    constructor(public value: number, public status: RollResultStatus, public dice: Array<number>) {}
+}
+
+export enum RollResultStatus {
+    FAILURE,
+    SUCCESS,
+    CRITICAL_FAILURE,
+    CRITICAL_SUCCESS,
+}

src/module/rolls/roll-executor.ts

@@ -1,4 +1,6 @@
+import { DefaultRollOptions, RollOptions, RollResult, RollResultStatus } from "./roll-data";
 import { DS4RollProvider, RollProvider } from "./roll-provider";
+import { isDiceSwapNecessary, isSlayingDiceRepetition } from "./roll-utils";
 
 export function ds4test(testValue: number, rollOptions: Partial<RollOptions> = {}): RollResult {
     const finalRollValue = testValue;
@@ -20,7 +22,7 @@ export function rollCheckSingleDie(
 
     if (roll <= usedOptions.maxCritSucc) {
         return new RollResult(testValue, RollResultStatus.CRITICAL_SUCCESS, dice);
-    } else if (roll >= usedOptions.minCritFail) {
+    } else if (roll >= usedOptions.minCritFail && !isSlayingDiceRepetition(usedOptions)) {
         return new RollResult(0, RollResultStatus.CRITICAL_FAILURE, dice);
     } else {
         if (roll <= testValue) {
@@ -44,7 +46,8 @@ export function rollCheckMultipleDice(
 
     const firstResult = dice[0];
 
-    if (firstResult >= usedOptions.minCritFail) {
+    // Slaying Dice require a different handling.
+    if (firstResult >= usedOptions.minCritFail && !isSlayingDiceRepetition(usedOptions)) {
         return new RollResult(0, RollResultStatus.CRITICAL_FAILURE, dice);
     }
 
@@ -61,13 +64,22 @@ export function rollCheckMultipleDice(
         .reduce(partitionCallback, [[], []])
         .map((a) => a.sort((r1, r2) => r2 - r1));
 
-    const sortedRollResults: Array<number> = critSuccesses.concat(otherRolls);
+    const swapLastWithCrit: boolean = isDiceSwapNecessary(critSuccesses, otherRolls, finalCheck);
+
+    let sortedRollResults: Array<number>;
+
+    if (swapLastWithCrit) {
+        const diceToMove = critSuccesses[0];
+        const remainingSuccesses = critSuccesses.slice(1);
+        sortedRollResults = remainingSuccesses.concat(otherRolls).concat([diceToMove]);
+    } else {
+        sortedRollResults = critSuccesses.concat(otherRolls);
+    }
 
     const evaluationResult = sortedRollResults
         .map((value, index) => {
             if (index == numberOfDice - 1) {
-                console.log(`Last dice: ${value}, checking against ${finalCheck}`);
+                if (value <= usedOptions.maxCritSucc) {
-                if (value == 1) {
                     return finalCheck;
                 } else if (value <= finalCheck) {
                     return value;
@@ -86,32 +98,3 @@ export function rollCheckMultipleDice(
 
     return new RollResult(evaluationResult, RollResultStatus.SUCCESS, sortedRollResults);
 }
-
-export interface RollOptions {
-    maxCritSucc: number;
-    minCritFail: number;
-    useSlayingDice: boolean;
-    slayingDiceRepetition: boolean;
-}
-
-class DefaultRollOptions implements RollOptions {
-    public maxCritSucc = 1;
-    public minCritFail = 20;
-    public useSlayingDice = false;
-    public slayingDiceRepetition = false;
-
-    mergeWith(other: Partial<RollOptions>): RollOptions {
-        return { ...this, ...other } as RollOptions;
-    }
-}
-
-export class RollResult {
-    constructor(public value: number, public status: RollResultStatus, public dice: Array<number>) {}
-}
-
-export enum RollResultStatus {
-    FAILURE,
-    SUCCESS,
-    CRITICAL_FAILURE,
-    CRITICAL_SUCCESS,
-}

src/module/rolls/roll-utils.ts

@@ -0,0 +1,22 @@
+import { RollOptions } from "./roll-data";
+
+export function isDiceSwapNecessary(
+    critSuccesses: Array<number>,
+    otherRolls: Array<number>,
+    finalRollValue: number,
+): boolean {
+    if (critSuccesses.length == 0 || otherRolls.length == 0) {
+        return false;
+    }
+    const amountOfOtherRolls = otherRolls.length;
+    const lastDice = otherRolls[amountOfOtherRolls - 1];
+    if (lastDice <= finalRollValue) {
+        return false;
+    }
+
+    return lastDice + finalRollValue > 20;
+}
+
+export function isSlayingDiceRepetition(opts: RollOptions): boolean {
+    return opts.useSlayingDice && opts.slayingDiceRepetition;
+}