From 9170234f9a503b0cb3a9822d8a3a861be4c387e4 Mon Sep 17 00:00:00 2001 From: Jake Potrebic Date: Wed, 22 Dec 2021 10:02:31 -0800 Subject: [PATCH] Fix duplicated BlockPistonRetractEvent call (#7111) --- ...onRetractEvent-for-all-empty-pistons.patch | 47 ---------- ...-pistons-and-BlockPistonRetractEvent.patch | 85 +++++++++++++++++++ ...istonRetractEvent-to-fix-duplication.patch | 49 ----------- 3 files changed, 85 insertions(+), 96 deletions(-) delete mode 100644 patches/server/Fire-BlockPistonRetractEvent-for-all-empty-pistons.patch create mode 100644 patches/server/Fix-sticky-pistons-and-BlockPistonRetractEvent.patch delete mode 100644 patches/server/Move-BlockPistonRetractEvent-to-fix-duplication.patch diff --git a/patches/server/Fire-BlockPistonRetractEvent-for-all-empty-pistons.patch b/patches/server/Fire-BlockPistonRetractEvent-for-all-empty-pistons.patch deleted file mode 100644 index aaa03eccd8..0000000000 --- a/patches/server/Fire-BlockPistonRetractEvent-for-all-empty-pistons.patch +++ /dev/null @@ -1,47 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Zach Brown <1254957+zachbr@users.noreply.github.com> -Date: Thu, 31 Jan 2019 16:33:36 -0500 -Subject: [PATCH] Fire BlockPistonRetractEvent for all empty pistons - -There is an explicit check in the handling code for empty pistons that -prevents sticky pistons from firing the event. However when we look back -at the history we see that this check was originally added so that ONLY -sticky pistons would fire the retract event. I'm not sure why. -https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/1092acbddf07edfa4100bc6824504ac75088e913 - -Over the course of several updates, the meaning of that field appears to -have changed from "is NOT sticky" to "is sticky". So now its having the -opposite effect. Only normal pistons fire the retraction event. And like -all things in CB, it's just been carried around since. - -If we are to believe the history, the correct fix for this issue is to -flip it so it only fires for sticky pistons, but that puts us in a -bind. It's already firing for non-sticky pistons, changing it now would -likely result in breakage. Furthermore, there is little documentation as -to WHY that was ever intended to be the case. - -Instead we opt to remove the check entirely so that the event fires for -all piston types. - -diff --git a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -+++ b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { - } - - // CraftBukkit start -- if (!this.isSticky) { -+ //if (!this.sticky) { // Paper - Prevents empty sticky pistons from firing retract - history behind is odd - org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); - BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); - world.getCraftServer().getPluginManager().callEvent(event); -@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { - if (event.isCancelled()) { - return; - } -- } -+ //} // Paper - // PAIL: checkME - what happened to setTypeAndData? - // CraftBukkit end - world.blockEvent(pos, this, b0, enumdirection.get3DDataValue()); diff --git a/patches/server/Fix-sticky-pistons-and-BlockPistonRetractEvent.patch b/patches/server/Fix-sticky-pistons-and-BlockPistonRetractEvent.patch new file mode 100644 index 0000000000..d652cddf9d --- /dev/null +++ b/patches/server/Fix-sticky-pistons-and-BlockPistonRetractEvent.patch @@ -0,0 +1,85 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jake Potrebic +Date: Wed, 22 Dec 2021 09:51:48 -0800 +Subject: [PATCH] Fix sticky pistons and BlockPistonRetractEvent + +There is an explicit check in the handling code for empty pistons that +prevents sticky pistons from firing the event. However when we look back +at the history we see that this check was originally added so that ONLY +sticky pistons would fire the retract event. I'm not sure why. +https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/1092acbddf07edfa4100bc6824504ac75088e913 + +Over the course of several updates, the meaning of that field appears to +have changed from "is NOT sticky" to "is sticky". So now its having the +opposite effect. Only normal pistons fire the retraction event. And like +all things in CB, it's just been carried around since. + +If we are to believe the history, the correct fix for this issue is to +flip it so it only fires for sticky pistons, but that puts us in a +bind. It's already firing for non-sticky pistons, changing it now would +likely result in breakage. Furthermore, there is little documentation as +to WHY that was ever intended to be the case. + +Instead we opt to remove the check entirely so that the event fires for +all piston types. + +Co-authored-by: Zach Brown <1254957+zachbr@users.noreply.github.com> +Co-authored-by: Madeline Miller + +diff --git a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java ++++ b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java +@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { + } + + // CraftBukkit start +- if (!this.isSticky) { +- org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); +- BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); +- world.getCraftServer().getPluginManager().callEvent(event); +- +- if (event.isCancelled()) { +- return; +- } +- } ++ // if (!this.isSticky) { // Paper - Move further down ++ // org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); ++ // BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); ++ // world.getCraftServer().getPluginManager().callEvent(event); ++ // ++ // if (event.isCancelled()) { ++ // return; ++ // } ++ // } + // PAIL: checkME - what happened to setTypeAndData? + // CraftBukkit end + world.blockEvent(pos, this, b0, enumdirection.get3DDataValue()); +@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { + + BlockState iblockdata1 = (BlockState) ((BlockState) Blocks.MOVING_PISTON.defaultBlockState().setValue(MovingPistonBlock.FACING, enumdirection)).setValue(MovingPistonBlock.TYPE, this.isSticky ? PistonType.STICKY : PistonType.DEFAULT); + ++ // Paper start - Move empty piston retract call to fix multiple event fires ++ if (!this.isSticky) { ++ if (!new BlockPistonRetractEvent(CraftBlock.at(world, pos), java.util.Collections.emptyList(), CraftBlock.notchToBlockFace(enumdirection)).callEvent()) { ++ return false; ++ } ++ } ++ // Paper end + world.setBlock(pos, iblockdata1, 20); + world.setBlockEntity(MovingPistonBlock.newMovingBlockEntity(pos, iblockdata1, (BlockState) this.defaultBlockState().setValue(PistonBaseBlock.FACING, Direction.from3DDataValue(data & 7)), enumdirection, false, true)); // Paper - diff on change + world.blockUpdated(pos, iblockdata1.getBlock()); +@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { + if (type == 1 && !iblockdata2.isAir() && PistonBaseBlock.isPushable(iblockdata2, world, blockposition1, enumdirection.getOpposite(), false, enumdirection) && (iblockdata2.getPistonPushReaction() == PushReaction.NORMAL || iblockdata2.is(Blocks.PISTON) || iblockdata2.is(Blocks.STICKY_PISTON))) { + this.moveBlocks(world, pos, enumdirection, false); + } else { ++ // Paper start - fire BlockPistonRetractEvent for sticky pistons retracting nothing (air) ++ if (type == TRIGGER_CONTRACT && iblockdata2.isAir()) { ++ if (!new BlockPistonRetractEvent(CraftBlock.at(world, pos), java.util.Collections.emptyList(), CraftBlock.notchToBlockFace(enumdirection)).callEvent()) { ++ return false; ++ } ++ } ++ // Paper end + world.removeBlock(pos.relative(enumdirection), false); + } + } diff --git a/patches/server/Move-BlockPistonRetractEvent-to-fix-duplication.patch b/patches/server/Move-BlockPistonRetractEvent-to-fix-duplication.patch deleted file mode 100644 index 32d75cb9fb..0000000000 --- a/patches/server/Move-BlockPistonRetractEvent-to-fix-duplication.patch +++ /dev/null @@ -1,49 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Madeline Miller -Date: Sun, 22 Aug 2021 22:17:18 +1000 -Subject: [PATCH] Move BlockPistonRetractEvent to fix duplication - - -diff --git a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -+++ b/src/main/java/net/minecraft/world/level/block/piston/PistonBaseBlock.java -@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { - } - - // CraftBukkit start -- //if (!this.sticky) { // Paper - Prevents empty sticky pistons from firing retract - history behind is odd -- org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); -- BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); -- world.getCraftServer().getPluginManager().callEvent(event); -- -- if (event.isCancelled()) { -- return; -- } -+ //if (!this.sticky) { // Paper - Prevents empty sticky pistons from firing retract - history behind is odd - Move further down -+ // org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); -+ // BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); -+ // world.getCraftServer().getPluginManager().callEvent(event); -+ -+ // if (event.isCancelled()) { -+ // return; -+ // } - //} // Paper - // PAIL: checkME - what happened to setTypeAndData? - // CraftBukkit end -@@ -0,0 +0,0 @@ public class PistonBaseBlock extends DirectionalBlock { - - BlockState iblockdata1 = (BlockState) ((BlockState) Blocks.MOVING_PISTON.defaultBlockState().setValue(MovingPistonBlock.FACING, enumdirection)).setValue(MovingPistonBlock.TYPE, this.isSticky ? PistonType.STICKY : PistonType.DEFAULT); - -+ // Paper - Move empty piston retract call to fix duplication -+ org.bukkit.block.Block block = world.getWorld().getBlockAt(pos.getX(), pos.getY(), pos.getZ()); -+ BlockPistonRetractEvent event = new BlockPistonRetractEvent(block, ImmutableList.of(), CraftBlock.notchToBlockFace(enumdirection)); -+ world.getCraftServer().getPluginManager().callEvent(event); -+ -+ if (event.isCancelled()) { -+ return false; -+ } // Paper -+ - world.setBlock(pos, iblockdata1, 20); - world.setBlockEntity(MovingPistonBlock.newMovingBlockEntity(pos, iblockdata1, (BlockState) this.defaultBlockState().setValue(PistonBaseBlock.FACING, Direction.from3DDataValue(data & 7)), enumdirection, false, true)); // Paper - diff on change - world.blockUpdated(pos, iblockdata1.getBlock());