Merge pull request #288 from parnas/feature/default-values

cs and cf with default comparison. A way to add default values in the modifier class.

src/dice/StandardDice.js

@@ -1,13 +1,10 @@
 import { RequiredArgumentError } from '../exceptions/index.js';
 import { isNumeric, isSafeNumber } from '../utilities/math.js';
 import { generator } from '../utilities/NumberGenerator.js';
-import ComparePoint from '../ComparePoint.js';
-import ExplodeModifier from '../modifiers/ExplodeModifier.js';
 import HasDescription from '../traits/HasDescription.js';
 import Modifier from '../modifiers/Modifier.js';
 import RollResult from '../results/RollResult.js';
 import RollResults from '../results/RollResults.js';
-import ReRollModifier from '../modifiers/ReRollModifier.js';
 
 const modifiersSymbol = Symbol('modifiers');
 const qtySymbol = Symbol('qty');
@@ -131,18 +128,6 @@ class StandardDice extends HasDescription {
     }
 
     this[modifiersSymbol] = modifiers;
-
-    // loop through each modifier and ensure that those that require it have compare points
-    // @todo find a better way of defining compare point on modifiers that don't have them
-    /* eslint-disable no-param-reassign */
-    this[modifiersSymbol].forEach((modifier) => {
-      if ((modifier instanceof ExplodeModifier) && !modifier.comparePoint) {
-        modifier.comparePoint = new ComparePoint('=', this.max);
-      } else if ((modifier instanceof ReRollModifier) && !modifier.comparePoint) {
-        modifier.comparePoint = new ComparePoint('=', this.min);
-      }
-    });
-    /* eslint-enable */
   }
 
   /**

src/modifiers/ComparisonModifier.js

@@ -76,6 +76,36 @@ class ComparisonModifier extends Modifier {
     return `${this.comparePoint || ''}`;
   }
 
+  /* eslint-disable class-methods-use-this */
+  /**
+   * Empty default compare point definition
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {null}
+   */
+  defaultComparePoint(_context) {
+    return {};
+  }
+  /* eslint-enable class-methods-use-this */
+
+  /**
+   * Eases processing of simple "compare point only" defaults
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {object}
+   */
+  defaults(_context) {
+    const comparePointConfig = this.defaultComparePoint(_context);
+
+    if (typeof comparePointConfig === 'object' && comparePointConfig.length === 2) {
+      return { comparePoint: new ComparePoint(...comparePointConfig) };
+    }
+
+    return {};
+  }
+
   /**
    * Check whether value matches the compare point or not.
    *

src/modifiers/CriticalFailureModifier.js

@@ -49,6 +49,19 @@ class CriticalFailureModifier extends ComparisonModifier {
     return `cf${super.notation}`;
   }
 
+  /* eslint-disable class-methods-use-this */
+  /**
+   * The default compare point definition
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {array}
+   */
+  defaultComparePoint(_context) {
+    return ['=', _context.min];
+  }
+  /* eslint-enable class-methods-use-this */
+
   /**
    * Run the modifier on the results.
    *
@@ -58,6 +71,8 @@ class CriticalFailureModifier extends ComparisonModifier {
    * @returns {RollResults} The modified results
    */
   run(results, _context) {
+    super.run(results, _context);
+
     results.rolls
       .forEach((roll) => {
         // add the modifier flag

src/modifiers/CriticalSuccessModifier.js

@@ -49,6 +49,19 @@ class CriticalSuccessModifier extends ComparisonModifier {
     return `cs${super.notation}`;
   }
 
+  /* eslint-disable class-methods-use-this */
+  /**
+   * The default compare point definition
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {array}
+   */
+  defaultComparePoint(_context) {
+    return ['=', _context.max];
+  }
+  /* eslint-enable class-methods-use-this */
+
   /**
    * Runs the modifier on the rolls.
    *
@@ -58,6 +71,8 @@ class CriticalSuccessModifier extends ComparisonModifier {
    * @returns {RollResults}
    */
   run(results, _context) {
+    super.run(results, _context);
+
     // loop through each roll and see if it's a critical success
     results.rolls
       .forEach((roll) => {

src/modifiers/ExplodeModifier.js

@@ -74,6 +74,19 @@ class ExplodeModifier extends ComparisonModifier {
     return this[penetrateSymbol];
   }
 
+  /* eslint-disable class-methods-use-this */
+  /**
+   * The default compare point definition
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {array}
+   */
+  defaultComparePoint(_context) {
+    return ['=', _context.max];
+  }
+  /* eslint-enable class-methods-use-this */
+
   /**
    * Run the modifier on the results.
    *
@@ -83,6 +96,8 @@ class ExplodeModifier extends ComparisonModifier {
    * @returns {RollResults} The modified results
    */
   run(results, _context) {
+    super.run(results, _context);
+
     // ensure that the dice can explode without going into an infinite loop
     if (_context.min === _context.max) {
       throw new DieActionValueError(_context, 'explode');

src/modifiers/Modifier.js

@@ -50,8 +50,34 @@ class Modifier {
   get maxIterations() {
     return 1000;
   }
+
+  /**
+   * No default values present
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {object}
+   */
+  defaults(_context) {
+    return {};
+  }
   /* eslint-enable class-methods-use-this */
 
+  /**
+   * Processing default values definitions
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {void}
+   */
+  useDefaultsIfNeeded(_context) {
+    Object.entries(this.defaults(_context)).forEach(([field, value]) => {
+      if (typeof this[field] === 'undefined') {
+        this[field] = value;
+      }
+    });
+  }
+
   /* eslint-disable class-methods-use-this */
   /**
    * Run the modifier on the results.
@@ -62,6 +88,7 @@ class Modifier {
    * @returns {RollResults} The modified results
    */
   run(results, _context) {
+    this.useDefaultsIfNeeded(_context);
     return results;
   }
   /* eslint-enable class-methods-use-this */

src/modifiers/ReRollModifier.js

@@ -69,6 +69,19 @@ class ReRollModifier extends ComparisonModifier {
     this[onceSymbol] = !!value;
   }
 
+  /* eslint-disable class-methods-use-this */
+  /**
+   * The default compare point definition
+   *
+   * @param {StandardDice|RollGroup} _context The object that the modifier is attached to
+   *
+   * @returns {array}
+   */
+  defaultComparePoint(_context) {
+    return ['=', _context.min];
+  }
+  /* eslint-enable class-methods-use-this */
+
   /**
    * Run the modifier on the results.
    *
@@ -78,6 +91,8 @@ class ReRollModifier extends ComparisonModifier {
    * @returns {RollResults} The modified results
    */
   run(results, _context) {
+    super.run(results, _context);
+
     // ensure that the dice can explode without going into an infinite loop
     if (_context.min === _context.max) {
       throw new DieActionValueError(_context, 're-roll');

src/parser/grammars/grammar.pegjs

@@ -113,13 +113,13 @@ UniqueModifier
 
 // Critical success setting
 CriticalSuccessModifier
-  = "cs" comparePoint:ComparePoint {
+  = "cs" comparePoint:ComparePoint? {
     return new Modifiers.CriticalSuccessModifier(comparePoint);
   }
 
 // Critical failure setting
 CriticalFailureModifier
-  = "cf" comparePoint:ComparePoint {
+  = "cf" comparePoint:ComparePoint? {
     return new Modifiers.CriticalFailureModifier(comparePoint);
   }
 

tests/modifiers/ComparisonModifier.test.js

@@ -154,13 +154,40 @@ describe('ComparisonModifier', () => {
   });
 
   describe('Run', () => {
+    const results = new RollResults();
+    const die = new StandardDice(6, 2);
+    let mod;
+
+    beforeEach(() => {
+      mod = new ComparisonModifier();
+    });
+
     test('returns RollResults object', () => {
-      const results = new RollResults();
-      const die = new StandardDice(6, 2);
-      const mod = new ComparisonModifier(new ComparePoint('=', 4));
+      mod.comparePoint = new ComparePoint('=', 4);
 
       expect(mod.run(results, die)).toBe(results);
     });
+
+    describe('Defaults', () => {
+      const testOperator = '=';
+      const testValue = 6;
+
+      beforeEach(() => {
+        mod.defaultComparePoint = () => [testOperator, testValue];
+      });
+
+      test('uses default comparison modifier', () => {
+        mod.run(results, die);
+        expect(mod.comparePoint.operator).toEqual(testOperator);
+        expect(mod.comparePoint.value).toEqual(testValue);
+      });
+
+      test('does not set default comparison modifier if value was provided', () => {
+        mod.comparePoint = new ComparePoint('=', 4);
+
+        expect(mod.run(results, die)).toBe(results);
+      });
+    });
   });
 
   describe('Readonly properties', () => {

tests/modifiers/ExplodeModifier.test.js

@@ -224,12 +224,12 @@ describe('ExplodeModifier', () => {
       expect(mod.run(results, die)).toBe(results);
     });
 
-    test('does not explode without compare point', () => {
+    test('can explode with default compare point', () => {
       const modifiedResults = mod.run(results, die).rolls;
 
       // assert that all the rolls exist
       expect(modifiedResults).toBeInstanceOf(Array);
-      expect(modifiedResults).toHaveLength(6);
+      expect(modifiedResults).toHaveLength(8);
 
       expect(modifiedResults[0].initialValue).toBe(8);
       expect(modifiedResults[0].value).toBe(8);
@@ -253,7 +253,15 @@ describe('ExplodeModifier', () => {
 
       expect(modifiedResults[5].initialValue).toBe(10);
       expect(modifiedResults[5].value).toBe(10);
-      expect(modifiedResults[5].modifiers).toEqual(new Set());
+      expect(modifiedResults[5].modifiers).toEqual(new Set(['explode']));
+
+      expect(modifiedResults[6].initialValue).toBe(10);
+      expect(modifiedResults[6].value).toBe(10);
+      expect(modifiedResults[6].modifiers).toEqual(new Set(['explode']));
+
+      expect(modifiedResults[7].initialValue).toBe(2);
+      expect(modifiedResults[7].value).toBe(2);
+      expect(modifiedResults[7].modifiers).toEqual(new Set());
     });
 
     test('can explode with compare point `>=8`', () => {

tests/modifiers/Modifier.test.js

@@ -186,6 +186,28 @@ describe('Modifier', () => {
       test('returns RollResults object', () => {
         expect(mod.run(results, die)).toBe(results);
       });
+
+      describe('Defaults', () => {
+        const testKey = 'testKey';
+        const defaultValue = 'defaultValue';
+
+        beforeEach(() => {
+          mod.defaults = () => ({ testKey: defaultValue });
+        });
+
+        test('uses default if value wasn\'t provided', () => {
+          mod.run(results, die);
+          expect(mod[testKey]).toEqual(defaultValue);
+        });
+
+        test('does not set default if value was provided', () => {
+          const providedValue = 'providedValue';
+          mod[testKey] = providedValue;
+
+          mod.run(results, die);
+          expect(mod[testKey]).toEqual(providedValue);
+        });
+      });
     });
   });
 });

tests/modifiers/ReRollModifier.test.js

@@ -182,7 +182,7 @@ describe('ReRollModifier', () => {
       expect(mod.run(results, die)).toBe(results);
     });
 
-    test('does not re-roll without compare point', () => {
+    test('does re-roll with default compare point', () => {
       const modifiedResults = mod.run(results, die).rolls;
 
       expect(modifiedResults).toBeInstanceOf(Array);
@@ -201,8 +201,8 @@ describe('ReRollModifier', () => {
       expect(modifiedResults[2].modifiers).toEqual(new Set());
 
       expect(modifiedResults[3].initialValue).toBe(1);
-      expect(modifiedResults[3].value).toBe(1);
-      expect(modifiedResults[3].modifiers).toEqual(new Set());
+      expect(modifiedResults[3].value).toBe(10);
+      expect(modifiedResults[3].modifiers).toEqual(new Set(new Set(['re-roll'])));
 
       expect(modifiedResults[4].initialValue).toBe(6);
       expect(modifiedResults[4].value).toBe(6);