Interkarma / daggerfall-unity

Open source recreation of Daggerfall in the Unity engine
http://www.dfworkshop.net
MIT License
2.7k stars 327 forks source link

Fixed "Weakens Armor" enchantment not being applied #2616

Open KABoissonneault opened 5 months ago

KABoissonneault commented 5 months ago

This PR has 3 changes related to "Weakens Armor", last one also including "Strengthens Armor".

Two uncontroversial fixes:

Fixed "Weakens Armor" enchantment not being applied.

The "Weakens Armor" decreased armor value was just not applied by DFU. In DF, lower armor value is better: players start at 100, and armors reduce the armor value by 5 for each armor class. Therefore, a "Decreased" armor value is a +5 malus to Amor Value.

When checking which "Decreased" armor value is the worst, we take the bigger value, not the smaller one. Otherwise, the base "DecreasedArmorValue" of 0 is always smaller than the +5 of "Weakens Armor", and fails to apply the actual debuff.

Properly apply "Bad" color for AC penalty in Paperdoll

The AC value shown in the paperdoll runs backward from the Armor Values used in formulas. AC starts at 0, and goes up when wearing better armors. +1 AC is equal to -5 Armor Value, or -5% chance to hit. So, an increased AC is a decreased armor value and is good. A decreased AC is an increased armor value and is bad.

DFU was not always like this. It used to get them backward until this issue: https://github.com/Interkarma/daggerfall-unity/issues/1740

and this changelist inverted them. https://github.com/Interkarma/daggerfall-unity/commit/baa983a83419b1ea1b8db7ac5105c060234331e9

So this old logic in Paperdoll was still accidentally thinking DecreasedArmorValue was a negative number - no, it's 0 or +5 (Weakens Armor). IncreasedArmorValue is 0 or -5. We instead add the two modifiers together, like in FormulaHelper.CalculateArmorToHit.

image

The more controversial fix:

2342

I initially reported that the bonus from "Strengthens Armor" and "Weakens Armor" are inconsistent between the shown AC and the effect on the formula. https://github.com/Interkarma/daggerfall-unity/issues/2342

In short, they both give ±5 to AC and ±5 to Armor Value (Hit). When it comes to the AC and armor value from armor, 1 AC is equal to -5 Armor Value (and therefore -5 Hit). This is inconsistent, either 5 AC should be -25 Armor Value, or -5 Armor Value should be +1 AC shown on the paperdoll.

Since I reported this issue, I tested in gameplay in DF, and this inconsistency is a classic issue. "Strengthens Armor" does not have as much of an impact on gameplay as a 25% difference would make.

When I reported this issue, we initially went with changing the effect to ±25 Armor Value, https://github.com/Interkarma/daggerfall-unity/commit/d0b3b2f7cdd9885b2b7ce8cf3f040407619e1beb

but IK reverted this here. And I now agree with reverting it. https://github.com/Interkarma/daggerfall-unity/commit/7825c4effabf825cad850932bf64073451758f77

Note: There is a mistake in that changelist's explanation. Daedric's base Armor Value is -5 because 21 AC means -105 Armor Value, and 100 - 105 = -5. The effect of "Strengthens Armor" is an extra -5, not an extra -105. So you go from 100 to -10, not 100 to -110. "Strengthens Armor" is nowhere near as impactful as wearing Daedric Armor, but it does add up.

I still agree with not changing the gameplay impact. However, I would rather we stop lying to players here, especially since it's not easy for mods to fix the Paperdoll display here.

So instead, I show the AC change as ±1. This does not visually match classic, but it matches it in gameplay, and has no impact on a playthrough otherwise. It does mean players will ask about it, and we'll need to send them the patch notes when that happens. Unlike the other DFU changes, this is purely visual.