From 0319635a8b6bae263f52e7228b01155b1a19930f Mon Sep 17 00:00:00 2001 From: Carson McManus Date: Mon, 20 Feb 2023 18:37:09 -0500 Subject: [PATCH] Implement Drop Item Events (#252) ## Description Dropping items is heavily coupled to inventories. Clients also predict state changes when they try to drop items, so we need to be able to replicate that change in order to stay in sync. This will also remove `DropItem` events in favor of just `DropItemStack` events. Having 2 event streams that basically mean the same thing seems verbose and error prone. As of right now, these changes require the event loop to have a reference to the client's inventory. This seems like something we are going to need to do a lot more of to complete #199. ## Test Plan
Playground ```rust use tracing::info; use valence::client::despawn_disconnected_clients; use valence::client::event::{default_event_handler, DropItemStack}; use valence::prelude::*; #[allow(unused_imports)] use crate::extras::*; const SPAWN_Y: i32 = 64; pub fn build_app(app: &mut App) { app.add_plugin(ServerPlugin::new(()).with_connection_mode(ConnectionMode::Offline)) .add_system_to_stage(EventLoop, default_event_handler) .add_startup_system(setup) .add_system(init_clients) .add_system(despawn_disconnected_clients) .add_system(toggle_gamemode_on_sneak) .add_system(drop_items); } fn setup(world: &mut World) { let mut instance = world .resource::() .new_instance(DimensionId::default()); for z in -5..5 { for x in -5..5 { instance.insert_chunk([x, z], Chunk::default()); } } for z in -25..25 { for x in -25..25 { instance.set_block([x, SPAWN_Y, z], BlockState::GRASS_BLOCK); } } world.spawn(instance); } fn init_clients( mut clients: Query<&mut Client, Added>, instances: Query>, ) { let instance = instances.get_single().unwrap(); for mut client in &mut clients { client.set_position([0.5, SPAWN_Y as f64 + 1.0, 0.5]); client.set_instance(instance); client.set_game_mode(GameMode::Creative); } } fn drop_items(clients: Query<&Client>, mut drop_stack_events: EventReader) { if drop_stack_events.is_empty() { return; } for event in drop_stack_events.iter() { info!("drop stack: {:?}", event); } } ```
Steps: 1. `cargo test -p valence --tests` 2. Run playground `cargo run -p playground` 3. Open creative menu 4. Pick an item and click to drop it outside of the creative menu 5. Pick an entire stack of an item, place it in your hotbar 6. Hover over the item, press your drop key to drop an item from the stack 7. Press shift to switch to survival 8. Select the item stack in your hotbar, press your drop key to drop an item from the stack 9. Open your inventory, grab the stack, hover outside the window and click to drop the entire stack 10. Grab another stack from creative, place it in your hotbar 11. switch back to survival, select the stack, and press your control + drop key to drop the entire stack 12. For each item you dropped, you should see a log message with the event #### Related --- crates/playground/src/extras.rs | 22 +++ crates/playground/src/main.rs | 1 + crates/valence/examples/chest.rs | 4 - crates/valence/src/client.rs | 2 +- crates/valence/src/client/event.rs | 116 +++++++++--- crates/valence/src/inventory.rs | 287 +++++++++++++++++++++++++++++ 6 files changed, 401 insertions(+), 31 deletions(-) diff --git a/crates/playground/src/extras.rs b/crates/playground/src/extras.rs index 7a43fdc..28612f5 100644 --- a/crates/playground/src/extras.rs +++ b/crates/playground/src/extras.rs @@ -1,2 +1,24 @@ //! Put stuff in here if you find that you have to write the same code for //! multiple playgrounds. + +use valence::client::event::StartSneaking; +use valence::prelude::*; + +/// Toggles client's game mode between survival and creative when they start +/// sneaking. +pub fn toggle_gamemode_on_sneak( + mut clients: Query<&mut Client>, + mut events: EventReader, +) { + for event in events.iter() { + let Ok(mut client) = clients.get_component_mut::(event.client) else { + continue; + }; + let mode = client.game_mode(); + client.set_game_mode(match mode { + GameMode::Survival => GameMode::Creative, + GameMode::Creative => GameMode::Survival, + _ => GameMode::Creative, + }); + } +} diff --git a/crates/playground/src/main.rs b/crates/playground/src/main.rs index 47dac45..ad21c8d 100644 --- a/crates/playground/src/main.rs +++ b/crates/playground/src/main.rs @@ -1,5 +1,6 @@ use valence::bevy_app::App; +#[allow(dead_code)] mod extras; mod playground; diff --git a/crates/valence/examples/chest.rs b/crates/valence/examples/chest.rs index 0e9b0b1..2d9e79b 100644 --- a/crates/valence/examples/chest.rs +++ b/crates/valence/examples/chest.rs @@ -38,10 +38,6 @@ fn setup(world: &mut World) { } } instance.set_block(CHEST_POS, BlockState::CHEST); - instance.set_block( - [CHEST_POS[0], CHEST_POS[1] - 1, CHEST_POS[2]], - BlockState::STONE, - ); world.spawn(instance); diff --git a/crates/valence/src/client.rs b/crates/valence/src/client.rs index 7669908..0081792 100644 --- a/crates/valence/src/client.rs +++ b/crates/valence/src/client.rs @@ -154,7 +154,7 @@ impl Client { window_id: 0, inventory_state_id: Wrapping(0), inventory_slots_modified: 0, - held_item_slot: 0, + held_item_slot: 36, } } diff --git a/crates/valence/src/client/event.rs b/crates/valence/src/client/event.rs index 3613916..62d9adc 100644 --- a/crates/valence/src/client/event.rs +++ b/crates/valence/src/client/event.rs @@ -22,6 +22,7 @@ use valence_protocol::{BlockFace, BlockPos, Ident, ItemStack}; use crate::client::Client; use crate::entity::{EntityAnimation, EntityKind, McEntity, TrackedData}; +use crate::inventory::Inventory; #[derive(Clone, Debug)] pub struct QueryBlockEntity { @@ -330,14 +331,11 @@ pub struct FinishDigging { pub sequence: i32, } -#[derive(Clone, Debug)] -pub struct DropItem { - pub client: Entity, -} - #[derive(Clone, Debug)] pub struct DropItemStack { pub client: Entity, + pub from_slot: Option, + pub stack: ItemStack, } /// Eating food, pulling back bows, using buckets, etc. @@ -647,7 +645,6 @@ events! { StartDigging CancelDigging FinishDigging - DropItem DropItemStack UpdateHeldItemState SwapItemInHand @@ -681,7 +678,7 @@ events! { } pub(crate) fn event_loop_run_criteria( - mut clients: Query<(Entity, &mut Client)>, + mut clients: Query<(Entity, &mut Client, &mut Inventory)>, mut clients_to_check: Local>, mut events: ClientEvents, ) -> ShouldRun { @@ -690,8 +687,9 @@ pub(crate) fn event_loop_run_criteria( update_all_event_buffers(&mut events); - for (entity, client) in &mut clients { + for (entity, client, inventory) in &mut clients { let client = client.into_inner(); + let inventory = inventory.into_inner(); let Ok(bytes) = client.conn.try_recv() else { // Client is disconnected. @@ -706,7 +704,7 @@ pub(crate) fn event_loop_run_criteria( client.dec.queue_bytes(bytes); - match handle_one_packet(client, entity, &mut events) { + match handle_one_packet(client, inventory, entity, &mut events) { Ok(had_packet) => { if had_packet { // We decoded one packet, but there might be more. @@ -729,12 +727,12 @@ pub(crate) fn event_loop_run_criteria( // Continue to filter the list of clients we need to check until there are none // left. clients_to_check.retain(|&entity| { - let Ok((_, mut client)) = clients.get_mut(entity) else { + let Ok((_, mut client, mut inventory)) = clients.get_mut(entity) else { // Client was deleted during the last run of the stage. return false; }; - match handle_one_packet(&mut client, entity, &mut events) { + match handle_one_packet(&mut client, &mut inventory, entity, &mut events) { Ok(had_packet) => had_packet, Err(e) => { // TODO: validate packets in separate systems. @@ -761,6 +759,7 @@ pub(crate) fn event_loop_run_criteria( fn handle_one_packet( client: &mut Client, + inventory: &mut Inventory, entity: Entity, events: &mut ClientEvents, ) -> anyhow::Result { @@ -859,16 +858,47 @@ fn handle_one_packet( }); } C2sPlayPacket::ClickContainer(p) => { - events.0.click_container.send(ClickContainer { - client: entity, - window_id: p.window_id, - state_id: p.state_id.0, - slot_id: p.slot_idx, - button: p.button, - mode: p.mode, - slot_changes: p.slots, - carried_item: p.carried_item, - }); + if p.slot_idx < 0 { + if let Some(stack) = client.cursor_item.take() { + events.2.drop_item_stack.send(DropItemStack { + client: entity, + from_slot: None, + stack, + }); + } + } else if p.mode == ClickContainerMode::DropKey { + let entire_stack = p.button == 1; + if let Some(stack) = inventory.slot(p.slot_idx as u16) { + let dropped = if entire_stack || stack.count() == 1 { + inventory.replace_slot(p.slot_idx as u16, None) + } else { + let mut stack = stack.clone(); + stack.set_count(stack.count() - 1); + let mut old_slot = inventory.replace_slot(p.slot_idx as u16, Some(stack)); + // we already checked that the slot was not empty and that the + // stack count is > 1 + old_slot.as_mut().unwrap().set_count(1); + old_slot + } + .expect("dropped item should exist"); // we already checked that the slot was not empty + events.2.drop_item_stack.send(DropItemStack { + client: entity, + from_slot: Some(p.slot_idx as u16), + stack: dropped, + }); + } + } else { + events.0.click_container.send(ClickContainer { + client: entity, + window_id: p.window_id, + state_id: p.state_id.0, + slot_id: p.slot_idx, + button: p.button, + mode: p.mode, + slot_changes: p.slots, + carried_item: p.carried_item, + }); + } } C2sPlayPacket::CloseContainerC2s(p) => { events.0.close_container.send(CloseContainer { @@ -1157,11 +1187,36 @@ fn handle_one_packet( face: p.face, sequence: p.sequence.0, }), - DiggingStatus::DropItemStack => events - .2 - .drop_item_stack - .send(DropItemStack { client: entity }), - DiggingStatus::DropItem => events.2.drop_item.send(DropItem { client: entity }), + DiggingStatus::DropItemStack => { + if let Some(stack) = inventory.replace_slot(client.held_item_slot(), None) { + client.inventory_slots_modified |= 1 << client.held_item_slot(); + events.2.drop_item_stack.send(DropItemStack { + client: entity, + from_slot: Some(client.held_item_slot()), + stack, + }); + } + } + DiggingStatus::DropItem => { + if let Some(stack) = inventory.slot(client.held_item_slot()) { + let mut old_slot = if stack.count() == 1 { + inventory.replace_slot(client.held_item_slot(), None) + } else { + let mut stack = stack.clone(); + stack.set_count(stack.count() - 1); + inventory.replace_slot(client.held_item_slot(), Some(stack.clone())) + } + .expect("old slot should exist"); // we already checked that the slot was not empty + client.inventory_slots_modified |= 1 << client.held_item_slot(); + old_slot.set_count(1); + + events.2.drop_item_stack.send(DropItemStack { + client: entity, + from_slot: Some(client.held_item_slot()), + stack: old_slot, + }); + } + } DiggingStatus::UpdateHeldItemState => events .2 .update_held_item_state @@ -1280,6 +1335,15 @@ fn handle_one_packet( }); } C2sPlayPacket::SetCreativeModeSlot(p) => { + if p.slot == -1 { + if let Some(stack) = p.clicked_item.as_ref() { + events.2.drop_item_stack.send(DropItemStack { + client: entity, + from_slot: None, + stack: stack.clone(), + }); + } + } events.3.set_creative_mode_slot.send(SetCreativeModeSlot { client: entity, slot: p.slot, diff --git a/crates/valence/src/inventory.rs b/crates/valence/src/inventory.rs index 2833349..43992ad 100644 --- a/crates/valence/src/inventory.rs +++ b/crates/valence/src/inventory.rs @@ -1088,4 +1088,291 @@ mod test { Ok(()) } + + mod dropping_items { + use valence_protocol::types::{ClickContainerMode, DiggingStatus}; + use valence_protocol::{BlockFace, BlockPos}; + + use super::*; + use crate::client::event::DropItemStack; + + #[test] + fn should_drop_item_player_action() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let mut inventory = app + .world + .get_mut::(client_ent) + .expect("could not find inventory"); + inventory.replace_slot(36, ItemStack::new(ItemKind::IronIngot, 3, None)); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::PlayerAction { + status: DiggingStatus::DropItem, + position: BlockPos::new(0, 0, 0), + face: BlockFace::Bottom, + sequence: VarInt(0), + }); + + app.update(); + + // Make assertions + let inventory = app + .world + .get::(client_ent) + .expect("could not find client"); + assert_eq!( + inventory.slot(36), + Some(&ItemStack::new(ItemKind::IronIngot, 2, None)) + ); + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, Some(36)); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 1, None) + ); + + let sent_packets = client_helper.collect_sent()?; + assert_packet_count!(sent_packets, 0, S2cPlayPacket::SetContainerSlot(_)); + + Ok(()) + } + + #[test] + fn should_drop_item_stack_player_action() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let mut inventory = app + .world + .get_mut::(client_ent) + .expect("could not find inventory"); + inventory.replace_slot(36, ItemStack::new(ItemKind::IronIngot, 32, None)); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::PlayerAction { + status: DiggingStatus::DropItemStack, + position: BlockPos::new(0, 0, 0), + face: BlockFace::Bottom, + sequence: VarInt(0), + }); + + app.update(); + + // Make assertions + let client = app + .world + .get::(client_ent) + .expect("could not find client"); + assert_eq!(client.held_item_slot(), 36); + let inventory = app + .world + .get::(client_ent) + .expect("could not find inventory"); + assert_eq!(inventory.slot(36), None); + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, Some(36)); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 32, None) + ); + + Ok(()) + } + + #[test] + fn should_drop_item_stack_set_creative_mode_slot() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::SetCreativeModeSlot { + slot: -1, + clicked_item: Some(ItemStack::new(ItemKind::IronIngot, 32, None)), + }); + + app.update(); + + // Make assertions + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, None); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 32, None) + ); + + Ok(()) + } + + #[test] + fn should_drop_item_stack_click_container_outside() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let mut client = app + .world + .get_mut::(client_ent) + .expect("could not find client"); + client.cursor_item = Some(ItemStack::new(ItemKind::IronIngot, 32, None)); + let state_id = client.inventory_state_id.0; + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::ClickContainer { + window_id: 0, + slot_idx: -999, + button: 0, + mode: ClickContainerMode::Click, + state_id: VarInt(state_id), + slots: vec![], + carried_item: None, + }); + + app.update(); + + // Make assertions + let client = app + .world + .get::(client_ent) + .expect("could not find client"); + assert_eq!(client.cursor_item(), None); + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, None); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 32, None) + ); + + Ok(()) + } + + #[test] + fn should_drop_item_click_container_with_dropkey_single() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let client = app + .world + .get_mut::(client_ent) + .expect("could not find client"); + let state_id = client.inventory_state_id.0; + let mut inventory = app + .world + .get_mut::(client_ent) + .expect("could not find inventory"); + inventory.replace_slot(40, ItemStack::new(ItemKind::IronIngot, 32, None)); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::ClickContainer { + window_id: 0, + slot_idx: 40, + button: 0, + mode: ClickContainerMode::DropKey, + state_id: VarInt(state_id), + slots: vec![], + carried_item: None, + }); + + app.update(); + + // Make assertions + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, Some(40)); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 1, None) + ); + + Ok(()) + } + + #[test] + fn should_drop_item_stack_click_container_with_dropkey() -> anyhow::Result<()> { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let client = app + .world + .get_mut::(client_ent) + .expect("could not find client"); + let state_id = client.inventory_state_id.0; + let mut inventory = app + .world + .get_mut::(client_ent) + .expect("could not find inventory"); + inventory.replace_slot(40, ItemStack::new(ItemKind::IronIngot, 32, None)); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + client_helper.send(&valence_protocol::packets::c2s::play::ClickContainer { + window_id: 0, + slot_idx: 40, + button: 1, // pressing control + mode: ClickContainerMode::DropKey, + state_id: VarInt(state_id), + slots: vec![], + carried_item: None, + }); + + app.update(); + + // Make assertions + let events = app + .world + .get_resource::>() + .expect("expected drop item stack events"); + let events = events.iter_current_update_events().collect::>(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].client, client_ent); + assert_eq!(events[0].from_slot, Some(40)); + assert_eq!( + events[0].stack, + ItemStack::new(ItemKind::IronIngot, 32, None) + ); + + Ok(()) + } + } }