search

raethkcj / RatingBuster

An item comparison tool for WoW Classic.
GNU General Public License v2.0
58 stars 14 forks source link

feral attack power #31

Closed Sigols closed 3 years ago

Sigols commented 3 years ago

Hi... The druid's agility does not turn into feral attack power!

mfg Thomas

ps: nice Addon thx

sry for bad english (google)

raethkcj commented 3 years ago

Hello,

This addon relies heavily on translations to read the stats on items in other languages. Would you mind sharing which language you play the game in?

Thanks!

Sigols commented 3 years ago

Hi i play with the german client

trickdaemon commented 3 years ago

Hi, this is also an issue with the English client. Previously in the 2.4.3 version I used to use on private servers it would only show agi into FAP while you're in cat form, as you don't receive attack power from agi in bearform. Thank you!

aeadio commented 3 years ago

Tested my PR above and the issue is fixed.

Agility -> AP conversion only happens when in Cat Form. This is annoying, but is in line with how RatingBuster originally functioned. StatLogic's API doesn't give any good mechanism for querying what the AP would be under different scenarios, like the player being in Cat Form.

For RatingBuster's purposes, I think it would be valuable to show these values at all times once Cat Form is learned. I think the Stat Logic API should be extended to account for this. I think Cat Form is the only exceptional situation like this, so it should be simple, and we can probably just tack on an optional parameter to :GetAPPerAgi() that always returns the shifted value. But if there's any other situations like this I'm not aware of, this might not be the right approach.

aeadio commented 3 years ago

Looking into it, I was considering updating the APIs to take a "formid" parameter and assume that form for all calculations. That prompted me to check out stamina and armor modifications for Bear Form, and opened up a can of worms -- Druids have a bunch of modifications per-form stemming from talents, and it all comes from the StatMods API.

The StatMods API inherently assumes you're expecting the current player conditions, such as buffs and talents learned. It doesn't even have a concept of shapeshift IDs, but just keys off active buff ID for Bear Form / Dire Bear Form. Hacking it to return modified values for shapeshift forms but not for anything else is a really bad approach. It also needs to be vetted for accuracy in tandem, because it's using a whole slew of hardcoded aura/spell IDs and spellbook pages/tabs, which are probably different between BCC and the original TBC client.

I think the correct approach to get the StatLogic side able to spit out the information wanted, would be to augment StatMods so that it can be supplied with some data structure describing the target state for which to perform its calculations -- ie, hand it a table saying "user is in Bear Form and has these buffs/talents/etc".

This is way out of scope of my available free time.

At the very least, I think an issue should be raised to audit the contents of the stat mods table and ensure all those hardcoded values are correct. Ie, stamina modifications in Bear Form aren't working as-is, at least going off what RatingBuster displays in the tooltip for items with +stamina on them.