From 9e250168e864678de0ab2914cb53503a5c927b11 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sat, 28 Jul 2018 12:13:52 -0400 Subject: [PATCH 1/5] Always process chunk removal in removeEntity Spigot might skip chunk registration changes in removeEntity which can keep them in the chunk when they shouldnt be if done during entity ticking. Should fix some cases where "Entity is still in another chunk section" Related to #1223 --- ...rocess-chunk-removal-in-removeEntity.patch | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Spigot-Server-Patches/0342-Always-process-chunk-removal-in-removeEntity.patch diff --git a/Spigot-Server-Patches/0342-Always-process-chunk-removal-in-removeEntity.patch b/Spigot-Server-Patches/0342-Always-process-chunk-removal-in-removeEntity.patch new file mode 100644 index 0000000000..c3b52fe623 --- /dev/null +++ b/Spigot-Server-Patches/0342-Always-process-chunk-removal-in-removeEntity.patch @@ -0,0 +1,33 @@ +From 529731e4258b21416a555ed6c2d0200b8449f1d7 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Sat, 28 Jul 2018 12:09:20 -0400 +Subject: [PATCH] Always process chunk removal in removeEntity + +Spigot might skip chunk registration changes in removeEntity +which can keep them in the chunk when they shouldnt be if done +during entity ticking. + +diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java +index d2d2ab794..df98a4f44 100644 +--- a/src/main/java/net/minecraft/server/World.java ++++ b/src/main/java/net/minecraft/server/World.java +@@ -1272,7 +1272,7 @@ public abstract class World implements IBlockAccess { + this.everyoneSleeping(); + } + +- if (!guardEntityList) { // Spigot - It will get removed after the tick if we are ticking ++ // if (!guardEntityList) { // Spigot - It will get removed after the tick if we are ticking // Paper - move down + int i = entity.ab; + int j = entity.ad; + +@@ -1280,6 +1280,7 @@ public abstract class World implements IBlockAccess { + this.getChunkAt(i, j).b(entity); + } + ++ if (!guardEntityList) { // Spigot - It will get removed after the tick if we are ticking // Paper - always remove from current chunk above + // CraftBukkit start - Decrement loop variable field if we've already ticked this entity + int index = this.entityList.indexOf(entity); + if (index != -1) { +-- +2.18.0 + From d98f6afef09b191038e37661c28744d84f259fe6 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sat, 28 Jul 2018 12:26:36 -0400 Subject: [PATCH 2/5] Ignore Dead Entities in entityList iteration A spigot change delays removal of entities from the entity list. This causes a change in behavior from Vanilla where getEntities type methods will return dead entities that they shouldn't otherwise be doing. This will ensure that dead entities are skipped from iteration since they shouldn't of been in the list in the first place. --- ...ead-Entities-in-entityList-iteration.patch | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 Spigot-Server-Patches/0343-Ignore-Dead-Entities-in-entityList-iteration.patch diff --git a/Spigot-Server-Patches/0343-Ignore-Dead-Entities-in-entityList-iteration.patch b/Spigot-Server-Patches/0343-Ignore-Dead-Entities-in-entityList-iteration.patch new file mode 100644 index 0000000000..d891b6e9f2 --- /dev/null +++ b/Spigot-Server-Patches/0343-Ignore-Dead-Entities-in-entityList-iteration.patch @@ -0,0 +1,103 @@ +From 3e0a26eb873fcf1fa259f775320ae0a6a6346ec0 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Sat, 28 Jul 2018 12:18:27 -0400 +Subject: [PATCH] Ignore Dead Entities in entityList iteration + +A spigot change delays removal of entities from the entity list. +This causes a change in behavior from Vanilla where getEntities type +methods will return dead entities that they shouldn't otherwise be doing. + +This will ensure that dead entities are skipped from iteration since +they shouldn't of been in the list in the first place. + +diff --git a/src/main/java/com/destroystokyo/paper/PaperCommand.java b/src/main/java/com/destroystokyo/paper/PaperCommand.java +index ecd1c65a9..1898ab897 100644 +--- a/src/main/java/com/destroystokyo/paper/PaperCommand.java ++++ b/src/main/java/com/destroystokyo/paper/PaperCommand.java +@@ -128,6 +128,7 @@ public class PaperCommand extends Command { + List entities = world.entityList; + entities.forEach(e -> { + MinecraftKey key = EntityTypes.getKey(e); ++ if (e.shouldBeRemoved) return; // Paper + + MutablePair> info = list.computeIfAbsent(key, k -> MutablePair.of(0, Maps.newHashMap())); + ChunkCoordIntPair chunk = new ChunkCoordIntPair(e.getChunkX(), e.getChunkZ()); +diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java +index 89f9bd347..d163acd8f 100644 +--- a/src/main/java/net/minecraft/server/Entity.java ++++ b/src/main/java/net/minecraft/server/Entity.java +@@ -124,6 +124,7 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper + protected boolean E; + private boolean aw; + public boolean dead; ++ public boolean shouldBeRemoved; // Paper + public float width; + public float length; + public float I; +diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java +index df98a4f44..8f82fbf67 100644 +--- a/src/main/java/net/minecraft/server/World.java ++++ b/src/main/java/net/minecraft/server/World.java +@@ -1279,6 +1279,7 @@ public abstract class World implements IBlockAccess { + if (entity.aa && this.isChunkLoaded(i, j, true)) { + this.getChunkAt(i, j).b(entity); + } ++ entity.shouldBeRemoved = true; // Paper + + if (!guardEntityList) { // Spigot - It will get removed after the tick if we are ticking // Paper - always remove from current chunk above + // CraftBukkit start - Decrement loop variable field if we've already ticked this entity +@@ -2633,6 +2634,7 @@ public abstract class World implements IBlockAccess { + + while (iterator.hasNext()) { + Entity entity = (Entity) iterator.next(); ++ if (entity.shouldBeRemoved) continue; // Paper + + if (oclass.isAssignableFrom(entity.getClass()) && predicate.apply((T) entity)) { + arraylist.add(entity); +@@ -2719,6 +2721,7 @@ public abstract class World implements IBlockAccess { + + while (iterator.hasNext()) { + Entity entity = (Entity) iterator.next(); ++ if (entity.shouldBeRemoved) continue; // Paper + // CraftBukkit start - Split out persistent check, don't apply it to special persistent mobs + if (entity instanceof EntityInsentient) { + EntityInsentient entityinsentient = (EntityInsentient) entity; +diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +index 210e3bc4e..e6ecd1796 100644 +--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java ++++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +@@ -678,6 +678,7 @@ public class CraftWorld implements World { + for (Object o : world.entityList) { + if (o instanceof net.minecraft.server.Entity) { + net.minecraft.server.Entity mcEnt = (net.minecraft.server.Entity) o; ++ if (mcEnt.shouldBeRemoved) continue; // Paper + Entity bukkitEntity = mcEnt.getBukkitEntity(); + + // Assuming that bukkitEntity isn't null +@@ -696,6 +697,7 @@ public class CraftWorld implements World { + for (Object o : world.entityList) { + if (o instanceof net.minecraft.server.Entity) { + net.minecraft.server.Entity mcEnt = (net.minecraft.server.Entity) o; ++ if (mcEnt.shouldBeRemoved) continue; // Paper + Entity bukkitEntity = mcEnt.getBukkitEntity(); + + // Assuming that bukkitEntity isn't null +@@ -720,6 +722,7 @@ public class CraftWorld implements World { + + for (Object entity: world.entityList) { + if (entity instanceof net.minecraft.server.Entity) { ++ if (((net.minecraft.server.Entity) entity).shouldBeRemoved) continue; // Paper + Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity(); + + if (bukkitEntity == null) { +@@ -742,6 +745,7 @@ public class CraftWorld implements World { + + for (Object entity: world.entityList) { + if (entity instanceof net.minecraft.server.Entity) { ++ if (((net.minecraft.server.Entity) entity).shouldBeRemoved) continue; // Paper + Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity(); + + if (bukkitEntity == null) { +-- +2.18.0 + From e674d3a00f651dc0947ebb1393de072a71960baf Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 29 Jul 2018 12:04:09 -0400 Subject: [PATCH 3/5] If Entity is added to chunk, look up the chunk if current isnt set Hopefully will (f)ix #1280... I'm suspicious that Citizens isn't calling things in the same order and causes the current chunk to not be set, which then bugs removals. Though this doesn't make any sense to me, so this likely won't fix it... But if the isAddedToChunk is true, we really should be returning a chunk anyways if its loaded. --- ...to-current-Chunk-for-Entity-and-Bloc.patch | 21 +++++++++++++------ ...entation-of-tile-entity-removal-list.patch | 15 +------------ 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Spigot-Server-Patches/0006-Store-reference-to-current-Chunk-for-Entity-and-Bloc.patch b/Spigot-Server-Patches/0006-Store-reference-to-current-Chunk-for-Entity-and-Bloc.patch index eb7b178c9b..b32bc55c29 100644 --- a/Spigot-Server-Patches/0006-Store-reference-to-current-Chunk-for-Entity-and-Bloc.patch +++ b/Spigot-Server-Patches/0006-Store-reference-to-current-Chunk-for-Entity-and-Bloc.patch @@ -1,4 +1,4 @@ -From 67c7a52969344e723be71cbd2fed330ca8ffa28d Mon Sep 17 00:00:00 2001 +From ab050c94d0a74f405ae49fce43faf3ee265d9a39 Mon Sep 17 00:00:00 2001 From: Aikar Date: Wed, 4 Jul 2018 02:10:36 -0400 Subject: [PATCH] Store reference to current Chunk for Entity and Block @@ -8,7 +8,7 @@ This enables us a fast reference to the entities current chunk instead of having to look it up by hashmap lookups. diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java -index 4bbebb25af..ea167a17bb 100644 +index 4bbebb25a..ea167a17b 100644 --- a/src/main/java/net/minecraft/server/Chunk.java +++ b/src/main/java/net/minecraft/server/Chunk.java @@ -25,7 +25,7 @@ public class Chunk { @@ -81,9 +81,18 @@ index 4bbebb25af..ea167a17bb 100644 // Keep this synced up with World.a(Class) if (entity instanceof EntityInsentient) { diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java -index 06c72b95f3..c107bd767f 100644 +index 06c72b95f..0e3a94ab8 100644 --- a/src/main/java/net/minecraft/server/Entity.java +++ b/src/main/java/net/minecraft/server/Entity.java +@@ -121,7 +121,7 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper + private static final DataWatcherObject aC = DataWatcher.a(Entity.class, DataWatcherRegistry.h); + private static final DataWatcherObject aD = DataWatcher.a(Entity.class, DataWatcherRegistry.h); + private static final DataWatcherObject aE = DataWatcher.a(Entity.class, DataWatcherRegistry.h); +- public boolean aa; ++ public boolean aa; public boolean isAddedToChunk() { return aa; } // Paper - OBFHELPER + public int ab; public int getChunkX() { return ab; } // Paper - OBFHELPER + public int ac; public int getChunkY() { return ac; } // Paper - OBFHELPER + public int ad; public int getChunkZ() { return ad; } // Paper - OBFHELPER @@ -1703,6 +1703,38 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper } @@ -98,7 +107,7 @@ index 06c72b95f3..c107bd767f 100644 + */ + public Chunk getCurrentChunk() { + final Chunk chunk = currentChunk != null ? currentChunk.get() : null; -+ return chunk != null && chunk.isLoaded() ? chunk : null; ++ return chunk != null && chunk.isLoaded() ? chunk : (isAddedToChunk() ? world.getChunkIfLoaded(getChunkX(), getChunkZ()) : null); + } + /** + * Returns the chunk at the location, using the entities local cache if avail @@ -124,7 +133,7 @@ index 06c72b95f3..c107bd767f 100644 private MinecraftKey entityKey = getMinecraftKey(); diff --git a/src/main/java/net/minecraft/server/TileEntity.java b/src/main/java/net/minecraft/server/TileEntity.java -index 0176ca530c..29069b753e 100644 +index 0176ca530..29069b753 100644 --- a/src/main/java/net/minecraft/server/TileEntity.java +++ b/src/main/java/net/minecraft/server/TileEntity.java @@ -28,6 +28,14 @@ public abstract class TileEntity implements KeyedObject { @@ -143,7 +152,7 @@ index 0176ca530c..29069b753e 100644 private MinecraftKey tileEntityKey = getMinecraftKey(); diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java -index c5a194ffea..833e3111de 100644 +index c5a194ffe..833e3111d 100644 --- a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java +++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java @@ -9,6 +9,7 @@ import java.util.UUID; diff --git a/Spigot-Server-Patches/0059-Change-implementation-of-tile-entity-removal-list.patch b/Spigot-Server-Patches/0059-Change-implementation-of-tile-entity-removal-list.patch index a6ecfb3a4c..ec39b43024 100644 --- a/Spigot-Server-Patches/0059-Change-implementation-of-tile-entity-removal-list.patch +++ b/Spigot-Server-Patches/0059-Change-implementation-of-tile-entity-removal-list.patch @@ -1,22 +1,9 @@ -From bc94a21d1941e2d5d173d0be455acef7c1b02ce8 Mon Sep 17 00:00:00 2001 +From abef5d63377c635fd7072ddf5e166aff23a86ead Mon Sep 17 00:00:00 2001 From: Joseph Hirschfeld Date: Thu, 3 Mar 2016 02:39:54 -0600 Subject: [PATCH] Change implementation of (tile)entity removal list -diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java -index aadc426fd..584501787 100644 ---- a/src/main/java/net/minecraft/server/Entity.java -+++ b/src/main/java/net/minecraft/server/Entity.java -@@ -122,7 +122,7 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper - private static final DataWatcherObject aC = DataWatcher.a(Entity.class, DataWatcherRegistry.h); - private static final DataWatcherObject aD = DataWatcher.a(Entity.class, DataWatcherRegistry.h); - private static final DataWatcherObject aE = DataWatcher.a(Entity.class, DataWatcherRegistry.h); -- public boolean aa; -+ public boolean aa; public boolean isAddedToChunk() { return aa; } // Paper - OBFHELPER - public int ab; public int getChunkX() { return ab; } // Paper - OBFHELPER - public int ac; public int getChunkY() { return ac; } // Paper - OBFHELPER - public int ad; public int getChunkZ() { return ad; } // Paper - OBFHELPER diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java index e85ed2e33..89197281e 100644 --- a/src/main/java/net/minecraft/server/World.java From c6d6773678486fc24ec0a784b37da9fc904bca61 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 29 Jul 2018 12:06:35 -0400 Subject: [PATCH 4/5] Always move Entity to its new Chunk even if unloaded Vanilla logic here would allow us to remvoe an entity from its current chunk, and if it was going to move into an unloaded chunk, that entity would not be added to the unloaded chunk. This is bad because this will result in the entity being lost! In almost all cases, the chunk will be loaded, but in the event it wasn't, instead of losing the entity, load the chunk to add the entity to it. --- .../0157-Chunk-registration-fixes.patch | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Spigot-Server-Patches/0157-Chunk-registration-fixes.patch b/Spigot-Server-Patches/0157-Chunk-registration-fixes.patch index 9b90846d88..432bdade41 100644 --- a/Spigot-Server-Patches/0157-Chunk-registration-fixes.patch +++ b/Spigot-Server-Patches/0157-Chunk-registration-fixes.patch @@ -1,4 +1,4 @@ -From e12d715554fa2684d9c5c7dcfc765fdf66f02654 Mon Sep 17 00:00:00 2001 +From 8133b92590c9b861b7342bcc1079c23f2e106450 Mon Sep 17 00:00:00 2001 From: Aikar Date: Wed, 21 Sep 2016 22:54:28 -0400 Subject: [PATCH] Chunk registration fixes @@ -8,7 +8,7 @@ World checks and the Chunk Add logic are inconsistent on how Y > 256, < 0, is tr Keep them consistent diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index 6e37c4366..000d2eeb9 100644 +index 6e37c4366..ea24a6e4c 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -1770,7 +1770,7 @@ public abstract class World implements IBlockAccess { @@ -20,6 +20,15 @@ index 6e37c4366..000d2eeb9 100644 int k = MathHelper.floor(entity.locZ / 16.0D); if (!entity.aa || entity.ab != i || entity.ac != j || entity.ad != k) { +@@ -1778,7 +1778,7 @@ public abstract class World implements IBlockAccess { + this.getChunkAt(entity.ab, entity.ad).a(entity, entity.ac); + } + +- if (!entity.bD() && !this.isChunkLoaded(i, k, true)) { ++ if (false && !entity.bD() && !this.isChunkLoaded(i, k, true)) { // Paper - Always send entities into a new chunk, never lose them + entity.aa = false; + } else { + this.getChunkAt(i, k).a(entity); -- 2.18.0 From f35324dbed150269aeb30e0b06366e72336d68e2 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 29 Jul 2018 12:10:20 -0400 Subject: [PATCH 5/5] Always process chunk registration after moving This will help guarantee that entities are always in the chunk that they are currently located at. Should hopefully also fix Citizens triggering the "Saved to wrong chunk" message --- ...cess-chunk-registration-after-moving.patch | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 Spigot-Server-Patches/0344-Always-process-chunk-registration-after-moving.patch diff --git a/Spigot-Server-Patches/0344-Always-process-chunk-registration-after-moving.patch b/Spigot-Server-Patches/0344-Always-process-chunk-registration-after-moving.patch new file mode 100644 index 0000000000..a50d657706 --- /dev/null +++ b/Spigot-Server-Patches/0344-Always-process-chunk-registration-after-moving.patch @@ -0,0 +1,67 @@ +From e7b5ab4f8db7b8af7de3f0f2cd3bc23fa2a2c83a Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Sun, 29 Jul 2018 11:58:05 -0400 +Subject: [PATCH] Always process chunk registration after moving + +This will help guarantee that entities are always in the +chunk that they are currently located at. + +diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java +index 45e149f4a..fce677f9f 100644 +--- a/src/main/java/net/minecraft/server/Entity.java ++++ b/src/main/java/net/minecraft/server/Entity.java +@@ -341,6 +341,7 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper + this.locX = d0; + this.locY = d1; + this.locZ = d2; ++ if (valid) world.entityJoinedWorld(this, false); // Paper - ensure Entity is moved to its proper chunk + float f = this.width / 2.0F; + float f1 = this.length; + +@@ -986,6 +987,7 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper + this.locX = (axisalignedbb.a + axisalignedbb.d) / 2.0D; + this.locY = axisalignedbb.b; + this.locZ = (axisalignedbb.c + axisalignedbb.f) / 2.0D; ++ if (valid) world.entityJoinedWorld(this, false); // Paper - ensure Entity is moved to its proper chunk + } + + protected SoundEffect ae() { +diff --git a/src/main/java/net/minecraft/server/EntityArrow.java b/src/main/java/net/minecraft/server/EntityArrow.java +index 8a9e16ad6..0b1d7a086 100644 +--- a/src/main/java/net/minecraft/server/EntityArrow.java ++++ b/src/main/java/net/minecraft/server/EntityArrow.java +@@ -360,6 +360,7 @@ public abstract class EntityArrow extends Entity implements IProjectile { + this.inGround = true; + this.shake = 7; + this.setCritical(false); ++ if (valid) world.entityJoinedWorld(this, false); // Paper - ensure Entity is moved to its proper chunk + if (iblockdata.getMaterial() != Material.AIR) { + this.av.a(this.world, blockposition, iblockdata, (Entity) this); + } +diff --git a/src/main/java/net/minecraft/server/EntityLeash.java b/src/main/java/net/minecraft/server/EntityLeash.java +index 7bd1c73bf..10a507595 100644 +--- a/src/main/java/net/minecraft/server/EntityLeash.java ++++ b/src/main/java/net/minecraft/server/EntityLeash.java +@@ -31,6 +31,7 @@ public class EntityLeash extends EntityHanging { + this.locX = (double) this.blockPosition.getX() + 0.5D; + this.locY = (double) this.blockPosition.getY() + 0.5D; + this.locZ = (double) this.blockPosition.getZ() + 0.5D; ++ if (valid) world.entityJoinedWorld(this, false); // Paper - ensure Entity is moved to its proper chunk + } + + public void setDirection(EnumDirection enumdirection) {} +diff --git a/src/main/java/net/minecraft/server/EntityShulker.java b/src/main/java/net/minecraft/server/EntityShulker.java +index 3ce843199..ad7c95924 100644 +--- a/src/main/java/net/minecraft/server/EntityShulker.java ++++ b/src/main/java/net/minecraft/server/EntityShulker.java +@@ -390,6 +390,7 @@ public class EntityShulker extends EntityGolem implements IMonster { + this.locX = (double) blockposition.getX() + 0.5D; + this.locY = (double) blockposition.getY(); + this.locZ = (double) blockposition.getZ() + 0.5D; ++ if (valid) world.entityJoinedWorld(this, false); // Paper - ensure Entity is moved to its proper chunk + this.lastX = this.locX; + this.lastY = this.locY; + this.lastZ = this.locZ; +-- +2.18.0 +