Look into HealthStatProvider
magicus opened this issue ยท 7 comments
Right now it returns getHealth() for CraftedGearItem and fixedStats().healthBuff() for normal gear. This is inconsistent. We need to separate the base health from the health buff. Possibly we need to be able to return both these separate, and have a "all health" that sums both of them.
Note that crafted gear can have both a base health (from materials) and a health buff stat (from ingredients).
@magicus It seems to me that it works correctly.
That is even more surprising, since the code looks so odd. :-) Don't close this bug just yet, I still want to investigate.
can we have a common superclass between all gear items for more consistency? There are some cases I noticed where that would improve the readability. It's easier to check for stats when you don't have to differentiate between normal and crafted items.
1 or more common interfaces for getting gear stats might work, but have to be looked into..
Possibly, but they are also very much different, so doing that risks just pushing logic to deal with two completely different cases from one place to another, and not improve anything.
With that said, I do think we can improve how we handle crafted gear. I know for a fact that we do not try to extract all possible information in the CraftedGearItem, for instance.
I admit that I did not give crafted items the full amount of love and care it needed while designing the stat and item annotation systems. However, I did think about them, and I ended up making them separate, since it seemed unclear what gains could be gotten from unifying them more.
In essence, "normal" items are mostly dictated by their template, the "gearinfo", and to a much smaller degree, the gearinstance which stores the few values that can change. In contrast, a crafted gear is a tabula rasa -- we know nothing about it, except what it brings along. So it has a zero "gearinfo", and everything there is to know is in the "gearinstance".
If we were to try and push all the info that is normally in the gearinfo into the gearinstance, for the benefit of unifying behavior between crafted and normal gears, we would get a very heavy gearinstance, which will sometimes contain data that is only available from the gearinfo for normal items. My worry is that this will create problems whenever we deal with normal items, and the added complexity here will outweigh any gains from presenting a unified view.
I believe it is much better that stuff that deals with items are prepared to deal with crafted gear as well as normal gear, just as it might deal with emerald pouches, ingredients, etc. It's just another kind of items in the Wynn universe that we need to know about. The fact that you can yield both a normal weapon and a crafted weapon does not necessarily mean that they should be represented as the same type.