Do some cleanup

src/module/actor/actor.ts

@@ -3,6 +3,9 @@ import { DS4Item } from "../item/item";
 import { ItemType } from "../item/item-data";
 import { DS4ActorData } from "./actor-data";
 
+/**
+ * The Actor class for DS4
+ */
 export class DS4Actor extends Actor<DS4ActorData, DS4Item> {
     /** @override */
     prepareDerivedData(): void {
@@ -45,7 +48,7 @@ export class DS4Actor extends Actor<DS4ActorData, DS4Item> {
 
     /**
      * Checks whether or not the given item type can be owned by the actor.
-     * @param itemType the item type to check
+     * @param itemType - The item type to check
      */
     canOwnItemType(itemType: ItemType): boolean {
         return this.ownableItemTypes.includes(itemType);

src/module/actor/sheets/actor-sheet.ts

@@ -4,7 +4,7 @@ import { DS4ItemData } from "../../item/item-data";
 import { DS4Actor } from "../actor";
 
 /**
- * Extend the basic ActorSheet with some very simple modifications
+ * The base Sheet class for all DS4 Actors
  */
 // TODO(types): Remove first generic parameter once https://github.com/League-of-Foundry-Developers/foundry-vtt-types/pull/273 is merged
 export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
@@ -43,13 +43,11 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
         return `${path}/${this.actor.data.type}-sheet.hbs`;
     }
 
-    /* -------------------------------------------- */
-
     /**
      * This method returns the data for the template of the actor sheet.
      * It explicitly adds the items of the object sorted by type in the
      * object itemsByType.
-     * @returns the data fed to the template of the actor sheet
+     * @returns The data fed to the template of the actor sheet
      */
     getData(): ActorSheet.Data<DS4Actor> {
         const data = {
@@ -62,8 +60,6 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
         return data;
     }
 
-    /* -------------------------------------------- */
-
     /** @override */
     activateListeners(html: JQuery): void {
         super.activateListeners(html);
@@ -94,14 +90,11 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
         html.find(".rollable").click(this._onRoll.bind(this));
     }
 
-    /* -------------------------------------------- */
-
     /**
      * Handle creating a new Owned Item for the actor using initial data defined in the HTML dataset
-     * @param event The originating click event
-     * @private
+     * @param event - The originating click event
      */
-    private _onItemCreate(event: JQuery.ClickEvent): Promise<DS4ItemData> {
+    protected _onItemCreate(event: JQuery.ClickEvent): Promise<DS4ItemData> {
         event.preventDefault();
         const header = event.currentTarget;
         // Get the type of item to create.
@@ -124,10 +117,9 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
      * Handle changes to properties of an Owned Item from within character sheet.
      * Can currently properly bind: see getValue().
      * Assumes the item property is given as the value of the HTML element property 'data-property'.
-     * @param ev The originating change event
-     * @private
+     * @param ev - The originating change event
      */
-    private _onItemChange(ev: JQuery.ChangeEvent): void {
+    protected _onItemChange(ev: JQuery.ChangeEvent): void {
         ev.preventDefault();
         console.log("Current target:", $(ev.currentTarget).get(0)["name"]);
         const el: HTMLFormElement = $(ev.currentTarget).get(0);
@@ -155,7 +147,7 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
      * - Checkbox: boolean
      * - Text input: string
      * - Number: number
-     * @param el the input element to collect the value of
+     * @param el - The input element to collect the value of
      */
     private getValue(el: HTMLFormElement): boolean | string | number {
         // One needs to differentiate between e.g. checkboxes (value="on") and select boxes etc.
@@ -206,10 +198,9 @@ export class DS4ActorSheet extends ActorSheet<unknown, DS4Actor> {
 
     /**
      * Handle clickable rolls.
-     * @param event The originating click event
-     * @private
+     * @param event - The originating click event
      */
-    private _onRoll(event: JQuery.ClickEvent): void {
+    protected _onRoll(event: JQuery.ClickEvent): void {
         event.preventDefault();
         const element = event.currentTarget;
         const dataset = element.dataset;

src/module/actor/sheets/character-sheet.ts

@@ -1,5 +1,8 @@
 import { DS4ActorSheet } from "./actor-sheet";
 
+/**
+ * The Sheet class for DS4 Character Actors
+ */
 export class DS4CharacterActorSheet extends DS4ActorSheet {
     /** @override */
     static get defaultOptions(): BaseEntitySheet.Options {

src/module/actor/sheets/creature-sheet.ts

@@ -1,5 +1,8 @@
 import { DS4ActorSheet } from "./actor-sheet";
 
+/**
+ * The Sheet class for DS4 Creature Actors
+ */
 export class DS4CreatureActorSheet extends DS4ActorSheet {
     /** @override */
     static get defaultOptions(): BaseEntitySheet.Options {

src/module/common/utils.ts

@@ -0,0 +1,37 @@
+/**
+ * Partition an array into two, following a predicate.
+ * @param input     - The Array to split.
+ * @param predicate - The predicate by which to split.
+ * @returns A tuple of two arrays, the first one containing all elements from `input` that match the predicate, the second one containing those that do not.
+ */
+export function partition<T>(input: Array<T>, predicate: (v: T) => boolean): [T[], T[]] {
+    return input.reduce(
+        (p: [Array<T>, Array<T>], cur: T) => {
+            if (predicate(cur)) {
+                p[0].push(cur);
+            } else {
+                p[1].push(cur);
+            }
+            return p;
+        },
+        [[], []],
+    );
+}
+
+/**
+ * Zips two Arrays to an array of pairs of elements with corresponding indices. Excessive elements are dropped.
+ * @param a1 - First array to zip.
+ * @param a2 - Second array to zip.
+ *
+ * @typeParam T - Type of elements contained in `a1`.
+ * @typeParam U - Type of elements contained in `a2`.
+ *
+ * @returns The array of pairs that had the same index in their source array.
+ */
+export 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]]);
+    } else {
+        return a2.map((e2, i) => [a1[i], e2]);
+    }
+}

src/module/config.ts

@@ -76,7 +76,7 @@ export const DS4 = {
         },
 
         /**
-         * Define the set of armor materials, used to determine if a characer may wear the armor without additional penalties
+         * Define the set of armor materials, used to determine if a character may wear the armor without additional penalties
          */
         armorMaterialTypes: {
             cloth: "DS4.ArmorMaterialTypeCloth",

src/module/ds4.ts

@@ -65,17 +65,13 @@ async function registerHandlebarsPartials() {
         "systems/ds4/templates/actor/partials/combat-values.hbs",
         "systems/ds4/templates/actor/partials/profile.hbs",
         "systems/ds4/templates/actor/partials/character-progression.hbs",
-        "systems/ds4/templates/actor/partials/special-creature-abilites-overview.hbs",
+        "systems/ds4/templates/actor/partials/special-creature-abilities-overview.hbs",
         "systems/ds4/templates/actor/partials/character-inventory.hbs",
         "systems/ds4/templates/actor/partials/creature-inventory.hbs",
     ];
     return loadTemplates(templatePaths);
 }
 
-/* -------------------------------------------- */
-/*  Foundry VTT Setup                           */
-/* -------------------------------------------- */
-
 /**
  * This function runs after game data has been requested and loaded from the servers, so entities exist
  */

src/module/item/item-sheet.ts

@@ -3,7 +3,7 @@ import { DS4Item } from "./item";
 import { isDS4ItemDataTypePhysical } from "./item-data";
 
 /**
- * Extend the basic ItemSheet with some very simple modifications
+ * The Sheet class for DS4 Items
  */
 // TODO(types): Remove first generic parameter once https://github.com/League-of-Foundry-Developers/foundry-vtt-types/pull/273 is merged
 export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
@@ -41,8 +41,6 @@ export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
         return `${path}/${this.item.data.type}-sheet.hbs`;
     }
 
-    /* -------------------------------------------- */
-
     /** @override */
     getData(): ItemSheet.Data<DS4Item> {
         const data = {
@@ -56,8 +54,6 @@ export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
         return data;
     }
 
-    /* -------------------------------------------- */
-
     /** @override */
     setPosition(options: Partial<Application.Position> = {}): Application.Position {
         const position = super.setPosition(options);
@@ -71,8 +67,6 @@ export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
         return position;
     }
 
-    /* -------------------------------------------- */
-
     /** @override */
     activateListeners(html: JQuery): void {
         super.activateListeners(html);
@@ -84,9 +78,9 @@ export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
 
     /**
      * Handle management of ActiveEffects.
-     * @param event The originating click event
+     * @param event - he originating click event
      */
-    private async _onManageActiveEffect(event: JQuery.ClickEvent): Promise<unknown> {
+    protected async _onManageActiveEffect(event: JQuery.ClickEvent): Promise<unknown> {
         event.preventDefault();
 
         if (this.item.isOwned) {
@@ -110,7 +104,7 @@ export class DS4ItemSheet extends ItemSheet<unknown, DS4Item> {
     /**
      * Create a new ActiveEffect for the item using default data.
      */
-    private async _createActiveEffect(): Promise<ActiveEffect.Data> {
+    protected async _createActiveEffect(): Promise<ActiveEffect.Data> {
         const label = `New Effect`;
 
         const createData = {

src/module/item/item.ts

@@ -1,21 +1,15 @@
 import { DS4ItemData } from "./item-data";
 
 /**
- * Extend the basic Item with some very simple modifications.
- * @extends {Item}
+ * The Item class for DS4
  */
 export class DS4Item extends Item<DS4ItemData> {
     /**
-     * Augment the basic Item data model with additional dynamic data.
+     * @override
      */
     prepareData(): void {
         super.prepareData();
         this.prepareDerivedData();
-
-        // Get the Item's data
-        // const itemData = this.data;
-        // const actorData = this.actor ? this.actor.data : {};
-        // const data = itemData.data;
     }
 
     prepareDerivedData(): void {

src/module/rolls/check-factory.ts

@@ -75,8 +75,8 @@ class CheckFactory {
 
 /**
  * Asks the user for all unknown/necessary information and passes them on to perform a roll.
- * @param targetValue   The Check Target Number ("CTN")
- * @param options       Options changing the behaviour of the roll and message.
+ * @param targetValue - The Check Target Number ("CTN")
+ * @param options     - Options changing the behavior of the roll and message.
  */
 export async function createCheckRoll(
     targetValue: number,
@@ -164,7 +164,7 @@ async function askGmModifier(
 
 /**
  * Extracts Dialog data from the returned DOM element.
- * @param formData  The filed dialog
+ * @param formData - The filed dialog
  */
 function parseDialogFormData(formData: HTMLFormElement): Partial<IntermediateGmModifierData> {
     return {

src/module/rolls/roll-executor.ts

@@ -4,9 +4,9 @@ import { calculateRollResult, isDiceSwapNecessary, isSlayingDiceRepetition, sepa
 
 /**
  * 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.
- * @param {Array<number>} dice optional, pass already thrown dice that are used instead of rolling new ones.
+ * @param checkTargetValue - the final CTN, including all static modifiers.
+ * @param rollOptions      - optional, final option override that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
+ * @param dice             - optional, pass already thrown dice that are used instead of rolling new ones.
  */
 export function ds4roll(
     checkTargetValue: number,
@@ -27,11 +27,11 @@ export function ds4roll(
  * This is not intended for direct usage. Use
  * {@link ds4roll | the function that is not bound to an amount of Dice} instead.
  *
- * @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 {Array<number>} dice optional, pass already thrown dice that are used instead of rolling new ones.
+ * @param checkTargetValue - The target value to check against.
+ * @param rollOptions      - Options that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
+ * @param dice             - optional, pass already thrown dice that are used instead of rolling new ones.
  *
- * @returns {RollResult} An object containing detailed information on the roll result.
+ * @returns An object containing detailed information on the roll result.
  */
 export function rollCheckSingleDie(
     checkTargetValue: number,
@@ -66,11 +66,11 @@ export function rollCheckSingleDie(
  * This is not intended for direct usage. Use
  * {@link ds4roll | the function that is not bound to an amount of Dice} instead.
  *
- * @param {number} targetValue- - 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 {Array<number>} dice - Optional array of dice values to consider instead of rolling new ones.
+ * @param targetValue - The target value to check against.
+ * @param rollOptions - Options that affect the checks outcome, e.g. different values for crits or whether slaying dice are used.
+ * @param dice        - Optional array of dice values to consider instead of rolling new ones.
  *
- * @returns {RollResult} An object containing detailed information on the roll result.
+ * @returns An object containing detailed information on the roll result.
  */
 export function rollCheckMultipleDice(
     targetValue: number,

src/module/rolls/roll-utils.ts

@@ -1,3 +1,4 @@
+import { partition, zip } from "../common/utils";
 import { RollOptions } from "./roll-data";
 
 /**
@@ -8,9 +9,9 @@ import { RollOptions } from "./roll-data";
  * @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.
+ * @param dice        - The dice values.
+ * @param usedOptions - Options that affect the check's behavior.
+ * @returns 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 descending by value.
  */
 export function separateCriticalHits(dice: Array<number>, usedOptions: RollOptions): CritsAndNonCrits {
     const [critSuccesses, otherRolls] = partition(dice, (v: number) => {
@@ -25,40 +26,19 @@ export function separateCriticalHits(dice: Array<number>, usedOptions: RollOptio
  */
 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:
- * ```
+ * ```ts
  * 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.
+ * @param critsAndNonCrits     - The dice values thrown. It is assumed that both critical successes and other rolls are sorted descending.
+ * @param remainingTargetValue - The target value for the last dice, that is the only one that can be less than 20.
+ * @returns Bool indicating whether a critical success has to be used as the last dice.
  */
 export function isDiceSwapNecessary(
     [critSuccesses, otherRolls]: CritsAndNonCrits,
@@ -81,7 +61,7 @@ export function isDiceSwapNecessary(
  *
  * @internal
  *
- * @param {RollOptions} opts the roll options to check against
+ * @param opts - The roll options to check against
  */
 export function isSlayingDiceRepetition(opts: RollOptions): boolean {
     return opts.useSlayingDice && opts.slayingDiceRepetition;
@@ -92,9 +72,9 @@ export function isSlayingDiceRepetition(opts: RollOptions): boolean {
  *
  * @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.
+ * @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.
  */
@@ -118,22 +98,3 @@ export function calculateRollResult(
         .map(([v]) => v)
         .reduce((a, b) => a + b);
 }
-
-// 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]]);
-    } else {
-        return a2.map((e2, i) => [a1[i], e2]);
-    }
-}

src/templates/actor/creature-sheet.hbs

@@ -64,7 +64,7 @@
         {{> systems/ds4/templates/actor/partials/creature-inventory.hbs}}
 
         {{!-- Special Creature Abilities Tab --}}
-        {{> systems/ds4/templates/actor/partials/special-creature-abilites-overview.hbs}}
+        {{> systems/ds4/templates/actor/partials/special-creature-abilities-overview.hbs}}
 
         {{!-- Spells Tab --}}
         {{> systems/ds4/templates/actor/partials/spells-overview.hbs}}