saltyhotdog / BattletechIssueTracker

Public issue tracker to communicate with modders and HBS.
MIT License
6 stars 0 forks source link

Mech tonnage checks for under/overweight are done as floating point comparisons with no epsilon #16

Open janxious opened 6 years ago

janxious commented 6 years ago

Describe the problem

In mech lab, it is possible to get into a situation where you have a mech that is fit perfectly to weight but you either cannot create a work order, save for skrimish, etc. because it is shown as overweight, or to fix that you must remove a pip of armor, so you have warnings for underweight in all screens where that mech appears.

This is because the tonnage that is checked by mech lab is a float but the check for under/over weight is done with full 32/64bit FP precision, which is a no-no when you have a known precision. In this case 1 pip of armor is 0.0125T.

There are items that exacerbate this issue significantly in mods, but it can infrequently happen in the vanilla game due to the nature of floating point math.

Provide an example of the code where the problem occurs

The offender is MechValidationRules.ValidateMechTonnage.

Specifically, the two boolean statements to check for underweight and overweight have no epsilon on them, so when they do >/<, "equal" FP values can be seen as under/over.

Provide an example of your proposed solution

One easy solution is to do an equality check with early return where the epsilon for comparison is large but not so large it will false positive for comparisons of the highest expected precision. It might look something like:

# yada
MechStatisticsRules.CalculateTonnage(mechDef, ref num, ref tonnage);
if (Math.Abs(num - mechDef.Chassis.Tonnage) < 0.0001) {
    return;
}
if (num > mechDef.Chassis.Tonnage)
#yada

Provide a link to a commit in the private repo for this issue

I did not make one on the private repo, but I have a mod that IL patches the class to run the comparison and early return

Leave any additional comments here

This frustrated me several times during my initial playthrough of the story and I was happy to find it had a relatively simple solution.