Skip to content

Fix #11919: Some Zombified Piglins don't forgive players after death #12379

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Guilherme-Corvelo
Copy link

Previously, Zombified Piglins did not properly reset their aggression after a player's death, causing some to remain hostile. To fix this, an instance of the ResetUniversalAngerTargetGoal class is now an attribute of the ZombifiedPiglin class and its start() method is called upon player death(in the startPersistentAngerTimer method). This ensures that only ZombifiedPiglin entities within a range are being reset. Additionally, the expected behavior of Zombified Piglins has been updated to consistently use this reset mechanism, ensuring proper forgiveness behavior.

Video showing the problem solved:
https://www.youtube.com/watch?v=LxUmljnKg40
The video is in Portuguese, due to the fact I am still in college and doing this for a course. Sorry for the inconvenience.

… death

Previously, Zombified Piglins did not properly reset their aggression
after a player's death, causing some to remain hostile. To fix this,
an instance of the ResetUniversalAngerTargetGoal class is now an
attribute of the ZombifiedPiglin class and its start() method is
called upon player death(in the startPersistentAngerTimer method).
This ensures that only ZombifiedPiglin entities within a range are
being reset. Additionally, the expected behavior of Zombified
Piglins has been updated to consistently use this reset mechanism,
ensuring proper forgiveness behavior.

Signed-off-by: Guilherme Corvelo <[email protected]>
@Guilherme-Corvelo Guilherme-Corvelo requested a review from a team as a code owner April 2, 2025 00:53
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Apr 2, 2025
@notTamion
Copy link
Member

notTamion commented Apr 2, 2025

This doesn't fix the issue (considering the issue isn't marked as accepted and i am personally not able to reproduce it i am not sure there even is an issue). It actually instead causes the server to deadlock if PigZombieAngerEvent is cancelled and there is at least one other zombie piglin around as startPersistentAngerTime -> resetUniversalTarget#start -> forgetCurrentTargetAndRefreshUniversalAnger -> startPersistentAngerTime after which the entire thing will repeat, now from the other piglin.

@AfonsoMendoncaJacinto
Copy link

AfonsoMendoncaJacinto commented Apr 2, 2025

This did actually fix the issue in my personal build and did not deadlock the server at all. I fear I have no idea what is happening on your side. Can you guide us with steps to reproduce that deadlock?

@Guilherme-Corvelo
Copy link
Author

Hello notTamion, I have a video I recorded for my course to confirm that this was indeed a bug. The recording was made before the 'solution' was implemented.
The video is very poor quality, but if you skip to the minute 7:04, where I died for piglins, I respawn, go through the nether portal and after a "tick" they start attacking me without me doing anything to them.
The video is still made in Portuguese, I apologize for that inconvenience.
Link: https://www.youtube.com/watch?v=FTUF9b1ps8A

@notTamion
Copy link
Member

Can you guide us with steps to reproduce that deadlock?

  1. cancel PigZombieAngerEvent via a plugin or debuggery
  2. spawn 2 zombified piglin next to each other
  3. hit one of them in survival mode
  4. see the console for StackOverflowErrors

i do not see how this could fix the issue. As the only time your code actually gets run is when the event is cancelled, in which case it leads to a deadlock. I will take another stab at reproducing the issue later.

@Guilherme-Corvelo
Copy link
Author

Hi @notTamion, thank you for the detailed reply and for taking the time to test the code.

I’ve carefully followed the exact steps you outlined to reproduce the potential deadlock:

Cancelled the PigZombieAngerEvent via a test plugin

Spawned two Zombified Piglins next to each other

Hit one of them in survival mode

Everything ran smoothly—no deadlocks, no StackOverflowError, and the server remained stable throughout. The reset behavior worked as expected.

If it helps, I’d be happy to record and share a video showing the test from start to finish, including the plugin setup and console output, to confirm that the issue doesn't occur on my end.

Let me know if you'd like me to go ahead with that.

@Guilherme-Corvelo
Copy link
Author

Also, it would be my pleasure to share the plugin I used to cancel the PigZombieAngerEvent, in case it helps with reproducing or debugging the issue on your end. Just let me know!

@notTamion
Copy link
Member

correction: it doesn't completely deadlock the server as it will exit the loop after some time due to the stated StackOverflowError. however it still hangs the server for a few seconds

2025-04-11_17-35-43.mp4

@Guilherme-Corvelo
Copy link
Author

Hi @notTamion,
Thanks for the clarification. I’ll be taking another look at the code sometime soon. Appreciate you pointing it out.

Copy link
Member

@Owen1212055 Owen1212055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue that fixes states that this is vanilla behavior and is documented on the wiki. Therefore this change will need a config option and be disabled by default.

@github-project-automation github-project-automation bot moved this from Awaiting review to Changes required in Paper PR Queue May 1, 2025
@Guilherme-Corvelo
Copy link
Author

Hello @Owen1212055,

Thank you for your comment.

Regarding the vanilla behavior, I’d just like to point out that according to the second paragraph of the Hostility section on the official Minecraft Wiki, it states the following:

“A player's death causes zombified piglins to become neutral toward the player if the gamerule forgiveDeadPlayers (true by default) is true.”

This means the expected behavior is that Zombified Piglins should become neutral after a player’s death, provided the forgiveDeadPlayers gamerule is enabled — which it is by default. This was the rationale behind the proposed fix.

Of course, I’m happy to adapt the solution based on your guidance.

Thank you again for your time and review!

Best regards,
Guilherme Corvelo

Screenshot from 2025-05-02 18-30-08

@Owen1212055
Copy link
Member

The wiki is player interpretation, if you are able to find a bug report this would allow us to get mojang's interpretation on the issue.

@Guilherme-Corvelo
Copy link
Author

Hello @Owen1212055,

Thank you again for the clarification.

Following your suggestion, I looked for an official Mojang bug report on this topic. I came across MC-148600, which specifically describes the issue where zombified piglins remain hostile even after the player dies — regardless of who dealt the killing blow.

This appears to align with the behavior I was addressing and seems to indicate that the current behavior might not be intended, or at the very least, not consistent with the gamerule forgiveDeadPlayers.

Best regards,
Guilherme Corvelo

@Owen1212055
Copy link
Member

Alright, well in which case this behavior should be under a configuration option since technically this is a vanilla break.

This also seems to be a limitation of the game rule, as the tellNeutralMobsThatIDied only extends 32 blocks out.

@Owen1212055
Copy link
Member

Yeah, infact this more looks like this is the issue.
The method above doesn't reach far enough in all cases, zombified piglins have a follow range of 35.0 so that means there are technically some pigs that will still have them targeted but not be correctly updated as they are outside the 32 block radius.

@Guilherme-Corvelo
Copy link
Author

Hi @Owen1212055,

Thanks for the follow-up and the insight into the tellNeutralMobsThatIDied range limitation — that makes a lot of sense and helps clarify the root of the issue.

Just to understand your perspective a bit better: since this patch is aimed at replicating the intended vanilla behavior (as per the gamerule forgiveDeadPlayers and the related Mojang bug report), could you clarify why this should still be behind a configuration option? Wouldn’t correcting this behavior bring Paper closer to vanilla, rather than further from it?

Really appreciate your guidance on this!

Best regards,
Guilherme Corvelo

@Owen1212055
Copy link
Member

Basically, you are "fixing" behavior on what is arguiably already vanilla behavior.

So, although you may be fixing what you consider a bug this is still a change in vanilla behavior. I would like to see a configuration option that allows us to configure the 32/10/32 block radius that way people who would like to change this behavior can do so accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Changes required
Development

Successfully merging this pull request may close these issues.

4 participants