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()) { 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) {} } 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..8df812f 100644 --- a/rust/src/java/native_callbacks/world.rs +++ b/rust/src/java/native_callbacks/world.rs @@ -23,12 +23,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 +71,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(())