Fix rate of fire modification (#497)

* Fix absolute setting of burst/burstrof

* Improve change display for stats with overwrite

* Module.getPretty takes synthetic getters into account

* Allow custom higherbetter for changes of stats when formatted

* Fix rof modding

src/app/components/Modification.jsx

@@ -3,7 +3,8 @@ import PropTypes from 'prop-types';
 import TranslatedComponent from './TranslatedComponent';
 import cn from 'classnames';
 import NumberEditor from 'react-number-editor';
-import { isValueBeneficial } from '../utils/BlueprintFunctions';
+import { isChangeValueBeneficial } from '../utils/BlueprintFunctions';
+import { Modifications } from 'coriolis-data/dist';
 
 /**
  * Modification
@@ -79,6 +80,7 @@ export default class Modification extends TranslatedComponent {
     let { translate, formats, units } = this.context.language;
     let { m, name } = this.props;
     let modValue = m.getChange(name);
+    let isOverwrite = Modifications.modifications[name].method === 'overwrite';
 
     if (name === 'damagedist') {
       // We don't show damage distribution
@@ -117,10 +119,10 @@ export default class Modification extends TranslatedComponent {
               </td>
               <td style={{ textAlign: 'center' }} className={
                 modValue ?
-                  isValueBeneficial(name, modValue) ? 'secondary' : 'warning' :
+                  isChangeValueBeneficial(name, modValue) ? 'secondary' : 'warning' :
                   ''
               }>
-                {formats.f2(modValue / 100) || 0}%
+                {formats.f2(modValue / 100) || 0}{isOverwrite ? '' : '%'}
               </td>
             </tr>
           </tbody>

src/app/shipyard/Module.js

@@ -59,7 +59,8 @@ export default class Module {
         } else if (modification.method === 'overwrite') {
           result = modifierActions[name];
         } else {
-          // rate of fire is special, as it's really burst interval.  Handle that here
+          // Rate of fire modifiers are special as they actually are modifiers
+          // for fire interval. Translate them accordingly here:
           let mod = null;
           if (name === 'rof') {
             mod = 1 / (1 + modifierActions[name]) - 1;
@@ -105,13 +106,6 @@ export default class Module {
         } else if (modification.method === 'overwrite') {
           value = null;
         } else {
-          // rate of fire is special, as it's really burst interval.  Handle that here
-          let mod = null;
-          if (name === 'rof') {
-            mod = 1 / (1 + modifierActions[name]) - 1;
-          } else {
-            mod = modifierActions[name];
-          }
           value = ((value / 10000 + 1) / (1 + mod) - 1) * 10000;
         }
       }
@@ -131,6 +125,13 @@ export default class Module {
    * @return {Number} The value queried
    */
   get(name, modified = true) {
+    if (name == 'rof' && isNaN(this[name])) {
+      let fireint = this['fireint'];
+      if (!isNaN(fireint)) {
+        this['rof'] = 1 / fireint;
+      }
+    }
+
     let val;
     if (modified) {
       val = this._getModifiedValue(name);
@@ -166,14 +167,20 @@ export default class Module {
       modValue = value - baseValue;
     } else if (name === 'shieldboost' || name === 'hullboost') {
       modValue = (1 + value) / (1 + baseValue) - 1;
+    } else if (name === 'rof') {
+      let burst = this.get('burst', true) || 1;
+      let burstInt = 1 / (this.get('burstrof', true) / 1);
+
+      let interval = burst / value;
+      let newFireint = (interval - (burst - 1) * burstInt);
+      modValue = newFireint / this['fireint'] - 1;
     } else { // multiplicative
       modValue = baseValue == 0 ? 0 : value / baseValue - 1;
     }
 
     if (modification.type === 'percentage') {
       modValue = modValue * 10000;
-    } else if (modification.type === 'numeric' && name !== 'burst' &&
+    } else if (modification.type === 'numeric') {
-      name !== 'burstrof') {
       modValue = modValue * 100;
     }
 
@@ -191,7 +198,14 @@ export default class Module {
    */
   getPretty(name, modified = true, places = 2) {
     const formattingOptions = STATS_FORMATTING[name];
-    let val = this.get(name, modified) || 0;
+    let val;
+    if (formattingOptions && formattingOptions.synthetic) {
+      val = (this[formattingOptions.synthetic]).call(this, modified);
+    } else {
+      val = this.get(name, modified);
+    }
+    val = val || 0;
+
     if (formattingOptions && formattingOptions.format.startsWith('pct')) {
       return 100 * val;
     }
@@ -250,6 +264,11 @@ export default class Module {
           } else if (name === 'shieldboost' || name === 'hullboost') {
             result = (1 + result) * (1 + modValue) - 1;
           } else {
+            // Rate of fire modifiers are special as they actually are modifiers
+            // for fire interval. Translate them accordingly here:
+            if (name == 'rof') {
+              modValue = 1 / (1 + modValue) - 1;
+            }
             result = result * (1 + modValue);
           }
         } else if (name === 'burstrof') {
@@ -277,11 +296,7 @@ export default class Module {
   formatModifiedValue(name, language, unit, val) {
     const formattingOptions = STATS_FORMATTING[name];
     if (val === undefined) {
-      if (formattingOptions && formattingOptions.synthetic) {
+      val = this.getPretty(name, true);
-        val = (this[formattingOptions.synthetic]).call(this, true);
-      } else {
-        val = this._getModifiedValue(name);
-      }
     }
 
     val = val || 0;
@@ -351,14 +366,18 @@ export default class Module {
 
     if (formattingOptions && formattingOptions.change) {
       let changeFormatting = formattingOptions.change;
-      let baseVal = this[name];
+      let baseVal = this[name] || 0;
       let absVal = this._getModifiedValue(name);
       if (changeFormatting === 'additive') {
         val = absVal - baseVal;
       } else if (changeFormatting === 'multiplicative') {
         val = absVal / baseVal - 1;
       }
-      val *= 10000;
+      if (Modifications.modifications[name].method === 'overwrite') {
+        val *= 100;
+      } else {
+        val *= 10000;
+      }
     }
     return val;
   }

src/app/shipyard/StatsFormatting.js

@@ -26,8 +26,8 @@ export const STATS_FORMATTING = {
   'ammo': { 'format': 'int', },
   'boot': { 'format': 'int', 'unit': 'secs' },
   'brokenregen': { 'format': 'round1', 'unit': 'ps' },
-  'burst': { 'format': 'int' },
+  'burst': { 'format': 'int', 'change': 'additive' },
-  'burstrof': { 'format': 'round1', 'unit': 'ps' },
+  'burstrof': { 'format': 'round1', 'unit': 'ps', 'change': 'additive' },
   'causres': { 'format': 'pct' },
   'clip': { 'format': 'int' },
   'damage': { 'format': 'round' },
@@ -61,7 +61,7 @@ export const STATS_FORMATTING = {
   'ranget': { 'format': 'f1', 'unit': 's' },
   'regen': { 'format': 'round1', 'unit': 'ps' },
   'reload': { 'format': 'int', 'unit': 's' },
-  'rof': { 'format': 'round1', 'unit': 'ps' },
+  'rof': { 'format': 'round1', 'unit': 'ps', 'synthetic': 'getRoF', 'higherbetter': true },
   'angle': { 'format': 'round1', 'unit': 'ang' },
   'scanrate': { 'format': 'int' },
   'scantime': { 'format': 'round1', 'unit': 's' },

src/app/utils/BlueprintFunctions.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import { Modifications } from 'coriolis-data/dist';
+import { STATS_FORMATTING } from '../shipyard/StatsFormatting';
 
 /**
  * Generate a tooltip with details of a blueprint's specials
@@ -280,6 +281,25 @@ export function isValueBeneficial(feature, value) {
   }
 }
 
+/**
+ * Is the change as shown beneficial?
+ * @param {string} feature The name of the feature
+ * @param {number} value The value of the feature as percentage change
+ * @returns True if the value is beneficial
+ */
+export function isChangeValueBeneficial(feature, value) {
+  let changeHigherBetter = STATS_FORMATTING[feature].higherbetter;
+  if (changeHigherBetter === undefined) {
+    return isValueBeneficial(feature, value);
+  }
+
+  if (changeHigherBetter) {
+    return value > 0;
+  } else {
+    return value < 0;
+  }
+}
+
 /**
  * Get a blueprint with a given name and an optional module
  * @param   {string} name    The name of the blueprint