Fix BlockHandler#onInteract return, simplify block placement code

src/main/java/net/minestom/server/instance/block/BlockHandler.java

@@ -1,11 +1,11 @@
 package net.minestom.server.instance.block;
 
+import net.minestom.server.coordinate.Point;
 import net.minestom.server.entity.Entity;
 import net.minestom.server.entity.Player;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.tag.Tag;
 import net.minestom.server.utils.NamespaceID;
-import net.minestom.server.coordinate.Point;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 
@@ -42,10 +42,10 @@ public interface BlockHandler {
      * menu, this prevents the player from placing a block when opening it for instance).
      *
      * @param interaction the interaction details
-     * @return true if this block blocks normal item use, false otherwise
+     * @return true to let the block interaction happens, false to cancel
      */
     default boolean onInteract(@NotNull Interaction interaction) {
-        return false;
+        return true;
     }
 
     /**

src/main/java/net/minestom/server/listener/BlockPlacementListener.java

@@ -2,7 +2,6 @@ package net.minestom.server.listener;
 
 import net.minestom.server.MinecraftServer;
 import net.minestom.server.coordinate.Point;
-import net.minestom.server.coordinate.Vec;
 import net.minestom.server.entity.Entity;
 import net.minestom.server.entity.EntityType;
 import net.minestom.server.entity.GameMode;
@@ -31,7 +30,6 @@ import net.minestom.server.utils.validate.Check;
 import java.util.Set;
 
 public class BlockPlacementListener {
-
     private static final BlockManager BLOCK_MANAGER = MinecraftServer.getBlockManager();
 
     public static void listener(ClientPlayerBlockPlacementPacket packet, Player player) {
@@ -58,133 +56,112 @@ public class BlockPlacementListener {
         // FIXME: onUseOnBlock
         PlayerBlockInteractEvent playerBlockInteractEvent = new PlayerBlockInteractEvent(player, hand, interactedBlock, blockPosition, blockFace);
         EventDispatcher.call(playerBlockInteractEvent);
+        boolean blockUse = playerBlockInteractEvent.isBlockingItemUse();
         if (!playerBlockInteractEvent.isCancelled()) {
             final var handler = interactedBlock.handler();
             if (handler != null) {
-                handler.onInteract(new BlockHandler.Interaction(interactedBlock, instance, blockPosition, player, hand));
+                blockUse |= !handler.onInteract(new BlockHandler.Interaction(interactedBlock, instance, blockPosition, player, hand));
             }
         }
-        if (playerBlockInteractEvent.isBlockingItemUse()) {
+        if (blockUse) {
             return;
         }
 
         final Material useMaterial = usedItem.getMaterial();
+        if (!useMaterial.isBlock()) {
+            // Player didn't try to place a block but interacted with one
+            PlayerUseItemOnBlockEvent event = new PlayerUseItemOnBlockEvent(player, hand, usedItem, blockPosition, direction);
+            EventDispatcher.call(event);
+            return;
+        }
 
         // Verify if the player can place the block
         boolean canPlaceBlock = true;
-        {
+        // Check if the player is allowed to place blocks based on their game mode
-            if (useMaterial == Material.AIR) { // Can't place air
+        if (player.getGameMode() == GameMode.SPECTATOR) {
-                return;
+            canPlaceBlock = false; // Spectators can't place blocks
-            }
+        } else if (player.getGameMode() == GameMode.ADVENTURE) {
-
+            //Check if the block can be placed on the block
-            //Check if the player is allowed to place blocks based on their game mode
+            canPlaceBlock = usedItem.getMeta().getCanPlaceOn().contains(interactedBlock);
-            if (player.getGameMode() == GameMode.SPECTATOR) {
-                canPlaceBlock = false; //Spectators can't place blocks
-            } else if (player.getGameMode() == GameMode.ADVENTURE) {
-                //Check if the block can placed on the block
-                canPlaceBlock = usedItem.getMeta().getCanPlaceOn().contains(interactedBlock);
-            }
         }
 
         // Get the newly placed block position
         final int offsetX = blockFace == BlockFace.WEST ? -1 : blockFace == BlockFace.EAST ? 1 : 0;
         final int offsetY = blockFace == BlockFace.BOTTOM ? -1 : blockFace == BlockFace.TOP ? 1 : 0;
         final int offsetZ = blockFace == BlockFace.NORTH ? -1 : blockFace == BlockFace.SOUTH ? 1 : 0;
-
+        final Point placementPosition = blockPosition.add(offsetX, offsetY, offsetZ);
-        final Point placementPosition = new Vec(offsetX, offsetY, offsetZ).add(blockPosition);
 
         if (!canPlaceBlock) {
-            if (useMaterial.isBlock()) {
+            // Send a block change with AIR as block to keep the client in sync,
-                //Send a block change with AIR as block to keep the client in sync,
+            // using refreshChunk results in the client not being in sync
-                //using refreshChunk results in the client not being in sync
+            // after rapid invalid block placements
-                //after rapid invalid block placements
+            player.getPlayerConnection().sendPacket(new BlockChangePacket(placementPosition, Block.AIR));
-                player.getPlayerConnection().sendPacket(new BlockChangePacket(placementPosition, Block.AIR.stateId()));
-            }
             return;
         }
 
         final Chunk chunk = instance.getChunkAt(placementPosition);
         Check.stateCondition(!ChunkUtils.isLoaded(chunk),
                 "A player tried to place a block in the border of a loaded chunk {0}", placementPosition);
-
+        if (chunk.isReadOnly()) {
-        // The concerned chunk will be send to the player if an error occur
+            chunk.sendChunk(player);
-        // This will ensure that the player has the correct version of the chunk
+            return;
-        boolean refreshChunk = false;
-
-        if (useMaterial.isBlock()) {
-            if (!chunk.isReadOnly()) {
-                final Block placedBlock = useMaterial.getBlock();
-                final Set<Entity> entities = instance.getChunkEntities(chunk);
-                // Check if the player is trying to place a block in an entity
-                boolean intersect = player.getBoundingBox().intersectWithBlock(placementPosition);
-                if (!intersect && placedBlock.isSolid()) {
-                    // TODO push entities too close to the position
-                    for (Entity entity : entities) {
-                        // 'player' has already been checked
-                        if (entity == player ||
-                                entity.getEntityType() == EntityType.ITEM)
-                            continue;
-                        // Marker Armor Stands should not prevent block placement
-                        if (entity.getEntityMeta() instanceof ArmorStandMeta) {
-                            ArmorStandMeta armorStandMeta = (ArmorStandMeta) entity.getEntityMeta();
-                            if (armorStandMeta.isMarker()) {
-                                continue;
-                            }
-                        }
-                        intersect = entity.getBoundingBox().intersectWithBlock(placementPosition);
-                        if (intersect)
-                            break;
-                    }
-                }
-
-                if (!intersect) {
-                    // BlockPlaceEvent check
-                    PlayerBlockPlaceEvent playerBlockPlaceEvent = new PlayerBlockPlaceEvent(player, placedBlock, blockFace, placementPosition, packet.hand);
-                    playerBlockPlaceEvent.consumeBlock(player.getGameMode() != GameMode.CREATIVE);
-
-                    EventDispatcher.call(playerBlockPlaceEvent);
-                    if (!playerBlockPlaceEvent.isCancelled()) {
-
-                        // BlockPlacementRule check
-                        Block resultBlock = playerBlockPlaceEvent.getBlock();
-                        final BlockPlacementRule blockPlacementRule = BLOCK_MANAGER.getBlockPlacementRule(resultBlock);
-                        if (blockPlacementRule != null) {
-                            // Get id from block placement rule instead of the event
-                            resultBlock = blockPlacementRule.blockPlace(instance, resultBlock, blockFace, blockPosition, player);
-                        }
-                        final boolean placementRuleCheck = resultBlock != null;
-                        if (placementRuleCheck) {
-                            // Place the block
-                            instance.placeBlock(player, resultBlock, placementPosition,
-                                    blockFace, packet.cursorPositionX, packet.cursorPositionY, packet.cursorPositionZ);
-                            // Block consuming
-                            if (playerBlockPlaceEvent.doesConsumeBlock()) {
-                                // Consume the block in the player's hand
-                                final ItemStack newUsedItem = usedItem.getStackingRule().apply(usedItem, usedItem.getAmount() - 1);
-                                playerInventory.setItemInHand(hand, newUsedItem);
-                            }
-                        } else {
-                            refreshChunk = true;
-                        }
-                    } else {
-                        refreshChunk = true;
-                    }
-                } else {
-                    refreshChunk = true;
-                }
-            } else {
-                refreshChunk = true;
-            }
-        } else {
-            // Player didn't try to place a block but interacted with one
-            PlayerUseItemOnBlockEvent event = new PlayerUseItemOnBlockEvent(player, hand, usedItem, blockPosition, direction);
-            EventDispatcher.call(event);
-            refreshChunk = true;
         }
 
-        // Refresh chunk section if needed
+        final Block placedBlock = useMaterial.getBlock();
-        if (refreshChunk) {
+        final Set<Entity> entities = instance.getChunkEntities(chunk);
+        // Check if the player is trying to place a block in an entity
+        boolean intersect = player.getBoundingBox().intersectWithBlock(placementPosition);
+        if (!intersect && placedBlock.isSolid()) {
+            // TODO push entities too close to the position
+            for (Entity entity : entities) {
+                // 'player' has already been checked
+                if (entity == player ||
+                        entity.getEntityType() == EntityType.ITEM)
+                    continue;
+                // Marker Armor Stands should not prevent block placement
+                if (entity.getEntityMeta() instanceof ArmorStandMeta) {
+                    ArmorStandMeta armorStandMeta = (ArmorStandMeta) entity.getEntityMeta();
+                    if (armorStandMeta.isMarker()) {
+                        continue;
+                    }
+                }
+                intersect = entity.getBoundingBox().intersectWithBlock(placementPosition);
+                if (intersect)
+                    break;
+            }
+        }
+        if (intersect) {
             chunk.sendChunk(player);
+            return;
+        }
+        // BlockPlaceEvent check
+        PlayerBlockPlaceEvent playerBlockPlaceEvent = new PlayerBlockPlaceEvent(player, placedBlock, blockFace, placementPosition, packet.hand);
+        playerBlockPlaceEvent.consumeBlock(player.getGameMode() != GameMode.CREATIVE);
+        EventDispatcher.call(playerBlockPlaceEvent);
+        if (playerBlockPlaceEvent.isCancelled()) {
+            chunk.sendChunk(player);
+            return;
+        }
+
+        // BlockPlacementRule check
+        Block resultBlock = playerBlockPlaceEvent.getBlock();
+        final BlockPlacementRule blockPlacementRule = BLOCK_MANAGER.getBlockPlacementRule(resultBlock);
+        if (blockPlacementRule != null) {
+            // Get id from block placement rule instead of the event
+            resultBlock = blockPlacementRule.blockPlace(instance, resultBlock, blockFace, blockPosition, player);
+        }
+        if (resultBlock == null) {
+            chunk.sendChunk(player);
+            return;
+        }
+        // Place the block
+        instance.placeBlock(player, resultBlock, placementPosition,
+                blockFace, packet.cursorPositionX, packet.cursorPositionY, packet.cursorPositionZ);
+        // Block consuming
+        if (playerBlockPlaceEvent.doesConsumeBlock()) {
+            // Consume the block in the player's hand
+            final ItemStack newUsedItem = usedItem.getStackingRule().apply(usedItem, usedItem.getAmount() - 1);
+            playerInventory.setItemInHand(hand, newUsedItem);
         }
     }
 }

src/main/java/net/minestom/server/network/packet/server/play/BlockChangePacket.java

@@ -1,11 +1,12 @@
 package net.minestom.server.network.packet.server.play;
 
+import net.minestom.server.coordinate.Point;
+import net.minestom.server.coordinate.Vec;
+import net.minestom.server.instance.block.Block;
 import net.minestom.server.network.packet.server.ServerPacket;
 import net.minestom.server.network.packet.server.ServerPacketIdentifier;
 import net.minestom.server.utils.binary.BinaryReader;
 import net.minestom.server.utils.binary.BinaryWriter;
-import net.minestom.server.coordinate.Point;
-import net.minestom.server.coordinate.Vec;
 import org.jetbrains.annotations.NotNull;
 
 public class BlockChangePacket implements ServerPacket {
@@ -18,6 +19,10 @@ public class BlockChangePacket implements ServerPacket {
         this.blockStateId = blockStateId;
     }
 
+    public BlockChangePacket(Point blockPosition, Block block) {
+        this(blockPosition, block.stateId());
+    }
+
     public BlockChangePacket() {
         this(Vec.ZERO, 0);
     }