feat: block predicate impl & some more tests

src/main/java/net/minestom/server/entity/Player.java

@@ -1755,12 +1755,12 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
      * It closes the player inventory (when opened) if {@link #getOpenInventory()} returns null.
      */
     public void closeInventory() {
-        closeInventory(true);
+        closeInventory(false);
     }
 
     @ApiStatus.Internal
-    public void closeInventory(boolean sendClosePacket) {
-        tryCloseInventory(sendClosePacket);
+    public void closeInventory(boolean skipClosePacket) {
+        tryCloseInventory(skipClosePacket);
         inventory.update();
     }
 

src/main/java/net/minestom/server/instance/block/predicate/BlockPredicate.java

@@ -3,13 +3,30 @@ package net.minestom.server.instance.block.predicate;
 import net.kyori.adventure.nbt.BinaryTag;
 import net.kyori.adventure.nbt.CompoundBinaryTag;
 import net.minestom.server.instance.block.Block;
+import net.minestom.server.instance.block.BlockHandler;
 import net.minestom.server.network.NetworkBuffer;
+import net.minestom.server.utils.block.BlockUtils;
 import net.minestom.server.utils.nbt.BinaryTagSerializer;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Objects;
 import java.util.function.Predicate;
 
+/**
+ * <p>A predicate to filter blocks based on their name, properties, and/or nbt.</p>
+ *
+ * <p>Note: Inline with vanilla, providing none of the filters will match any block.</p>
+ *
+ * <p>Note: To match the vanilla behavior of comparing block NBT, the NBT predicate
+ * will ONLY match data which would be sent to the client eg with
+ * {@link BlockHandler#getBlockEntityTags()}. This is relevant because this structure
+ * is used for matching adventure mode blocks and must line up with client prediction.</p>
+ *
+ * @param blocks The block names/tags to match.
+ * @param state The block properties to match.
+ * @param nbt The block nbt to match.
+ */
 public record BlockPredicate(
         @Nullable BlockTypeFilter blocks,
         @Nullable PropertiesPredicate state,
@@ -82,7 +99,11 @@ public record BlockPredicate(
 
     @Override
     public boolean test(@NotNull Block block) {
-        throw new UnsupportedOperationException("not implemented");
+        if (blocks != null && !blocks.test(block))
+            return false;
+        if (state != null && !state.test(block))
+            return false;
+        return nbt == null || Objects.equals(nbt, BlockUtils.extractClientNbt(block));
     }
 
 }

src/main/java/net/minestom/server/instance/block/predicate/PropertiesPredicate.java

@@ -3,6 +3,7 @@ package net.minestom.server.instance.block.predicate;
 import net.kyori.adventure.nbt.BinaryTag;
 import net.kyori.adventure.nbt.CompoundBinaryTag;
 import net.kyori.adventure.nbt.StringBinaryTag;
+import net.minestom.server.instance.block.Block;
 import net.minestom.server.network.NetworkBuffer;
 import net.minestom.server.utils.nbt.BinaryTagSerializer;
 import org.jetbrains.annotations.NotNull;
@@ -10,8 +11,9 @@ import org.jetbrains.annotations.Nullable;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.function.Predicate;
 
-public record PropertiesPredicate(@NotNull Map<String, ValuePredicate> properties) {
+public record PropertiesPredicate(@NotNull Map<String, ValuePredicate> properties) implements Predicate<Block> {
 
     public static final NetworkBuffer.Type<PropertiesPredicate> NETWORK_TYPE = new NetworkBuffer.Type<>() {
         @Override
@@ -58,15 +60,39 @@ public record PropertiesPredicate(@NotNull Map<String, ValuePredicate> propertie
         properties = Map.copyOf(properties);
     }
 
-    public sealed interface ValuePredicate permits ValuePredicate.Exact, ValuePredicate.Range {
+    @Override
+    public boolean test(@NotNull Block block) {
+        for (Map.Entry<String, ValuePredicate> entry : properties.entrySet()) {
+            final String value = block.getProperty(entry.getKey());
+            if (!entry.getValue().test(value))
+                return false;
+        }
+        return true;
+    }
+
+    public sealed interface ValuePredicate extends Predicate<@Nullable String> permits ValuePredicate.Exact, ValuePredicate.Range {
 
         record Exact(@Nullable String value) implements ValuePredicate {
 
             public static final NetworkBuffer.Type<Exact> NETWORK_TYPE = NetworkBuffer.STRING.map(Exact::new, Exact::value);
             public static final BinaryTagSerializer<Exact> NBT_TYPE = BinaryTagSerializer.STRING.map(Exact::new, Exact::value);
 
+            @Override
+            public boolean test(@Nullable String prop) {
+                return prop != null && prop.equals(value);
+            }
         }
 
+        /**
+         * <p>Vanilla has some fancy behavior to get integer properties as ints, but seems to just compare the value
+         * anyway if its a string. Our behavior here is to attempt to parse the values as an integer and default
+         * to a string.compareTo otherwise.</p>
+         *
+         * <p>Providing no min or max or a property which does exist results in a constant false.</p>
+         *
+         * @param min The min value to match, inclusive
+         * @param max The max value to match, exclusive
+         */
         record Range(@Nullable String min, @Nullable String max) implements ValuePredicate {
 
             public static final NetworkBuffer.Type<Range> NETWORK_TYPE = new NetworkBuffer.Type<>() {
@@ -92,6 +118,21 @@ public record PropertiesPredicate(@NotNull Map<String, ValuePredicate> propertie
                         return builder.build();
                     }
             );
+
+            @Override
+            public boolean test(@Nullable String prop) {
+                if (prop == null || (min == null && max == null)) return false;
+                try {
+                    // Try to match as integers
+                    int value = Integer.parseInt(prop);
+                    return (min == null || value >= Integer.parseInt(min))
+                            && (max == null || value < Integer.parseInt(max));
+                } catch (NumberFormatException e) {
+                    // Not an integer, just compare the strings
+                    return (min == null || prop.compareTo(min) >= 0)
+                            && (max == null || prop.compareTo(max) < 0);
+                }
+            }
         }
 
         NetworkBuffer.Type<ValuePredicate> NETWORK_TYPE = new NetworkBuffer.Type<>() {

src/test/java/net/minestom/server/entity/player/PlayerBlockPlacementIntegrationTest.java

@@ -22,7 +22,6 @@ import org.junit.jupiter.params.provider.MethodSource;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 @EnvTest
 public class PlayerBlockPlacementIntegrationTest {
@@ -30,7 +29,6 @@ public class PlayerBlockPlacementIntegrationTest {
     @ParameterizedTest
     @MethodSource("placeBlockFromAdventureModeParams")
     public void placeBlockFromAdventureMode(Block baseBlock, BlockPredicates canPlaceOn, Env env) {
-        assumeTrue(false);
         var instance = env.createFlatInstance();
         var connection = env.createConnection();
         var player = connection.connect(instance, new Pos(0, 42, 0)).join();
@@ -55,7 +53,6 @@ public class PlayerBlockPlacementIntegrationTest {
     private static Stream<Arguments> placeBlockFromAdventureModeParams() {
         return Stream.of(
                 Arguments.of(Block.ACACIA_STAIRS.withProperty("facing", "south"), new BlockPredicates(new BlockPredicate(new BlockTypeFilter.Blocks(Block.ACACIA_STAIRS)))),
-                Arguments.of(Block.ACACIA_STAIRS, new BlockPredicates(new BlockPredicate(new BlockTypeFilter.Blocks(Block.ACACIA_STAIRS), PropertiesPredicate.exact("facing", "south"), null))),
                 Arguments.of(Block.ACACIA_STAIRS.withProperty("facing", "south"), new BlockPredicates(new BlockPredicate(new BlockTypeFilter.Blocks(Block.ACACIA_STAIRS), PropertiesPredicate.exact("facing", "south"), null))),
                 Arguments.of(Block.AMETHYST_BLOCK, new BlockPredicates(new BlockPredicate(new BlockTypeFilter.Blocks(Block.AMETHYST_BLOCK))))
         );

src/test/java/net/minestom/server/instance/block/SuspiciousGravelBlockHandler.java

@@ -0,0 +1,33 @@
+package net.minestom.server.instance.block;
+
+import net.minestom.server.item.ItemStack;
+import net.minestom.server.tag.Tag;
+import net.minestom.server.utils.NamespaceID;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collection;
+import java.util.List;
+
+public class SuspiciousGravelBlockHandler implements BlockHandler {
+    public static final SuspiciousGravelBlockHandler INSTANCE = new SuspiciousGravelBlockHandler(true);
+    public static final SuspiciousGravelBlockHandler INSTANCE_NO_TAGS = new SuspiciousGravelBlockHandler(false);
+
+    public static final Tag<String> LOOT_TABLE = Tag.String("LootTable");
+    public static final Tag<ItemStack> ITEM = Tag.ItemStack("item");
+
+    private final boolean hasTags;
+
+    public SuspiciousGravelBlockHandler(boolean hasTags) {
+        this.hasTags = hasTags;
+    }
+
+    @Override
+    public @NotNull NamespaceID getNamespaceId() {
+        return NamespaceID.from("minecraft:suspicious_gravel");
+    }
+
+    @Override
+    public @NotNull Collection<Tag<?>> getBlockEntityTags() {
+        return hasTags ? List.of(LOOT_TABLE, ITEM) : List.of();
+    }
+}

src/test/java/net/minestom/server/instance/block/predicate/BlockPredicateTest.java

@@ -0,0 +1,134 @@
+package net.minestom.server.instance.block.predicate;
+
+import net.kyori.adventure.nbt.CompoundBinaryTag;
+import net.minestom.server.MinecraftServer;
+import net.minestom.server.instance.block.Block;
+import net.minestom.server.instance.block.SuspiciousGravelBlockHandler;
+import net.minestom.server.item.ItemStack;
+import net.minestom.server.item.Material;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class BlockPredicateTest {
+
+    static {
+        MinecraftServer.init();
+    }
+
+    // See sibling files for blocks and properties tests
+
+    @Nested
+    class NbtPredicate {
+        private static final Block SUS_GRAVEL = Block.SUSPICIOUS_GRAVEL.withHandler(SuspiciousGravelBlockHandler.INSTANCE);
+
+        @Test
+        public void testMatching() {
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .build());
+            var block = SUS_GRAVEL.withNbt(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .build());
+            assertTrue(predicate.test(block));
+        }
+
+        @Test
+        public void testEmptyTarget() {
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .build());
+            var block = SUS_GRAVEL.withNbt(CompoundBinaryTag.builder()
+                    .build());
+            assertFalse(predicate.test(block));
+        }
+
+        @Test
+        public void testEmptySource() {
+            var itemNbt = ItemStack.of(Material.STONE).toItemNBT();
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .put("item", itemNbt)
+                    .build());
+            var block = SUS_GRAVEL.withNbt(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .put("item", itemNbt)
+                    .build());
+            assertTrue(predicate.test(block));
+        }
+
+        @Test
+        public void testNoMatchDeep() {
+            var itemNbt1 = ItemStack.of(Material.STONE).toItemNBT();
+            var itemNbt2 = ItemStack.of(Material.STONE).withAmount(2).toItemNBT();
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .put("item", itemNbt1)
+                    .build());
+            var block = SUS_GRAVEL.withNbt(CompoundBinaryTag.builder()
+                    .putString("LootTable", "minecraft:test")
+                    .put("item", itemNbt2)
+                    .build());
+            assertFalse(predicate.test(block));
+        }
+
+        @Test
+        public void testNoBlockEntity() {
+            // Never match if the block has no client block entity
+
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder().build());
+            var block = Block.STONE;
+            assertFalse(predicate.test(block), "stone should not match empty");
+        }
+
+        @Test
+        public void testNoExposedTags() {
+            var predicate = new BlockPredicate(CompoundBinaryTag.builder().putString("LootTable", "minecraft:stone").build());
+            // No exposed tags because no block handler so cannot match
+            assertFalse(predicate.test(Block.SUSPICIOUS_GRAVEL.withHandler(SuspiciousGravelBlockHandler.INSTANCE_NO_TAGS)
+                    .withNbt(CompoundBinaryTag.builder().putString("LootTable", "minecraft:stone").build())));
+
+            // In this case its fine because when there is no block handler we send the entire block entity
+            assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withNbt(CompoundBinaryTag.builder().putString("LootTable", "minecraft:stone").build())));
+        }
+    }
+
+
+    // Combinations
+
+    @Test
+    public void emptyMatchAnything() {
+        var predicate = new BlockPredicate(null, null, null);
+        assertTrue(predicate.test(Block.STONE_STAIRS));
+        assertTrue(predicate.test(Block.STONE_STAIRS.withProperty("facing", "east")));
+        assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withHandler(SuspiciousGravelBlockHandler.INSTANCE)));
+        assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withNbt(CompoundBinaryTag.builder().build())));
+        assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withNbt(CompoundBinaryTag.builder().putString("LootTable", "minecraft:test").build())));
+        assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withHandler(SuspiciousGravelBlockHandler.INSTANCE)
+                .withNbt(CompoundBinaryTag.builder().putString("LootTable", "minecraft:test").build())));
+    }
+
+    @Test
+    public void blockAlone() {
+        var predicate = new BlockPredicate(new BlockTypeFilter.Blocks(Block.STONE));
+        assertTrue(predicate.test(Block.STONE));
+        assertFalse(predicate.test(Block.DIRT));
+    }
+
+    @Test
+    public void propsAlone() {
+        var predicate = new BlockPredicate(PropertiesPredicate.exact("facing", "east"));
+        assertTrue(predicate.test(Block.STONE_STAIRS.withProperty("facing", "east")));
+        assertTrue(predicate.test(Block.FURNACE.withProperty("facing", "east")));
+        assertFalse(predicate.test(Block.FURNACE));
+    }
+
+    @Test
+    public void nbtAlone() {
+        var predicate = new BlockPredicate(CompoundBinaryTag.builder().putString("LootTable", "minecraft:stone").build());
+        assertTrue(predicate.test(Block.SUSPICIOUS_GRAVEL.withHandler(SuspiciousGravelBlockHandler.INSTANCE)
+                .withNbt(CompoundBinaryTag.builder().putString("LootTable", "minecraft:stone").build())));
+    }
+}

src/test/java/net/minestom/server/instance/block/predicate/BlockTypeFilterTest.java

@@ -0,0 +1,75 @@
+package net.minestom.server.instance.block.predicate;
+
+import net.minestom.server.MinecraftServer;
+import net.minestom.server.instance.block.Block;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class BlockTypeFilterTest {
+
+    static {
+        MinecraftServer.init();
+    }
+
+    @Test
+    public void testBlockExact() {
+        var filter = new BlockTypeFilter.Blocks(Block.STONE);
+        var block = Block.STONE;
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testBlockExactMulti() {
+        var filter = new BlockTypeFilter.Blocks(Block.STONE, Block.STONE_STAIRS);
+        var block = Block.STONE_STAIRS;
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testBlockExactMultiMissing() {
+        var filter = new BlockTypeFilter.Blocks(Block.STONE, Block.STONE_STAIRS);
+        var block = Block.DIRT;
+        assertFalse(filter.test(block));
+    }
+
+    @Test
+    public void testBlockExactDifferentPropertyA() {
+        var filter = new BlockTypeFilter.Blocks(Block.STONE_STAIRS);
+        var block = Block.STONE_STAIRS.withProperty("shape", "inner_left");
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testBlockExactDifferentPropertyB() {
+        var filter = new BlockTypeFilter.Blocks(Block.STONE_STAIRS.withProperty("shape", "inner_left"));
+        var block = Block.STONE_STAIRS;
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testTag() {
+        var filter = new BlockTypeFilter.Tag("minecraft:doors");
+        var block = Block.OAK_DOOR;
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testTagDifferentProperty() {
+        var filter = new BlockTypeFilter.Tag("minecraft:doors");
+        var block = Block.OAK_DOOR.withProperty("half", "upper");
+        assertTrue(filter.test(block));
+    }
+
+    @Test
+    public void testTagMissing() {
+        var filter = new BlockTypeFilter.Tag("minecraft:doors");
+        var block = Block.STONE;
+        assertFalse(filter.test(block));
+    }
+
+    @Test
+    public void testTagUnknown() {
+        assertThrows(NullPointerException.class, () -> new BlockTypeFilter.Tag("minecraft:not_a_tag"));
+    }
+}

src/test/java/net/minestom/server/instance/block/predicate/PropertiesPredicateTest.java

@@ -0,0 +1,75 @@
+package net.minestom.server.instance.block.predicate;
+
+import net.minestom.server.instance.block.Block;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Map;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+public class PropertiesPredicateTest {
+
+    @Test
+    public void testMultiMatch() {
+        var predicate = new PropertiesPredicate(Map.of("facing", new PropertiesPredicate.ValuePredicate.Exact("east"),
+                "shape", new PropertiesPredicate.ValuePredicate.Exact("inner_left")));
+        assertTrue(predicate.test(Block.STONE_STAIRS.withProperties(Map.of("facing", "east", "shape", "inner_left"))));
+        assertFalse(predicate.test(Block.STONE_STAIRS.withProperties(Map.of("facing", "east"))));
+        assertFalse(predicate.test(Block.STONE));
+    }
+
+    @Nested
+    class ValuePredicate {
+
+        private static Stream<Arguments> exactTests() {
+            return Stream.of(
+                    // name, expected, actual, valid
+                    arguments("success", "value", "value", true),
+                    arguments("fail", "value", "other", false),
+                    arguments("missing exp", null, "value", false),
+                    arguments("missing act", "value", null, false)
+            );
+        }
+
+        @ParameterizedTest(name = "{0}")
+        @MethodSource("exactTests")
+        public void matchExact(String name, String expected, String actual, boolean valid) {
+            var predicate = new PropertiesPredicate.ValuePredicate.Exact(expected);
+            assertEquals(valid, predicate.test(actual));
+        }
+
+        private static Stream<Arguments> rangeTests() {
+            return Stream.of(
+                    // name, min, max, value, valid
+                    arguments("int / min exact", "0", null, "0", true),
+                    arguments("int / min too low (inclusive)", "1", null, "0", false),
+                    arguments("int / max exact", null, "1", "0", true),
+                    arguments("int / max too high (exclusive)", null, "1", "1", false),
+                    arguments("int / range good a", "0", "2", "1", true),
+                    arguments("int / range good b", "0", "20", "11", true),
+                    arguments("int / range too low", "0", "2", "-1", false),
+                    arguments("int / range too high", "0", "2", "3", false),
+
+                    arguments("string / min exact", "a", null, "a", true),
+                    arguments("string / max exact", null, "b", "a", true),
+                    arguments("string / range good", "c", "g", "e", true),
+                    arguments("string / range bad low", "c", "g", "a", false),
+                    arguments("string / range bad high", "c", "g", "z", false)
+            );
+        }
+
+        @ParameterizedTest(name = "{0}")
+        @MethodSource("rangeTests")
+        public void matchRange(String name, String min, String max, String value, boolean valid) {
+            var predicate = new PropertiesPredicate.ValuePredicate.Range(min, max);
+            assertEquals(valid, predicate.test(value));
+        }
+
+    }
+}