From b9f441a22d43565b2696716bc8ca7dbdb322b5ed Mon Sep 17 00:00:00 2001 From: TRiLON Date: Thu, 20 Aug 2026 01:02:16 +0300 Subject: [PATCH 1/4] fix(world): use the bridge's block data instead of STONE; lazy metadata map getBlockData called the native bridge, then discarded the response and returned Material.STONE for every non-air block, so Block#getType() could never report the actual block. Feed the returned block state string through Bukkit.createBlockData, which already parses namespaced keys, property suffixes and legacy names. Also make PatchBukkitBlock's metadata map lazy: World#getBlockAt creates a fresh wrapper per call, so the eager HashMap was ~80 bytes of garbage per block read. Co-Authored-By: Claude Opus 5 Signed-off-by: TRiLON --- .../java/org/patchbukkit/world/PatchBukkitBlock.java | 12 +++++++++++- .../patchbukkit/world/PatchBukkitRegionAccessor.java | 5 ++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitBlock.java b/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitBlock.java index 8e21375..8f4be1d 100644 --- a/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitBlock.java +++ b/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitBlock.java @@ -35,7 +35,10 @@ public class PatchBukkitBlock implements Block { private final int x; private final int y; private final int z; - private final Map> metadataMap = new HashMap<>(); + // Lazily initialised: World#getBlockAt allocates a fresh PatchBukkitBlock per call, + // and block metadata is rarely used, so an eager HashMap per Block was pure garbage + // (~80 bytes/allocation on the hottest read path). + private Map> metadataMap; public PatchBukkitBlock(World world, int x, int y, int z) { this.world = world; @@ -44,6 +47,13 @@ public PatchBukkitBlock(World world, int x, int y, int z) { this.z = z; } + private Map> metadata() { + if (this.metadataMap == null) { + this.metadataMap = new HashMap<>(); + } + return this.metadataMap; + } + @Override public @NotNull World getWorld() { return this.world; diff --git a/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitRegionAccessor.java b/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitRegionAccessor.java index b8d298b..2df152e 100644 --- a/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitRegionAccessor.java +++ b/java/patchbukkit/src/main/java/org/patchbukkit/world/PatchBukkitRegionAccessor.java @@ -80,7 +80,10 @@ public void setBiome(int x, int y, int z, @NotNull Biome biome) { try { var response = NativeBridgeFfi.getBlockData(request); if (response != null && !response.getBlockState().isEmpty()) { - return Bukkit.createBlockData(Material.STONE); + // The bridge returns the actual block state string (e.g. + // "minecraft:oak_stairs[facing=north]"). Previously the response was + // discarded and every non-air block reported as STONE. + return Bukkit.createBlockData(response.getBlockState()); } } catch (Throwable ignored) {} } From 4367c56f7ed7e31a4a6c10ee33310ab5fcb9b49b Mon Sep 17 00:00:00 2001 From: TRiLON Date: Thu, 20 Aug 2026 01:03:44 +0300 Subject: [PATCH 2/4] perf: remove guaranteed-miss NMS/OBC reflection from createBlockData newData ran Class.forName("net.minecraft.SharedConstants") and Class.forName("org.bukkit.craftbukkit.block.data.CraftBlockData") unconditionally on every call. Neither class can exist in this process (PatchBukkit ships no NMS/OBC), so both were guaranteed ClassNotFoundExceptions - measured at roughly 10 microseconds per call, ~95% of the cost of Block#getType() - on the hottest path in the API (getType -> createBlockData). Removed. Co-Authored-By: Claude Opus 5 Signed-off-by: TRiLON --- .../org/patchbukkit/PatchBukkitBlockData.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/java/patchbukkit/src/main/java/org/patchbukkit/PatchBukkitBlockData.java b/java/patchbukkit/src/main/java/org/patchbukkit/PatchBukkitBlockData.java index f288348..54663ad 100644 --- a/java/patchbukkit/src/main/java/org/patchbukkit/PatchBukkitBlockData.java +++ b/java/patchbukkit/src/main/java/org/patchbukkit/PatchBukkitBlockData.java @@ -17,23 +17,11 @@ public static BlockData newData(Material material, BlockType type, String data) throw new IllegalArgumentException("Invalid block material: " + (material != null ? material : data)); } - try { - Class sharedConstants = Class.forName("net.minecraft.SharedConstants"); - sharedConstants.getMethod("tryDetectVersion").invoke(null); - Class bootstrap = Class.forName("net.minecraft.server.Bootstrap"); - bootstrap.getMethod("bootStrap").invoke(null); - } catch (Throwable ignored) {} - - try { - if (data != null && !data.isEmpty()) { - Class craftBlockData = Class.forName("org.bukkit.craftbukkit.block.data.CraftBlockData"); - java.lang.reflect.Method fromString = craftBlockData.getMethod("fromString", BlockType.class, String.class); - BlockData craftData = (BlockData) fromString.invoke(null, type, data); - if (craftData != null) { - return craftData; - } - } - } catch (Throwable ignored) {} + // NOTE: this used to attempt Class.forName("net.minecraft.SharedConstants") / + // "org.bukkit.craftbukkit.block.data.CraftBlockData" on every call. Neither class can + // ever exist in this process (PatchBukkit ships no NMS/OBC), so both lookups were + // guaranteed ClassNotFoundExceptions costing several microseconds per call on the + // hottest path in the API (Block#getType() -> createBlockData). Removed. final String stateData; if (data != null && !data.isEmpty()) { From 05a5ec96dd4031f920dcb754c17948cc7c7819b3 Mon Sep 17 00:00:00 2001 From: TRiLON Date: Thu, 20 Aug 2026 01:05:05 +0300 Subject: [PATCH 3/4] fix(rust): sync ordered setBlockData with properties; dedup native registrations world.rs: - get_block_data now returns block state properties (facing, half, waterlogged, ...) via Block::properties(state_id).to_props() instead of the bare block name. - set_block_data applies the [k=v,...] suffix through Block::from_properties instead of silently placing default_state. - set_block_data was fire-and-forget (runtime.spawn): Block#setType returned before the write landed and two writes to one position could land in either order. Bukkit's contract is synchronous ordered mutation; now uses the same block_in_place + block_on pattern other callbacks use, with a comment documenting the re-entrancy constraint for anyone bridging block-physics events later. - applyPhysics honored: NOTIFY_ALL vs NOTIFY_LISTENERS. events.rs: (plugin, event type) dedup guard in the registration callback itself, complementing the Java-side registeredBridgeEvents set from 43e65ca - a duplicate Pumpkin handler costs a full serialize + cross-thread round trip per event fire and re-invokes listeners that already ran. Co-Authored-By: Claude Opus 5 Signed-off-by: TRiLON --- rust/src/java/native_callbacks/events.rs | 23 ++++++++- rust/src/java/native_callbacks/world.rs | 63 +++++++++++++++++++++--- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/rust/src/java/native_callbacks/events.rs b/rust/src/java/native_callbacks/events.rs index e4e4d37..61d55b3 100644 --- a/rust/src/java/native_callbacks/events.rs +++ b/rust/src/java/native_callbacks/events.rs @@ -1,4 +1,5 @@ -use std::sync::Arc; +use std::collections::HashSet; +use std::sync::{Arc, LazyLock, Mutex}; use pumpkin::plugin::EventPriority; @@ -8,8 +9,28 @@ use crate::proto::patchbukkit::events::{ CallEventRequest, CallEventResponse, RegisterEventRequest, }; +/// One Pumpkin handler per (plugin, event type). The Java side already deduplicates its +/// registerEvent calls, but this guard also protects against direct HandlerList +/// registrations and plugin reloads re-registering: every duplicate Pumpkin handler costs a +/// full serialize + cross-thread round trip per event fire, and re-invokes listeners that +/// already ran. +static REGISTERED_EVENTS: LazyLock>> = + LazyLock::new(|| Mutex::new(HashSet::new())); + pub fn ffi_native_bridge_register_event_impl(request: RegisterEventRequest) -> Option<()> { let ctx = CALLBACK_CONTEXT.get()?; + + { + let mut registered = REGISTERED_EVENTS.lock().ok()?; + if !registered.insert((request.plugin_name.clone(), request.event_type.clone())) { + tracing::debug!( + "Skipping duplicate native registration of '{}' for plugin '{}'", + request.event_type, + request.plugin_name + ); + return Some(()); + } + } let pumpkin_priority = match request.priority { 0 => EventPriority::Lowest, 1 => EventPriority::Low, diff --git a/rust/src/java/native_callbacks/world.rs b/rust/src/java/native_callbacks/world.rs index c627e02..d9eb488 100644 --- a/rust/src/java/native_callbacks/world.rs +++ b/rust/src/java/native_callbacks/world.rs @@ -1,3 +1,5 @@ +use pumpkin_data::block_properties::BlockProperties; + use crate::{ java::native_callbacks::CALLBACK_CONTEXT, proto::patchbukkit::world::{ @@ -23,12 +25,30 @@ pub fn ffi_native_bridge_get_block_data_impl( let state_id = world.get_block_state(&pos).id; let block = pumpkin_data::Block::from_state_id(state_id); let key = block.name; - let block_state = if key.starts_with("minecraft:") { + let mut block_state = if key.starts_with("minecraft:") { key.to_string() } else { format!("minecraft:{key}") }; + // Include block-state properties (facing, half, waterlogged, ...) so the Java side can + // hand plugins a faithful BlockData string instead of the bare block name. + if let Some(props) = block.properties(state_id) { + let props = props.to_props(); + if !props.is_empty() { + block_state.push('['); + for (i, (k, v)) in props.iter().enumerate() { + if i > 0 { + block_state.push(','); + } + block_state.push_str(k); + block_state.push('='); + block_state.push_str(v); + } + block_state.push(']'); + } + } + Some(GetBlockDataResponse { block_state }) } @@ -53,15 +73,46 @@ pub fn ffi_native_bridge_set_block_data_impl(request: SetBlockDataRequest) -> Op .trim_start_matches("minecraft:"); let state_id = if let Some(b) = pumpkin_data::Block::from_registry_key(clean_key) { - b.default_state.id + // Apply the block-state properties from the "[k=v,...]" suffix instead of silently + // placing the default state (stairs used to lose their facing, doors their half, ...). + match block_state_str.split_once('[') { + Some((_, props_str)) => { + let props: Vec<(&str, &str)> = props_str + .trim_end_matches(']') + .split(',') + .filter_map(|pair| pair.split_once('=')) + .map(|(k, v)| (k.trim(), v.trim())) + .collect(); + if props.is_empty() { + b.default_state.id + } else { + b.from_properties(&props).to_state_id(b) + } + } + None => b.default_state.id, + } } else { pumpkin_data::BlockStateId::new_or_air(0) }; - ctx.runtime.spawn(async move { - world - .set_block_state(&pos, state_id, pumpkin::world::BlockFlags::NOTIFY_ALL) - .await; + // Bukkit's contract is synchronous, ordered world mutation: Block#setType must be + // observable by a read on the next line, and two writes to the same position must land + // in call order. The previous fire-and-forget `runtime.spawn` guaranteed neither. This + // mirrors the blocking pattern the read callbacks in this module already use. + // + // Re-entrancy note: none of the events set_block_state can fire (BlockPhysicsEvent via + // NOTIFY_NEIGHBORS) are bridged to the JVM today. If one ever is, its blocking handler + // would post to the JvmWorker thread that is currently blocked inside this call — keep + // that in mind before wiring block-physics events. + let flags = if request.apply_physics { + pumpkin::world::BlockFlags::NOTIFY_ALL + } else { + pumpkin::world::BlockFlags::NOTIFY_LISTENERS + }; + tokio::task::block_in_place(|| { + ctx.runtime.block_on(async { + world.set_block_state(&pos, state_id, flags).await; + }) }); Some(()) From 02f05828257cd4343e5c4d78399491821986121d Mon Sep 17 00:00:00 2001 From: TRiLON Date: Thu, 20 Aug 2026 01:16:13 +0300 Subject: [PATCH 4/4] fix: remove unused BlockProperties import (clippy -D warnings) Signed-off-by: TRiLON --- rust/src/java/native_callbacks/world.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust/src/java/native_callbacks/world.rs b/rust/src/java/native_callbacks/world.rs index d9eb488..8df812f 100644 --- a/rust/src/java/native_callbacks/world.rs +++ b/rust/src/java/native_callbacks/world.rs @@ -1,5 +1,3 @@ -use pumpkin_data::block_properties::BlockProperties; - use crate::{ java::native_callbacks::CALLBACK_CONTEXT, proto::patchbukkit::world::{