fix(inventory): Drap and drop on already equipped slot

src/main/java/fr/quatrevieux/araknemu/game/fight/fighter/player/PlayerFighter.java

@@ -33,6 +33,7 @@
 import fr.quatrevieux.araknemu.game.item.type.Weapon;
 import fr.quatrevieux.araknemu.game.player.GamePlayer;
 import fr.quatrevieux.araknemu.game.player.PlayerSessionScope;
+import fr.quatrevieux.araknemu.game.player.inventory.InventoryEntry;
 import fr.quatrevieux.araknemu.game.player.inventory.slot.WeaponSlot;
 import fr.quatrevieux.araknemu.game.spell.boost.DispatcherSpellsBoosts;
 import fr.quatrevieux.araknemu.game.spell.boost.SimpleSpellsBoosts;
@@ -142,6 +143,7 @@ public CastableWeapon weapon() {
 
         return weapon = player.inventory()
             .bySlot(WeaponSlot.SLOT_ID)
+            .map(InventoryEntry::item)
             .map(Weapon.class::cast)
             .map(CastableWeapon::new)
             .orElseThrow(() -> new FightException("The fighter do not have any weapon"))

src/main/java/fr/quatrevieux/araknemu/game/handler/object/MoveObject.java

@@ -20,27 +20,39 @@
 package fr.quatrevieux.araknemu.game.handler.object;
 
 import fr.quatrevieux.araknemu.core.network.parser.PacketHandler;
+import fr.quatrevieux.araknemu.game.item.inventory.ItemEntry;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.AlreadyEquippedException;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.BadLevelException;
+import fr.quatrevieux.araknemu.game.player.inventory.InventoryEntry;
+import fr.quatrevieux.araknemu.game.player.inventory.PlayerInventory;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.InventorySlots;
 import fr.quatrevieux.araknemu.network.game.GameSession;
 import fr.quatrevieux.araknemu.network.game.in.object.ObjectMoveRequest;
 import fr.quatrevieux.araknemu.network.game.out.object.AddItemError;
 import org.checkerframework.checker.nullness.util.NullnessUtil;
+import org.checkerframework.common.value.qual.IntRange;
 
 /**
  * Move an object from the repository
  */
 public final class MoveObject implements PacketHandler<GameSession, ObjectMoveRequest> {
     @Override
     public void handle(GameSession session, ObjectMoveRequest packet) throws Exception {
+        final PlayerInventory inventory = NullnessUtil.castNonNull(session.player()).inventory();
+        final @IntRange(from = -1, to = InventorySlots.SLOT_MAX) int position = packet.position();
+
+        if (position != ItemEntry.DEFAULT_POSITION) {
+            // Unequip the item from the target position
+            inventory.bySlot(position)
+                .filter(entry -> entry.id() != packet.id())
+                .ifPresent(InventoryEntry::unequip)
+            ;
+        }
+
         try {
-            NullnessUtil.castNonNull(session.player())
-                .inventory()
+            inventory
                 .get(packet.id())
-                .move(
-                    packet.position(),
-                    packet.quantity()
-                )
+                .move(position, packet.quantity())
             ;
         } catch (BadLevelException e) {
             session.send(new AddItemError(AddItemError.Error.TOO_LOW_LEVEL));

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/InventoryEntry.java

@@ -26,6 +26,7 @@
 import fr.quatrevieux.araknemu.game.item.inventory.event.ObjectMoved;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.InventoryException;
 import fr.quatrevieux.araknemu.game.player.inventory.slot.InventorySlots;
+import fr.quatrevieux.araknemu.util.Asserter;
 import org.checkerframework.checker.index.qual.Positive;
 import org.checkerframework.common.value.qual.IntRange;
 import org.checkerframework.dataflow.qual.Pure;
@@ -86,9 +87,23 @@ public void move(@IntRange(from = -1, to = InventorySlots.SLOT_MAX) int position
         remove(quantity);
     }
 
+    /**
+     * Move all the entry to the default position
+     * This can be used to unequip the item
+     *
+     * This is equivalent of calling `move(DEFAULT_POSITION, quantity())`
+     *
+     * This method will do nothing if the item is already on the default position
+     */
+    public void unequip() {
+        move(DEFAULT_POSITION, Asserter.assertPositive(quantity()));
+    }
+
     /**
      * Set the item to the default position
      * Note: this method is internal and should not be called
+     *
+     * @see #unequip() To unequip the item (this method should be called instead)
      */
     public void setToDefaultPosition() {
         entity.setPosition(DEFAULT_POSITION);

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/PlayerInventory.java

@@ -127,14 +127,14 @@ public Collection<AbstractEquipment> equipments() {
     }
 
     /**
-     * Get an item by the slot id
+     * Get an entry by the slot id
      *
      * @param slotId The slot of the item
      *
-     * @return The item contained in the slot, or an empty Optional
+     * @return The entry if the slot is full, empty otherwise
      */
-    public Optional<Item> bySlot(@IntRange(from = 0, to = InventorySlots.SLOT_MAX) int slotId) throws InventoryException {
-        return slots.get(slotId).entry().map(InventoryEntry::item);
+    public Optional<InventoryEntry> bySlot(@IntRange(from = 0, to = InventorySlots.SLOT_MAX) int slotId) throws InventoryException {
+        return slots.get(slotId).entry();
     }
 
     /**

src/test/java/fr/quatrevieux/araknemu/game/handler/object/MoveObjectTest.java

@@ -136,6 +136,25 @@ void handleErrorRingAlreadyEquipped() throws Exception {
         assertEquals(1, entry.quantity());
     }
 
+    @Test
+    void handleSlotNotEmpty() throws Exception {
+        requestStack.clear();
+        handler.handle(session, new ObjectMoveRequest(3, 0, 1));
+
+        requestStack.assertOne("OM1|-1");
+        requestStack.assertOne("OQ3|9");
+        requestStack.assertOne("OAKO4~979~1~0~7e#a#0#0#0d0+10,76#a#0#0#0d0+10");
+
+        assertEquals(2425, player.inventory().bySlot(0).get().item().template().id());
+        assertEquals(9, player.inventory().get(3).quantity());
+        assertEquals(2425, player.inventory().get(4).item().template().id());
+        assertEquals(1, player.inventory().get(4).quantity());
+
+        assertEquals(39, player.inventory().get(1).item().template().id());
+        assertEquals(-1, player.inventory().get(1).position());
+        assertEquals(1, player.inventory().get(1).quantity());
+    }
+
     @Test
     void functionalNotAllowedOnActiveFight() throws Exception {
         Fight fight = createFight();

src/test/java/fr/quatrevieux/araknemu/game/player/inventory/InventoryEntryTest.java

@@ -33,9 +33,11 @@
 import fr.quatrevieux.araknemu.game.item.inventory.event.ObjectQuantityChanged;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.InventoryException;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.ItemNotFoundException;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.UsableSlot;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.concurrent.atomic.AtomicReference;
@@ -159,6 +161,44 @@ public Class<ObjectQuantityChanged> event() {
         assertSame(item, ref1.get().item());
     }
 
+    @Test
+    void unequipItem() throws InventoryException, ContainerException {
+        InventoryEntry entry = inventory.add(
+            container.get(ItemService.class).create(40),
+            1,
+            1
+        );
+
+        AtomicReference<ItemEntry> ref = new AtomicReference<>();
+        dispatcher.add(ObjectMoved.class, objectMoved -> ref.set(objectMoved.entry()));
+
+        entry.unequip();
+
+        assertSame(ref.get(), entry);
+        assertEquals(-1, entry.position());
+        assertEquals(1, entry.quantity());
+    }
+
+    @Test
+    void unequipUsableSlot() throws InventoryException, ContainerException, SQLException {
+        dataSet.pushUsableItems();
+
+        InventoryEntry entry = inventory.add(
+            container.get(ItemService.class).create(283),
+            10,
+            UsableSlot.SLOT_ID_START
+        );
+
+        AtomicReference<ItemEntry> ref = new AtomicReference<>();
+        dispatcher.add(ObjectMoved.class, objectMoved -> ref.set(objectMoved.entry()));
+
+        entry.unequip();
+
+        assertSame(ref.get(), entry);
+        assertEquals(-1, entry.position());
+        assertEquals(10, entry.quantity());
+    }
+
     @Test
     void add() throws InventoryException, ContainerException {
         InventoryEntry entry = inventory.add(

src/test/java/fr/quatrevieux/araknemu/game/player/inventory/PlayerInventoryTest.java

@@ -301,7 +301,7 @@ void bySlot() throws SQLException, ContainerException, InventoryException {
 
         InventoryEntry entry = inventory.add(container.get(ItemService.class).create(2416), 1, 1);
 
-        assertEquals(Optional.of(entry.item()), inventory.bySlot(1));
+        assertEquals(Optional.of(entry), inventory.bySlot(1));
         assertFalse(inventory.bySlot(5).isPresent());
     }
 
@@ -456,7 +456,7 @@ void equipSameRingWithoutItemSetIsAllowed() throws SQLException {
         inventory.add(ring2, 1, RingSlot.RING2);
 
         assertCount(2, inventory.equipments());
-        assertEquals(ring1, inventory.bySlot(RingSlot.RING1).get());
-        assertEquals(ring1, inventory.bySlot(RingSlot.RING2).get());
+        assertEquals(ring1, inventory.bySlot(RingSlot.RING1).get().item());
+        assertEquals(ring1, inventory.bySlot(RingSlot.RING2).get().item());
     }
 }