Paper icon indicating copy to clipboard operation
Paper copied to clipboard

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

Open Guilherme-Corvelo opened this issue 9 months ago • 17 comments

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.

Guilherme-Corvelo avatar Apr 02 '25 00:04 Guilherme-Corvelo

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.

notTamion avatar Apr 02 '25 15:04 notTamion

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?

AfonsoMendoncaJacinto avatar Apr 02 '25 21:04 AfonsoMendoncaJacinto

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

Guilherme-Corvelo avatar Apr 02 '25 22:04 Guilherme-Corvelo

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.

notTamion avatar Apr 03 '25 05:04 notTamion

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 avatar Apr 11 '25 00:04 Guilherme-Corvelo

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!

Guilherme-Corvelo avatar Apr 11 '25 00:04 Guilherme-Corvelo

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

https://github.com/user-attachments/assets/ae582df3-16c5-4aae-9b30-a4b09a6d86fa

notTamion avatar Apr 11 '25 15:04 notTamion

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

Guilherme-Corvelo avatar Apr 11 '25 22:04 Guilherme-Corvelo

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

Guilherme-Corvelo avatar May 02 '25 17:05 Guilherme-Corvelo

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.

Owen1212055 avatar May 02 '25 17:05 Owen1212055

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

Guilherme-Corvelo avatar May 02 '25 23:05 Guilherme-Corvelo

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 avatar May 02 '25 23:05 Owen1212055

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.

Owen1212055 avatar May 02 '25 23:05 Owen1212055

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

Guilherme-Corvelo avatar May 03 '25 15:05 Guilherme-Corvelo

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.

Owen1212055 avatar May 03 '25 17:05 Owen1212055

Hi @Owen1212055,

Apologies for the delay in getting back to you — a few other things came up that unfortunately prevented me from dedicating the necessary time to debug this code earlier.

In the meantime, I’ve gone over your last comment again and had a quick question regarding the configuration option you mentioned. When you say "configuration," are you referring to adding a proper boolean flag in WorldConfiguration.java (e.g., zombifiedPiglinsForgiveBeyondVanillaRange), or would a local boolean variable in the class itself suffice?

Just want to make sure I’m aligned with your expectations before proceeding with the change.

Thanks again for the thorough feedback and guidance!

Best regards, Guilherme Corvelo

Guilherme-Corvelo avatar Jun 16 '25 22:06 Guilherme-Corvelo

in the configuration file, that way server owners can configure it

electronicboy avatar Jun 16 '25 22:06 electronicboy

Hi everyone, I pushed a commit last week based on the previous feedback. If anyone gets a chance, I’d really appreciate it if you could take a quick look and let me know if it’s okay as is — or if there’s anything else I should add. Thanks again for all the support! Best regards, Guilherme Corvelo

Guilherme-Corvelo avatar Jun 24 '25 14:06 Guilherme-Corvelo

This doesn't fix the issue. Just from looking over the diff it seems like you did the same thing that i previously said will stall the server. Now its just behind a config flag.

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.

^ this is what owen asked for and would fix the issue

notTamion avatar Jun 24 '25 16:06 notTamion

Due to this being a pretty low priority issue because we are not currently breaking vanilla behavior and there being a lot of back and forth we have decided to not move forward with this PR. Thank you for your effort anyways!

notTamion avatar Jun 26 '25 19:06 notTamion