Merge pull request #39 from eps1lon/fix-zero-behavior

Remove skip_if_zero in favor of defaulting to it
eps1lon · Mar 20, 2018 · 5943eeb · 5943eeb
2 parents c264821 + 7f58385
commit 5943eeb
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,13 +9,17 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 - `locale-data` for Path Of Exile@3.2.0 ([#28](https://github.com/eps1lon/poe-i18n/pull/28))
 - `groupMods()` to generate a fitting translation for a collection of mods 
   (e.g. mods of a `correct_group`). ([#30](https://github.com/eps1lon/poe-i18n/pull/30))
-- `skip_if_zero` fallback for `formatStats()` which suppresses thrown errors
-  if the stat has a value that is equivilant to zero. ([#29](https://github.com/eps1lon/poe-i18n/pull/29))
 - `textToStats` (also available in `Format`) which finds every combination
   of stats that could've produced a given text. Check the API docs for more info. ([#24](https://github.com/eps1lon/poe-i18n/pull/24))
 
 ### Changed
-- Value ranges should now be displayed ordered. This caused [#32](https://github.com/eps1lon/poe-i18n/issues/32) and was fixed with [#37](https://github.com/eps1lon/poe-i18n/pull/37).
+- Value ranges should now be displayed ordered. 
+  This caused [#32](https://github.com/eps1lon/poe-i18n/issues/32) and 
+  was fixed with [#37](https://github.com/eps1lon/poe-i18n/pull/37).
+
+- Stats with zero or equivalent (range 0 - 0) are now ignored by default. 
+  This matches ingame behavior. ([#29](https://github.com/eps1lon/poe-i18n/pull/29)
+  and [#39](https://github.com/eps1lon/poe-i18n/pull/39))
 
 ### Fixes
 - Some translations had standard printf syntax which is not understood by

diff --git a/dist/index.d.ts b/dist/index.d.ts
@@ -154,7 +154,6 @@ declare module "format/stats" {
         throw = 0,
         id = 1,
         skip = 2,
-        skip_if_zero = 3,
     }
     const formatStats: (stats: Stat[], options?: Partial<Options>) => string[];
     export default formatStats;

diff --git a/dist/index.js b/dist/index.js
@@ -209,7 +209,8 @@ System.register("localize/formatters", ["types/StatValue"], function (exports_4,
                     return String(formatter(value[0]));
                 }
                 else {
-                    return "(" + formatter(value[0]) + " - " + formatter(value[1]) + ")";
+                    var _a = __read(valueOrder(value, formatter_id), 2), min = _a[0], max = _a[1];
+                    return "(" + formatter(min) + " - " + formatter(max) + ")";
                 }
             }
             else {
@@ -218,6 +219,24 @@ System.register("localize/formatters", ["types/StatValue"], function (exports_4,
         };
     }
     exports_4("default", factory);
+    /**
+     * orders the given values so that the smallest displayed is min
+     *
+     * reduced stats are given as negative values and then negated for display
+     * whichs results in [-30, -15] being displayed as "(30 - 15) reduced"
+     * @param param0
+     *
+     */
+    function valueOrder(_a, formatter_id) {
+        var _b = __read(_a, 2), left = _b[0], right = _b[1];
+        var sign = Math.sign(left);
+        if ((left < right && sign === 1) || (left > right && sign === -1)) {
+            return [left, right];
+        }
+        else {
+            return [right, left];
+        }
+    }
     var StatValue_1, item_classes, formatters, inverse_formatters, number, formatter_regexp;
     return {
         setters: [
@@ -629,7 +648,7 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
     exports_11("createDescriptionFindStrategies", createDescriptionFindStrategies);
     function formatWithFinder(stats, find, options) {
         if (options === void 0) { options = {}; }
-        var _a = options.ignore_if_zero, ignore_if_zero = _a === void 0 ? false : _a, _b = options.getFormatters, getFormatters = _b === void 0 ? function (t) { return t.formatters; } : _b;
+        var _a = options.getFormatters, getFormatters = _a === void 0 ? function (t) { return t.formatters; } : _a;
         var lines = [];
         var translated = new Set();
         var _loop_1 = function (stat_id, stat) {
@@ -646,7 +665,7 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
                         var value = _a.value;
                         return StatValue_2.isZero(value);
                     });
-                    if (!ignore_if_zero || !requiredStatsAreZero) {
+                    if (!requiredStatsAreZero) {
                         throw new Error("matching translation not found for '" + stat.id + "'");
                     }
                 }
@@ -666,20 +685,20 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
             }
         };
         try {
-            for (var _c = __values(Array.from(stats.entries())), _d = _c.next(); !_d.done; _d = _c.next()) {
-                var _e = __read(_d.value, 2), stat_id = _e[0], stat = _e[1];
+            for (var _b = __values(Array.from(stats.entries())), _c = _b.next(); !_c.done; _c = _b.next()) {
+                var _d = __read(_c.value, 2), stat_id = _d[0], stat = _d[1];
                 _loop_1(stat_id, stat);
             }
         }
         catch (e_1_1) { e_1 = { error: e_1_1 }; }
         finally {
             try {
-                if (_d && !_d.done && (_f = _c.return)) _f.call(_c);
+                if (_c && !_c.done && (_e = _b.return)) _e.call(_b);
             }
             finally { if (e_1) throw e_1.error; }
         }
         return lines;
-        var e_1, _f;
+        var e_1, _e;
     }
     function requiredStats(description, provided) {
         // intersect the required stat_ids from the desc with the provided
@@ -700,37 +719,41 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
             .filter(function (stat) { return stat !== null; });
     }
     function formatWithFallback(stats, fallback) {
+        var non_zero_stats = Array.from(stats.entries()).filter(function (_a) {
+            var _b = __read(_a, 2), stat = _b[1];
+            return !StatValue_2.isZero(stat.value);
+        });
+        if (non_zero_stats.length === 0) {
+            return [];
+        }
         if (fallback === Fallback.throw) {
             if (stats.size > 0) {
-                throw new NoDescriptionFound(Array.from(stats.values()));
+                throw new NoDescriptionFound(non_zero_stats.map(function (_a) {
+                    var _b = __read(_a, 2), stat = _b[1];
+                    return stat;
+                }));
             }
             else {
                 return [];
             }
         }
         else if (fallback === Fallback.id) {
-            return Array.from(stats.keys());
+            return non_zero_stats.map(function (_a) {
+                var _b = __read(_a, 1), key = _b[0];
+                return key;
+            });
         }
         else if (fallback === Fallback.skip) {
             return [];
         }
         else if (typeof fallback === 'function') {
-            return Array.from(stats.entries())
+            return non_zero_stats
                 .map(function (_a) {
                 var _b = __read(_a, 2), id = _b[0], stat = _b[1];
                 return fallback(id, stat);
             })
                 .filter(function (line) { return typeof line === 'string'; });
         }
-        else if (fallback === Fallback.skip_if_zero) {
-            var non_zero_stats = Array.from(stats.values()).filter(function (stat) { return !StatValue_2.isZero(stat.value); });
-            if (non_zero_stats.length > 0) {
-                throw new NoDescriptionFound(non_zero_stats);
-            }
-            else {
-                return [];
-            }
-        }
         else {
             // should ts recognize that this is unreachable code? enums can prob
             // be extended at runtime an therfore somebody could mess with them
@@ -763,9 +786,6 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
                 Fallback[Fallback["throw"] = 0] = "throw";
                 Fallback[Fallback["id"] = 1] = "id";
                 Fallback[Fallback["skip"] = 2] = "skip";
-                // ignore if no matching translation is found in stat description
-                // if the stat value is equiv to zero (e.g. 0 or [0, 9])
-                Fallback[Fallback["skip_if_zero"] = 3] = "skip_if_zero";
             })(Fallback || (Fallback = {}));
             exports_11("Fallback", Fallback);
             initial_options = {
@@ -788,8 +808,7 @@ System.register("format/stats", ["translate/index", "types/StatValue"], function
                         for (var _b = __values(createDescriptionFindStrategies(data)), _c = _b.next(); !_c.done; _c = _b.next()) {
                             var descriptionFinder = _c.value;
                             lines.push.apply(lines, __spread(formatWithFinder(untranslated, descriptionFinder, {
-                                getFormatters: getFormatters,
-                                ignore_if_zero: fallback === Fallback.skip_if_zero
+                                getFormatters: getFormatters
                             })));
                         }
                     }
@@ -5108,7 +5127,7 @@ System.register("format/groupMods", ["format/stats"], function (exports_16, cont
                     arg: i + 1,
                     id: 'placeholder'
                 }); });
-            }, fallback: stats_2.Fallback.skip_if_zero }));
+            } }));
         // collapes value ranges into single placeholder
         return lines.map(function (line) { return line.replace(/\(# - #\)/g, '#'); }).join(' / ');
     }

diff --git a/src/format/__tests__/formatStats.ts b/src/format/__tests__/formatStats.ts
@@ -3,15 +3,15 @@ import { StatLocaleData } from '../../types/StatDescription';
 import datas from '../../__fixtures__/english';
 import formatStats, { Fallback, Stat } from '../stats';
 
-it('should translate single stat line', () => {
+it('should throw if the values dont match', () => {
   expect(() =>
-    formatStats([{ id: 'weapon_physical_damage_+%', value: 0 }], { datas })
+    formatStats([{ id: 'weapon_physical_damage_+%', value: -1 }], { datas })
   ).toThrowError(
     "matching translation not found for 'weapon_physical_damage_+%'"
   );
 });
 
-it('should throw if the values dont match', () => {
+it('should translate single stat line', () => {
   expect(
     formatStats([{ id: 'weapon_physical_damage_+%', value: 25 }], { datas })
   ).toEqual(['25% increased Physical Damage with Weapons']);
@@ -177,19 +177,14 @@ it('should support skipping fallback', () => {
   ).toEqual([]);
 });
 
-describe('skip_if_zero fallback', () => {
+describe('zero stats', () => {
   it('ignores stats with 0 value', () => {
     const stats = [{ id: 'weapon_physical_damage_+%', value: 0 }];
 
-    expect(() =>
-      formatStats(stats, {
-        datas
-      })
-    ).toThrow("matching translation not found for 'weapon_physical_damage_+%'");
     expect(
       formatStats(stats, {
         datas,
-        fallback: Fallback.skip_if_zero
+        fallback: Fallback.throw
       })
     ).toEqual([]);
   });
@@ -203,7 +198,7 @@ describe('skip_if_zero fallback', () => {
         ],
         {
           datas,
-          fallback: Fallback.skip_if_zero
+          fallback: Fallback.throw
         }
       )
     ).toThrow("matching translation not found for 'local_energy_shield_+%'");
@@ -213,7 +208,7 @@ describe('skip_if_zero fallback', () => {
     expect(
       formatStats([{ id: 'non_existing_zero', value: 0 }], {
         datas,
-        fallback: Fallback.skip_if_zero
+        fallback: Fallback.throw
       })
     ).toEqual([]);
   });
@@ -222,7 +217,7 @@ describe('skip_if_zero fallback', () => {
     expect(() =>
       formatStats([{ id: 'non_existing_zero', value: [0, 1] }], {
         datas,
-        fallback: Fallback.skip_if_zero
+        fallback: Fallback.throw
       })
     ).toThrow('no descriptions found for non_existing_zero');
   });

diff --git a/src/format/groupMods.ts b/src/format/groupMods.ts
@@ -1,8 +1,4 @@
-import formatStats, {
-  Fallback,
-  Options as FormatStatsOptions,
-  Stat
-} from './stats';
+import formatStats, { Options as FormatStatsOptions, Stat } from './stats';
 
 // arg types
 export { Stat } from './stats';
@@ -59,8 +55,7 @@ function groupStats(
         arg: i + 1,
         id: 'placeholder'
       }));
-    },
-    fallback: Fallback.skip_if_zero
+    }
   });
 
   // collapes value ranges into single placeholder

diff --git a/src/format/stats.ts b/src/format/stats.ts
@@ -33,10 +33,7 @@ export class NoDescriptionFound extends Error {
 export enum Fallback {
   throw, // throw if no stat was found
   id,
-  skip,
-  // ignore if no matching translation is found in stat description
-  // if the stat value is equiv to zero (e.g. 0 or [0, 9])
-  skip_if_zero
+  skip
 }
 
 const initial_options: Options = {
@@ -71,8 +68,7 @@ const formatStats = (
     for (const descriptionFinder of createDescriptionFindStrategies(data)) {
       lines.push(
         ...formatWithFinder(untranslated, descriptionFinder, {
-          getFormatters,
-          ignore_if_zero: fallback === Fallback.skip_if_zero
+          getFormatters
         })
       );
     }
@@ -113,17 +109,13 @@ interface FormatWithFinderOptions {
     stat: Stat,
     n: number
   ) => Translation['formatters'];
-  ignore_if_zero: boolean;
 }
 function formatWithFinder(
   stats: Map<string, Stat>,
   find: (stat: Stat) => Description | undefined,
   options: Partial<FormatWithFinderOptions> = {}
 ): string[] {
-  const {
-    ignore_if_zero = false,
-    getFormatters = (t: Translation) => t.formatters
-  } = options;
+  const { getFormatters = (t: Translation) => t.formatters } = options;
   const lines: string[] = [];
   const translated: Set<string> = new Set();
 
@@ -145,7 +137,7 @@ function formatWithFinder(
           stats
         ).every(({ value }) => isZero(value));
 
-        if (!ignore_if_zero || !requiredStatsAreZero) {
+        if (!requiredStatsAreZero) {
           throw new Error(`matching translation not found for '${stat.id}'`);
         }
       } else {
@@ -193,30 +185,27 @@ function formatWithFallback(
   stats: Map<string, Stat>,
   fallback: Fallback | FallbackCallback
 ): string[] {
+  const non_zero_stats = Array.from(stats.entries()).filter(
+    ([, stat]) => !isZero(stat.value)
+  );
+  if (non_zero_stats.length === 0) {
+    return [];
+  }
+
   if (fallback === Fallback.throw) {
     if (stats.size > 0) {
-      throw new NoDescriptionFound(Array.from(stats.values()));
+      throw new NoDescriptionFound(non_zero_stats.map(([, stat]) => stat));
     } else {
       return [];
     }
   } else if (fallback === Fallback.id) {
-    return Array.from(stats.keys());
+    return non_zero_stats.map(([key]) => key);
   } else if (fallback === Fallback.skip) {
     return [];
   } else if (typeof fallback === 'function') {
-    return Array.from(stats.entries())
+    return non_zero_stats
       .map(([id, stat]) => fallback(id, stat))
       .filter((line): line is string => typeof line === 'string');
-  } else if (fallback === Fallback.skip_if_zero) {
-    const non_zero_stats = Array.from(stats.values()).filter(
-      stat => !isZero(stat.value)
-    );
-
-    if (non_zero_stats.length > 0) {
-      throw new NoDescriptionFound(non_zero_stats);
-    } else {
-      return [];
-    }
   } else {
     // should ts recognize that this is unreachable code? enums can prob
     // be extended at runtime an therfore somebody could mess with them