From 83083d121a4a89a571e8e6bc018f8c67933a5e9e Mon Sep 17 00:00:00 2001 From: ZlordHUN <137647604+ZlordHUN@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:18:39 +0200 Subject: [PATCH] fix(bedrock): block breaking and item crafting (#2859) * Fix Bedrock block breaking and crafting * Fix Bedrock workbench synchronization --- .../pumpkin-inventory/src/screen_handler.rs | 7 +- crates/pumpkin-inventory/src/sync_handler.rs | 16 +- crates/pumpkin/src/command/commands/item.rs | 2 +- crates/pumpkin/src/entity/player.rs | 48 ++-- crates/pumpkin/src/net/bedrock/play.rs | 222 +++++++++++------- 5 files changed, 181 insertions(+), 114 deletions(-) diff --git a/crates/pumpkin-inventory/src/screen_handler.rs b/crates/pumpkin-inventory/src/screen_handler.rs index 510da0425..f15e15192 100644 --- a/crates/pumpkin-inventory/src/screen_handler.rs +++ b/crates/pumpkin-inventory/src/screen_handler.rs @@ -157,7 +157,12 @@ pub trait InventoryPlayer: Send + Sync { ) -> PlayerFuture<'a, ()>; /// Sends a single slot update packet. - fn enqueue_slot_packet<'a>(&'a self, packet: &'a CSetContainerSlot) -> PlayerFuture<'a, ()>; + fn enqueue_slot_packet<'a>( + &'a self, + packet: &'a CSetContainerSlot, + window_type: Option, + total_slots: usize, + ) -> PlayerFuture<'a, ()>; /// Sends a cursor item update packet. fn enqueue_cursor_packet<'a>(&'a self, packet: &'a CSetCursorItem) -> PlayerFuture<'a, ()>; diff --git a/crates/pumpkin-inventory/src/sync_handler.rs b/crates/pumpkin-inventory/src/sync_handler.rs index 71e42c7a6..694958753 100644 --- a/crates/pumpkin-inventory/src/sync_handler.rs +++ b/crates/pumpkin-inventory/src/sync_handler.rs @@ -134,12 +134,16 @@ impl SyncHandler { ) { if let Some(player) = self.player.lock().await.as_ref() { player - .enqueue_slot_packet(&CSetContainerSlot::new( - screen_handler.sync_id as i8, - next_revision as i32, - slot as i16, - &ItemStackSerializer::from(stack.clone()), - )) + .enqueue_slot_packet( + &CSetContainerSlot::new( + screen_handler.sync_id as i8, + next_revision as i32, + slot as i16, + &ItemStackSerializer::from(stack.clone()), + ), + screen_handler.window_type, + screen_handler.slots.len(), + ) .await; } } diff --git a/crates/pumpkin/src/command/commands/item.rs b/crates/pumpkin/src/command/commands/item.rs index 23d2cadb5..bdff6404a 100644 --- a/crates/pumpkin/src/command/commands/item.rs +++ b/crates/pumpkin/src/command/commands/item.rs @@ -186,7 +186,7 @@ impl CommandExecutor for EntityReplaceExecutor { let stack_serializer = ItemStackSerializer::from(item_stack.clone()); let packet = CSetContainerSlot::new(0, 0, slot as i16, &stack_serializer); - player.enqueue_slot_packet(&packet).await; + player.enqueue_slot_packet(&packet, None, 0).await; let eq_slot = if slot == 36 { Some(EquipmentSlot::FEET) diff --git a/crates/pumpkin/src/entity/player.rs b/crates/pumpkin/src/entity/player.rs index 5e690f41f..f5cfde7e6 100644 --- a/crates/pumpkin/src/entity/player.rs +++ b/crates/pumpkin/src/entity/player.rs @@ -2133,16 +2133,7 @@ impl Player { let state = world.get_block_state(&pos); // Is the block broken? if state.is_air() { - world - .set_block_breaking( - &self.living_entity.entity, - pos, - BlockBreakingProgress::Stop, - ) - .await; - self.current_block_destroy_stage - .store(-1, Ordering::Relaxed); - self.mining.store(false, Ordering::Relaxed); + self.stop_mining().await; } else { let finished = self .continue_mining( @@ -2153,16 +2144,7 @@ impl Player { ) .await; if finished && matches!(self.client.as_ref(), ClientPlatform::Bedrock(_)) { - self.mining.store(false, Ordering::Relaxed); - self.current_block_destroy_stage - .store(-1, Ordering::Relaxed); - world - .set_block_breaking( - &self.living_entity.entity, - pos, - BlockBreakingProgress::Stop, - ) - .await; + self.stop_mining().await; let block = Block::from_state_id(state.id); let can_harvest = self.can_harvest(state, block).await; @@ -2259,6 +2241,20 @@ impl Player { total_progress >= 1.0 } + pub(crate) async fn stop_mining(&self) { + let was_mining = self.mining.swap(false, Ordering::Relaxed); + let stage = self.current_block_destroy_stage.swap(-1, Ordering::Relaxed); + self.current_block_breaking_speed + .store(0, Ordering::Relaxed); + + if was_mining || stage >= 0 { + let pos = *self.mining_pos.lock().await; + self.world() + .set_block_breaking(&self.living_entity.entity, pos, BlockBreakingProgress::Stop) + .await; + } + } + pub async fn jump(&self) { self.stats .lock() @@ -5512,7 +5508,12 @@ impl InventoryPlayer for Player { }) } - fn enqueue_slot_packet<'a>(&'a self, packet: &'a CSetContainerSlot) -> PlayerFuture<'a, ()> { + fn enqueue_slot_packet<'a>( + &'a self, + packet: &'a CSetContainerSlot, + window_type: Option, + total_slots: usize, + ) -> PlayerFuture<'a, ()> { Box::pin(async move { match self.client.as_ref() { ClientPlatform::Java(java) => { @@ -5547,11 +5548,6 @@ impl InventoryPlayer for Player { let slot_idx = packet.slot as usize; let item_desc = NetworkItemStackDescriptor::from(&*packet.slot_data.0); - // Container screen - let current_handler = self.current_screen_handler.lock().await.clone(); - let handler = current_handler.lock().await; - let window_type = handler.window_type(); - let total_slots = handler.get_behaviour().slots.len(); let bedrock_info = if total_slots >= 36 { let container_slots = total_slots - 36; if slot_idx < container_slots { diff --git a/crates/pumpkin/src/net/bedrock/play.rs b/crates/pumpkin/src/net/bedrock/play.rs index 6aa57fb18..a05bda6b3 100644 --- a/crates/pumpkin/src/net/bedrock/play.rs +++ b/crates/pumpkin/src/net/bedrock/play.rs @@ -9,7 +9,10 @@ use pumpkin_data::{ }, item_stack::ItemStack, }; -use pumpkin_inventory::screen_handler::{InventoryPlayer, ScreenHandler}; +use pumpkin_inventory::{ + player::player_inventory::PlayerInventory, + screen_handler::{InventoryPlayer, ScreenHandler}, +}; use pumpkin_protocol::bedrock::{ client::inventory_content::CInventoryContent, network_item::{ @@ -545,8 +548,7 @@ impl BedrockClient { dynamic_id: None, }, slot_id, - 0, - VarInt(0), + ItemStack::EMPTY, ); inventory_updated = true; } @@ -1100,6 +1102,12 @@ impl BedrockClient { let world = entity.world.load_full(); let (block, state) = world.get_block_and_state(&location); + if player.mining.load(Ordering::Relaxed) + && *player.mining_pos.lock().await != location + { + player.stop_mining().await; + } + if player.gamemode.load() == GameMode::Creative { let new_state = world .break_block( @@ -1117,6 +1125,7 @@ impl BedrockClient { } else if !state.is_air() { let speed = crate::block::calc_block_breaking(player, state, block).await; if speed >= 1.0 { + player.stop_mining().await; let broken_state = world.get_block_state(&location); let can_harvest = player.can_harvest(broken_state, block).await; let new_state = world @@ -1184,7 +1193,7 @@ impl BedrockClient { } } } - PlayerAction::PredictDestroyBlock | PlayerAction::StopBreak => { + action @ (PlayerAction::PredictDestroyBlock | PlayerAction::StopBreak) => { let location = packet.block_pos; if !player.can_interact_with_block_at(&location, 1.0) { return; @@ -1204,13 +1213,7 @@ impl BedrockClient { && same_block && speed * elapsed as f32 >= MIN_PREDICTED_BREAK_PROGRESS { - player.mining.store(false, Ordering::Relaxed); - player - .current_block_destroy_stage - .store(-1, Ordering::Relaxed); - world - .set_block_breaking(entity, location, BlockBreakingProgress::Stop) - .await; + player.stop_mining().await; let can_harvest = player.can_harvest(state, block).await; let flags = if can_harvest { @@ -1236,19 +1239,25 @@ impl BedrockClient { let runtime_id = pumpkin_data::BlockState::to_be_network_id(state.id); self.enqueue_packet(&CUpdateBlock::new(location, runtime_id as u32)) .await; - world - .set_block_breaking( - entity, - location, - BlockBreakingProgress::Update { - stage: player - .current_block_destroy_stage - .load(Ordering::Relaxed), - speed: Some(speed), - }, - ) - .await; + if matches!(action, PlayerAction::StopBreak) { + player.stop_mining().await; + } else { + world + .set_block_breaking( + entity, + location, + BlockBreakingProgress::Update { + stage: player + .current_block_destroy_stage + .load(Ordering::Relaxed), + speed: Some(speed), + }, + ) + .await; + } } + } else if matches!(action, PlayerAction::StopBreak) { + player.stop_mining().await; } } PlayerAction::CrackBreak => { @@ -1256,14 +1265,7 @@ impl BedrockClient { // cracking is done fully server-side. } PlayerAction::AbortBreak => { - let location = packet.block_pos; - let entity = &player.get_entity(); - let world = entity.world.load(); - - player.mining.store(false, Ordering::Relaxed); - world - .set_block_breaking(entity, location, BlockBreakingProgress::Stop) - .await; + player.stop_mining().await; } PlayerAction::DropItem => { player.drop_held_item(false).await; @@ -1353,6 +1355,7 @@ impl BedrockClient { for request in packet.requests { let mut created_item: Option = None; + let mut crafting_inputs_consumed = false; let mut updates = Vec::new(); let mut result = 0u8; // 0 = Success, 1 = Error @@ -1437,26 +1440,11 @@ impl BedrockClient { source_stack.decrement(count); if source.container_name.container_name == ContainerName::CreatedOutput + && let Some(ref mut stack) = created_item { - if let Some(ref mut stack) = created_item { - stack.decrement(count); - if stack.is_empty() { - created_item = None; - } - } - } else if source.container_name.container_name == ContainerName::Cursor - { - let cursor_is_empty = screen_handler - .get_behaviour() - .cursor_stack - .lock() - .await - .is_empty(); - if cursor_is_empty && let Some(ref mut stack) = created_item { - stack.decrement(count); - if stack.is_empty() { - created_item = None; - } + stack.decrement(count); + if stack.is_empty() { + created_item = None; } } let source_stack = if source_stack.is_empty() { @@ -1484,15 +1472,13 @@ impl BedrockClient { &mut updates, source.container_name.clone(), source.slot_id, - source_stack.item_count, - source.stack_id, + &source_stack, ); record_update( &mut updates, destination.container_name.clone(), destination.slot_id, - dest_stack.item_count, - destination.stack_id, + &dest_stack, ); } } @@ -1511,15 +1497,13 @@ impl BedrockClient { &mut updates, slot1.container_name.clone(), slot1.slot_id, - stack2.item_count, - slot2.stack_id, + &stack2, ); record_update( &mut updates, slot2.container_name.clone(), slot2.slot_id, - stack1.item_count, - slot1.stack_id, + &stack1, ); } ItemStackRequestAction::Drop { @@ -1557,13 +1541,27 @@ impl BedrockClient { &mut updates, source.container_name.clone(), source.slot_id, - source_stack.item_count, - source.stack_id, + &source_stack, ); } } ItemStackRequestAction::Destroy { count, source } | ItemStackRequestAction::Consume { count, source } => { + if crafting_inputs_consumed + && source.container_name.container_name == ContainerName::CraftingInput + { + let source_stack = + get_slot_stack(&*screen_handler, &source, created_item.as_ref()) + .await; + record_update( + &mut updates, + source.container_name.clone(), + source.slot_id, + &source_stack, + ); + continue; + } + let mut source_stack = get_slot_stack(&*screen_handler, &source, created_item.as_ref()).await; if source_stack.is_empty() { @@ -1591,8 +1589,7 @@ impl BedrockClient { &mut updates, source.container_name.clone(), source.slot_id, - source_stack.item_count, - source.stack_id, + &source_stack, ); } } @@ -1610,6 +1607,7 @@ impl BedrockClient { let is_player = screen_handler.window_type().is_none(); let grid_size = if is_player { 4 } else { 9 }; + let bedrock_grid_start = if is_player { 28 } else { 32 }; for i in 0..grid_size { let grid_slot_index = 1 + i; let grid_slot = @@ -1625,8 +1623,10 @@ impl BedrockClient { let output_slot = screen_handler.get_behaviour().slots[0].clone(); let output_stack = output_slot.get_cloned_stack().await; - if output_stack.is_empty() { - tracing::warn!("Client tried to craft, but output slot is empty!"); + if output_stack.is_empty() + || repetitions > output_slot.get_max_item_count().await + { + tracing::warn!("Client sent an invalid crafting request"); result = 1; break; } @@ -1641,6 +1641,7 @@ impl BedrockClient { .on_take_item(player.as_ref(), &output_stack) .await; } + crafting_inputs_consumed = true; // Record updates for all grid slots so Bedrock client is notified of consumed ingredients! let is_player = screen_handler.window_type().is_none(); @@ -1656,9 +1657,8 @@ impl BedrockClient { container_name: ContainerName::CraftingInput, dynamic_id: None, }, - i as u8, - grid_stack.item_count, - VarInt(0), + (bedrock_grid_start + i) as u8, + &grid_stack, ); } } @@ -1938,8 +1938,12 @@ fn map_bedrock_container_slot( container_name: ContainerName, slot_id: u8, ) -> Option { - let container_slots = screen_handler.get_behaviour().container_slots; let is_player_screen = screen_handler.window_type().is_none(); + let container_slots = screen_handler + .get_behaviour() + .slots + .len() + .saturating_sub(PlayerInventory::MAIN_SIZE); match container_name { ContainerName::HotBar => { @@ -2197,22 +2201,26 @@ fn record_update( updates: &mut Vec, container_name: FullContainerName, slot_id: u8, - count: u8, - stack_id: VarInt, + stack: &ItemStack, ) { - let final_stack_id = if count == 0 { VarInt(0) } else { stack_id }; + let count = stack.item_count; + let stack_id = if stack.is_empty() { + VarInt(0) + } else { + VarInt(stack.uid.get()) + }; if let Some(existing) = updates .iter_mut() .find(|u| u.container_name == container_name && u.slot_id == slot_id) { existing.count = count; - existing.stack_id = final_stack_id; + existing.stack_id = stack_id; } else { updates.push(SlotUpdate { container_name, slot_id, count, - stack_id: final_stack_id, + stack_id, }); } } @@ -2228,13 +2236,12 @@ async fn get_slot_stack( return stack.clone(); } if slot_info.container_name.container_name == ContainerName::Cursor { - let cursor_lock = screen_handler.get_behaviour().cursor_stack.lock().await; - if cursor_lock.is_empty() - && let Some(stack) = created_item - { - return stack.clone(); - } - return cursor_lock.clone(); + return screen_handler + .get_behaviour() + .cursor_stack + .lock() + .await + .clone(); } if let Some(screen_slot) = map_bedrock_container_slot( screen_handler, @@ -2301,3 +2308,58 @@ async fn update_slot_stack( screen_handler.set_received_stack(screen_slot, new_stack); } } + +#[cfg(test)] +mod tests { + use super::*; + use pumpkin_data::item::Item; + use pumpkin_inventory::{ + build_equipment_slots, crafting::crafting_screen_handler::CraftingTableScreenHandler, + entity_equipment::EntityEquipment, + }; + use tokio::sync::Mutex; + + #[tokio::test] + async fn crafting_table_maps_bedrock_player_inventory_after_its_ten_slots() { + let inventory = Arc::new(PlayerInventory::new( + Arc::new(Mutex::new(EntityEquipment::new())), + Arc::new(build_equipment_slots()), + )); + let handler = CraftingTableScreenHandler::new(1, &inventory, None).await; + + assert_eq!( + map_bedrock_container_slot(&handler, ContainerName::Inventory, 26), + Some(27) + ); + assert_eq!( + map_bedrock_container_slot(&handler, ContainerName::HotBar, 0), + Some(37) + ); + assert_eq!( + map_bedrock_container_slot(&handler, ContainerName::CraftingInput, 32), + Some(1) + ); + assert_eq!( + map_bedrock_container_slot(&handler, ContainerName::CraftingInput, 40), + Some(9) + ); + } + + #[test] + fn item_stack_response_uses_the_authoritative_stack_id() { + let stack = ItemStack::new(3, &Item::SPRUCE_DOOR); + let container = FullContainerName { + container_name: ContainerName::Cursor, + dynamic_id: None, + }; + let mut updates = Vec::new(); + + record_update(&mut updates, container.clone(), 0, &stack); + assert_eq!(updates[0].count, 3); + assert_eq!(updates[0].stack_id, VarInt(stack.uid.get())); + + record_update(&mut updates, container, 0, ItemStack::EMPTY); + assert_eq!(updates[0].count, 0); + assert_eq!(updates[0].stack_id, VarInt(0)); + } +}