From bcb0301fb1c4649c4a4462774cdaa7c0bf3b3b40 Mon Sep 17 00:00:00 2001 From: boxic Date: Mon, 21 Oct 2024 11:00:05 -0400 Subject: [PATCH] Duplicate block property ID fix + unit test (#2433) * Duplicate property check * increased bits per index * fixed MAX_STATES error * Block properties are now long, and new test for block state ID conversion * Block properties are now long, and new test for block state ID conversion * Block properties are now long, and new test for block state ID conversion * excessive bits, and assertion fix --------- Co-authored-by: Matt Worzala <35708499+mworzala@users.noreply.github.com> --- .../server/instance/block/BlockImpl.java | 38 ++++++++++--------- .../minestom/server/instance/BlockTest.java | 21 ++++++++++ 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/main/java/net/minestom/server/instance/block/BlockImpl.java b/src/main/java/net/minestom/server/instance/block/BlockImpl.java index 672dd6c7a..b4667de17 100644 --- a/src/main/java/net/minestom/server/instance/block/BlockImpl.java +++ b/src/main/java/net/minestom/server/instance/block/BlockImpl.java @@ -1,6 +1,6 @@ package net.minestom.server.instance.block; -import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap; import it.unimi.dsi.fastutil.objects.Object2ObjectArrayMap; import it.unimi.dsi.fastutil.objects.Object2ObjectMaps; import net.kyori.adventure.nbt.CompoundBinaryTag; @@ -20,7 +20,7 @@ import java.util.Map; import java.util.Objects; record BlockImpl(@NotNull Registry.BlockEntry registry, - int propertiesArray, + long propertiesArray, @Nullable CompoundBinaryTag nbt, @Nullable BlockHandler handler) implements Block { /** @@ -28,16 +28,17 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, *

* Block states are all stored within a single number. */ - private static final int BITS_PER_INDEX = 4; + private static final int BITS_PER_INDEX = 5; - private static final int MAX_STATES = Integer.SIZE / BITS_PER_INDEX; + private static final int MAX_STATES = Long.SIZE / BITS_PER_INDEX; + private static final int MAX_VALUES = 1 << BITS_PER_INDEX; // Block state -> block object private static final ObjectArray BLOCK_STATE_MAP = ObjectArray.singleThread(); // Block id -> valid property keys (order is important for lookup) private static final ObjectArray PROPERTIES_TYPE = ObjectArray.singleThread(); // Block id -> Map - private static final ObjectArray> POSSIBLE_STATES = ObjectArray.singleThread(); + private static final ObjectArray> POSSIBLE_STATES = ObjectArray.singleThread(); private static final Registry.Container CONTAINER = Registry.createStaticContainer(Registry.Resource.BLOCKS, (namespace, properties) -> { final int blockId = properties.getInt("id"); @@ -57,6 +58,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, for (var entry : stateProperties) { final var k = entry.getKey(); final var v = (List) entry.getValue(); + assert v.size() < MAX_VALUES; propertyTypes[i++] = new PropertyType(k, v); } } else { @@ -68,7 +70,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, // Retrieve block states { final int propertiesCount = stateObject.size(); - int[] propertiesKeys = new int[propertiesCount]; + long[] propertiesKeys = new long[propertiesCount]; BlockImpl[] blocksValues = new BlockImpl[propertiesCount]; int propertiesOffset = 0; for (var stateEntry : stateObject) { @@ -76,7 +78,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, final var stateOverride = (Map) stateEntry.getValue(); final var propertyMap = BlockUtils.parseProperties(query); assert propertyTypes.length == propertyMap.size(); - int propertiesValue = 0; + long propertiesValue = 0; for (Map.Entry entry : propertyMap.entrySet()) { final byte keyIndex = findKeyIndex(propertyTypes, entry.getKey(), null); final byte valueIndex = findValueIndex(propertyTypes[keyIndex], entry.getValue(), null); @@ -90,7 +92,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, propertiesKeys[propertiesOffset] = propertiesValue; blocksValues[propertiesOffset++] = block; } - POSSIBLE_STATES.set(blockId, new Int2ObjectArrayMap<>(propertiesKeys, blocksValues, propertiesOffset)); + POSSIBLE_STATES.set(blockId, new Long2ObjectArrayMap<>(propertiesKeys, blocksValues, propertiesOffset)); } // Register default state final int defaultState = properties.getInt("defaultStateId"); @@ -129,7 +131,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, assert propertyTypes != null; final byte keyIndex = findKeyIndex(propertyTypes, property, this); final byte valueIndex = findValueIndex(propertyTypes[keyIndex], value, this); - final int updatedProperties = updateIndex(propertiesArray, keyIndex, valueIndex); + final long updatedProperties = updateIndex(propertiesArray, keyIndex, valueIndex); return compute(updatedProperties); } @@ -138,7 +140,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, if (properties.isEmpty()) return this; final PropertyType[] propertyTypes = PROPERTIES_TYPE.get(id()); assert propertyTypes != null; - int updatedProperties = this.propertiesArray; + long updatedProperties = this.propertiesArray; for (Map.Entry entry : properties.entrySet()) { final byte keyIndex = findKeyIndex(propertyTypes, entry.getKey(), this); final byte valueIndex = findValueIndex(propertyTypes[keyIndex], entry.getValue(), this); @@ -178,8 +180,8 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, for (int i = 0; i < length; i++) { PropertyType property = propertyTypes[i]; keys[i] = property.key(); - final int index = extractIndex(propertiesArray, i); - values[i] = property.values().get(index); + final long index = extractIndex(propertiesArray, i); + values[i] = property.values().get((int) index); } return Object2ObjectMaps.unmodifiable(new Object2ObjectArrayMap<>(keys, values, length)); } @@ -199,7 +201,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, return tag.read(Objects.requireNonNullElse(nbt, CompoundBinaryTag.empty())); } - private Int2ObjectArrayMap possibleProperties() { + private Long2ObjectArrayMap possibleProperties() { return POSSIBLE_STATES.get(id()); } @@ -220,7 +222,7 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, return Objects.hash(stateId(), nbt, handler); } - private Block compute(int updatedProperties) { + private Block compute(long updatedProperties) { if (updatedProperties == this.propertiesArray) return this; final BlockImpl block = possibleProperties().get(updatedProperties); assert block != null; @@ -255,15 +257,15 @@ record BlockImpl(@NotNull Registry.BlockEntry registry, private record PropertyType(String key, List values) { } - static int updateIndex(int value, int index, byte newValue) { + static long updateIndex(long value, int index, byte newValue) { final int position = index * BITS_PER_INDEX; final int mask = (1 << BITS_PER_INDEX) - 1; - value &= ~(mask << position); // Clear the bits at the specified position - value |= (newValue & mask) << position; // Set the new bits + value &= ~((long) mask << position); // Clear the bits at the specified position + value |= (long) (newValue & mask) << position; // Set the new bits return value; } - static int extractIndex(int value, int index) { + static long extractIndex(long value, int index) { final int position = index * BITS_PER_INDEX; final int mask = (1 << BITS_PER_INDEX) - 1; return ((value >> position) & mask); diff --git a/src/test/java/net/minestom/server/instance/BlockTest.java b/src/test/java/net/minestom/server/instance/BlockTest.java index eadc2e821..085ca6db8 100644 --- a/src/test/java/net/minestom/server/instance/BlockTest.java +++ b/src/test/java/net/minestom/server/instance/BlockTest.java @@ -7,10 +7,12 @@ import net.minestom.server.instance.block.Block; import net.minestom.server.tag.Tag; import org.junit.jupiter.api.Test; +import java.util.HashSet; import java.util.Map; import java.util.Objects; import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; public class BlockTest { @@ -84,4 +86,23 @@ public class BlockTest { assertEquals(start, new Vec(0.3125, 0, 0.3125)); assertEquals(end, new Vec(0.6875, 0.5625, 0.6875)); } + + @Test + public void testDuplicateProperties() { + HashSet assignedStates = new HashSet<>(); + for (Block block : Block.values()) { + for (Block blockWithState : block.possibleStates()) { + assertTrue(assignedStates.add(blockWithState.stateId())); + } + } + } + + @Test + public void testStateIdConversion() { + for (Block block : Block.values()) { + for (Block blockWithState : block.possibleStates()) { + assertEquals(blockWithState, Block.fromStateId(blockWithState.stateId())); + } + } + } }