search

javierriveracastro / betteroll-swade

A Better Rolls port for SWADE
GNU General Public License v3.0
15 stars 31 forks source link

Fix for armor calculation bug #674

Closed Blackcloud010 closed 5 months ago

Blackcloud010 commented 5 months ago

Found a bug where armor would not be calculated correctly when targeting arms/legs/head Renamed change_location to match system config Refactored armorPerLocation lookup to not favor num > 0 over a result of 0, and to correctly handle NaN.

javierriveracastro commented 5 months ago

That breaks most NPCs and all vehicles. They all have torso armor 0 (not NaN) and a global armor set. With your fix they armor will be set to 0.

The bug is real, but it needs a different fix.

Blackcloud010 commented 5 months ago

These changes shouldn't affect Vehicles, their calculation is handled in a different logical section.

For NPCs/Any character with a fixed/global armor override set, it will now always take that value.

Still new to SWADE so apologies if it shouldn't work that way, or I'm missing some other logic piece.

javierriveracastro commented 5 months ago

You are right about vehicles.

But, no autoCalcTougness is not related to this.

It is just that NPCs (or at least quite a lot of them) have armor 0 in all locations, but have their armor value set in actor.system.stats.toughness.armor. They can have autoCalcToughness set to true or false, but this irrelevant.

I think the right approach here is to make torso armor equivalent to actor..armor if it is set to 0. But I'm not really sure.

Blackcloud010 commented 5 months ago

When autoCalcToughness is set the value stats.toughness.armor will still have a value set. However the (at least default) sheet will show the armor as 0. Not sure if this is the intended behavior or not.

Also found another bug, where armor added through active effects will not be properly added. (Both in this PR and in the original)

javierriveracastro commented 5 months ago

There are now four issues in the same PR, this is getting messing. Ok, let's recap.

Issue 1: Spelling of legs and arms. That's the most important thing in this PR. Maybe is not technically brilliant but it is a bug that if you didn't find it, it will probably remain there for ages, as legs and arms called shots don't usually happen in SW. Thanks a lot.

Issue 2: Then Nan checking. The bug is there but I'm ambivalent about this. Theoretically since version 3.2, system checks that the types inside actors are right. So instead of checking for Nan, maybe we should trust the system and drop the parseInt. Or no, as historically type safety has been quite lacking.

Issue 4: Active effects. Need to check into this, but probably it is going to be messy.

Issue 3: Non zero preference. Actually I believe that this is not a bug, but it will probably need a longer explanation. But for starter took a look at any npc from the core rules. Most will have 0 in all locations and a value in toughness.armor. They will also have autocalc off, but this is a different thing as it also affects Vigor calculation.

javierriveracastro commented 5 months ago

I have cherry picked issue 1 solve.

You should probably add an issue about the armor problem.