fix cursor item updates not being sent if an inventory is open

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::<CursorItem>(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::<ScreenHandlerSlotUpdateS2c>(1);
+    for pkt in sent_packets.0 {
+        if let Ok(pkt) = pkt.decode::<ScreenHandlerSlotUpdateS2c>() {
+            assert_eq!(pkt.window_id, -1);
+            assert_eq!(pkt.slot_idx, -1);
+        }
+    }
+
+    let cursor_item = app
+        .world
+        .get_mut::<CursorItem>(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();

crates/valence_inventory/src/lib.rs

@@ -607,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),
@@ -627,7 +628,7 @@ fn update_open_inventories(
         Entity,
         &mut Client,
         &mut ClientInventoryState,
-        &CursorItem,
+        Ref<CursorItem>,
         &mut OpenInventory,
     )>,
     mut inventories: Query<&mut Inventory>,
@@ -692,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),
@@ -706,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;
     }
 }