Master Branch: CRITICAL_DAMAGE_POS
Posted: Mon Nov 02, 2015 4:04 pm
I was browsing out of curiosity, and found a post dated a while back
viewtopic.php?f=103&t=31576&p=189658&hi ... as#p189658
Looking over the formulas, I question the that the intended effect is achieved.
If Focus Death and Focus Power are meant to add only half of their effect to blows, then
double criticalModPos = (((attacker.calcStat(Stats.CRITICAL_DAMAGE_POS, 1, target, skill) - 1) / 2) + 1);
Is not a good way to achieve that. It may work OK now, because there are not many buffs with POS damage mods, but it is not a future-proof implementation.
Here is why:
assume you have 2 buffs that give 1.6x position damage. So you expect that on normal hit its 1.6*1.6, and on blow hit is 1.3*1.3 (because both are multiplying function).
but
calcStat will do 1.6*1.6 in both cases first, then the rest.
(1.6*1.6 - 1) = 1.56
1.56 / 2 = 0.78
0.78 + 1 = 1.78
------------------
1.3*1.3 = 1.69 != 1.78
The difference is minor (5% == 1.78/1.69) but it will exacerbate the more buffs you have.
------------------
My suggestion is
double criticalModPos = (((attacker.calcStat((Stats.CRITICAL_DAMAGE_POS - 1)/2 + 1, 1, target, skill)); //count the brackets, might be wrong amount
(Heh, you CAN'T actually do that. Why can't you do computation in method argument?)
Which will ensure that the stat pulled from XML is always halved, i.e. (1.6 - 1)/2 + 1 = 1.3.
Some questions:
there used to be a physicalBlowDamage param for XML skills, same way there is physicalSkillDamage for Final Secret (917). Why did the physicalBlowDamage get removed? It's a very useful stat. For the above topic, could have fixed without Core changes.
Example:
Focus death gives 1.9x crit dmg.
But for physicalBlowDamage 0.765x.
Total for blow skills 1.9*0.765 = 1.45x
Overall not very happy about the refactor (just opinion). Skill structure seems to be less abstract. I remember Cubic Summon skills had cubic cast skills attached. Now I have to look in Core for what cubic skills have. Loads of other changes that make it more difficult to change skills to get desired effect.
Kind Regards
P.S. all the formula is ****ed. Why do you include critical damage twice in calcBlowDamage()? Once when you pull criticalMod, then again criticalModPos. Won't both exist in XML skill?
viewtopic.php?f=103&t=31576&p=189658&hi ... as#p189658
Looking over the formulas, I question the that the intended effect is achieved.
If Focus Death and Focus Power are meant to add only half of their effect to blows, then
double criticalModPos = (((attacker.calcStat(Stats.CRITICAL_DAMAGE_POS, 1, target, skill) - 1) / 2) + 1);
Is not a good way to achieve that. It may work OK now, because there are not many buffs with POS damage mods, but it is not a future-proof implementation.
Here is why:
assume you have 2 buffs that give 1.6x position damage. So you expect that on normal hit its 1.6*1.6, and on blow hit is 1.3*1.3 (because both are multiplying function).
but
calcStat will do 1.6*1.6 in both cases first, then the rest.
(1.6*1.6 - 1) = 1.56
1.56 / 2 = 0.78
0.78 + 1 = 1.78
------------------
1.3*1.3 = 1.69 != 1.78
The difference is minor (5% == 1.78/1.69) but it will exacerbate the more buffs you have.
------------------
My suggestion is
double criticalModPos = (((attacker.calcStat((Stats.CRITICAL_DAMAGE_POS - 1)/2 + 1, 1, target, skill)); //count the brackets, might be wrong amount
(Heh, you CAN'T actually do that. Why can't you do computation in method argument?)
Which will ensure that the stat pulled from XML is always halved, i.e. (1.6 - 1)/2 + 1 = 1.3.
Some questions:
there used to be a physicalBlowDamage param for XML skills, same way there is physicalSkillDamage for Final Secret (917). Why did the physicalBlowDamage get removed? It's a very useful stat. For the above topic, could have fixed without Core changes.
Example:
Focus death gives 1.9x crit dmg.
But for physicalBlowDamage 0.765x.
Total for blow skills 1.9*0.765 = 1.45x
Overall not very happy about the refactor (just opinion). Skill structure seems to be less abstract. I remember Cubic Summon skills had cubic cast skills attached. Now I have to look in Core for what cubic skills have. Loads of other changes that make it more difficult to change skills to get desired effect.
Kind Regards
P.S. all the formula is ****ed. Why do you include critical damage twice in calcBlowDamage()? Once when you pull criticalMod, then again criticalModPos. Won't both exist in XML skill?