fix packet order for ServerboundSetCarriedItem

5 files changed, 172 insertions, 19 deletions

diff --git a/azalea-client/src/plugins/interact/pick.rs b/azalea-client/src/plugins/interact/pick.rs
index a0a75910..c32ac5c7 100644
--- a/azalea-client/src/plugins/interact/pick.rs
+++ b/azalea-client/src/plugins/interact/pick.rs
@@ -201,6 +201,8 @@ fn filter_hit_result(hit_result: HitResult, eye_position: Vec3, range: f64) -> H
 /// Get the block that a player would be looking at if their eyes were at the
 /// given direction and position.
 ///
+/// This does not consider entities.
+///
 /// Also see [`pick`].
 pub fn pick_block(
     look_direction: LookDirection,
diff --git a/azalea-client/src/plugins/inventory.rs b/azalea-client/src/plugins/inventory.rs
index 8037f8fc..7dfe42c4 100644
--- a/azalea-client/src/plugins/inventory.rs
+++ b/azalea-client/src/plugins/inventory.rs
@@ -1,6 +1,7 @@
 use std::{cmp, collections::HashSet};
 
 use azalea_chat::FormattedText;
+use azalea_core::tick::GameTick;
 use azalea_entity::PlayerAbilities;
 pub use azalea_inventory::*;
 use azalea_inventory::{
@@ -46,6 +47,10 @@ impl Plugin for InventoryPlugin {
                     .chain()
                     .in_set(InventorySet)
                     .before(perform_respawn),
+            )
+            .add_systems(
+                GameTick,
+                ensure_has_sent_carried_item.after(super::mining::handle_mining_queued),
             );
     }
 }
@@ -927,15 +932,17 @@ fn handle_set_container_content_event(
     }
 }
 
+/// An ECS event to switch our hand to a different hotbar slot.
+///
+/// This is equivalent to using the scroll wheel or number keys in Minecraft.
 #[derive(Event)]
 pub struct SetSelectedHotbarSlotEvent {
     pub entity: Entity,
     /// The hotbar slot to select. This should be in the range 0..=8.
     pub slot: u8,
 }
-fn handle_set_selected_hotbar_slot_event(
+pub fn handle_set_selected_hotbar_slot_event(
     mut events: EventReader<SetSelectedHotbarSlotEvent>,
-    mut commands: Commands,
     mut query: Query<&mut Inventory>,
 ) {
     for event in events.read() {
@@ -947,12 +954,42 @@ fn handle_set_selected_hotbar_slot_event(
         }
 
         inventory.selected_hotbar_slot = event.slot;
-        commands.trigger(SendPacketEvent::new(
-            event.entity,
-            ServerboundSetCarriedItem {
-                slot: event.slot as u16,
-            },
-        ));
+    }
+}
+
+/// The item slot that the server thinks we have selected.
+///
+/// See [`ensure_has_sent_carried_item`].
+#[derive(Component)]
+pub struct LastSentSelectedHotbarSlot {
+    pub slot: u8,
+}
+/// A system that makes sure that [`LastSentSelectedHotbarSlot`] is in sync with
+/// [`Inventory::selected_hotbar_slot`].
+///
+/// This is necessary to make sure that [`ServerboundSetCarriedItem`] is sent in
+/// the right order, since it's not allowed to happen outside of a tick.
+pub fn ensure_has_sent_carried_item(
+    mut commands: Commands,
+    query: Query<(Entity, &Inventory, Option<&LastSentSelectedHotbarSlot>)>,
+) {
+    for (entity, inventory, last_sent) in query.iter() {
+        if let Some(last_sent) = last_sent {
+            if last_sent.slot == inventory.selected_hotbar_slot {
+                continue;
+            }
+
+            commands.trigger(SendPacketEvent::new(
+                entity,
+                ServerboundSetCarriedItem {
+                    slot: inventory.selected_hotbar_slot as u16,
+                },
+            ));
+        }
+
+        commands.entity(entity).insert(LastSentSelectedHotbarSlot {
+            slot: inventory.selected_hotbar_slot,
+        });
     }
 }
 
diff --git a/azalea-client/src/plugins/mining.rs b/azalea-client/src/plugins/mining.rs
index b6ac113a..f9bd50f6 100644
--- a/azalea-client/src/plugins/mining.rs
+++ b/azalea-client/src/plugins/mining.rs
@@ -181,6 +181,9 @@ fn handle_start_mining_block_event(
             // we're looking at the block
             (block_hit_result.direction, false)
         } else {
+            debug!(
+                "Got StartMiningBlockEvent but we're not looking at the block. Picking an arbitrary direction instead."
+            );
             // we're not looking at the block, arbitrary direction
             (Direction::Down, true)
         };
@@ -201,7 +204,7 @@ pub struct MiningQueued {
     pub force: bool,
 }
 #[allow(clippy::too_many_arguments, clippy::type_complexity)]
-fn handle_mining_queued(
+pub fn handle_mining_queued(
     mut commands: Commands,
     mut attack_block_events: EventWriter<AttackBlockEvent>,
     mut mine_block_progress_events: EventWriter<MineBlockProgressEvent>,
diff --git a/azalea-client/tests/mine_block_timing.rs b/azalea-client/tests/mine_block_timing.rs
index 1216f178..45648a83 100644
--- a/azalea-client/tests/mine_block_timing.rs
+++ b/azalea-client/tests/mine_block_timing.rs
@@ -65,23 +65,24 @@ fn test_mine_block_timing() {
     sent_packets.clear();
     simulation.tick();
     sent_packets.expect("ServerboundPlayerAction", |p| {
-        p == &ServerboundGamePacket::PlayerAction(ServerboundPlayerAction {
+        p == &ServerboundPlayerAction {
             action: s_player_action::Action::StartDestroyBlock,
             pos,
             direction: Direction::Up,
             seq: 1,
-        })
+        }
+        .into_variant()
     });
     sent_packets.expect("Swing 1", |p| {
-        p == &ServerboundGamePacket::Swing(ServerboundSwing {
+        p == &ServerboundSwing {
             hand: InteractionHand::MainHand,
-        })
+        }
         .into_variant()
     });
     sent_packets.expect("Swing 2", |p| {
-        p == &ServerboundGamePacket::Swing(ServerboundSwing {
+        p == &ServerboundSwing {
             hand: InteractionHand::MainHand,
-        })
+        }
         .into_variant()
     });
     sent_packets.expect_tick_end();
@@ -90,9 +91,9 @@ fn test_mine_block_timing() {
     for i in 3..=151 {
         simulation.tick();
         sent_packets.expect(&format!("Swing {i}"), |p| {
-            p == &ServerboundGamePacket::Swing(ServerboundSwing {
+            p == &ServerboundSwing {
                 hand: InteractionHand::MainHand,
-            })
+            }
             .into_variant()
         });
         sent_packets.maybe_expect(|p| matches!(p, ServerboundGamePacket::MovePlayerPos(_)));
@@ -112,9 +113,9 @@ fn test_mine_block_timing() {
         },
     );
     sent_packets.expect("Last swing", |p| {
-        p == &ServerboundGamePacket::Swing(ServerboundSwing {
+        p == &ServerboundSwing {
             hand: InteractionHand::MainHand,
-        })
+        }
         .into_variant()
     });
 
diff --git a/azalea-client/tests/packet_order_set_carried_item.rs b/azalea-client/tests/packet_order_set_carried_item.rs
new file mode 100644
index 00000000..358b7b73
--- /dev/null
+++ b/azalea-client/tests/packet_order_set_carried_item.rs
@@ -0,0 +1,110 @@
+use azalea_client::{
+    inventory::SetSelectedHotbarSlotEvent, mining::StartMiningBlockEvent, test_utils::prelude::*,
+};
+use azalea_core::{
+    direction::Direction,
+    position::{BlockPos, ChunkPos, Vec3},
+    resource_location::ResourceLocation,
+};
+use azalea_entity::LookDirection;
+use azalea_protocol::{
+    common::movements::{PositionMoveRotation, RelativeMovements},
+    packets::{
+        ConnectionProtocol, Packet,
+        game::{
+            ClientboundBlockUpdate, ClientboundPlayerPosition, ServerboundPlayerAction,
+            ServerboundSetCarriedItem, ServerboundSwing, s_interact::InteractionHand,
+            s_player_action,
+        },
+    },
+};
+use azalea_registry::{Block, DataRegistry, DimensionType};
+
+#[test]
+fn test_packet_order_set_carried_item() {
+    init_tracing();
+
+    let mut simulation = Simulation::new(ConnectionProtocol::Game);
+    let sent_packets = SentPackets::new(&mut simulation);
+    simulation.receive_packet(make_basic_login_packet(
+        DimensionType::new_raw(0),
+        ResourceLocation::new("azalea:overworld"),
+    ));
+
+    simulation.receive_packet(make_basic_empty_chunk(ChunkPos::new(0, 0), (384 + 64) / 16));
+    simulation.tick();
+
+    let pos = BlockPos::new(0, 2, 0);
+    simulation.receive_packet(ClientboundBlockUpdate {
+        pos,
+        block_state: Block::Stone.into(),
+    });
+    simulation.receive_packet(ClientboundPlayerPosition {
+        id: 1,
+        change: PositionMoveRotation {
+            pos: pos.up(1).center_bottom(),
+            delta: Vec3::ZERO,
+            look_direction: LookDirection::default(),
+        },
+        relative: RelativeMovements::all_absolute(),
+    });
+    simulation.tick();
+    assert_eq!(simulation.get_block_state(pos), Some(Block::Stone.into()));
+    simulation.with_component_mut::<LookDirection>(|look| {
+        // look down
+        look.update_x_rot(90.);
+    });
+
+    simulation.tick();
+    simulation.tick();
+    simulation.tick();
+
+    simulation.send_event(SetSelectedHotbarSlotEvent {
+        entity: simulation.entity,
+        slot: 1,
+    });
+    simulation.send_event(StartMiningBlockEvent {
+        entity: simulation.entity,
+        position: pos,
+    });
+
+    sent_packets.clear();
+    simulation.tick();
+    sent_packets.expect("ServerboundPlayerAction", |p| {
+        p == &ServerboundPlayerAction {
+            action: s_player_action::Action::StartDestroyBlock,
+            pos,
+            direction: Direction::Up,
+            seq: 1,
+        }
+        .into_variant()
+    });
+    sent_packets.expect("Swing 1", |p| {
+        p == &ServerboundSwing {
+            hand: InteractionHand::MainHand,
+        }
+        .into_variant()
+    });
+    sent_packets.expect("SetCarriedItem", |p| {
+        p == &ServerboundSetCarriedItem { slot: 1 }.into_variant()
+    });
+    sent_packets.expect("Swing 2", |p| {
+        p == &ServerboundSwing {
+            hand: InteractionHand::MainHand,
+        }
+        .into_variant()
+    });
+    sent_packets.expect_tick_end();
+    sent_packets.expect_empty();
+
+    simulation.tick();
+
+    sent_packets.expect("Swing", |p| {
+        p == &ServerboundSwing {
+            hand: InteractionHand::MainHand,
+        }
+        .into_variant()
+    });
+    sent_packets.expect_tick_end();
+    sent_packets.expect_empty();
+}