fix(inventory): Disallow equip same item twice #312

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

@@ -20,8 +20,8 @@
 package fr.quatrevieux.araknemu.game.handler.object;
 
 import fr.quatrevieux.araknemu.core.network.parser.PacketHandler;
+import fr.quatrevieux.araknemu.game.item.inventory.exception.AlreadyEquippedException;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.BadLevelException;
-import fr.quatrevieux.araknemu.game.item.inventory.exception.MoveException;
 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;
@@ -44,7 +44,7 @@ public void handle(GameSession session, ObjectMoveRequest packet) throws Excepti
             ;
         } catch (BadLevelException e) {
             session.send(new AddItemError(AddItemError.Error.TOO_LOW_LEVEL));
-        } catch (MoveException e) {
+        } catch (AlreadyEquippedException e) {
             session.send(new AddItemError(AddItemError.Error.ALREADY_EQUIPED));
         }
     }

src/main/java/fr/quatrevieux/araknemu/game/item/inventory/exception/MoveException.java → src/main/java/fr/quatrevieux/araknemu/game/item/inventory/exception/AlreadyEquippedException.java

@@ -14,31 +14,16 @@
  * You should have received a copy of the GNU Lesser General Public License
  * along with Araknemu.  If not, see <https://www.gnu.org/licenses/>.
  *
- * Copyright (c) 2017-2019 Vincent Quatrevieux
+ * Copyright (c) 2017-2023 Vincent Quatrevieux
  */
 
 package fr.quatrevieux.araknemu.game.item.inventory.exception;
 
 /**
- * Exception raised when cannot move the item
+ * Raise when the item is already equipped
+ * When this exception is raised, an error "ALREADY_EQUIPPED" should be sent to the client, and nothing should be done
+ *
+ * @see fr.quatrevieux.araknemu.game.player.inventory.slot.constraint.EquipOnceConstraint The constraint that raise this exception
  */
-public class MoveException extends InventoryException {
-    public MoveException() {
-    }
-
-    public MoveException(String message) {
-        super(message);
-    }
-
-    public MoveException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public MoveException(Throwable cause) {
-        super(cause);
-    }
-
-    public MoveException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
-        super(message, cause, enableSuppression, writableStackTrace);
-    }
+public class AlreadyEquippedException extends InventoryException {
 }

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

@@ -25,7 +25,6 @@
 import fr.quatrevieux.araknemu.game.item.inventory.AbstractItemEntry;
 import fr.quatrevieux.araknemu.game.item.inventory.event.ObjectMoved;
 import fr.quatrevieux.araknemu.game.item.inventory.exception.InventoryException;
-import fr.quatrevieux.araknemu.game.item.inventory.exception.MoveException;
 import fr.quatrevieux.araknemu.game.player.inventory.slot.InventorySlots;
 import org.checkerframework.checker.index.qual.Positive;
 import org.checkerframework.common.value.qual.IntRange;
@@ -57,19 +56,22 @@ public InventoryEntry(PlayerInventory inventory, PlayerItem entity, Item item) {
 
     /**
      * Move the entry to a new position
+     * If the item is already in the given position, nothing is done
      *
      * @param position The new position
      * @param quantity Quantity to move
      *
-     * @throws MoveException When the item is already on the requested position
+     * @throws InventoryException When cannot move the item
+     * @throws fr.quatrevieux.araknemu.game.item.inventory.exception.BadLevelException When the player level is too low for the item
+     * @throws fr.quatrevieux.araknemu.game.item.inventory.exception.AlreadyEquippedException When the item is already equipped
      */
     public void move(@IntRange(from = -1, to = InventorySlots.SLOT_MAX) int position, @Positive int quantity) throws InventoryException {
         if (quantity > quantity() || quantity <= 0) {
             throw new InventoryException("Invalid quantity given");
         }
 
         if (position == position()) {
-            throw new MoveException("The item is already on the requested position");
+            return;
         }
 
         if (quantity == quantity()) {

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/slot/AbstractWearableSlot.java

@@ -36,19 +36,38 @@
  * Base slot class for wearable
  */
 public abstract class AbstractWearableSlot extends AbstractEquipmentSlot {
-    public AbstractWearableSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int id, SuperType type) {
+    public AbstractWearableSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int id, SuperType type, SlotConstraint[] customConstraints) {
         super(
             dispatcher,
             new SimpleSlot(
                 id,
-                new SlotConstraint[] {
-                    new SingleItemConstraint(),
-                    new ItemClassConstraint(Wearable.class),
-                    new ItemTypeConstraint(type),
-                    new EquipmentLevelConstraint(owner),
-                },
+                makeConstraints(owner, type, customConstraints),
                 storage
             )
         );
     }
+
+    public AbstractWearableSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int id, SuperType type) {
+        this(dispatcher, storage, owner, id, type, new SlotConstraint[0]);
+    }
+
+    private static SlotConstraint[] makeConstraints(GamePlayer owner, SuperType type, SlotConstraint[] customConstraints) {
+        final SlotConstraint[] defaultConstraints = new SlotConstraint[] {
+            new SingleItemConstraint(),
+            new ItemClassConstraint(Wearable.class),
+            new ItemTypeConstraint(type),
+            new EquipmentLevelConstraint(owner),
+        };
+
+        if (customConstraints.length == 0) {
+            return defaultConstraints;
+        }
+
+        final SlotConstraint[] constraints = new SlotConstraint[defaultConstraints.length + customConstraints.length];
+
+        System.arraycopy(defaultConstraints, 0, constraints, 0, defaultConstraints.length);
+        System.arraycopy(customConstraints, 0, constraints, defaultConstraints.length, customConstraints.length);
+
+        return constraints;
+    }
 }

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/slot/DofusSlot.java

@@ -24,6 +24,8 @@
 import fr.quatrevieux.araknemu.game.item.inventory.ItemStorage;
 import fr.quatrevieux.araknemu.game.player.GamePlayer;
 import fr.quatrevieux.araknemu.game.player.inventory.InventoryEntry;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.constraint.EquipOnceConstraint;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.constraint.SlotConstraint;
 import org.checkerframework.common.value.qual.IntRange;
 
 /**
@@ -32,7 +34,9 @@
 public final class DofusSlot extends AbstractWearableSlot {
     public static final @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int[] SLOT_IDS = new int[] {9, 10, 11, 12, 13, 14};
 
-    public DofusSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int id) {
-        super(dispatcher, storage, owner, id, SuperType.DOFUS);
+    public DofusSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, InventorySlots inventorySlots, @IntRange(from = 0, to = InventorySlots.SLOT_MAX) int id) {
+        super(dispatcher, storage, owner, id, SuperType.DOFUS, new SlotConstraint[] {
+            new EquipOnceConstraint(inventorySlots, SLOT_IDS, false),
+        });
     }
 }

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/slot/InventorySlots.java

@@ -97,8 +97,8 @@ private void init(GamePlayer owner) {
     private void initSlots(GamePlayer owner) {
         add(new AmuletSlot(dispatcher, storage, owner));
         add(new WeaponSlot(dispatcher, storage, owner));
-        add(new RingSlot(dispatcher, storage, owner, RingSlot.RING1));
-        add(new RingSlot(dispatcher, storage, owner, RingSlot.RING2));
+        add(new RingSlot(dispatcher, storage, owner, this, RingSlot.RING1));
+        add(new RingSlot(dispatcher, storage, owner, this, RingSlot.RING2));
         add(new BeltSlot(dispatcher, storage, owner));
         add(new BootsSlot(dispatcher, storage, owner));
         add(new HelmetSlot(dispatcher, storage, owner));
@@ -112,7 +112,7 @@ private void initSlots(GamePlayer owner) {
 
     private void initDofusSlots(GamePlayer owner) {
         for (int id : DofusSlot.SLOT_IDS) {
-            add(new DofusSlot(dispatcher, storage, owner, id));
+            add(new DofusSlot(dispatcher, storage, owner, this, id));
         }
     }
 

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/slot/RingSlot.java

@@ -24,6 +24,8 @@
 import fr.quatrevieux.araknemu.game.item.inventory.ItemStorage;
 import fr.quatrevieux.araknemu.game.player.GamePlayer;
 import fr.quatrevieux.araknemu.game.player.inventory.InventoryEntry;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.constraint.EquipOnceConstraint;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.constraint.SlotConstraint;
 import org.checkerframework.common.value.qual.IntVal;
 
 /**
@@ -33,7 +35,9 @@ public final class RingSlot extends AbstractWearableSlot {
     public static final int RING1 = 2;
     public static final int RING2 = 4;
 
-    public RingSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, @IntVal({RING1, RING2}) int id) {
-        super(dispatcher, storage, owner, id, SuperType.RING);
+    public RingSlot(Dispatcher dispatcher, ItemStorage<InventoryEntry> storage, GamePlayer owner, InventorySlots inventorySlots, @IntVal({RING1, RING2}) int id) {
+        super(dispatcher, storage, owner, id, SuperType.RING, new SlotConstraint[] {
+            new EquipOnceConstraint(inventorySlots, new int[] {RING1, RING2}, true),
+        });
     }
 }

src/main/java/fr/quatrevieux/araknemu/game/player/inventory/slot/constraint/EquipOnceConstraint.java

@@ -0,0 +1,60 @@
+/*
+ * This file is part of Araknemu.
+ *
+ * Araknemu is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Araknemu is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Araknemu.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * Copyright (c) 2017-2023 Vincent Quatrevieux
+ */
+
+package fr.quatrevieux.araknemu.game.player.inventory.slot.constraint;
+
+import fr.quatrevieux.araknemu.game.item.Item;
+import fr.quatrevieux.araknemu.game.item.inventory.exception.AlreadyEquippedException;
+import fr.quatrevieux.araknemu.game.item.inventory.exception.InventoryException;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.InventorySlots;
+import org.checkerframework.common.value.qual.IntRange;
+
+/**
+ * Check that the item can be equipped only once
+ * This constraint will raise {@link AlreadyEquippedException} if the item is already equipped on another slot
+ */
+public final class EquipOnceConstraint implements SlotConstraint {
+    private final InventorySlots inventorySlots;
+    private final @IntRange(from = -1, to = InventorySlots.SLOT_MAX) int[] slotIds;
+    private final boolean onlyWithinItemSet;
+
+    /**
+     * @param inventorySlots the player inventory
+     * @param slotIds the slots ids to check
+     * @param onlyWithinItemSet if true, only check items that are in an item set (use for rings)
+     */
+    public EquipOnceConstraint(InventorySlots inventorySlots, @IntRange(from = -1, to = InventorySlots.SLOT_MAX) int[] slotIds, boolean onlyWithinItemSet) {
+        this.inventorySlots = inventorySlots;
+        this.slotIds = slotIds;
+        this.onlyWithinItemSet = onlyWithinItemSet;
+    }
+
+    @Override
+    public void check(Item item, int quantity) throws InventoryException {
+        if (onlyWithinItemSet && !item.set().isPresent()) {
+            return;
+        }
+
+        for (int slot : slotIds) {
+            if (inventorySlots.get(slot).entry().filter(entry -> item.template().equals(entry.item().template())).isPresent()) {
+                throw new AlreadyEquippedException();
+            }
+        }
+    }
+}

src/test/java/fr/quatrevieux/araknemu/data/world/repository/implementation/local/ItemTemplateRepositoryCacheTest.java

@@ -85,7 +85,7 @@ void hasCached() {
     void load() {
         Collection<ItemTemplate> templates = repository.load();
 
-        assertCount(18, templates);
+        assertCount(19, templates);
 
         for (ItemTemplate template : templates) {
             assertSame(

src/test/java/fr/quatrevieux/araknemu/game/GameDataSet.java

@@ -321,6 +321,8 @@ public GameDataSet pushItemTemplates() throws SQLException, ContainerException {
         pushItemTemplate(new ItemTemplate(8237, 10, "Ceinture du Piou Rouge", 1, Arrays.asList(new ItemTemplateEffectEntry(Effect.ADD_INTELLIGENCE, 1, 0, 0, "0d0+1")), 1, "", 60, "", 1));
         pushItemTemplate(new ItemTemplate(8243, 16, "Chapeau du Piou Rouge", 1, Arrays.asList(new ItemTemplateEffectEntry(Effect.ADD_INTELLIGENCE, 1, 0, 0, "0d0+1")), 1, "", 60, "", 1));
 
+        pushItemTemplate(new ItemTemplate(849, 9, "Anneau Forcesque", 1, Arrays.asList(new ItemTemplateEffectEntry(Effect.ADD_STRENGTH, 2, 0, 0, "0d0+2")), 1, "", 0, "", 1));
+
         return this;
     }
 

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

@@ -25,7 +25,9 @@
 import fr.quatrevieux.araknemu.game.fight.turn.order.AlternateTeamFighterOrder;
 import fr.quatrevieux.araknemu.game.item.ItemService;
 import fr.quatrevieux.araknemu.game.player.inventory.InventoryEntry;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.DofusSlot;
 import fr.quatrevieux.araknemu.game.player.inventory.slot.MantleSlot;
+import fr.quatrevieux.araknemu.game.player.inventory.slot.RingSlot;
 import fr.quatrevieux.araknemu.network.game.in.object.ObjectMoveRequest;
 import fr.quatrevieux.araknemu.network.game.out.info.Error;
 import fr.quatrevieux.araknemu.network.game.out.object.AddItemError;
@@ -99,15 +101,41 @@ void handleErrorTooLowLevel() throws Exception {
     }
 
     @Test
-    void handleErrorAlreadyOnRequestPosition() throws Exception {
+    void handleAlreadyOnRequestedPositionShouldDoesNothing() throws Exception {
         handler.handle(session, new ObjectMoveRequest(1, 0, 1));
 
-        requestStack.assertLast(new AddItemError(AddItemError.Error.ALREADY_EQUIPED));
+        requestStack.assertEmpty();
 
         assertEquals(0, gamePlayer().inventory().get(1).position());
         assertEquals(1, gamePlayer().inventory().get(1).quantity());
     }
 
+    @Test
+    void handleErrorDofusAlreadyEquipped() throws Exception {
+        gamePlayer().inventory().add(itemService.create(694), 1, DofusSlot.SLOT_IDS[0]);
+        InventoryEntry entry = gamePlayer().inventory().add(itemService.create(694), 1);
+
+        handler.handle(session, new ObjectMoveRequest(entry.id(), DofusSlot.SLOT_IDS[1], 1));
+
+        requestStack.assertLast(new AddItemError(AddItemError.Error.ALREADY_EQUIPED));
+
+        assertEquals(-1, entry.position());
+        assertEquals(1, entry.quantity());
+    }
+
+    @Test
+    void handleErrorRingAlreadyEquipped() throws Exception {
+        gamePlayer().inventory().add(itemService.create(2419), 1, RingSlot.RING1);
+        InventoryEntry entry = gamePlayer().inventory().add(itemService.create(2419), 1);
+
+        handler.handle(session, new ObjectMoveRequest(entry.id(), RingSlot.RING2, 1));
+
+        requestStack.assertLast(new AddItemError(AddItemError.Error.ALREADY_EQUIPED));
+
+        assertEquals(-1, entry.position());
+        assertEquals(1, entry.quantity());
+    }
+
     @Test
     void functionalNotAllowedOnActiveFight() throws Exception {
         Fight fight = createFight();

src/test/java/fr/quatrevieux/araknemu/game/item/ItemServiceTest.java

@@ -78,7 +78,7 @@ void preload() {
         Mockito.verify(logger).info("Successfully load {} item sets", 3);
 
         Mockito.verify(logger).info("Loading items...");
-        Mockito.verify(logger).info("Successfully load {} items", 18);
+        Mockito.verify(logger).info("Successfully load {} items", 19);
     }
 
     @Test

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

@@ -93,10 +93,13 @@ void moveTooHighQuantity() throws InventoryException, ContainerException {
     }
 
     @Test
-    void moveAlreadyOnRequestedPosition() throws InventoryException, ContainerException {
+    void moveAlreadyOnRequestedPositionShouldDoesNothing() throws InventoryException, ContainerException {
         InventoryEntry entry = inventory.add(container.get(ItemService.class).create(284));
 
-        assertThrowsWithMessage(InventoryException.class, "The item is already on the requested position", () -> entry.move(-1, 1));
+        entry.move(-1, 1);
+
+        assertEquals(-1, entry.position());
+        assertEquals(1, entry.quantity());
     }
 
     @Test