diff --git a/crates/valence/src/tests/inventory.rs b/crates/valence/src/tests/inventory.rs index 19fea190e..fdaf04988 100644 --- a/crates/valence/src/tests/inventory.rs +++ b/crates/valence/src/tests/inventory.rs @@ -339,6 +339,48 @@ fn test_should_modify_open_inventory_server_side() { ); } +#[test] +fn test_should_modify_cursor_item_open_inventory_server_side() { + let mut app = App::new(); + let (client_ent, mut client_helper) = scenario_single_client(&mut app); + let inventory_ent = set_up_open_inventory(&mut app, client_ent); + + // Process a tick to get past the "on join" logic. + app.update(); + client_helper.clear_sent(); + + // Modify the cursor item. + let mut cursor_item = app + .world + .get_mut::(client_ent) + .expect("could not find inventory for client"); + cursor_item.0 = Some(ItemStack::new(ItemKind::IronIngot, 3, None)); + + app.update(); + + // Make assertions + let sent_packets = client_helper.collect_sent(); + + // because the cursor item was modified server side, the client needs to be + // updated with the change. + sent_packets.assert_count::(1); + for pkt in sent_packets.0 { + if let Ok(pkt) = pkt.decode::() { + assert_eq!(pkt.window_id, -1); + assert_eq!(pkt.slot_idx, -1); + } + } + + let cursor_item = app + .world + .get_mut::(client_ent) + .expect("could not find inventory for client"); + assert_eq!( + cursor_item.0, + Some(ItemStack::new(ItemKind::IronIngot, 3, None)) + ); +} + #[test] fn test_should_sync_entire_open_inventory() { let mut app = App::new(); diff --git a/crates/valence_inventory/src/lib.rs b/crates/valence_inventory/src/lib.rs index 1bb6fe3f0..68dc05fa9 100644 --- a/crates/valence_inventory/src/lib.rs +++ b/crates/valence_inventory/src/lib.rs @@ -25,6 +25,7 @@ use std::ops::Range; use bevy_app::prelude::*; use bevy_ecs::prelude::*; +use menu::OpenMenu; use packet::{ ClickMode, ClickSlotC2s, CloseHandledScreenC2s, CloseScreenS2c, CreativeInventoryActionC2s, InventoryS2c, OpenScreenS2c, ScreenHandlerSlotUpdateS2c, SlotChange, UpdateSelectedSlotC2s, @@ -40,6 +41,7 @@ use valence_core::protocol::encode::WritePacket; use valence_core::protocol::var_int::VarInt; use valence_core::text::Text; +pub mod menu; pub mod packet; mod validate; @@ -65,7 +67,8 @@ impl Plugin for InventoryPlugin { .add_systems( ( handle_update_selected_slot, - handle_click_slot, + emit_events_from_packets, + handle_click_slot.after(emit_events_from_packets), handle_creative_inventory_action, handle_close_handled_screen, handle_player_actions, @@ -332,11 +335,11 @@ pub struct ClientInventoryState { state_id: Wrapping, /// Tracks what slots have been changed by this client in this tick, so we /// don't need to send updates for them. - slots_changed: u64, + pub(crate) slots_changed: u64, /// Whether the client has updated the cursor item in this tick. This is not /// on the `CursorItem` component to make maintaining accurate change /// detection for end users easier. - client_updated_cursor_item: bool, + pub(crate) client_updated_cursor_item: bool, // TODO: make this a separate modifiable component. held_item_slot: u16, } @@ -604,6 +607,7 @@ fn update_player_inventories( // state ID here because the client doesn't actually acknowledge the // state_id change for this packet specifically. See #304. + debug!("sending cursor item update"); client.write_packet(&ScreenHandlerSlotUpdateS2c { window_id: -1, state_id: VarInt(inv_state.state_id.0), @@ -624,7 +628,7 @@ fn update_open_inventories( Entity, &mut Client, &mut ClientInventoryState, - &CursorItem, + Ref, &mut OpenInventory, )>, mut inventories: Query<&mut Inventory>, @@ -689,6 +693,7 @@ fn update_open_inventories( for (i, slot) in inventory.slots.iter().enumerate() { if (changed_filtered >> i) & 1 == 1 { + debug!("sending slot update for slot {}", i); client.write_packet(&ScreenHandlerSlotUpdateS2c { window_id: inv_state.window_id as i8, state_id: VarInt(inv_state.state_id.0), @@ -703,8 +708,23 @@ fn update_open_inventories( open_inventory.client_changed = 0; inv_state.slots_changed = 0; - inv_state.client_updated_cursor_item = false; inventory.changed = 0; + + if cursor_item.is_changed() && !inv_state.client_updated_cursor_item { + // Contrary to what you might think, we actually don't want to increment the + // state ID here because the client doesn't actually acknowledge the + // state_id change for this packet specifically. See #304. + + debug!("sending cursor item update"); + client.write_packet(&ScreenHandlerSlotUpdateS2c { + window_id: -1, + state_id: VarInt(inv_state.state_id.0), + slot_idx: -1, + slot_data: Cow::Borrowed(&cursor_item.0), + }); + } + + inv_state.client_updated_cursor_item = false; } } @@ -754,7 +774,7 @@ pub struct DropItemStack { pub stack: ItemStack, } -fn handle_click_slot( +fn emit_events_from_packets( mut packets: EventReader, mut clients: Query<( &mut Client, @@ -969,22 +989,6 @@ fn handle_click_slot( continue; } - - cursor_item.set_if_neq(CursorItem(pkt.carried_item.clone())); - - for slot in pkt.slot_changes.clone() { - if (0i16..target_inventory.slot_count() as i16).contains(&slot.idx) { - // The client is interacting with a slot in the target inventory. - target_inventory.set_slot(slot.idx as u16, slot.item); - open_inventory.client_changed |= 1 << slot.idx; - } else { - // The client is interacting with a slot in their own inventory. - let slot_id = - convert_to_player_slot_id(target_inventory.kind, slot.idx as u16); - client_inv.set_slot(slot_id, slot.item); - inv_state.slots_changed |= 1 << slot_id; - } - } } else { // The client is interacting with their own inventory. @@ -1004,23 +1008,6 @@ fn handle_click_slot( continue; } - - cursor_item.set_if_neq(CursorItem(pkt.carried_item.clone())); - inv_state.client_updated_cursor_item = true; - - for slot in pkt.slot_changes.clone() { - if (0i16..client_inv.slot_count() as i16).contains(&slot.idx) { - client_inv.set_slot(slot.idx as u16, slot.item); - inv_state.slots_changed |= 1 << slot.idx; - } else { - // The client is trying to interact with a slot that does not exist, - // ignore. - warn!( - "Client attempted to interact with slot {} which does not exist", - slot.idx - ); - } - } } click_slot_events.send(ClickSlot { @@ -1037,6 +1024,75 @@ fn handle_click_slot( } } +fn handle_click_slot( + mut click_slots: EventReader, + mut clients: Query<( + &mut Client, + &mut Inventory, + &mut ClientInventoryState, + Option<&mut OpenInventory>, + &mut CursorItem, + )>, + mut inventories: Query<&mut Inventory, Without>, +) { + for click in click_slots.iter() { + // The player is clicking a slot in an inventory. + let Ok(( + mut client, + mut client_inv, + mut inv_state, + open_inventory, + mut cursor_item + )) = clients.get_mut(click.client) else { + // The client does not exist, ignore. + continue; + }; + + if let Some(mut open_inventory) = open_inventory { + // The player is interacting with an inventory that is open. + + let Ok(mut target_inventory) = inventories.get_mut(open_inventory.entity) else { + // The inventory does not exist, ignore. + continue; + }; + + cursor_item.set_if_neq(CursorItem(click.carried_item.clone())); + inv_state.client_updated_cursor_item = true; + + for slot in click.slot_changes.clone() { + if (0i16..target_inventory.slot_count() as i16).contains(&slot.idx) { + // The client is interacting with a slot in the target inventory. + target_inventory.set_slot(slot.idx as u16, slot.item); + open_inventory.client_changed |= 1 << slot.idx; + } else { + // The client is interacting with a slot in their own inventory. + let slot_id = convert_to_player_slot_id(target_inventory.kind, slot.idx as u16); + client_inv.set_slot(slot_id, slot.item); + inv_state.slots_changed |= 1 << slot_id; + } + } + } else { + // The client is interacting with their own inventory. + cursor_item.set_if_neq(CursorItem(click.carried_item.clone())); + inv_state.client_updated_cursor_item = true; + + for slot in click.slot_changes.clone() { + if (0i16..client_inv.slot_count() as i16).contains(&slot.idx) { + client_inv.set_slot(slot.idx as u16, slot.item); + inv_state.slots_changed |= 1 << slot.idx; + } else { + // The client is trying to interact with a slot that does not exist, + // ignore. + warn!( + "Client attempted to interact with slot {} which does not exist", + slot.idx + ); + } + } + } + } +} + fn handle_player_actions( mut packets: EventReader, mut clients: Query<(&mut Inventory, &mut ClientInventoryState)>, diff --git a/crates/valence_inventory/src/menu.rs b/crates/valence_inventory/src/menu.rs new file mode 100644 index 000000000..e4d6d1f79 --- /dev/null +++ b/crates/valence_inventory/src/menu.rs @@ -0,0 +1,98 @@ +use std::borrow::Cow; + +use bevy_app::{CoreSet, IntoSystemAppConfig, Plugin}; +use bevy_ecs::prelude::*; +use tracing::debug; +use valence_client::event_loop::{EventLoopSchedule, PacketEvent}; +use valence_client::Client; +use valence_core::__private::VarInt; +use valence_core::item::ItemStack; +use valence_core::protocol::encode::WritePacket; +use valence_core::text::Text; + +use crate::packet::{ + ClickMode, ClickSlotC2s, InventoryS2c, OpenScreenS2c, ScreenHandlerSlotUpdateS2c, WindowType, +}; +use crate::{ + validate, ClickSlot, ClientInventoryState, CursorItem, DropItemStack, Inventory, InventoryKind, + OpenInventory, +}; + +#[derive(Debug, Default)] +pub struct InventoryMenuPlugin; + +impl Plugin for InventoryMenuPlugin { + fn build(&self, app: &mut bevy_app::App) { + app.add_system( + handle_click_slot + .in_schedule(EventLoopSchedule) + .in_base_set(CoreSet::PreUpdate), + ) + .add_event::(); + } +} + +#[derive(Debug, Component)] +pub struct OpenMenu; + +#[derive(Debug, Component)] +pub struct InventoryMenu; + +fn handle_click_slot( + mut clicks: EventReader, + mut clients: Query< + ( + &mut Client, + &mut Inventory, + &mut ClientInventoryState, + &mut CursorItem, + &mut OpenInventory, + ), + With, + >, + mut inventories: Query<&mut Inventory, (Without, With)>, + mut menu_clicks: EventWriter, +) { + for click in clicks.iter() { + if click.mode != ClickMode::Click { + continue; + } + println!("menu click: {:?}", click); + if let Ok((mut client, mut inv, mut inv_state, mut cursor_item, open_inv)) = + clients.get_mut(click.client) + { + let Ok(mut target) = inventories.get_mut(open_inv.entity) else { + continue; + }; + if click.slot_id < 0 || click.slot_id >= (target.slot_count() as i16) { + continue; + } + println!("reset clicked slot"); + target.set_slot(click.slot_id as u16, click.carried_item.clone()); + inv_state.slots_changed &= !(1 << click.slot_id); + client.write_packet(&ScreenHandlerSlotUpdateS2c { + window_id: inv_state.window_id as i8, + state_id: VarInt(inv_state.state_id.0), + slot_idx: click.slot_id, + slot_data: Cow::Borrowed(&click.carried_item), + }); + + println!("clearing cursor item"); + cursor_item.0 = None; + inv_state.client_updated_cursor_item = false; + + menu_clicks.send(MenuClickEvent { + client: click.client, + slot_id: click.slot_id as u16, + button: click.button, + }); + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MenuClickEvent { + pub client: Entity, + pub slot_id: u16, + pub button: i8, +}