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

fix: fleeing entities must have a CharacterMovement component #85

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

skaldarnar
Copy link
Contributor

Problem: An entity with a death animation may already be stripped off its CharacterMovementComponent when receiving additional damage. This may cause an NPE when accessing the component without guarding check.

Solution: Require the CharacterMovementComponent in the event handler to ensure that it is present. Skip the event handler if the component is not present, as fleeing without character movement does make much sense anyways.

**Problem:** An entity with a death animation may already be stripped off its CharacterMovementComponent when receiving additional damage. This may cause an NPE when accessing the component without guarding check.

**Solution:** Require the CharacterMovementComponent in the event handler to ensure that it is present. Skip the event handler if the component is not present, as fleeing without character movement does make much sense anyways.
@skaldarnar
Copy link
Contributor Author

@casals @jdrueckert I printed the full description (entity dump) of the entity right before the crash and noticed that the CharacterMovementComponent was just being removed from the entity. Upon closer inspection I noticed that it holds the DestroyAtAnimationEndComponent component, which is added by the same piece of code that already removes the character movement component. If I understand it correctly this is to avoid having the entity move while their death animation is played.

https://github.com/Terasology/WildAnimals/blob/develop/src/main/java/org/terasology/wildAnimals/Death/DeathSystem.java#L68-L91

This might just hint to another underlying issue, though. Please also check out Terasology/Health#98 which tries to fix one possible root cause that might confuse systems (unnecessarily).

@jdrueckert jdrueckert merged commit 33560b2 into develop Dec 2, 2021
@jdrueckert jdrueckert deleted the fix/npe-on-dying-entity branch December 2, 2021 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants