diff --git a/crates/valence_equipment/src/inventory_sync.rs b/crates/valence_equipment/src/inventory_sync.rs index 65df9aba3..672a3a7e2 100644 --- a/crates/valence_equipment/src/inventory_sync.rs +++ b/crates/valence_equipment/src/inventory_sync.rs @@ -1,5 +1,5 @@ use valence_inventory::player_inventory::PlayerInventory; -use valence_inventory::{HeldItem, Inventory}; +use valence_inventory::{HeldItem, Inventory, UpdateSelectedSlotEvent}; use valence_server::entity::player::PlayerEntity; use super::*; @@ -9,6 +9,8 @@ pub struct EquipmentInventorySync; /// Syncs the player [`Equipment`] with the [`Inventory`]. /// If a change in the player's inventory and in the equipment occurs in the /// same tick, the equipment change has priority. +/// Note: This system only handles direct changes to the held item (not actual +/// changes from the client) see [`equipment_held_item_sync_from_client`] pub(crate) fn equipment_inventory_sync( mut clients: Query< (&mut Equipment, &mut Inventory, &mut HeldItem), @@ -18,6 +20,7 @@ pub(crate) fn equipment_inventory_sync( With, ), >, + // mut change_slot_events ) { for (mut equipment, mut inventory, held_item) in &mut clients { // Equipment change has priority over held item changes @@ -25,6 +28,8 @@ pub(crate) fn equipment_inventory_sync( let item = equipment.main_hand().clone(); inventory.set_slot(held_item.slot(), item); } else if held_item.is_changed() { + // This will only be called if we change the held item from valence, + // the client change is handled in `equipment_held_item_sync_from_client` let item = inventory.slot(held_item.slot()).clone(); equipment.set_main_hand(item); } @@ -37,12 +42,15 @@ pub(crate) fn equipment_inventory_sync( (Equipment::FEET_IDX, PlayerInventory::SLOT_FEET), ]; + // We cant rely on the changed bitfield of inventory here + // because that gets reset when the client changes the inventory + for (equipment_slot, inventory_slot) in slots { // Equipment has priority over inventory changes if equipment.changed & (1 << equipment_slot) != 0 { let item = equipment.slot(equipment_slot).clone(); inventory.set_slot(inventory_slot, item); - } else if inventory.changed & (1 << inventory_slot) != 0 { + } else if inventory.is_changed() { let item = inventory.slot(inventory_slot).clone(); equipment.set_slot(equipment_slot, item); } @@ -50,6 +58,23 @@ pub(crate) fn equipment_inventory_sync( } } +/// Handles the case where the client changes the slot (the bevy change is +/// suppressed for this). See +/// [`valence_inventory::handle_update_selected_slot`]. +pub(crate) fn equipment_held_item_sync_from_client( + mut clients: Query<(&HeldItem, &Inventory, &mut Equipment), With>, + mut events: EventReader, +) { + for event in events.read() { + let Ok((held_item, inventory, mut equipment)) = clients.get_mut(event.client) else { + continue; + }; + + let item = inventory.slot(held_item.slot()).clone(); + equipment.set_main_hand(item); + } +} + pub(crate) fn on_attach_inventory_sync( entities: Query, (Added, With)>, ) { diff --git a/crates/valence_equipment/src/lib.rs b/crates/valence_equipment/src/lib.rs index e4f2ef571..be01b8864 100644 --- a/crates/valence_equipment/src/lib.rs +++ b/crates/valence_equipment/src/lib.rs @@ -22,6 +22,7 @@ impl Plugin for EquipmentPlugin { on_entity_init, inventory_sync::on_attach_inventory_sync, inventory_sync::equipment_inventory_sync, + inventory_sync::equipment_held_item_sync_from_client, ), ) .add_systems( diff --git a/crates/valence_inventory/src/lib.rs b/crates/valence_inventory/src/lib.rs index 19841bfa0..5918ce0b2 100644 --- a/crates/valence_inventory/src/lib.rs +++ b/crates/valence_inventory/src/lib.rs @@ -1408,6 +1408,9 @@ fn handle_update_selected_slot( for packet in packets.read() { if let Some(pkt) = packet.decode::() { if let Ok(mut mut_held) = clients.get_mut(packet.client) { + // We bypass the change detection here because the server listens for changes + // of `HeldItem` in order to send the update to the client. + // This is not required here because the update is coming from the client. let held = mut_held.bypass_change_detection(); if pkt.slot > 8 { // The client is trying to interact with a slot that does not exist, ignore. diff --git a/src/tests/equipment.rs b/src/tests/equipment.rs index 43b285173..7460b78f2 100644 --- a/src/tests/equipment.rs +++ b/src/tests/equipment.rs @@ -1,12 +1,14 @@ use valence_equipment::{Equipment, EquipmentInventorySync}; use valence_inventory::player_inventory::PlayerInventory; -use valence_inventory::Inventory; +use valence_inventory::{ClickMode, ClientInventoryState, Inventory, SlotChange}; use valence_server::entity::armor_stand::ArmorStandEntityBundle; use valence_server::entity::item::ItemEntityBundle; use valence_server::entity::zombie::ZombieEntityBundle; use valence_server::entity::{EntityLayerId, Position}; use valence_server::math::DVec3; -use valence_server::protocol::packets::play::EntityEquipmentUpdateS2c; +use valence_server::protocol::packets::play::{ + ClickSlotC2s, EntityEquipmentUpdateS2c, UpdateSelectedSlotC2s, +}; use valence_server::{ItemKind, ItemStack}; use crate::testing::ScenarioSingleClient; @@ -298,7 +300,7 @@ fn test_inventory_sync_from_equipment() { } #[test] -fn test_inventory_sync_from_inventory() { +fn test_equipment_sync_from_inventory() { let ScenarioSingleClient { mut app, client, @@ -405,3 +407,137 @@ fn test_equipment_priority_over_inventory() { ItemStack::new(ItemKind::GoldenChestplate, 1, None) ); } + +#[test] +fn test_equipment_change_from_player() { + let ScenarioSingleClient { + mut app, + client, + mut helper, + .. + } = ScenarioSingleClient::new(); + + // Process a tick to get past the "on join" logic. + app.update(); + helper.clear_received(); + + app.world_mut() + .entity_mut(client) + .insert(EquipmentInventorySync); + + let mut player_inventory = app + .world_mut() + .get_mut::(client) + .expect("could not get player equipment"); + + player_inventory.set_slot(36, ItemStack::new(ItemKind::DiamondChestplate, 1, None)); + app.update(); + helper.clear_received(); + + let state_id = app + .world() + .get::(client) + .expect("could not get player equipment") + .state_id(); + + app.update(); + + helper.send(&ClickSlotC2s { + window_id: 0, + button: 0, + mode: ClickMode::Hotbar, + state_id: state_id.0.into(), + slot_idx: 36, + slot_changes: vec![ + SlotChange { + idx: 36, + stack: ItemStack::EMPTY, + }, + SlotChange { + idx: PlayerInventory::SLOT_CHEST as i16, + stack: ItemStack::new(ItemKind::DiamondChestplate, 1, None), + }, + ] + .into(), + carried_item: ItemStack::EMPTY, + }); + + app.update(); + app.update(); + + let player_inventory = app + .world() + .get::(client) + .expect("could not get player equipment"); + + let player_equipment = app + .world() + .get::(client) + .expect("could not get player equipment"); + + assert_eq!( + player_inventory.slot(PlayerInventory::SLOT_CHEST), + &ItemStack::new(ItemKind::DiamondChestplate, 1, None) + ); + + assert_eq!(player_inventory.slot(36), &ItemStack::EMPTY); + + assert_eq!( + player_equipment.chest(), + &ItemStack::new(ItemKind::DiamondChestplate, 1, None) + ); +} + +#[test] +fn test_held_item_change_from_client() { + let ScenarioSingleClient { + mut app, + client, + mut helper, + .. + } = ScenarioSingleClient::new(); + + // Process a tick to get past the "on join" logic. + app.update(); + helper.clear_received(); + + app.world_mut() + .entity_mut(client) + .insert(EquipmentInventorySync); + + let mut player_inventory = app + .world_mut() + .get_mut::(client) + .expect("could not get player equipment"); + + player_inventory.set_slot(36, ItemStack::new(ItemKind::DiamondSword, 1, None)); + player_inventory.set_slot(37, ItemStack::new(ItemKind::IronSword, 1, None)); + + app.update(); + + let player_equipment = app + .world() + .get::(client) + .expect("could not get player equipment"); + + assert_eq!( + player_equipment.main_hand(), + &ItemStack::new(ItemKind::DiamondSword, 1, None) + ); + + // Change the held item from the client + helper.send(&UpdateSelectedSlotC2s { slot: 1 }); + + app.update(); // handle change slot + app.update(); // handle change equipment + + let player_equipment = app + .world() + .get::(client) + .expect("could not get player equipment"); + + assert_eq!( + player_equipment.main_hand(), + &ItemStack::new(ItemKind::IronSword, 1, None) + ); +}