Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use potions #492

Open
wheybags opened this issue May 16, 2020 · 7 comments
Open

Use potions #492

wheybags opened this issue May 16, 2020 · 7 comments

Comments

@wheybags
Copy link
Owner

No description provided.

@neveza
Copy link
Contributor

neveza commented May 22, 2020

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

@wheybags
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link
Contributor

neveza commented May 23, 2020

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
Copy link
Owner Author

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
Copy link
Contributor

neveza commented May 25, 2020

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
Copy link
Owner Author

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.

wheybags added a commit that referenced this issue Jun 11, 2020
* current addition of potion of healing. May include mana later.

* For issue #492 : Added potion.cpp/.h for cleaner code instead of jamming all the logic in the triggerItem function which would have gotten very ugly very quickly. Also added FullHealing as well as mana and rejuvination equivalents since they had related features.

* for #492, All potions and elixers added along with functions to add to attributes.

* Update player.cpp

* Update guimanager.cpp

* Update player.cpp

* added cases to satisfy the errors in appveyor

* Update guimanager.cpp

* Updated the add Attributes functions to use std::min()

* Updated functions to use FixedPoint

* Update potion.h

* Update guimanager.cpp

* Update potion.h

* Update guimanager.cpp

* Update potion.cpp

* Update potion.cpp

* Update playerinput.h

* Update playerinput.cpp

* Update playerbehaviour.cpp

* Update guimanager.cpp

* Update playerbehaviour.cpp

* Update CMakeLists.txt

* Update guimanager.cpp

* compile fixes

* clang-format

* changelog

Co-authored-by: neveza <[email protected]>
Co-authored-by: Tom Mason <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants