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 - Fix entity damage effect #2761

Merged
merged 3 commits into from
Feb 3, 2020
Merged

HealthUtils - Fix entity damage effect #2761

merged 3 commits into from
Feb 3, 2020

Conversation

ShaneBeee
Copy link
Contributor

Description

This PR fixes an issue with a change in HealthUtils, which prevents entities taking damage when using the damage effect, due to setting the entities health rather than damaging them. Their health is dropped, but they won't visibly take damage.


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

@ShaneBeee ShaneBeee added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jan 13, 2020
@Whimsyturtle
Copy link
Member

This will cause other issues... maybe use setHealth() and e.damage(0)? To get the damage effect

@ShaneBeee
Copy link
Contributor Author

ShaneBeee commented Jan 13, 2020

This will cause other issues... maybe use setHealth() and e.damage(0)? To get the damage effect

what issue would this cause?
Damaging an entity should well, damage an entity, not set its health.
This shouldn't have been changed in the first place.

@Whimsyturtle
Copy link
Member

This will cause other issues... maybe use setHealth() and e.damage(0)? To get the damage effect

what issue would this cause?
Damaging an entity should well, damage an entity, not set its health.
This shouldn't have been changed in the first place.

Look at the Java CI results and you'll see that this code fails the syntax test https://github.com/SkriptLang/Skript/blob/master/src/test/skript/tests/syntaxes/effects/EffHealth.sk. I guess I exchanged 1 bug for another.

@ShaneBeee
Copy link
Contributor Author

ShaneBeee commented Jan 13, 2020

Look at the Java CI results and you'll see that this code fails the syntax test https://github.com/SkriptLang/Skript/blob/master/src/test/skript/tests/syntaxes/effects/EffHealth.sk. I guess I exchanged 1 bug for another.

That test should be re-written then. The healthUtils class was written properly before. We shouldn't be changing classes strictly to get a test to run.

- This is a spigot/MC issue (not a skript issue), damaging an entity in quick succession to many times causes the entity to not take all damage
@bensku bensku merged commit cd2abe2 into SkriptLang:master Feb 3, 2020
@ShaneBeee ShaneBeee deleted the fix/healthUtils branch February 9, 2020 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants