server
server copied to clipboard
🐛 HPP and MPP are calculated incorrectly
I affirm:
- [x] I understand that if I do not agree to the following points by completing the checkboxes my issue will be ignored.
- [x] I have read and understood the Contributing Guide and the Code of Conduct.
- [x] I have searched existing issues to see if the issue has already been opened, and I have checked the commit log to see if the issue has been resolved since my server was last updated.
OS / platform the server is running (if known)
Windows10
Branch affected by issue
base
Steps to reproduce
- Calculate your max yellow HP threshold, which is the largest HP that is less than 75% of your max HP. (For example, with 1000 max HP, you should be yellow at 749 HP or less).
- Set your HP to this max yellow HP threshold (749 HP, in the previous example).
- Observe your HP is white. (
/echo <hpp>
will output 75%). - Keep reducing your HP until it finally turns yellow (this will be at 74% HP or less, rather than <75% HP).
Alternatively:
- Set your current HP to your max HP.
- Do
/echo <hpp>
(Observe 100%) - Set your HP to 1 less than max HP.
- Do
/echo <hpp>
(Observe 100%)
Expected behavior
- Your HP turns yellow.
/echo <hpp>
will output 74%.
Alternatively:
-
/echo <hpp>
outputs 99%.
Analysis
The problem is in CBattleEntity::GetHPP()
and CBattleEntity::GetMPP()
. These functions directly feed into the CCharHealthPacket
/0x0DF
packet which informs the client of their HPP (and hence red/orange/yellow/white HP and the output of /echo <hpp>
).
uint8 CBattleEntity::GetHPP() const
{
return (uint8)ceil(((float)health.hp / (float)GetMaxHP()) * 100);
}
It is doing ceil()
whereas it should be doing floor()
. To account for retail only yielding 0 HPP when your HP is 0 (that is, you're dead), it should look something like this:
uint8 CBattleEntity::GetHPP() const
{
if (health.hp == 0)
{
return 0;
}
return std::max<uint8>(1, (uint8)floor((float)health.hp / (float)GetMaxHP()) * 100);
}
There could potentially be a variety of knock-on effects from other systems having previously compensated for this inaccuracy, so it probably needs a bit of discussion before fixing.
Possibly affected systems:
- Undead aggro threshold
- HPP latent effects
- Sublimation deactivation
- Mob TP behaviors
- Breath damage
- Spirits Within damage
- Wyvern healing breath threshold (should be 1/2, 1/3, 1/4 rather than HPP from my testing, but that's a separate issue).
- ???
@DiscipleOfEris Do you happen to know if in retail things such as latent effects use the same HPP logic? I am making a PR to fix this but unclear if the HPP logic for all the systems you mention use the same floor logic.
@TracentEden There are actually two types of HP latent triggers. One is based on your HPP (e.g. Medicine Ring), and another is based on a rather arcane formula (e.g. Sorcerer's Ring). You can see where I discussed this a little bit in #5127 comments.
From the glances at the code I've done, player systems fall into two categories: using HPP assuming it works the same as retail and will thus have been wrong all this time and will be fixed by this change, or already coded HP% independently of HPP and will be unaffected by this change. I might have missed something though.
@TracentEden There are actually two types of HP latent triggers. One is based on your HPP (e.g. Medicine Ring), and another is based on a rather arcane formula (e.g. Sorcerer's Ring). You can see where I discussed this a little bit in #5127 comments.
From the glances at the code I've done, player systems fall into two categories: using HPP assuming it works the same as retail and will thus have been wrong all this time and will be fixed by this change, or already coded HP% independently of HPP and will be unaffected by this change. I might have missed something though.
Yeah, I will change the latents to also use HPP formula (except the arcane ones which will use a different HPP formula that will also be in the PR), thanks for your info
I did notice healing breath was slightly off (it's single to double digit more in favor of the player) but didn't realize it was not tied to hpp interesting. Probably the reasoning I didn't notice during the audit was I was only off by so little that i just chalked it up to rounding errors.
On retail it was always max HP / 2
for healing sub + drachen armet, and max hp * .33
without (or on hybrid sub), floored of course.