search

jvd33 / ClassicThreat

Estimates tank threat based off of warcraft logs data
https://classicthreat.com
3 stars 1 forks source link

Paladin Melee Damage is being multiplied by Righteous Fury modifier #40

Closed dimlithegwarf closed 4 years ago

dimlithegwarf commented 4 years ago

Title - as Righteous Fury only affects holy damage, the melee damage should not be increased by 1.9, but kept at 1.

jvd33 commented 4 years ago

Big yikes, good looks. I'll fix this asap

Thanks for opening an issue!

dimlithegwarf commented 4 years ago

Just realised as well, this will apply to mana generation and any debuffs applied. Worth having a 'holy/non holy' tag for pally spells?

Anselm123 commented 4 years ago

Just a word of warning that certain mechanics within our toolkit are affected by RF and some aren't. For example Judgement of wisdom and seal of wisdom mana gained effects are counted as "holy" iirc but blessing of wisdom isn't (might be wrong about one of those but there's something about this)

jvd33 commented 4 years ago

I'm going to opt for the holy vs non holy flag, definitely. Judgement of Wisdom and Seal of Wisdom mana gains would be nice to get a confirmation on (my gut feeling says it probably benefits from RF, but I also play a Warrior), but according to old patch notes Blessing of Wisdom mana regens generate no threat as of patch 1.9: https://www.engadget.com/2011/08/16/wow-archivist-patch-1-9-the-gates-of-ahnqiraj/

That's currently how the tool treats blessing of wisdom, so we should be all good there

Anselm123 commented 4 years ago

Might be right about that, it's hard to know what does and doesn't these days with how jigsawed classic is

jvd33 commented 4 years ago

I just pushed a change that appears to resolve it. I also noticed I wasn't handling Paladin spell healing properly, so that's also fixed in this change.

Check it out and let me know if it checks out- I manually refreshed some of the logs for Paladins in BWL, but I wasn't very thorough so any logs that still show the error (e.g. ALL the MC logs I didn't feel like doing) can just be recalculated and it'll fix em without needing to refetch the events from Warcraft Logs. An automatic recalculation feature for me to use in this situation is on my radar, but for now this is what I've got.

Thanks guys!