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

Adjust recalc_hp to avoid migrating hp levels #28692

Merged
merged 1 commit into from Mar 14, 2019

Conversation

Projects
None yet
3 participants
@kevingranade
Copy link
Member

kevingranade commented Mar 14, 2019

Looks like cumulative rounding errors in recalc_hp were causing characters using the new JSON-based STS to suddenly die.

Summary

SUMMARY: None

Purpose of change

Fixes #28680
It seems there was a latent rounding error in Character::recalc_hp(), but it was never triggered before because it was only called on character creation.

Describe the solution

Most importantly, this avoids updating hp levels if the max hp has not changed, this should prevent the bug on it's own.
Additionally, I changed the interpolation of current HP to be a bit more generous, taking the ceiling of the interpolated value instead of the (implicit) floor. In order to prevent out-of range HP values, I also clamped the value between 1 and the new max HP.

Describe alternatives you've considered

Remove STS because it's terrible.

Additional context

Introduced by #28637

Adjust recalc_hp to avoid migrating hp levels
Looks like cumulative rounding errors in recalc_hp were causing characters using the new JSON-based STS to suddenly die.

@kevingranade kevingranade force-pushed the kevingranade-fix-sts-instant-death branch from 05a9345 to afae3ff Mar 14, 2019

@Rivet-the-Zombie Rivet-the-Zombie merged commit 5b6a055 into master Mar 14, 2019

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
gorgon-ghprb Build finished.
Details
@kevingranade

This comment has been minimized.

Copy link
Member Author

kevingranade commented Mar 14, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/game-breaking-issue-with-stats-through-skills-enabled-on-latest-builds/19213/4

@ZhilkinSerg ZhilkinSerg deleted the kevingranade-fix-sts-instant-death branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.