Basically redid equip and unequip functions().

I made the system handle the fact that equipment item
are completely unlinked to the inventory items.
Equip items now have a unique itemInstance number permitting
to equip the same item type multiple time when the slot capacity
is wide enough to do so.

I also prepared the functions to welcome in the near tests against
scripted equipment.

The equip process is known to be working server-side but the unequip
process has yet to be reviewed, even if implemented.

4 files changed, 270 insertions, 156 deletions

diff --git a/src/common/inventorydata.h b/src/common/inventorydata.h
index e7c8117..e1c5bfa 100644
--- a/src/common/inventorydata.h
+++ b/src/common/inventorydata.h
@@ -49,6 +49,12 @@ struct EquipmentItem
         itemId(0), itemInstance(0)
     {}
 
+    EquipmentItem(unsigned int itemId, unsigned int itemInstance)
+    {
+        this->itemId = itemId;
+        this->itemInstance = itemInstance;
+    }
+
     // The item id taken from the item db.
     unsigned int itemId;
     // A unique instance number used to separate items when equipping the same
diff --git a/src/common/manaserv_protocol.h b/src/common/manaserv_protocol.h
index 786330c..6c51aef 100644
--- a/src/common/manaserv_protocol.h
+++ b/src/common/manaserv_protocol.h
@@ -94,8 +94,8 @@ enum {
     PGMSG_UNEQUIP                  = 0x0113, // W equipment slot

     PGMSG_MOVE_ITEM                = 0x0114, // W slot1, W slot2, W amount

     GPMSG_INVENTORY                = 0x0120, // { W slot, W item id [, W amount] (if item id is nonzero) }*

-    GPMSG_INVENTORY_FULL           = 0x0121, // W inventory slot count { W slot, W itemId, W amount }, { W equip slot, W invy slot}*

-    GPMSG_EQUIP                    = 0x0122, // { W Invy slot, W equip slot type count { W equip slot, W number used} }*

+    GPMSG_INVENTORY_FULL           = 0x0121, // W inventory slot count { W slot, W itemId, W amount }, { W equip slot, W item Id, W item Instance}*

+    GPMSG_EQUIP                    = 0x0122, // W item Id, W equip slot type count { W equip slot, W capacity used}*

     GPMSG_PLAYER_ATTRIBUTE_CHANGE  = 0x0130, // { W attribute, D base value (in 1/256ths), D modified value (in 1/256ths)}*

     GPMSG_PLAYER_EXP_CHANGE        = 0x0140, // { W skill, D exp got, D exp needed }*

     GPMSG_LEVELUP                  = 0x0150, // W new level, W character points, W correction points

diff --git a/src/game-server/inventory.cpp b/src/game-server/inventory.cpp
index 75294a1..1eaf8b9 100644
--- a/src/game-server/inventory.cpp
+++ b/src/game-server/inventory.cpp
@@ -55,9 +55,9 @@ void Inventory::sendFull() const
          k != k_end;
          ++k)
     {
-        m.writeInt16(k->first);                  // Equip slot id
-        m.writeInt16(k->second.itemId);          // Item id
-        m.writeInt16(k->second.itemInstance);    // Item instance
+        m.writeInt16(k->first);                 // Equip slot id
+        m.writeInt16(k->second.itemId);         // Item id
+        m.writeInt16(k->second.itemInstance);   // Item instance
     }
 
     gameHandler->sendTo(mCharacter, m);
@@ -106,9 +106,6 @@ void Inventory::initialize()
     EquipData::iterator it2;
     for (it2 = mPoss->equipSlots.begin(); it2 != mPoss->equipSlots.end();)
     {
-        /*
-         * TODO: Check that all needed slots are available here.
-         */
         ItemClass *item = itemManager->getItem(it2->second.itemId);
         if (item)
         {
@@ -541,170 +538,258 @@ void Inventory::updateEquipmentTrigger(ItemClass *oldI, ItemClass *newI)
         newI->useTrigger(mCharacter, ITT_EQUIP);
 }
 
-bool Inventory::equip(int slot, bool override)
+unsigned int Inventory::getNewEquipItemInstance()
+{
+    unsigned int itemInstance = 1;
+
+    for (EquipData::const_iterator it = mPoss->equipSlots.begin(),
+        it_end = mPoss->equipSlots.end(); it != it_end; ++it)
+    {
+        if (it->second.itemInstance == itemInstance)
+        {
+            ++itemInstance;
+            it = mPoss->equipSlots.begin();
+        }
+    }
+
+    return itemInstance;
+}
+
+bool Inventory::checkEquipmentCapacity(unsigned int equipmentSlot,
+                                       unsigned int capacityRequested)
 {
-    if (mPoss->equipSlots.count(slot))
+    int capacity = itemManager->getEquipSlotCapacity(equipmentSlot);
+
+    // If the equipement slot doesn't exist, we can't equip on it.
+    if (capacity <= 0)
         return false;
+
+    // Test whether the slot capacity requested is reached.
+    for (EquipData::const_iterator it = mPoss->equipSlots.begin(),
+        it_end = mPoss->equipSlots.end(); it != it_end; ++it)
+    {
+        if (it->first == equipmentSlot)
+        {
+            if (it->second.itemInstance != 0)
+            {
+                capacity--;
+            }
+        }
+    }
+
+    assert(capacity >= 0); // A should never happen case.
+
+    if (capacity < (int)capacityRequested)
+        return false;
+
+    return true;
+}
+
+bool Inventory::equip(int inventorySlot)
+{
+    // Test inventory slot existence
     InventoryData::iterator it;
-    if ((it = mPoss->inventory.find(slot)) == mPoss->inventory.end())
+    if ((it = mPoss->inventory.find(inventorySlot)) == mPoss->inventory.end())
+    {
         return false;
-    const ItemEquipsInfo &eq = itemManager->getItem(it->second.itemId)
-                                    ->getItemEquipData();
-    if (eq.empty())
+        LOG_DEBUG("No existing item in inventory at slot: " << inventorySlot);
+    }
+
+    // Test the equipment scripted requirements
+    if (!testEquipScriptRequirements(it->second.itemId))
         return false;
-    ItemEquipInfo const *ovd = 0;
 
-    MessageOut equipMsg(GPMSG_EQUIP);
-    // Iterate through all possible combinations of slots
-/*    for (ItemEquipsInfo::const_iterator it2 = eq.begin(),
-         it2_end = eq.end(); it2 != it2_end; ++it2)
+    // Test the equip requirements. If none, it's not an equipable item.
+    const ItemEquipsInfo &equipInfoList =
+        itemManager->getItem(it->second.itemId)->getItemEquipData();
+    if (equipInfoList.empty())
     {
-        // Iterate through this combination of slots.
-        /*
-         * 0 = all ok, slots free
-         * 1 = possible if other items are unequipped first
-         * 2 = impossible, requires too many slots
-         *     even with other equipment being removed
-         */
-        int fail = 0; /*
-        ItemEquipInfo::const_iterator it3, it3_end;
-        for (it3 = it2->begin(),
-             it3_end = it2->end();
-             it3 != it3_end;
-             ++it3)
+        LOG_DEBUG("No equip requirements for item id: " << it->second.itemId
+            << " at slot: " << inventorySlot);
+        return false;
+    }
+
+    // Iterate through all slots requirements.
+    std::list<unsigned int> equipSlotsToUnequipFirst;
+    for (ItemEquipsInfo::const_iterator it2 = equipInfoList.begin(),
+         it2_end = equipInfoList.end(); it2 != it2_end; ++it2)
+    {
+        // We first check the equipment slots for:
+        // - 1. whether enough total equip slot space is available.
+        // - 2. whether some other equipment is to be unequipped first.
+
+        // If not enough total space in the equipment slot is available,
+        // we cannot equip.
+        if (itemManager->getEquipSlotCapacity(it2->first) < it2->second)
         {
-            // it3 -> { slot id, number required }
-            unsigned int max = itemManager->getEquipSlotCapacity(it3->first),
-                         used = mPoss->equipSlots.count(it3->first);
-            if (max - used >= it3->second)
-                continue;
-            else if (max >= it3->second)
+            LOG_DEBUG("Not enough equip capacity at slot: " << it2->first
+                      << ", total available: "
+                      << itemManager->getEquipSlotCapacity(it2->first)
+                      << ", required: " << it2->second);
+            return false;
+        }
+
+        // Test whether some item(s) is(are) to be unequipped first.
+        if (!checkEquipmentCapacity(it2->first, it2->second))
+        {
+            // And test whether the unequip action would succeed first.
+            if (testUnequipScriptRequirements(it2->first)
+                && hasInventoryEnoughSpace(it2->first))
             {
-                fail |= 1;
-                if (override)
-                    continue;
-                else
-                    break;
+                equipSlotsToUnequipFirst.push_back(it2->first);
             }
             else
             {
-                fail |= 2;
+                // Some non-unequippable equipment is to be unequipped first.
+                // Can be the case of cursed items,
+                // or when the inventory is full, for instance.
+                return false;
+            }
+        }
+    }
+
+    // Potential Pre-unequipment process
+    for (std::list<unsigned int>::const_iterator it3 =
+            equipSlotsToUnequipFirst.begin();
+            it3 != equipSlotsToUnequipFirst.end(); ++it3)
+    {
+        if (!unequip(*it3))
+        {
+            // Something went wrong even when we tested the unequipment process.
+            LOG_WARN("Unable to unequip even when unequip was tested. "
+                     "Character : " << mCharacter->getName()
+                     << ", unequip slot: " << *it3);
+            return false;
+        }
+    }
+
+    // Actually equip the item now that the requirements has met.
+    //W equip slot type count, W item id, { W equip slot, W capacity used}*
+    MessageOut equipMsg(GPMSG_EQUIP);
+    equipMsg.writeInt16(it->second.itemId); // Item Id
+    equipMsg.writeInt16(equipInfoList.size()); // Number of equip slot changed.
+
+    // Compute an unique equip item Instance id (unicity is per character only.)
+    int itemInstance = getNewEquipItemInstance();
+
+    for (ItemEquipsInfo::const_iterator it2 = equipInfoList.begin(),
+         it2_end = equipInfoList.end(); it2 != it2_end; ++it2)
+    {
+        unsigned int capacityLeft = it2->second;
+        unsigned int capacityUsed = 0;
+        // Apply equipment changes
+        for (EquipData::iterator it4 = mPoss->equipSlots.begin(),
+             it4_end = mPoss->equipSlots.end(); it4 != it4_end; ++it4)
+        {
+            if (!capacityLeft)
                 break;
+
+            // We've found an existing equip slot
+            if (it4->first == it2->first)
+            {
+                // We've found an empty slot
+                if (it4->second.itemInstance == 0)
+                {
+                    it4->second.itemId = it->second.itemId;
+                    it4->second.itemInstance = itemInstance;
+                    --capacityLeft;
+                }
+                else // The slot is already in use.
+                {
+                    ++capacityUsed;
+                }
             }
         }
-        switch (fail)
+
+        // When there is still something to apply even when out of that loop,
+        // It means that the equip multimapis missing empty slots.
+        // Hence, we add them back
+        if(capacityLeft)
         {
-            case 0:
-            /*
-             * Clean fit. Equip and apply immediately.
-             */ /*
-            equipMsg.writeInt16(slot);           // Inventory slot
-            equipMsg.writeInt16(it2->size());     // Equip slot type count
-            for (it3 = it2->begin(),
-                 it3_end = it2->end();
-                 it3 != it3_end;
-                 ++it3)
+            unsigned int maxCapacity =
+                itemManager->getEquipSlotCapacity(it2->first);
+
+            // A should never happen case
+            assert(maxCapacity >= capacityUsed + capacityLeft);
+
+            while (capacityLeft)
             {
-                equipMsg.writeInt16(it3->first);  // Equip slot
-                equipMsg.writeInt16(it3->second); // How many are used
-                /*
-                 * This bit can be somewhat inefficient, but is far better for
-                 *  average case assuming most equip use one slot max for each
-                 *  type and infrequently (<1/3) two of each type max.
-                 * If the reader cares, you're more than welcome to add
-                 *  compile time options optimising for other usage.
-                 * For now, this is adequate assuming `normal' usage.
-                 */
-                /** Part disabled until reimplemented*/
-                /*for (unsigned int i = 0; i < it3->second; ++i)
-                    mPoss->equipSlots.insert(
-                            std::make_pair(it3->first, slot));*/
-         /*   }
-
-            updateEquipmentTrigger(0, it->second.itemId);
-            return true;
-            case 1:
-            /*
-             * Definitions earlier in the item file have precedence (even if it
-             *      means requiring unequipping more), so no need to store more
-             *      than the first.
-             */ /*
-            if (override && !ovd)
-                ovd = &*it2; // Iterator -> object -> pointer.
-            break;
-            case 2:
-            default:
-            /*
-             * Since slots are currently static (and I don't see any reason to
-             *      change this right now), something probably went wrong.
-             * The logic to catch this is here rather than in the item manager
-             *      just in case non-static equip slots do want to be
-             *      implemented later. This would not be a trivial task,
-             *      however.
-             */ /*
-            LOG_WARN("Inventory - item '" << it->second.itemId <<
-                     "' cannot be equipped, even by unequipping other items!");
-            break;
+                EquipmentItem equipItem(it->second.itemId, itemInstance);
+                mPoss->equipSlots.insert(
+                    std::make_pair<unsigned int, EquipmentItem>
+                        (it2->first, equipItem));
+                --capacityLeft;
+            }
         }
+
+        // Equip slot
+        equipMsg.writeInt16(it2->first);
+        // Capacity used
+        equipMsg.writeInt16(it2->second);
     }
 
-    if (equipMsg.getLength() > 2)
-        gameHandler->sendTo(mCharacter, equipMsg);
-    // We didn't find a clean equip.
-    if (ovd)
-    {
-        /*
-         * We did find an equip that works if we unequip other items,
-         * and we can override.
-         * Process unequip triggers for all items we have to unequip.
-         * Process equip triggers for new item.
-         * Attempt to reequip any equipment we had to remove,
-         * but disallowing override.
-         */
+    // New item trigger
+    updateEquipmentTrigger(0, it->second.itemId);
 
-        // TODO - this would increase ease of use substatially, add as soon as
-        // there is time to do so.
+    // Remove item from inventory
+    removeFromSlot(inventorySlot, 1);
 
-        return false; // Return true when this section is complete
-/*    }
-    /*
-     * We cannot equip, either because we could not find any valid equip process
-     *     or because we found a dirty equip and weren't allowed to override.
-     */
-    return false;
-}
+    if (equipMsg.getLength() > 2)
+        gameHandler->sendTo(mCharacter, equipMsg);
 
-bool Inventory::unequip(EquipData::iterator it)
-{
-    return unequip(it->first, &it);
+    return true;
 }
 
-bool Inventory::unequip(unsigned int slot, EquipData::iterator *itp)
+bool Inventory::unequip(unsigned int equipmentSlot)
 {
-    EquipData::iterator it = itp ? *itp : mPoss->equipSlots.begin(),
-                        it_end = mPoss->equipSlots.end();
+    // map of { itemInstance, itemId }
+    std::map<unsigned int, unsigned int> itemIdListToInventory;
     bool changed = false;
 
     MessageOut equipMsg(GPMSG_EQUIP);
-    // Erase all equip entries that point to the given inventory slot
-    while (it != it_end)
+    equipMsg.writeInt16(0); // Item Id, useless in case of unequip.
+    equipMsg.writeInt16(1); // Number of slot types touched,
+                            // 1 in case of unequip.
+
+    // Empties all equip entries that point to the given equipment slot
+    // The equipment slots should NEVER be erased after initialization!
+    for (EquipData::iterator it = mPoss->equipSlots.begin(),
+            it_end = mPoss->equipSlots.end(); it != it_end; ++it)
     {
-        if (it->first == slot)
+        if (it->first == equipmentSlot && it->second.itemId != 0)
         {
+            if (!it->second.itemInstance)
+                continue;
+
+            // Add the item to the inventory list if not already present there
+            std::map<unsigned int, unsigned int>::const_iterator it2 =
+                itemIdListToInventory.find(it->second.itemInstance);
+            if (it2 == itemIdListToInventory.end())
+            {
+                itemIdListToInventory.insert(
+                    std::make_pair<unsigned int, unsigned int>
+                        (it->second.itemInstance, it->second.itemId));
+            }
+
             changed = true;
-            mPoss->equipSlots.erase(it++);
-        }
-        else
-        {
-            ++it;
+            it->second.itemId = 0;
+            it->second.itemInstance = 0;
         }
     }
 
+    // Apply unequip trigger(s), and move the item(s) back to inventory.
+    for (std::map<unsigned int, unsigned int>::const_iterator it2 =
+         itemIdListToInventory.begin(), it2_end = itemIdListToInventory.end();
+         it2 != it2_end; ++it2)
+    {
+        updateEquipmentTrigger(it2->second, 0);
+        insert(it2->second, 1);
+    }
+
     if (changed)
     {
-        updateEquipmentTrigger(mPoss->inventory.at(slot).itemId, 0);
-        equipMsg.writeInt16(slot);
-        equipMsg.writeInt16(0);
+        equipMsg.writeInt16(equipmentSlot);
+        equipMsg.writeInt16(0); // Capacity used, set to 0 to unequip.
     }
 
     if (equipMsg.getLength() > 2)
diff --git a/src/game-server/inventory.h b/src/game-server/inventory.h
index 24d4694..06f55e0 100644
--- a/src/game-server/inventory.h
+++ b/src/game-server/inventory.h
@@ -58,25 +58,17 @@ class Inventory
 
         /**
          * Equips item from given inventory slot.
-         * @param slot The slot in which the target item is in.
-         * @param override Whether this item can unequip other items to equip
-         *            itself. If true, items that are unequipped will be
-         *            attempted to be reequipped, but with override disabled.
+         * @param inventorySlot The slot in which the target item is in.
          * @returns whether the item could be equipped.
          */
-        bool equip(int slot, bool override = true);
+        bool equip(int inventorySlot);
 
         /**
          * Unequips item from given equipment slot.
-         * @param it Starting iterator. When the only parameter, also extracts
-         *           slot number from it.
-         *           Used so that when we already have an iterator to the first
-         *           occurence from a previous operation we can start from
-         *           there.
+         * @param equipmentSlot Where to remove item(s).
          * @returns Whether it was unequipped.
          */
-        bool unequip(EquipData::iterator it);
-        bool unequip(unsigned int slot, EquipData::iterator *itp = 0);
+        bool unequip(unsigned int equipmentSlot);
 
         /**
          * Inserts some items into the inventory.
@@ -115,6 +107,45 @@ class Inventory
 
     private:
         /**
+         * Tell whether the equipment slot has enough room in an equipment slot.
+         * @param equipmentSlot the slot in equipement to check.
+         * @param capacityRequested the capacity needed.
+         */
+        bool checkEquipmentCapacity(unsigned int equipmentSlot,
+                                    unsigned int capacityRequested);
+
+        /**
+         * Test whether the inventory has enough space to welcome
+         * the willing-to-be equipment slot.
+         * @todo
+         */
+        bool hasInventoryEnoughSpace(unsigned int equipmentSlot)
+        { return false; }
+
+        /**
+         * Test the items unequipment requirements.
+         * This is especially useful for scripted equipment.
+         * @todo
+         */
+        bool testUnequipScriptRequirements(unsigned int equipementSlot)
+        { return true; }
+
+        /**
+         * Test the items equipment for scripted requirements.
+         * @todo
+         */
+        bool testEquipScriptRequirements(unsigned int itemId)
+        { return true; }
+
+        /**
+         * Return an equip item instance id unique to the item used,
+         * per character.
+         * This is used to differenciate some items that can be equipped
+         * multiple times, like one-handed weapons for instance.
+         */
+        unsigned int getNewEquipItemInstance();
+
+        /**
          * Check the inventory is within the slot limit and capacity.
          * Forcibly delete items from the end if it is not.
          * @todo Drop items instead?
@@ -122,15 +153,7 @@ class Inventory
         void checkInventorySize();
 
         /**
-         * Helper function for equip() when computing changes to equipment
-         * When newCount is 0, the item is being unequipped.
-         */
-        // inventory slot -> {equip slots}
-        typedef std::multimap<unsigned int, unsigned short> IdSlotMap;
-        void equip_sub(unsigned int newCount, IdSlotMap::const_iterator &it);
-
-        /**
-         * Changes equipment and adjusts character attributes.
+         * Apply equipment triggers.
          */
         void updateEquipmentTrigger(unsigned int oldId, unsigned int itemId);
         void updateEquipmentTrigger(ItemClass *oldI, ItemClass *newI);