From 92d12bf7bc69917f42143f805f2627cb69a60896 Mon Sep 17 00:00:00 2001 From: Eclipse Date: Tue, 1 Jul 2025 12:33:37 +0000 Subject: [PATCH] Fix block by Java key lookup (#5637) * Fix block by Java key lookup, optimise hardcoded lookup a bit, clarify Javadocs * Rename the registry too --- .../world/GeyserSpigotBlockPlaceListener.java | 2 +- .../manager/GeyserSpigotWorldManager.java | 4 +-- .../geyser/registry/BlockRegistries.java | 4 +-- .../mappings/versions/MappingsReader_v1.java | 8 ++--- .../populator/BlockRegistryPopulator.java | 4 +-- .../CustomBlockRegistryPopulator.java | 6 ++-- .../cache/registry/JavaRegistries.java | 36 ++++++++++--------- .../org/geysermc/geyser/util/SoundUtils.java | 2 +- 8 files changed, 34 insertions(+), 32 deletions(-) diff --git a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/GeyserSpigotBlockPlaceListener.java b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/GeyserSpigotBlockPlaceListener.java index 1cdd77c64..7a4caabd7 100644 --- a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/GeyserSpigotBlockPlaceListener.java +++ b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/GeyserSpigotBlockPlaceListener.java @@ -59,7 +59,7 @@ public class GeyserSpigotBlockPlaceListener implements Listener { event.getBlockPlaced().getX(), event.getBlockPlaced().getY(), event.getBlockPlaced().getZ()))); } else { String javaBlockId = event.getBlockPlaced().getBlockData().getAsString(); - placeBlockSoundPacket.setExtraData(session.getBlockMappings().getBedrockBlockId(BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().getOrDefault(javaBlockId, Block.JAVA_AIR_ID))); + placeBlockSoundPacket.setExtraData(session.getBlockMappings().getBedrockBlockId(BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().getOrDefault(javaBlockId, Block.JAVA_AIR_ID))); } placeBlockSoundPacket.setIdentifier(":"); session.sendUpstreamPacket(placeBlockSoundPacket); diff --git a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/manager/GeyserSpigotWorldManager.java b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/manager/GeyserSpigotWorldManager.java index 54b5b9178..ea36dd1e7 100644 --- a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/manager/GeyserSpigotWorldManager.java +++ b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/manager/GeyserSpigotWorldManager.java @@ -76,9 +76,9 @@ public class GeyserSpigotWorldManager extends WorldManager { // Terrible behavior, but this is basically what's always been happening behind the scenes anyway. CompletableFuture blockData = new CompletableFuture<>(); Bukkit.getRegionScheduler().execute(this.plugin, block.getLocation(), () -> blockData.complete(block.getBlockData().getAsString())); - return BlockRegistries.JAVA_IDENTIFIER_TO_ID.getOrDefault(blockData.join(), org.geysermc.geyser.level.block.type.Block.JAVA_AIR_ID); + return BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.getOrDefault(blockData.join(), org.geysermc.geyser.level.block.type.Block.JAVA_AIR_ID); } - return BlockRegistries.JAVA_IDENTIFIER_TO_ID.getOrDefault(block.getBlockData().getAsString(), org.geysermc.geyser.level.block.type.Block.JAVA_AIR_ID); // TODO could just make this a BlockState lookup? + return BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.getOrDefault(block.getBlockData().getAsString(), org.geysermc.geyser.level.block.type.Block.JAVA_AIR_ID); // TODO could just make this a BlockState lookup? } @Override diff --git a/core/src/main/java/org/geysermc/geyser/registry/BlockRegistries.java b/core/src/main/java/org/geysermc/geyser/registry/BlockRegistries.java index 2c04930be..e0e61bdce 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/BlockRegistries.java +++ b/core/src/main/java/org/geysermc/geyser/registry/BlockRegistries.java @@ -78,9 +78,9 @@ public class BlockRegistries { public static final ListRegistry JAVA_BLOCKS = ListRegistry.create(RegistryLoaders.empty(ArrayList::new)); /** - * A mapped registry containing the Java identifiers to IDs. + * A mapped registry containing the Java block state identifiers to IDs. */ - public static final MappedRegistry> JAVA_IDENTIFIER_TO_ID = MappedRegistry.create(RegistryLoaders.empty(Object2IntOpenHashMap::new)); + public static final MappedRegistry> JAVA_BLOCK_STATE_IDENTIFIER_TO_ID = MappedRegistry.create(RegistryLoaders.empty(Object2IntOpenHashMap::new)); /** * A registry containing non-vanilla block IDS. diff --git a/core/src/main/java/org/geysermc/geyser/registry/mappings/versions/MappingsReader_v1.java b/core/src/main/java/org/geysermc/geyser/registry/mappings/versions/MappingsReader_v1.java index b5e25a4ba..c09399980 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/mappings/versions/MappingsReader_v1.java +++ b/core/src/main/java/org/geysermc/geyser/registry/mappings/versions/MappingsReader_v1.java @@ -262,7 +262,7 @@ public class MappingsReader_v1 extends MappingsReader { .creativeCategory(creativeCategory) .creativeGroup(creativeGroup); - if (BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().containsKey(identifier)) { + if (BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().containsKey(identifier)) { // There is only one Java block state to override CustomBlockComponentsMapping componentsMapping = createCustomBlockComponentsMapping(node, identifier, name); CustomBlockData blockData = customBlockDataBuilder @@ -280,7 +280,7 @@ public class MappingsReader_v1 extends MappingsReader { while (fields.hasNext()) { Map.Entry overrideEntry = fields.next(); String state = identifier + "[" + overrideEntry.getKey() + "]"; - if (!BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().containsKey(state)) { + if (!BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().containsKey(state)) { throw new InvalidCustomMappingsFileException("Unknown Java block state: " + state + " for state_overrides."); } componentsMap.put(state, createCustomBlockComponentsMapping(overrideEntry.getValue(), state, name)); @@ -292,7 +292,7 @@ public class MappingsReader_v1 extends MappingsReader { if (!onlyOverrideStates) { // Create components for any remaining Java block states - BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().keySet() + BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().keySet() .stream() .filter(s -> s.startsWith(identifier + "[")) .filter(Predicate.not(componentsMap::containsKey)) @@ -365,7 +365,7 @@ public class MappingsReader_v1 extends MappingsReader { */ private CustomBlockComponentsMapping createCustomBlockComponentsMapping(JsonNode node, String stateKey, String name) { // This is needed to find the correct selection box for the given block - int id = BlockRegistries.JAVA_IDENTIFIER_TO_ID.getOrDefault(stateKey, -1); + int id = BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.getOrDefault(stateKey, -1); BoxComponent boxComponent = createBoxComponent(id); BoxComponent extendedBoxComponent = createExtendedBoxComponent(id); CustomBlockComponents.Builder builder = new GeyserCustomBlockComponents.Builder() diff --git a/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java b/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java index a44e4deb6..50dd483c6 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java +++ b/core/src/main/java/org/geysermc/geyser/registry/populator/BlockRegistryPopulator.java @@ -421,7 +421,7 @@ public final class BlockRegistryPopulator { javaRuntimeId++; String javaId = javaBlockState.toString().intern(); - BlockRegistries.JAVA_IDENTIFIER_TO_ID.register(javaId, javaRuntimeId); + BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.register(javaId, javaRuntimeId); } BLOCKS_NBT = blocksNbt; @@ -441,7 +441,7 @@ public final class BlockRegistryPopulator { private static BitSet toBlockStateSet(ArrayNode node) { BitSet blockStateSet = new BitSet(node.size()); for (JsonNode javaIdentifier : node) { - blockStateSet.set(BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().getInt(javaIdentifier.textValue())); + blockStateSet.set(BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().getInt(javaIdentifier.textValue())); } return blockStateSet; } diff --git a/core/src/main/java/org/geysermc/geyser/registry/populator/CustomBlockRegistryPopulator.java b/core/src/main/java/org/geysermc/geyser/registry/populator/CustomBlockRegistryPopulator.java index d672d1732..d38204aaa 100644 --- a/core/src/main/java/org/geysermc/geyser/registry/populator/CustomBlockRegistryPopulator.java +++ b/core/src/main/java/org/geysermc/geyser/registry/populator/CustomBlockRegistryPopulator.java @@ -188,7 +188,7 @@ public class CustomBlockRegistryPopulator { } for(Map.Entry entry : BLOCK_STATE_OVERRIDES_QUEUE.entrySet()) { - int id = BlockRegistries.JAVA_IDENTIFIER_TO_ID.getOrDefault(entry.getKey(), -1); + int id = BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.getOrDefault(entry.getKey(), -1); if (id == -1) { GeyserImpl.getInstance().getLogger().warning("Custom block state override for Java Identifier: " + entry.getKey() + " could not be registered as it is not a valid block state."); @@ -212,7 +212,7 @@ public class CustomBlockRegistryPopulator { CUSTOM_BLOCK_ITEM_OVERRIDES.put(block.javaIdentifier(), block.data()); } block.states().forEach((javaIdentifier, customBlockState) -> { - int id = BlockRegistries.JAVA_IDENTIFIER_TO_ID.getOrDefault(javaIdentifier, -1); + int id = BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.getOrDefault(javaIdentifier, -1); blockStateOverrides.put(id, customBlockState.state()); BoxComponent extendedCollisionBox = customBlockState.extendedCollisionBox(); if (extendedCollisionBox != null) { @@ -302,7 +302,7 @@ public class CustomBlockRegistryPopulator { block.setJavaId(javaBlockState.stateGroupId()); BlockRegistries.JAVA_BLOCKS.registerWithAnyIndex(javaBlockState.stateGroupId(), block, Blocks.AIR); - BlockRegistries.JAVA_IDENTIFIER_TO_ID.register(javaId, stateRuntimeId); + BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.register(javaId, stateRuntimeId); BlockRegistries.NON_VANILLA_BLOCK_IDS.register(set -> set.set(stateRuntimeId)); // TODO register different collision types? diff --git a/core/src/main/java/org/geysermc/geyser/session/cache/registry/JavaRegistries.java b/core/src/main/java/org/geysermc/geyser/session/cache/registry/JavaRegistries.java index 25acb493f..e6c6a05de 100644 --- a/core/src/main/java/org/geysermc/geyser/session/cache/registry/JavaRegistries.java +++ b/core/src/main/java/org/geysermc/geyser/session/cache/registry/JavaRegistries.java @@ -64,15 +64,17 @@ public class JavaRegistries { private static final List> VALUES = new ArrayList<>(); public static final JavaRegistryKey BLOCK = createHardcoded("block", BlockRegistries.JAVA_BLOCKS, - Block::javaId, Block::javaIdentifier, key -> Optional.ofNullable(BlockRegistries.JAVA_IDENTIFIER_TO_ID.get(key.asString())).orElse(-1)); + Block::javaId, Block::javaIdentifier, key -> BlockRegistries.JAVA_BLOCKS.get().stream() + .filter(block -> block.javaIdentifier().equals(key)) + .findFirst()); public static final JavaRegistryKey ITEM = createHardcoded("item", Registries.JAVA_ITEMS, - Item::javaId, Item::javaKey, key -> Optional.ofNullable(Registries.JAVA_ITEM_IDENTIFIERS.get(key.asString())).map(Item::javaId).orElse(-1)); + Item::javaId, Item::javaKey, key -> Optional.ofNullable(Registries.JAVA_ITEM_IDENTIFIERS.get(key.asString()))); public static JavaRegistryKey ENTITY_TYPE = createHardcoded("entity_type", Arrays.asList(EntityType.values()), EntityType::ordinal, type -> MinecraftKey.key(type.name().toLowerCase(Locale.ROOT)), key -> { try { - return EntityType.valueOf(key.value().toUpperCase(Locale.ROOT)).ordinal(); + return Optional.of(EntityType.valueOf(key.value().toUpperCase(Locale.ROOT))); } catch (IllegalArgumentException exception) { - return -1; // Non-existent entity type + return Optional.empty(); // Non-existent entity type } }); @@ -105,13 +107,13 @@ public class JavaRegistries { } private static JavaRegistryKey createHardcoded(String key, ListRegistry registry, RegistryNetworkMapper networkSerializer, - RegistryIdentifierMapper identifierMapper, RegistryIdMapper idMapper) { - return createHardcoded(key, registry.get(), networkSerializer, identifierMapper, idMapper); + RegistryObjectIdentifierMapper objectIdentifierMapper, RegistryIdentifierObjectMapper identifierObjectMapper) { + return createHardcoded(key, registry.get(), networkSerializer, objectIdentifierMapper, identifierObjectMapper); } private static JavaRegistryKey createHardcoded(String key, List registry, RegistryNetworkMapper networkSerializer, - RegistryIdentifierMapper identifierMapper, RegistryIdMapper idMapper) { - return create(key, new HardcodedLookup<>(registry, networkSerializer, identifierMapper, idMapper)); + RegistryObjectIdentifierMapper objectIdentifierMapper, RegistryIdentifierObjectMapper identifierObjectMapper) { + return create(key, new HardcodedLookup<>(registry, networkSerializer, objectIdentifierMapper, identifierObjectMapper)); } private static JavaRegistryKey create(String key) { @@ -135,37 +137,37 @@ public class JavaRegistries { } @FunctionalInterface - interface RegistryIdentifierMapper { + interface RegistryObjectIdentifierMapper { Key get(T object); } @FunctionalInterface - interface RegistryIdMapper { + interface RegistryIdentifierObjectMapper { - int get(Key key); + Optional get(Key key); } - private record HardcodedLookup(List registry, RegistryNetworkMapper networkMapper, RegistryIdentifierMapper identifierMapper, - RegistryIdMapper idMapper) implements JavaRegistryKey.RegistryLookup { + private record HardcodedLookup(List registry, RegistryNetworkMapper networkMapper, RegistryObjectIdentifierMapper objectIdentifierMapper, + RegistryIdentifierObjectMapper identifierObjectMapper) implements JavaRegistryKey.RegistryLookup { @Override public Optional> entry(GeyserSession session, JavaRegistryKey registryKey, int networkId) { return Optional.ofNullable(registry.get(networkId)) - .map(value -> new RegistryEntryData<>(networkId, Objects.requireNonNull(identifierMapper.get(value)), value)); + .map(value -> new RegistryEntryData<>(networkId, Objects.requireNonNull(objectIdentifierMapper.get(value)), value)); } @Override public Optional> entry(GeyserSession session, JavaRegistryKey registryKey, Key key) { - int id = idMapper.get(key); - return Optional.ofNullable(registry.get(id)).map(value -> new RegistryEntryData<>(id, key, value)); + Optional object = identifierObjectMapper.get(key); + return object.map(value -> new RegistryEntryData<>(networkMapper.get(value), key, value)); } @Override public Optional> entry(GeyserSession session, JavaRegistryKey registryKey, T object) { int id = networkMapper.get(object); return Optional.ofNullable(registry.get(id)) - .map(value -> new RegistryEntryData<>(id, Objects.requireNonNull(identifierMapper.get(value)), value)); + .map(value -> new RegistryEntryData<>(id, Objects.requireNonNull(objectIdentifierMapper.get(value)), value)); } } diff --git a/core/src/main/java/org/geysermc/geyser/util/SoundUtils.java b/core/src/main/java/org/geysermc/geyser/util/SoundUtils.java index bfb163b66..898899760 100644 --- a/core/src/main/java/org/geysermc/geyser/util/SoundUtils.java +++ b/core/src/main/java/org/geysermc/geyser/util/SoundUtils.java @@ -148,7 +148,7 @@ public final class SoundUtils { soundPacket.setExtraData(soundMapping.extraData() + (int) (Math.round((Math.log10(pitch) / Math.log10(2)) * 12)) + 12); } else if (sound == SoundEvent.PLACE && soundMapping.extraData() == -1) { if (!soundMapping.identifier().equals(":")) { - int javaId = BlockRegistries.JAVA_IDENTIFIER_TO_ID.get().getOrDefault(soundMapping.identifier(), Block.JAVA_AIR_ID); + int javaId = BlockRegistries.JAVA_BLOCK_STATE_IDENTIFIER_TO_ID.get().getOrDefault(soundMapping.identifier(), Block.JAVA_AIR_ID); soundPacket.setExtraData(session.getBlockMappings().getBedrockBlockId(javaId)); } else { session.getGeyser().getLogger().debug("PLACE sound mapping identifier was invalid! Please report: " + soundMapping);