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

Stat sub and heal commands use the wrong value to calculate percentage #1480

Closed
mark-b5 opened this issue Mar 16, 2025 · 2 comments
Closed

Comments

@mark-b5
Copy link
Contributor

mark-b5 commented Mar 16, 2025

The following commands

  • stat_sub
  • stat_heal
  • npc_statsub
  • npc_statheal
  • Others potentially?

All do something along the lines of

const current = npc.levels[stat];
const subbed = current - (constant + (current * percent) / 100);

They take a percent argument and then modify the stat by currentstat * percent / 100. This is no good!!!!

The proper behavior should be modifying the stat by basestat * percent / 100. You can see an example of this in the following runescript

Image

Here, stat_heal(hitpoints,0,100) should heal the player to full hitpoints. However in our current engine, if we run the following debugproc while at 99/99 hp,

[debugproc,dmg_heal]
if (p_finduid(uid) = true) {
    p_delay(1);
    stat_sub(hitpoints, 98, 0);
    p_delay(1);
    stat_heal(hitpoints,0,100);
}

the player is left at 2/99 hitpoints rather than 99/99 hitpoints.

All these engine commands needs to be updated, and then all calls in runescript that pass in a percent parameter need to be updated to match the new engine behavior

So, in short:

  1. Figure out if there are more than these 4 commands where this problem exists
  2. Update these commands to use base_stat rather than current_stat when multiplying by percentage
  3. Sort through all calls to these commands in runescript and check if they need updated to match new behavior
@hex-agon
Copy link
Contributor

Might also consider reworking this logic inside STAT_HEAL and move it into Player.changeStat or into runescript (think there was some discussion around if they could clear heropoints from runescript).

if (stat === 3 && player.levels[3] >= player.baseLevels[3]) {
    player.heroPoints.clear();
}

@mark-b5
Copy link
Contributor Author

mark-b5 commented Mar 16, 2025

I think it should be inside Player.changeStat because hp regen is all handled in engine

Wasn't there some discussion that resetting hp when npcs heal to full isn't even authentic before osrs?

It also makes sense to expose herePoints.clear() to runescript also, but not for this use-case because of the hp regen thing

@mark-b5 mark-b5 closed this as completed Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants