CritEffectBonus undervaluing crit damage/healing modifiers

[Pewtro]

The way crit damage calculations are done in the game is a bit weird. Some effects will affect the total damage, whereas others will only affect the extra damage and their interactions are pretty messed up.

For the SimC implementation see: https://github.com/simulationcraft/simc/blob/ff6ebe350d366cb5d650ba5d85fedd1c1ae2efb5/engine/action/action.cpp#L1243

But for the effects that affect the total damage/healing, the CritEffectBonus helper is undervaluing their contribution. This is because (from what I understand of the hook), it's summing up crit effects and adding them onto the base crit mod of 2.

So in a super simplistic example of having a single total damage modifier that says "X spell school does 2% more crit damage" (see Might of the Mountain) the game would make a base hit of 100 do 204 damage when critting (base * 2 * (1+0.02)), whereas CritEffectBonus would calculate that as base * 2.02, or 202 damage.

This means that when calculating backwards, we are undervaluing the value of SOME crit effect modifiers. In other words this means an effect that would add 2% total damage in-game, is only adding 1% damage according to CritEffectBonus (roughly anyways).

Therefore, we probably need better support for crit effects in general. A quick test shows that changing the BASE_CRIT_EFFECT_MOD from 2 to 1, fixes the issue for something like Might of the Mountain for DPS, as that causes the appropriate ratio the 2% passed in as the crit effect and the BASE_CRIT_EFFECT_MOD value. However, I would assume this would have uglier consequences for all the healers since they use this helper a lot, and they might rely on current numbers.

Most effects that only apply to certain spells, would primarily affect only the increased damage part, and as thus should be accurate as is - which would also mean breakage if changing BASE_CRIT_EFFECT_MOD value.

File in question: https://github.com/WoWAnalyzer/WoWAnalyzer/blob/dragonflight/src/parser/shared/modules/helpers/CritEffectBonus.ts