wheybags / freeablo

[ARCHIVED] Modern reimplementation of the Diablo 1 game engine
GNU General Public License v3.0
2.16k stars 195 forks source link

Use potions #492

Open wheybags opened 4 years ago

neveza commented 4 years ago

Currently working on this one. I've implemented healing, mana, rejuvination, and the full equivalents of the potions except for elixers.

wheybags commented 4 years ago

Nice! Feel free to open a PR even if it's not finished yet, I won't merge until you let me know it's done.

wheybags commented 4 years ago

Also, just so you know, I'm currently working on a refactor of the item handling code (at #495), so don't worry too much about hacking around the not-so-nice item class.

neveza commented 4 years ago

While working on the elixirs, I may have gone a bit above and beyond. I noticed the baseStats weren't using MaxCurrentItem such like Hp/Mana, so I decided that instead of repeatedly implementing switch/ifs checks, I updated the BaseStats to use MaxCurrentItem as I figured it'd help in the long run while it's early.

So, instead of int32_t strength = 0;

   it is this
    `mutable Misc::MaxCurrentItem<int32_t> strength;`

then code such as: stats.baseStats.strength.current = charStats.strength.current +itemStats.baseStats.strength.current; now won't go out of character maxes stats.baseStats.strength.add(charStats.strength.current + itemStats.baseStats.strength.current);

I've done a rough dirty patch up to see if I didn't break anything obvious. So far, nothing seems out of the norm. That said, if this is disagreeable, I will revert back, otherwise I can continue seeing if anything can be updated appropriately then commit and pull request.

wheybags commented 4 years ago

I'm not quite sure what you're trying to do with that? IIRC attributes don't have a current/max status like life and mana? Also, why mutable?

neveza commented 4 years ago

Attributes have a max. You have the base max and then the max after any bonuses from equipment if factored in. Each character has their own specific maxes. http://www.bigd-online.com/JG/JGFrame.html ; so doing this, would allow the max restriction for the base, then for the livestats, maybe have the same setup for the 'after bonus' attributes. Otherwise, it's going to be a lot of case statement checks for not just the base, but the after bonus too. So that's double the checks any time the attribute is adjusted be it equipment or level up. That's a lot of redundant code down the line when we already got this nifty MaxCurrentItem that we can set up on initial for the max and never have to worry about it.

As for the mutable, I'm just following suite with what was there. After all, this has to be mutable data, but unsure if it was needed or not.

wheybags commented 4 years ago

Ah, ok I think I get what you mean. BTW, we have a more usable copy of Jarulf's guide available here: https://wheybags.gitlab.io/jarulfs-guide/#maximum-stats, that supports links to each section. From my reading of this, it seems like there is only an actual limit placed on your base stats, and that the "max with items" numbers are a result of calculations based off using the best items in the game on top of the max base value. I would prefer not so use MaxCurrentItem for this. As the limitation only applies to base, it would only be applicable there, and it is cleaner to just handle it some other way IMO. Also the max values are static, MaxCurrentItem is designed for items with variable maximum values.