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

Living entities now take fire damage #888

Merged
merged 2 commits into from Apr 11, 2018

Conversation

@smcconke
Copy link
Contributor

@smcconke smcconke commented Mar 28, 2018

This pull request aims to implement fire damage for living entities:

Observed Behavior: Entities would not catch fire when standing in a fire block. Since entities were not being detected in the fire block, fire ticks were not updated and the entity takes zero damage.

Expected Behavior: Entities will catch fire when standing in fire. While standing in fire, the entity will take 1 tick of damage, equivalence of half a heart in-game. Once the entity is no longer standing in fire, they will receive 8 ticks of damage, equivalence of four hearts in-game. If the entity were to enter the fire again, they will continue to take damage and add an additional 8 ticks of fire damage once they exit.

This was implemented in a similar manner as lava damage, checking if the entity is standing in fire and dealing damage to the entity. We must check if the entity is dead or not, that way if they were to die in the fire they do not receive the fire ticks after death. Once exiting the fire if they were previously standing in fire, then their fire ticks will increment by 180.

@@ -309,6 +313,18 @@ public void pulse() {
damage(1, DamageCause.SUFFOCATION);
}

// fire damage
if (getLocation().getBlock().getType() == Material.FIRE) {
damage(1, DamageCause.FIRE);

This comment has been minimized.

@aramperes

aramperes Mar 28, 2018
Member

The pulse method is executed once per tick, which is every 50ms. This check would damage the entity 20 times per second if they step in fire. Also, the setFireTicks call below would increment the fire duration by 180 ticks every tick it isn't on fire.

edit: actually the invincibility timer takes care of the damaging, but the fireTicks counter issue would still be problematic

This comment has been minimized.

@smcconke

smcconke Mar 28, 2018
Author Contributor

The fireTicks is only incremented when the entity has just recently exited the fire block, so by assigning False to stoodInFire we won't have a fireTicks counter incrementing by 180 with every tick. Sorry about the confusion, I tested this on a live server where the damage in the fire and out of the fire proved successful. Please let me know if I should make any changes.

This comment has been minimized.

@aramperes

aramperes Mar 28, 2018
Member

Hey, it's my bad for the confusion. After putting second thought to this, the logic does make sense 😊

Although you can simplify the } else { if (stoodInFire) { branches to an else-if statement.

This comment has been minimized.

@smcconke

smcconke Mar 28, 2018
Author Contributor

Thank you for the feedback though! It seems I will have to do some more work in this implementation.

@Pr0methean
Copy link
Contributor

@Pr0methean Pr0methean commented Mar 28, 2018

Shouldn't we be searching at least a 3x2x3 neighborhood for fire? I'm pretty sure standing on a block that's next to a fire block will ignite an entity in vanilla.

@aramperes
Copy link
Member

@aramperes aramperes commented Mar 28, 2018

@Pr0methean I don't think so about standing next to a block, but it should take the bounding-box of the entity into consideration.

@smcconke
Copy link
Contributor Author

@smcconke smcconke commented Mar 28, 2018

I was thinking this as well. I was basing this off of the lava implementation and noticed you will not take damage or catch fire if you are adjacent to the block.

@smcconke
Copy link
Contributor Author

@smcconke smcconke commented Apr 6, 2018

Hi All,

My most recent commit took into consideration the bounding-box of the entity for fire and lava. I tried to refactor the code in order to incorporate the lava damage with the fire. Please let me know if I can make any changes.

Thanks.

@aramperes aramperes merged commit 1512bc9 into GlowstoneMC:dev Apr 11, 2018
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details
@aramperes
Copy link
Member

@aramperes aramperes commented Apr 11, 2018

Works like a charm, thank you for your contribution (and apologies for the delay)

XuZhen86 added a commit to Glowminers/Glowstone that referenced this pull request Apr 16, 2018
Shane McConkey:
Kill: GlowstoneMC#888

Xu Zhen:
Kill: GlowstoneMC#897
Assist: Jordan Murray
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants