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

HealthUtils - update #2584

Merged
merged 2 commits into from
Nov 4, 2019
Merged

HealthUtils - update #2584

merged 2 commits into from
Nov 4, 2019

Conversation

ShaneBeee
Copy link
Contributor

@ShaneBeee ShaneBeee commented Oct 20, 2019

Description

  • Removed some really old code that was used before MC 1.6 (prior to MC 1.6, the damageable classes methods used ints, 1.6 introduced using doubles. The reflection methods were introduced by Njol years ago to help handle the difference. Since we don't support below 1.9.4 they are most definitely not needed)
  • Updated the java doc notes
  • Updated to use attributes to make this class more future proof in the event the deprecated Damageable#getMaxHealth and Damageable#setMaxHealth methods are removed in future Spigot releases
  • I have tested this on both 1.9.4 and 1.14.4, everything works swimmingly.

Target Minecraft Versions: any
Requirements: none
Related Issues: none

Special Note:

This file was using CRLF, I wanted to switch it to LF but figured the PR would be hard to see what was changed.
If this is to be approved and you guys so choose, I can easily switch it to LF and push before merging.

- Removed some really old code that was used before MC 1.6
- Updated to use newer attributes to make this class more future proof in the event the deprecated Damageable#getMaxHealth and Damageable#setMaxHealth methods are removed
@ShaneBeee ShaneBeee added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Oct 20, 2019
@bensku
Copy link
Member

bensku commented Oct 21, 2019

Good call on not changing line endings. Now that we're changing entity damage code, testing that everything still works (a misc test) would be great.

- Added a test script for health utils
@ShaneBeee
Copy link
Contributor Author

Good call on not changing line endings. Now that we're changing entity damage code, testing that everything still works (a misc test) would be great.

Okay I added the health utils test for a pig, ran it using gradlew skriptTest and the test passed for all three versions

@bensku bensku merged commit cac4eff into SkriptLang:master Nov 4, 2019
@ShaneBeee ShaneBeee deleted the feature/health-utils-refresh branch November 4, 2019 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants