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

Heal signs do not account for custom max health values #5751

Closed
ImDarkLaw opened this issue Mar 31, 2024 · 3 comments · Fixed by #5752
Closed

Heal signs do not account for custom max health values #5751

ImDarkLaw opened this issue Mar 31, 2024 · 3 comments · Fixed by #5752
Labels
bug: confirmed Confirmed bugs in EssentialsX. status: open to PR Low priority enhancements that anyone is welcome to contribute.

Comments

@ImDarkLaw
Copy link
Contributor

Type of bug

Other unexpected behaviour

/ess dump all output

https://essentialsx.net/dump.html?id=4d77c9610f5d470ab46f0142b21415fd

Error log (if applicable)

No response

Bug description

The onSignInteract method inside the SignHeal class responsible for handling interactions with heal signs, currently, sets the player's health to 20 without considering potential increases in amount of health due to the generic max health attribute.

Steps to reproduce

Suggested course of action to reproduce this behavior:

  1. Ensure heal signs are enabled in the config:
enabledSigns:
  - heal
  1. Create a heal sign by placing a sign and typing "[Heal]" on the first line.
  2. Use /attribute @p minecraft:generic.max_health base set 40 to change our base maximum health value, in this case, doubling it to 40.
  3. Right click on the sign and notice that our health is set back to 20 and the health buff removed.

Expected behaviour

One would expect that onSignInteract the player's health would be set to the maximum amount respecting modified maximum health values.

Actual behaviour

Upon interacting with a heal sign, health value is being set to 20 no matter what, without accounting for an increased amount of hearts.

Additional Information

No response

@ImDarkLaw ImDarkLaw added the bug: unconfirmed Potential bugs that need replicating to verify. label Mar 31, 2024
@Chew
Copy link
Member

Chew commented Mar 31, 2024

Interesting how this is fixed in the /heal but not here. Good catch!

https://github.com/EssentialsX/Essentials/blob/2.x/Essentials/src/main/java/com/earth2me/essentials/commands/Commandheal.java#L43-L76

@ImDarkLaw
Copy link
Contributor Author

Yes, it seems like there's already a solution in the /heal command implementation.

I'm currently in the process of implementing a quick fix for the SignHeal class to address this.

The proposed solution involves retrieving the player's maximum health attribute and setting their health accordingly, instead of the hardcoded 20 value. I've shared the implementation details in this pastebin. Let me know your thoughts and i can proceed to submit a PR.

@pop4959 pop4959 added bug: confirmed Confirmed bugs in EssentialsX. and removed bug: unconfirmed Potential bugs that need replicating to verify. labels Mar 31, 2024
@pop4959
Copy link
Member

pop4959 commented Mar 31, 2024

Something like that should be fine. Doesn't hurt to open a PR so more users can test.

I will note that this may require a version check for feature / API not present in old versions of the game - but aside from that this fix is fairly straightforward (set the proper health, not just 20).

@pop4959 pop4959 added the status: open to PR Low priority enhancements that anyone is welcome to contribute. label Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: confirmed Confirmed bugs in EssentialsX. status: open to PR Low priority enhancements that anyone is welcome to contribute.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants