PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.69k stars 2.26k forks source link

[1.17 Build #46] Zombies holding eggs (Due to 1.17 bug of chickens spawning underground) #5994

Closed bun20381 closed 3 years ago

bun20381 commented 3 years ago

Is your feature request related to a problem?

Yes, due to chickens spawning underground. It is leading to 100's and 100's of zombies holding eggs [Vanilla bug] not despawning underground which leads to TPS drops over time.
Can confirm this is a vanilla bug though, and not related to Paper, as it occurs on Spigot server and Vanilla server jars as well. 2021-06-24_17 29 19

Describe the solution you'd like.

Allow an option in the config file to disallow zombies from picking up eggs until Mojang provides a fix.

Describe alternatives you've considered.

I tried searching for a plugin that could achieve the same effect, but to no avail. There is also the option of not allowing mob griefing, however that ruins many Minecraft aspects.

Agreements

Other

No response 2021-06-24_17 50 53

Machine-Maker commented 3 years ago

Can you link the mojira issue for underground chucking spawning?

The better thing, would be to fix them spawning underground, instead of trying to fix anything resulting from that.

bun20381 commented 3 years ago

https://bugs.mojang.com/browse/MC-229490?jql=project%20%3D%20MC%20AND%20affectedVersion%20%3D%201.17%20AND%20text%20~%20%22Chicken%22 - The bug that is most likely leading to this occurring. Community consensus confirmed bug.

Machine-Maker commented 3 years ago

I can't seem to replicate the chickens spawning underground. I'm not using the Caves & Cliffs datapack, are you? If you had some solid repro steps to get them to spawn underground, that'd be appreciated.

bun20381 commented 3 years ago

No, data pack. Just 1.17 and nothing else. Not sure how to give solid reproduce tips, it just happens on every single world I've generated. It's a confirmed Mojang bug, and maybe it's unique to certain seeds, hard to say without Mojang looking into it more. I've definitely had fairly consistent results though. Keep in mind tho, this doesn't happen immediately. Usually starts randomly, and can take hours and hours before chickens begin spawning underground and leading to zombies holding eggs and not despawning.

Also note: I'm using default Bukkit, Spigot, Paper and Vanilla config files.

Here's a world seed and coords I know for a fact it occurs on. Seed: -3178201063967011175 Coords: Around X: 400 Y: 63 Z: -300

Chew commented 3 years ago

The reason chickens "spawn" underground is because of baby zombie Jockies, is it not? The baby zombie dies, somehow, and the chicken is left. How or why it dies is beyond me. Unless this is a completely different issue I'm not sure how else they would spawn

BillyGalbreath commented 3 years ago

Turns out this is not even a vanilla bug even though it was reported to Mojang. It's caused by CraftBukkit's addition of a persist field to Entities.

Here's how I fixed it: https://github.com/pl3xgaming/Purpur/commit/881d7c221aff2f65e198fee34557999202bb5c7f

I was easily able to reproduce the problem on Purpur because of my jockey chance options (I just set them to 100% and allowed non-babies to be jockeys). Otherwise you'd be waiting all day for a 0.05% chance when a baby spawns. :P

Scratch all this. I was close, but wrong. See my next comment.

electronicboy commented 3 years ago

Remind me when we hard fork to nuke the persistence changes from CB and make it a tristate...

On Sat, 26 Jun 2021, 04:19 BillyGalbreath, @.***> wrote:

Turns out this is not even a vanilla bug. It's caused by CraftBukkit's addition of a persist field to Entities.

Here's how I fixed it: @.*** https://github.com/pl3xgaming/Purpur/commit/881d7c221aff2f65e198fee34557999202bb5c7f

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/5994#issuecomment-868938201, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZFQEXMUT66S5FUX5O3TUVBKTANCNFSM47IZS3VA .

BillyGalbreath commented 3 years ago

Actually, looks like I was in the right area, but wrong about the problem and fix.

Turns out this is a remapping issue. After talking internally about the /data command, I set out to find out why the jockey flag was always showing false. Here is what I found out:

The field Chicken#isChickenJockey remaps back to EntityChicken#bZ.

The field Mob#persistenceRequired remaps back to EntityInsentient#bZ

Inside Chicken's aiStep() method CB added this piece of code:

        // CraftBukkit start
        if (this.isChickenJockey()) {
            this.persistenceRequired = !this.removeWhenFarAway(0);
        }
        // CraftBukkit end

And Chicken#removeWhenFarAway() just returns the value of the Chicken#isChickenJockey field.

So this effectively remaps back to

        // CraftBukkit start
        if (this.isChickenJockey) {
            this.isChickenJockey = !this.isChickenJockey;
        }
        // CraftBukkit end

And boom. There you have it. The field gets toggled back to false on the first aiStep() tick, which marks the chicken as persistent and has the ability to lay eggs.

drops mic

walks off stage

Edit: I just decompiled the Spigot and CraftBukkit jars and can confirm that the bug is indeed upstream in those jars. So it is definitely a decompile error on CraftBukkit's part.

BillyGalbreath commented 3 years ago

Here's an updated fix I came up with. I simply use a different named field for chicken jockeys. I'm sure someone smarter can come up with a better way (like maybe fixing the decompile/remapping issue at the root in paperweight)

https://github.com/pl3xgaming/Purpur/blob/bd133948ec082ba79ecdd4117b18340740084a9a/patches/server/0215-Fix-MC-229490.patch

kennytv commented 3 years ago

You clearly have no idea how any of the Paper and Spigot development processes work - including updating to new Minecraft releases, updating upstream, what a GitHub project and todo column is what it is used for, how issues are resolved, and why Paper exists in the first place. If you want to find out, you can join our Discord and continue the discussion there... or look below.

electronicboy commented 3 years ago

1) adding to todo was for a pending investigation, we avoid gross workarounds for the sake of workarounds, stuff takes time, this is not our day job and so often takes the back seat towards quite practically everything else in our lives. Conversations also happen outside of the issue tracker to prevent the flood of emails for essentially useless back and forth.

2) See 1, we thank him for the work and are glad he spotted the issue, with everything else in the works, we had other priorities, so this definitely helped to solve the issue.

3) The workaround in part is already needed in paperweight to deal with inconsistencies which spigot causes with their mappings, but, also applies a wider set of fixes automatically which are pretty much required with spigots tooling

4) We have tried to work with upstream in the past, however no longer bother after being repeatedly ignored over the years as well as blatantly insulted and dismissed by them, our inclination to spend time debugging their tooling issues when they refuse to listen to us elsewhere is pretty much a non-starter.

5) determining what patches are still needed is a constant cycle of working over the top of vanilla, a code-base we have 0 control over, especially when much of our criticism which leads to the patches we make goes to mojang to allow them to approve the code for the entire ecosystem.

1.17 itself was not a great release, and on the basis that this is not a full-time job for us, we weren't exactly inclined to work ourselves to burnout for the sake of tossing out a release, this is an open-source project in which pretty much anybody can contribute to and become an active part of the team to help with this.