Small diff cleanup

4 files changed, 57 insertions, 114 deletions

diff --git a/paper-server/patches/sources/net/minecraft/server/level/ServerPlayerGameMode.java.patch b/paper-server/patches/sources/net/minecraft/server/level/ServerPlayerGameMode.java.patch
index fd053c54f5..99bc1d539e 100644
--- a/paper-server/patches/sources/net/minecraft/server/level/ServerPlayerGameMode.java.patch
+++ b/paper-server/patches/sources/net/minecraft/server/level/ServerPlayerGameMode.java.patch
@@ -91,7 +91,7 @@
              this.debugLogging(pos, false, sequence, "too far");
          } else if (pos.getY() > maxBuildHeight) {
              this.player.connection.send(new ClientboundBlockUpdatePacket(pos, this.level.getBlockState(pos)));
-@@ -138,16 +_,40 @@
+@@ -138,16 +_,39 @@
          } else {
              if (action == ServerboundPlayerActionPacket.Action.START_DESTROY_BLOCK) {
                  if (!this.level.mayInteract(this.player, pos)) {
@@ -126,7 +126,6 @@
 +                // Spigot start - handle debug stick left click for non-creative
 +                if (this.player.getMainHandItem().is(net.minecraft.world.item.Items.DEBUG_STICK)
 +                    && ((net.minecraft.world.item.DebugStickItem) net.minecraft.world.item.Items.DEBUG_STICK).handleInteraction(this.player, this.level.getBlockState(pos), this.level, pos, false, this.player.getMainHandItem())) {
-+                    // this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos)); // Paper - Don't resync block
 +                    return;
 +                }
 +                // Spigot end
@@ -134,45 +133,28 @@
                  if (this.player.blockActionRestricted(this.level, pos, this.gameModeForPlayer)) {
                      this.player.connection.send(new ClientboundBlockUpdatePacket(pos, this.level.getBlockState(pos)));
                      this.debugLogging(pos, false, sequence, "block action restricted");
-@@ -157,7 +_,21 @@
+@@ -157,7 +_,7 @@
                  this.destroyProgressStart = this.gameTicks;
                  float f = 1.0F;
                  BlockState blockState = this.level.getBlockState(pos);
 -                if (!blockState.isAir()) {
-+                // CraftBukkit start - Swings at air do *NOT* exist.
-+                if (event.useInteractedBlock() == org.bukkit.event.Event.Result.DENY) {
-+                    // If we denied a door from opening, we need to send a correcting update to the client, as it already opened the door.
-+                    // Paper start - Don't resync blocks
-+                    //BlockState data = this.level.getBlockState(pos);
-+                    //if (data.getBlock() instanceof DoorBlock) {
-+                    //    // For some reason *BOTH* the bottom/top part have to be marked updated.
-+                    //    boolean bottom = data.getValue(DoorBlock.HALF) == DoubleBlockHalf.LOWER;
-+                    //    this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos));
-+                    //    this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, bottom ? pos.above() : pos.below()));
-+                    //} else if (data.getBlock() instanceof TrapDoorBlock) {
-+                    //    this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos));
-+                    //}
-+                    // Paper end - Don't resync blocks
-+                } else if (!blockState.isAir()) {
++                if (event.useInteractedBlock() != org.bukkit.event.Event.Result.DENY && !blockState.isAir()) { // Paper
                      EnchantmentHelper.onHitBlock(
                          this.level,
                          this.player.getMainHandItem(),
-@@ -172,6 +_,26 @@
+@@ -172,6 +_,23 @@
                      f = blockState.getDestroyProgress(this.player, this.player.level(), pos);
                  }
  
++                // CraftBukkit start
++                // Note that we don't need to resync blocks, block acks will handle it properly for everything but block entities already
 +                if (event.useItemInHand() == org.bukkit.event.Event.Result.DENY) {
-+                    // If we 'insta destroyed' then the client needs to be informed.
-+                    if (f > 1.0f) {
-+                        // this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos)); // Paper - Don't resync blocks
-+                    }
 +                    return;
 +                }
++
 +                org.bukkit.event.block.BlockDamageEvent blockEvent = org.bukkit.craftbukkit.event.CraftEventFactory.callBlockDamageEvent(this.player, pos, face, this.player.getInventory().getSelected(), f >= 1.0f); // Paper - Add BlockFace to BlockDamageEvent
 +
 +                if (blockEvent.isCancelled()) {
-+                    // Let the client know the block still exists
-+                    // this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos)); // Paper - Don't resync block
 +                    return;
 +                }
 +
@@ -184,20 +166,22 @@
                  if (!blockState.isAir() && f >= 1.0F) {
                      this.destroyAndAck(pos, sequence, "insta mine");
                  } else {
-@@ -212,14 +_,18 @@
+@@ -212,14 +_,22 @@
                  this.debugLogging(pos, true, sequence, "stopped destroying");
              } else if (action == ServerboundPlayerActionPacket.Action.ABORT_DESTROY_BLOCK) {
                  this.isDestroyingBlock = false;
 -                if (!Objects.equals(this.destroyPos, pos)) {
 -                    LOGGER.warn("Mismatch in destroy block pos: {} {}", this.destroyPos, pos);
--                    this.level.destroyBlockProgress(this.player.getId(), this.destroyPos, -1);
--                    this.debugLogging(pos, true, sequence, "aborted mismatched destroying");
++                // Paper start - Don't allow digging into unloaded chunks
 +                if (!Objects.equals(this.destroyPos, pos) && !BlockPos.ZERO.equals(this.destroyPos)) { // Paper
 +                    ServerPlayerGameMode.LOGGER.debug("Mismatch in destroy block pos: {} {}", this.destroyPos, pos); // CraftBukkit - SPIGOT-5457 sent by client when interact event cancelled
-+                    BlockState type = this.level.getBlockStateIfLoaded(this.destroyPos); // Paper - don't load unloaded chunks for stale records here
-+                    if (type != null) this.level.destroyBlockProgress(this.player.getId(), this.destroyPos, -1);
-+                    if (type != null) this.debugLogging(pos, true, sequence, "aborted mismatched destroying");
-+                    this.destroyPos = BlockPos.ZERO; // Paper
++                    BlockState type = this.level.getBlockStateIfLoaded(this.destroyPos); // Don't load unloaded chunks for stale records here
++                    if (type != null) {
+                     this.level.destroyBlockProgress(this.player.getId(), this.destroyPos, -1);
+                     this.debugLogging(pos, true, sequence, "aborted mismatched destroying");
++                    }
++                    this.destroyPos = BlockPos.ZERO;
++                    // Paper end - Don't allow digging into unloaded chunks
                  }
  
                  this.level.destroyBlockProgress(this.player.getId(), pos, -1);
@@ -207,7 +191,7 @@
              }
          }
      }
-@@ -235,36 +_,127 @@
+@@ -235,36 +_,108 @@
  
      public boolean destroyBlock(BlockPos pos) {
          BlockState blockState = this.level.getBlockState(pos);
@@ -217,54 +201,36 @@
 +        org.bukkit.event.block.BlockBreakEvent event = null;
 +        if (this.player instanceof ServerPlayer) {
 +            // Sword + Creative mode pre-cancel
-+            boolean isSwordNoBreak = !this.player.getMainHandItem().getItem().canAttackBlock(blockState, this.level, pos, this.player);
-+
-+            // Tell client the block is gone immediately then process events
-+            // Don't tell the client if its a creative sword break because its not broken!
-+            if (false && this.level.getBlockEntity(pos) == null && !isSwordNoBreak) { // Paper - Don't resync block
-+                ClientboundBlockUpdatePacket packet = new ClientboundBlockUpdatePacket(pos, net.minecraft.world.level.block.Blocks.AIR.defaultBlockState());
-+                this.player.connection.send(packet);
-+            }
-+
++            boolean canAttackBlock = !this.player.getMainHandItem().getItem().canAttackBlock(blockState, this.level, pos, this.player);
 +            event = new org.bukkit.event.block.BlockBreakEvent(bblock, this.player.getBukkitEntity());
 +
 +            // Sword + Creative mode pre-cancel
-+            event.setCancelled(isSwordNoBreak);
++            event.setCancelled(canAttackBlock);
 +
 +            // Calculate default block experience
-+            BlockState nmsData = this.level.getBlockState(pos);
-+            Block nmsBlock = nmsData.getBlock();
-+
-+            ItemStack itemstack = this.player.getItemBySlot(EquipmentSlot.MAINHAND);
++            BlockState updatedBlockState = this.level.getBlockState(pos);
++            Block block = updatedBlockState.getBlock();
 +
-+            if (nmsBlock != null && !event.isCancelled() && !this.isCreative() && this.player.hasCorrectToolForDrops(nmsBlock.defaultBlockState())) {
-+                event.setExpToDrop(nmsBlock.getExpDrop(nmsData, this.level, pos, itemstack, true));
++            if (!event.isCancelled() && !this.isCreative() && this.player.hasCorrectToolForDrops(block.defaultBlockState())) {
++                ItemStack itemInHand = this.player.getItemBySlot(EquipmentSlot.MAINHAND);
++                event.setExpToDrop(block.getExpDrop(updatedBlockState, this.level, pos, itemInHand, true));
 +            }
 +
 +            this.level.getCraftServer().getPluginManager().callEvent(event);
 +
 +            if (event.isCancelled()) {
-+                if (isSwordNoBreak) {
++                if (canAttackBlock) {
 +                    return false;
 +                }
-+                // Paper start - Don't resync blocks
-+                // Let the client know the block still exists
-+                //this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos));
-+
-+                // Brute force all possible updates
-+                //for (Direction dir : Direction.values()) {
-+                //    this.player.connection.send(new ClientboundBlockUpdatePacket(this.level, pos.relative(dir)));
-+                //}
-+                // Paper end - Don't resync blocks
 +
-+                // Update any tile entity data for this block
-+                if (!this.captureSentBlockEntities) { // Paper - Send block entities after destroy prediction
++                // Block entity data is not reset by the block acks, send after destroy prediction
++                if (!this.captureSentBlockEntities) {
 +                    BlockEntity blockEntity = this.level.getBlockEntity(pos);
 +                    if (blockEntity != null) {
 +                        this.player.connection.send(blockEntity.getUpdatePacket());
 +                    }
 +                } else {
-+                    this.capturedBlockEntity = true; // Paper - Send block entities after destroy prediction
++                    this.capturedBlockEntity = true;
 +                }
 +                return false;
 +            }
@@ -325,10 +291,9 @@
 +                if (event.isDropItems()) {
 +                    org.bukkit.craftbukkit.event.CraftEventFactory.handleBlockDropItemEvent(bblock, state, this.player, itemsToDrop); // Paper - capture all item additions to the world
 +                }
-+                //this.level.captureDrops = null; // Paper - capture all item additions to the world; move up
 +
 +                // Drop event experience
-+                if (flag && event != null) {
++                if (flag) {
 +                    blockState.getBlock().popExperience(this.level, pos, event.getExpToDrop(), this.player); // Paper
 +                }
 +                // Paper start - Trigger bee_nest_destroyed trigger in the correct place (check impls of block#playerDestroy)
@@ -344,7 +309,7 @@
              }
          }
      }
-@@ -307,15 +_,61 @@
+@@ -307,15 +_,47 @@
          }
      }
  
@@ -379,32 +344,18 @@
 +        this.interactItemStack = stack.copy();
 +
 +        if (event.useInteractedBlock() == org.bukkit.event.Event.Result.DENY) {
-+            // If we denied a door from opening, we need to send a correcting update to the client, as it already opened the door.
-+            if (blockState.getBlock() instanceof net.minecraft.world.level.block.DoorBlock) {
-+                // Paper start - Don't resync blocks
-+                // boolean bottom = iblockdata.getValue(DoorBlock.HALF) == DoubleBlockHalf.LOWER;
-+                // player.connection.send(new ClientboundBlockUpdatePacket(world, bottom ? blockposition.above() : blockposition.below()));
-+                // Paper end - Don't resync blocks
-+            } else if (blockState.getBlock() instanceof net.minecraft.world.level.block.CakeBlock) {
++            // Block acks will take care of most of it, just handle some special cases here
++            if (blockState.getBlock() instanceof net.minecraft.world.level.block.CakeBlock) {
 +                player.getBukkitEntity().sendHealthUpdate(); // SPIGOT-1341 - reset health for cake
-+            } else if (this.interactItemStack.getItem() instanceof net.minecraft.world.item.DoubleHighBlockItem) {
-+                // send a correcting update to the client, as it already placed the upper half of the bisected item
-+                //player.connection.send(new ClientboundBlockUpdatePacket(world, blockposition.relative(hitResult.getDirection()).above())); // Paper - Don't resync blocks
-+
-+                // send a correcting update to the client for the block above as well, this because of replaceable blocks (such as grass, sea grass etc)
-+                //player.connection.send(new ClientboundBlockUpdatePacket(world, blockposition.above())); // Paper - Don't resync blocks
-+                // Paper start - extend Player Interact cancellation // TODO: consider merging this into the extracted method
 +            } else if (blockState.is(net.minecraft.world.level.block.Blocks.JIGSAW) || blockState.is(net.minecraft.world.level.block.Blocks.STRUCTURE_BLOCK) || blockState.getBlock() instanceof net.minecraft.world.level.block.CommandBlock) {
 +                player.connection.send(new net.minecraft.network.protocol.game.ClientboundContainerClosePacket(this.player.containerMenu.containerId));
 +            }
-+            // Paper end - extend Player Interact cancellation
 +            player.getBukkitEntity().updateInventory(); // SPIGOT-2867
 +            this.player.resyncUsingItem(this.player); // Paper - Properly cancel usable items
 +            return (event.useItemInHand() != org.bukkit.event.Event.Result.ALLOW) ? InteractionResult.SUCCESS : InteractionResult.PASS;
 +        } else if (this.gameModeForPlayer == GameType.SPECTATOR) {
-+            MenuProvider itileinventory = blockState.getMenuProvider(level, blockPos);
-+
-+            if (itileinventory != null && player.openMenu(itileinventory).isPresent()) { // Paper - Fix InventoryOpenEvent cancellation
++            MenuProvider menuProvider = blockState.getMenuProvider(level, blockPos);
++            if (menuProvider != null && player.openMenu(menuProvider).isPresent()) { // Paper - Fix InventoryOpenEvent cancellation
                  return InteractionResult.CONSUME;
              } else {
                  return InteractionResult.PASS;
diff --git a/paper-server/patches/sources/net/minecraft/server/network/LegacyQueryHandler.java.patch b/paper-server/patches/sources/net/minecraft/server/network/LegacyQueryHandler.java.patch
index 5e6a096b22..31868aa929 100644
--- a/paper-server/patches/sources/net/minecraft/server/network/LegacyQueryHandler.java.patch
+++ b/paper-server/patches/sources/net/minecraft/server/network/LegacyQueryHandler.java.patch
@@ -48,30 +48,25 @@
                      sendFlushAndClose(context, createLegacyDisconnectPacket(context.alloc(), string));
                  } else {
                      if (byteBuf.readUnsignedByte() != 1) {
-@@ -43,16 +_,35 @@
+@@ -43,16 +_,39 @@
                      }
  
                      if (byteBuf.isReadable()) {
 -                        if (!readCustomPayloadPacket(byteBuf)) {
-+                        // Paper start - Replace with improved version below
++                        // Paper start - Replace below
 +                        if (byteBuf.readUnsignedByte() != LegacyProtocolUtils.CUSTOM_PAYLOAD_PACKET_ID) {
 +                            string = this.readLegacy1_6(context, byteBuf);
 +                            if (string == null) {
 +                                return;
 +                            }
 +                        }
-+                        // if (!readCustomPayloadPacket(byteBuf)) {
-+                        //     return;
-+                        // }
-+
-+                        // LOGGER.debug("Ping: (1.6) from {}", socketAddress);
-+                        // Paper end - Replace with improved version below
++                        // Paper end - Replace below
 +                    } else {
 +                        LOGGER.debug("Ping: (1.4-1.5.x) from {}", net.minecraft.server.MinecraftServer.getServer().logIPs() ? socketAddress : "<ip address withheld>"); // Paper - Respect logIPs option
 +                    }
 +
++                    // Paper start - Call PaperServerListPingEvent and use results
 +                    if (string == null) {
-+                        // Paper start - Call PaperServerListPingEvent and use results
 +                        com.destroystokyo.paper.event.server.PaperServerListPingEvent event = com.destroystokyo.paper.network.PaperLegacyStatusClient.processRequest(net.minecraft.server.MinecraftServer.getServer(), (java.net.InetSocketAddress) socketAddress, 127, null); // Paper
 +                        if (event == null) {
 +                            context.close();
@@ -79,11 +74,19 @@
 +                            flag = false;
                              return;
                          }
--
+ 
 -                        LOGGER.debug("Ping: (1.6) from {}", socketAddress);
 -                    } else {
 -                        LOGGER.debug("Ping: (1.4-1.5.x) from {}", socketAddress);
-+                        string = String.format(Locale.ROOT, "§1\u0000%d\u0000%s\u0000%s\u0000%d\u0000%d", event.getProtocolVersion(), this.server.getServerVersion(), event.getMotd(), event.getNumPlayers(), event.getMaxPlayers()); // CraftBukkit
++                        // See createVersion1Response
++                        string = String.format(
++                            Locale.ROOT,
++                            "§1\u0000%d\u0000%s\u0000%s\u0000%d\u0000%d",
++                            event.getProtocolVersion(), this.server.getServerVersion(),
++                            event.getMotd(),
++                            event.getNumPlayers(),
++                            event.getMaxPlayers()
++                        );
 +                        // Paper end - Call PaperServerListPingEvent and use results
                      }
 -
diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch
index 81c6160738..4d1d822b0f 100644
--- a/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch
+++ b/paper-server/patches/sources/net/minecraft/server/network/ServerCommonPacketListenerImpl.java.patch
@@ -95,7 +95,7 @@
          }
      }
  
-@@ -88,37 +_,124 @@
+@@ -88,30 +_,117 @@
      public void handlePong(ServerboundPongPacket packet) {
      }
  
@@ -206,31 +206,20 @@
  
      protected void keepConnectionAlive() {
          Profiler.get().push("keepAlive");
--        long millis = Util.getMillis();
+         long millis = Util.getMillis();
 -        if (!this.isSingleplayerOwner() && millis - this.keepAliveTime >= 15000L) {
 -            if (this.keepAlivePending) {
 -                this.disconnect(TIMEOUT_DISCONNECTION_MESSAGE);
--            } else if (this.checkIfClosed(millis)) {
 +        // Paper start - give clients a longer time to respond to pings as per pre 1.12.2 timings
 +        // This should effectively place the keepalive handling back to "as it was" before 1.12.2
-+        long currentTime = Util.getMillis();
-+        long elapsedTime = currentTime - this.keepAliveTime;
-+        if (!this.isSingleplayerOwner() && elapsedTime >= 15000L) { // Paper - use vanilla's 15000L between keep alive packets
-+            if (this.keepAlivePending && !this.processedDisconnect && elapsedTime >= KEEPALIVE_LIMIT) { // Paper - check keepalive limit, don't fire if already disconnected
++        final long elapsedTime = millis - this.keepAliveTime;
++        if (!this.isSingleplayerOwner() && elapsedTime >= 15000L) { // use vanilla's 15000L between keep alive packets
++            if (this.keepAlivePending && !this.processedDisconnect && elapsedTime >= KEEPALIVE_LIMIT) { // check keepalive limit, don't fire if already disconnected
++                // Paper end - give clients a longer time to respond to pings as per pre 1.12.2 timings
 +                this.disconnect(TIMEOUT_DISCONNECTION_MESSAGE, org.bukkit.event.player.PlayerKickEvent.Cause.TIMEOUT); // Paper - kick event cause
-+            } else if (this.checkIfClosed(currentTime)) { // Paper
+             } else if (this.checkIfClosed(millis)) {
                  this.keepAlivePending = true;
--                this.keepAliveTime = millis;
--                this.keepAliveChallenge = millis;
-+                this.keepAliveTime = currentTime;
-+                this.keepAliveChallenge = currentTime;
-                 this.send(new ClientboundKeepAlivePacket(this.keepAliveChallenge));
-             }
-         }
-+        // Paper end - give clients a longer time to respond to pings as per pre 1.12.2 timings
- 
-         Profiler.get().pop();
-     }
+                 this.keepAliveTime = millis;
 @@ -126,7 +_,7 @@
      private boolean checkIfClosed(long time) {
          if (this.closed) {
diff --git a/paper-server/patches/sources/net/minecraft/server/network/ServerConnectionListener.java.patch b/paper-server/patches/sources/net/minecraft/server/network/ServerConnectionListener.java.patch
index 97e6b6ed34..78b623e0aa 100644
--- a/paper-server/patches/sources/net/minecraft/server/network/ServerConnectionListener.java.patch
+++ b/paper-server/patches/sources/net/minecraft/server/network/ServerConnectionListener.java.patch
@@ -68,7 +68,6 @@
                                          ? new RateKickingConnection(rateLimitPacketsPerSecond)
                                          : new Connection(PacketFlow.SERVERBOUND));
 -                                    ServerConnectionListener.this.connections.add(connection);
-+                                    // ServerConnectionListener.this.connections.add(connection); // Paper
 +                                    // Paper start - Add support for Proxy Protocol
 +                                    if (io.papermc.paper.configuration.GlobalConfiguration.get().proxies.proxyProtocol) {
 +                                        channel.pipeline().addAfter("timeout", "haproxy-decoder", new io.netty.handler.codec.haproxy.HAProxyMessageDecoder());
@@ -98,6 +97,7 @@
 +                                        });
 +                                    }
 +                                    // Paper end - Add support for proxy protocol
++                                    // ServerConnectionListener.this.connections.add(connection); // Paper - prevent blocking on adding a new connection while the server is ticking
 +                                    ServerConnectionListener.this.pending.add(connection); // Paper - prevent blocking on adding a new connection while the server is ticking
                                      connection.configurePacketHandler(channelPipeline);
                                      connection.setListenerForServerboundHandshake(