Fix Block#getType returning STONE, ordered synchronous setBlockData, block-state properties, and bridge hot-path costs

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()) {

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<String, List<MetadataValue>> 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<String, List<MetadataValue>> 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<String, List<MetadataValue>> metadata() {
+        if (this.metadataMap == null) {
+            this.metadataMap = new HashMap<>();
+        }
+        return this.metadataMap;
+    }
+
     @Override
     public @NotNull World getWorld() {
         return this.world;

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) {}
         }

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<Mutex<HashSet<(String, String)>>> =
+    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,

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(())