Send changes to client when cancelled and test fix

src/main/java/net/minestom/server/inventory/ContainerInventory.java

@@ -135,15 +135,19 @@ public non-sealed class ContainerInventory extends InventoryImpl {
     public @Nullable List<Click.Change> handleClick(@NotNull Player player, Click.@NotNull Info info, @Nullable List<Click.Change> clientPrediction) {
         // We can use the client prediction if it's conservative (i.e. doesn't create or delete items) or the client is in creative.
         // Otherwise, we make our own.
+        List<Click.Change> changes;
         if (clientPrediction != null && (ClickUtils.conservative(clientPrediction, this, player.getInventory()) || player.isCreative())) {
-            return ContainerInventory.handleClick(this, player, info, (i, g) -> clientPrediction);
+            changes = ContainerInventory.handleClick(this, player, info, (i, g) -> clientPrediction);
         } else {
-            var results = ContainerInventory.handleClick(this, player, info,
+            changes = ContainerInventory.handleClick(this, player, info,
                     ClickProcessors.PROCESSORS_MAP.getOrDefault(inventoryType, ClickProcessors.GENERIC_PROCESSOR));
+        }
+
+        if (changes == null || !changes.equals(clientPrediction)) {
             update(player);
             player.getInventory().update(player);
-            return results;
         }
+        return changes;
     }
 
     @Override

src/main/java/net/minestom/server/inventory/PlayerInventory.java

@@ -162,13 +162,17 @@ public non-sealed class PlayerInventory extends InventoryImpl {
     public @Nullable List<Click.Change> handleClick(@NotNull Player player, Click.@NotNull Info info, @Nullable List<Click.Change> clientPrediction) {
         // We can use the client prediction if it's conservative (i.e. doesn't create or delete items) or the client is in creative.
         // Otherwise, we make our own.
+        List<Click.Change> changes;
         if (clientPrediction != null && (ClickUtils.conservative(clientPrediction, this, this) || player.isCreative())) {
-            return ContainerInventory.handleClick(this, player, info, (i, g) -> clientPrediction);
+            changes = ContainerInventory.handleClick(this, player, info, (i, g) -> clientPrediction);
         } else {
-            var results = ContainerInventory.handleClick(this, player, info, ClickProcessors.PLAYER_PROCESSOR);
-            update(player);
-            return results;
+            changes = ContainerInventory.handleClick(this, player, info, ClickProcessors.PLAYER_PROCESSOR);
         }
+
+        if (changes == null || !changes.equals(clientPrediction)) {
+            update(player);
+        }
+        return changes;
     }
 
     public @NotNull ItemStack getEquipment(@NotNull EquipmentSlot slot, int heldSlot) {

src/test/java/net/minestom/server/inventory/InventoryIntegrationTest.java

@@ -1,6 +1,8 @@
 package net.minestom.server.inventory;
 
 import net.kyori.adventure.text.Component;
+import net.minestom.server.event.inventory.InventoryPreClickEvent;
+import net.minestom.server.inventory.click.Click;
 import net.minestom.server.utils.inventory.PlayerInventoryUtils;
 import net.minestom.testing.Env;
 import net.minestom.testing.EnvTest;
@@ -13,6 +15,8 @@ import net.minestom.server.network.packet.server.play.SetSlotPacket;
 import net.minestom.server.network.packet.server.play.WindowItemsPacket;
 import org.junit.jupiter.api.Test;
 
+import java.util.List;
+
 import static org.junit.jupiter.api.Assertions.*;
 
 @EnvTest
@@ -187,4 +191,47 @@ public class InventoryIntegrationTest {
         });
     }
 
+    @Test
+    public void testClientUpdatesInventoryWhenCancelled(Env env) {
+        var instance = env.createFlatInstance();
+        var connection = env.createConnection();
+        var player = connection.connect(instance, new Pos(0, 42, 0)).join();
+        assertEquals(instance, player.getInstance());
+
+        var inventory = player.getInventory();
+
+        player.getInventory().setCursorItem(MAGIC_STACK);
+
+        var packetTracker = connection.trackIncoming(WindowItemsPacket.class);
+
+        inventory.handleClick(player, new Click.Info.Left(0), List.of(
+                new Click.Change.Container(0, MAGIC_STACK),
+                new Click.Change.Cursor(ItemStack.AIR)
+        ));
+
+        // Should not have sent any packets because client predictions can be followed
+        packetTracker.assertEmpty();
+
+        // Reset inventory to previous state
+        inventory.setItemStack(0, ItemStack.AIR);
+        player.getInventory().setCursorItem(MAGIC_STACK);
+
+        packetTracker = connection.trackIncoming(WindowItemsPacket.class);
+
+        // Cancelling the event should send a packet back
+        var listener = env.listen(InventoryPreClickEvent.class);
+        listener.followup(event -> event.setCancelled(true));
+
+        inventory.handleClick(player, new Click.Info.Left(0), List.of(
+                new Click.Change.Container(0, MAGIC_STACK),
+                new Click.Change.Cursor(ItemStack.AIR)
+        ));
+
+        // Should not have sent any packets because client predictions can be followed
+        packetTracker.assertSingle(packet -> {
+            packet.items().forEach(item -> assertEquals(ItemStack.AIR, item));
+            assertEquals(MAGIC_STACK, packet.carriedItem());
+        });
+    }
+
 }