From 4128c348c24702f2883f6c50b55de16bb032582a Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 2 Aug 2018 23:06:03 -0400 Subject: [PATCH 1/3] Add "Safe Regen" Duplicate UUID resolver and make default After witnessing behavior of the regeneration logs, its clear that Vanilla has had bugs with saving duplicate entities for a while.... Some entities are saved in multiple chunks, and now we are bringing those duplicates out that use to never surface. This mode will analyze if the entity appears to be a duplicate (near the other dupe uuid) and delete the entity instead. This should reduce regenerations to entities that are nowhere near each other, and therefore more likely to be subject to real UUID collisions due to our previous bug, and therefor should survive the chunk load. --- ...dd-some-Debug-to-Chunk-Entity-slices.patch | 2 +- .../Duplicate-UUID-Resolve-Option.patch | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Spigot-Server-Patches/Add-some-Debug-to-Chunk-Entity-slices.patch b/Spigot-Server-Patches/Add-some-Debug-to-Chunk-Entity-slices.patch index 6b4e7c518f..a1f35d958c 100644 --- a/Spigot-Server-Patches/Add-some-Debug-to-Chunk-Entity-slices.patch +++ b/Spigot-Server-Patches/Add-some-Debug-to-Chunk-Entity-slices.patch @@ -9,7 +9,7 @@ This should hopefully avoid duplicate entities ever being created if the entity was to end up in 2 different chunk slices diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java -index 27b73acdee..be3ac2d940 100644 +index ccb30d5bfd..94b294e87b 100644 --- a/src/main/java/net/minecraft/server/Chunk.java +++ b/src/main/java/net/minecraft/server/Chunk.java @@ -0,0 +0,0 @@ public class Chunk { diff --git a/Spigot-Server-Patches/Duplicate-UUID-Resolve-Option.patch b/Spigot-Server-Patches/Duplicate-UUID-Resolve-Option.patch index 5377509981..e0cd02936b 100644 --- a/Spigot-Server-Patches/Duplicate-UUID-Resolve-Option.patch +++ b/Spigot-Server-Patches/Duplicate-UUID-Resolve-Option.patch @@ -33,7 +33,7 @@ But for those who are ok with leaving this inconsistent behavior, you may use WA It is recommended you regenerate the entities, as these were legit entities, and deserve your love. diff --git a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java -index 14c8edeffc..1f4e438c1f 100644 +index 14c8edeffc..46ec852b6c 100644 --- a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java +++ b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java @@ -0,0 +0,0 @@ public class PaperWorldConfig { @@ -42,12 +42,16 @@ index 14c8edeffc..1f4e438c1f 100644 } + + public enum DuplicateUUIDMode { -+ REGEN, DELETE, NOTHING, WARN ++ SAFE_REGEN, REGEN, DELETE, NOTHING, WARN + } -+ public DuplicateUUIDMode duplicateUUIDMode = DuplicateUUIDMode.REGEN; ++ public DuplicateUUIDMode duplicateUUIDMode = DuplicateUUIDMode.SAFE_REGEN; + private void repairDuplicateUUID() { -+ String desiredMode = getString("duplicate-uuid-resolver", "regenerate").toLowerCase().trim(); ++ String desiredMode = getString("duplicate-uuid-resolver", "saferegen").toLowerCase().trim(); + switch (desiredMode.toLowerCase()) { ++ case "saferegen": ++ case "saferegenerate": ++ duplicateUUIDMode = DuplicateUUIDMode.SAFE_REGEN; ++ log("Duplicate UUID Resolve: Safer Regenerate New UUID (Delete likely duplicates)"); + case "regen": + case "regenerate": + duplicateUUIDMode = DuplicateUUIDMode.REGEN; @@ -78,7 +82,7 @@ index 14c8edeffc..1f4e438c1f 100644 + } } diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java -index f1815d3766..74612a3924 100644 +index 4757081090..4018410485 100644 --- a/src/main/java/net/minecraft/server/Chunk.java +++ b/src/main/java/net/minecraft/server/Chunk.java @@ -0,0 +0,0 @@ @@ -123,8 +127,19 @@ index f1815d3766..74612a3924 100644 + if (other == null || other.dead || world.getEntityUnloadQueue().contains(other)) { + other = thisChunk.get(entity.uniqueID); + } ++ ++ if (mode == DuplicateUUIDMode.SAFE_REGEN && other != null && !other.dead && ++ !world.getEntityUnloadQueue().contains(other) ++ && java.util.Objects.equals(other.getSaveID(), entity.getSaveID()) ++ && entity.getBukkitEntity().getLocation().distance(other.getBukkitEntity().getLocation()) < 24 ++ ) { ++ logger.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", deleted entity " + entity + " because it was near the duplicate and likely an actual duplicate. See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); ++ entity.die(); ++ continue; ++ } + if (other != null && !other.dead) { + switch (mode) { ++ case SAFE_REGEN: + case REGEN: { + entity.setUUID(UUID.randomUUID()); + logger.warn("[DUPE-UUID] Duplicate UUID found used by " + other + ", regenerated UUID for " + entity + ". See https://github.com/PaperMC/Paper/issues/1223 for discussion on what this is about."); @@ -149,7 +164,7 @@ index f1815d3766..74612a3924 100644 this.world.a((Collection) entityslice); } diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java -index 93ab050fa6..fdabb1e369 100644 +index 7b856cad91..eb8904a728 100644 --- a/src/main/java/net/minecraft/server/Entity.java +++ b/src/main/java/net/minecraft/server/Entity.java @@ -0,0 +0,0 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper @@ -161,7 +176,7 @@ index 93ab050fa6..fdabb1e369 100644 this.uniqueID = uuid; this.ar = this.uniqueID.toString(); diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index 52adee8806..b4de3b515a 100644 +index d5cd289c21..57217bec2b 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { From 712ded51a2bafcc3d026a22967a0594008ec1ce0 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 3 Aug 2018 00:06:20 -0400 Subject: [PATCH 2/3] Fix EXP orb merging causing values to go negative - Closes #1169 --- ...rocess-chunk-removal-in-removeEntity.patch | 2 +- ...ups-for-Entity-TileEntity-Current-Ch.patch | 4 ++-- .../ExperienceOrbMergeEvent.patch | 14 ++++++------- ...ead-Entities-in-entityList-iteration.patch | 8 +++---- ...-maximum-exp-value-when-merging-orbs.patch | 21 +++++++++++-------- ...revent-Saving-Bad-entities-to-chunks.patch | 2 +- 6 files changed, 27 insertions(+), 24 deletions(-) diff --git a/Spigot-Server-Patches/Always-process-chunk-removal-in-removeEntity.patch b/Spigot-Server-Patches/Always-process-chunk-removal-in-removeEntity.patch index 1a96ccc29b..845d2407eb 100644 --- a/Spigot-Server-Patches/Always-process-chunk-removal-in-removeEntity.patch +++ b/Spigot-Server-Patches/Always-process-chunk-removal-in-removeEntity.patch @@ -8,7 +8,7 @@ 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 +index 0aa2f99838..8822d17745 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { diff --git a/Spigot-Server-Patches/Avoid-Chunk-Lookups-for-Entity-TileEntity-Current-Ch.patch b/Spigot-Server-Patches/Avoid-Chunk-Lookups-for-Entity-TileEntity-Current-Ch.patch index 1f82c2e531..bbf73d5dfe 100644 --- a/Spigot-Server-Patches/Avoid-Chunk-Lookups-for-Entity-TileEntity-Current-Ch.patch +++ b/Spigot-Server-Patches/Avoid-Chunk-Lookups-for-Entity-TileEntity-Current-Ch.patch @@ -10,7 +10,7 @@ to the object directly on the Entity/TileEntity object we can directly grab. Use that local value instead to reduce lookups in many hot places. diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java -index 81bf60efa..04adf4e3c 100644 +index d24d45fdd3..4757081090 100644 --- a/src/main/java/net/minecraft/server/Chunk.java +++ b/src/main/java/net/minecraft/server/Chunk.java @@ -0,0 +0,0 @@ public class Chunk { @@ -22,7 +22,7 @@ index 81bf60efa..04adf4e3c 100644 this.a(entity, entity.ac); } diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index c0816b9f8..52adee880 100644 +index 986670f689..a01488e985 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { diff --git a/Spigot-Server-Patches/ExperienceOrbMergeEvent.patch b/Spigot-Server-Patches/ExperienceOrbMergeEvent.patch index 57b7840c67..0c371550f4 100644 --- a/Spigot-Server-Patches/ExperienceOrbMergeEvent.patch +++ b/Spigot-Server-Patches/ExperienceOrbMergeEvent.patch @@ -8,16 +8,16 @@ Plugins can cancel this if they want to ensure experience orbs do not lose impor metadata such as spawn reason, or conditionally move data from source to target. diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index 39b90fb4c..c0816b9f8 100644 +index c0e79796bd..986670f689 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { - for (Entity e : entities) { if (e instanceof EntityExperienceOrb) { EntityExperienceOrb loopItem = (EntityExperienceOrb) e; -- if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue)) { // Paper -+ if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue) && new com.destroystokyo.paper.event.entity.ExperienceOrbMergeEvent((org.bukkit.entity.ExperienceOrb) entity.getBukkitEntity(), (org.bukkit.entity.ExperienceOrb) loopItem.getBukkitEntity()).callEvent()) { // Paper - xp.value += loopItem.value; - // Paper start - if (!mergeUnconditionally && xp.value > maxValue) { + // Paper start +- if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue)) { ++ if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue) && new com.destroystokyo.paper.event.entity.ExperienceOrbMergeEvent((org.bukkit.entity.ExperienceOrb) entity.getBukkitEntity(), (org.bukkit.entity.ExperienceOrb) loopItem.getBukkitEntity()).callEvent()) { + long newTotal = (long)xp.value + (long)loopItem.value; + if (newTotal > (long)maxValue) { + loopItem.value = xp.value - maxValue; -- \ No newline at end of file diff --git a/Spigot-Server-Patches/Ignore-Dead-Entities-in-entityList-iteration.patch b/Spigot-Server-Patches/Ignore-Dead-Entities-in-entityList-iteration.patch index 3ea5bf4e6e..949ed5abcf 100644 --- a/Spigot-Server-Patches/Ignore-Dead-Entities-in-entityList-iteration.patch +++ b/Spigot-Server-Patches/Ignore-Dead-Entities-in-entityList-iteration.patch @@ -11,7 +11,7 @@ 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 +index ecd1c65a98..1898ab897c 100644 --- a/src/main/java/com/destroystokyo/paper/PaperCommand.java +++ b/src/main/java/com/destroystokyo/paper/PaperCommand.java @@ -0,0 +0,0 @@ public class PaperCommand extends Command { @@ -23,7 +23,7 @@ index ecd1c65a9..1898ab897 100644 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 1e64d5fcd..45e149f4a 100644 +index 1e64d5fcd6..45e149f4a3 100644 --- a/src/main/java/net/minecraft/server/Entity.java +++ b/src/main/java/net/minecraft/server/Entity.java @@ -0,0 +0,0 @@ public abstract class Entity implements ICommandListener, KeyedObject { // Paper @@ -35,7 +35,7 @@ index 1e64d5fcd..45e149f4a 100644 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 0dca11b2c..dadd4b839 100644 +index 8822d17745..3ce6f9778b 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { @@ -71,7 +71,7 @@ index 0dca11b2c..dadd4b839 100644 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 +index 210e3bc4e6..e6ecd1796c 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java @@ -0,0 +0,0 @@ public class CraftWorld implements World { diff --git a/Spigot-Server-Patches/Option-for-maximum-exp-value-when-merging-orbs.patch b/Spigot-Server-Patches/Option-for-maximum-exp-value-when-merging-orbs.patch index cbe2e8a8c8..dc1dcec081 100644 --- a/Spigot-Server-Patches/Option-for-maximum-exp-value-when-merging-orbs.patch +++ b/Spigot-Server-Patches/Option-for-maximum-exp-value-when-merging-orbs.patch @@ -5,7 +5,7 @@ Subject: [PATCH] Option for maximum exp value when merging orbs diff --git a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java -index 4d30cdbc8..535a8d3ed 100644 +index 4d30cdbc8b..535a8d3ed1 100644 --- a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java +++ b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java @@ -0,0 +0,0 @@ public class PaperWorldConfig { @@ -20,7 +20,7 @@ index 4d30cdbc8..535a8d3ed 100644 + } } diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index 29dffc3ac..2c69ae748 100644 +index 3561507de1..b6b4d52718 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { @@ -29,7 +29,7 @@ index 29dffc3ac..2c69ae748 100644 if (radius > 0) { + // Paper start - Maximum exp value when merging - Whole section has been tweaked, see comments for specifics + final int maxValue = paperConfig.expMergeMaxValue; -+ final boolean mergeUnconditionally = maxValue <= 0; ++ final boolean mergeUnconditionally = paperConfig.expMergeMaxValue <= 0; + if (mergeUnconditionally || xp.value < maxValue) { // Paper - Skip iteration if unnecessary + List entities = this.getEntities(entity, entity.getBoundingBox().grow(radius, radius, radius)); @@ -37,16 +37,19 @@ index 29dffc3ac..2c69ae748 100644 if (e instanceof EntityExperienceOrb) { EntityExperienceOrb loopItem = (EntityExperienceOrb) e; - if (!loopItem.dead) { -+ if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue)) { // Paper - xp.value += loopItem.value; -+ // Paper start -+ if (!mergeUnconditionally && xp.value > maxValue) { +- xp.value += loopItem.value; +- loopItem.die(); ++ // Paper start ++ if (!loopItem.dead && !(maxValue > 0 && loopItem.value >= maxValue)) { ++ long newTotal = (long)xp.value + (long)loopItem.value; ++ if (newTotal > (long)maxValue) { + loopItem.value = xp.value - maxValue; + xp.value = maxValue; -+ break; ++ } else { ++ xp.value += loopItem.value; ++ loopItem.die(); + } + // Paper end - loopItem.die(); } } } diff --git a/Spigot-Server-Patches/Prevent-Saving-Bad-entities-to-chunks.patch b/Spigot-Server-Patches/Prevent-Saving-Bad-entities-to-chunks.patch index 9013fb129e..5185e24566 100644 --- a/Spigot-Server-Patches/Prevent-Saving-Bad-entities-to-chunks.patch +++ b/Spigot-Server-Patches/Prevent-Saving-Bad-entities-to-chunks.patch @@ -57,7 +57,7 @@ index bcce5e8b7e..bad287fca4 100644 nbttagcompound.set("Entities", nbttaglist1); NBTTagList nbttaglist2 = new NBTTagList(); diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index b4de3b515a..d2d2ab794b 100644 +index 3012768cb9..0aa2f99838 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { From 3522a633d610b9ac5130d648c35bb1bff9183714 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 3 Aug 2018 00:07:02 -0400 Subject: [PATCH 3/3] MC-135506: Experience should save as Integers A large orb will lose its EXP value if it went over 32k due to short truncation. --- ...6-Experience-should-save-as-Integers.patch | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Spigot-Server-Patches/MC-135506-Experience-should-save-as-Integers.patch diff --git a/Spigot-Server-Patches/MC-135506-Experience-should-save-as-Integers.patch b/Spigot-Server-Patches/MC-135506-Experience-should-save-as-Integers.patch new file mode 100644 index 0000000000..f9c9e8467b --- /dev/null +++ b/Spigot-Server-Patches/MC-135506-Experience-should-save-as-Integers.patch @@ -0,0 +1,28 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Fri, 3 Aug 2018 00:04:54 -0400 +Subject: [PATCH] MC-135506: Experience should save as Integers + + +diff --git a/src/main/java/net/minecraft/server/EntityExperienceOrb.java b/src/main/java/net/minecraft/server/EntityExperienceOrb.java +index 1c59fd9661..c179fe516e 100644 +--- a/src/main/java/net/minecraft/server/EntityExperienceOrb.java ++++ b/src/main/java/net/minecraft/server/EntityExperienceOrb.java +@@ -0,0 +0,0 @@ public class EntityExperienceOrb extends Entity { + public void b(NBTTagCompound nbttagcompound) { + nbttagcompound.setShort("Health", (short) this.d); + nbttagcompound.setShort("Age", (short) this.b); +- nbttagcompound.setShort("Value", (short) this.value); ++ nbttagcompound.setInt("Value", (short) this.value); // Paper - save as Integer + savePaperNBT(nbttagcompound); // Paper + } + + public void a(NBTTagCompound nbttagcompound) { + this.d = nbttagcompound.getShort("Health"); + this.b = nbttagcompound.getShort("Age"); +- this.value = nbttagcompound.getShort("Value"); ++ this.value = nbttagcompound.getInt("Value"); // Paper - load as Integer + loadPaperNBT(nbttagcompound); // Paper + } + +-- \ No newline at end of file