Some Inventory and TradeWindow cleanups

Mainly using std::vector<std::unique_ptr<Item>> to automate memory
management.

5 files changed, 56 insertions, 81 deletions

diff --git a/src/gui/itemamountwindow.cpp b/src/gui/itemamountwindow.cpp
index 49cdafba..de204da3 100644
--- a/src/gui/itemamountwindow.cpp
+++ b/src/gui/itemamountwindow.cpp
@@ -35,6 +35,9 @@
 #include "gui/widgets/slider.h"
 #include "gui/widgets/icon.h"
 
+#include "net/net.h"
+#include "net/tradehandler.h"
+
 #include "utils/gettext.h"
 
 void ItemAmountWindow::finish(Item *item, int amount, Usage usage)
@@ -42,7 +45,7 @@ void ItemAmountWindow::finish(Item *item, int amount, Usage usage)
     switch (usage)
     {
         case TradeAdd:
-            tradeWindow->tradeItem(item, amount);
+            Net::getTradeHandler()->addItem(item, amount);
             break;
         case ItemDrop:
             item->doEvent(Event::DoDrop, amount);
diff --git a/src/gui/tradewindow.cpp b/src/gui/tradewindow.cpp
index db4b6490..e24845b7 100644
--- a/src/gui/tradewindow.cpp
+++ b/src/gui/tradewindow.cpp
@@ -24,7 +24,6 @@
 #include "event.h"
 #include "inventory.h"
 #include "item.h"
-#include "localplayer.h"
 #include "playerinfo.h"
 #include "units.h"
 
@@ -39,7 +38,6 @@
 #include "gui/widgets/textfield.h"
 #include "gui/widgets/layout.h"
 
-#include "net/inventoryhandler.h"
 #include "net/net.h"
 #include "net/tradehandler.h"
 
@@ -48,8 +46,6 @@
 
 #include <guichan/font.hpp>
 
-#include <sstream>
-
 #define CAPTION_PROPOSE _("Propose trade")
 #define CAPTION_CONFIRMED _("Confirmed. Waiting...")
 #define CAPTION_ACCEPT _("Agree trade")
@@ -125,9 +121,7 @@ TradeWindow::TradeWindow():
     reset();
 }
 
-TradeWindow::~TradeWindow()
-{
-}
+TradeWindow::~TradeWindow() = default;
 
 void TradeWindow::setMoney(int amount)
 {
@@ -138,7 +132,10 @@ void TradeWindow::setMoney(int amount)
 
 void TradeWindow::addItem(int id, bool own, int quantity)
 {
-    (own ? mMyInventory : mPartnerInventory)->addItem(id, quantity);
+    if (own)
+        mMyInventory->addItem(id, quantity);
+    else
+        mPartnerInventory->addItem(id, quantity);
 }
 
 void TradeWindow::changeQuantity(int index, bool own, int quantity)
@@ -186,22 +183,15 @@ void TradeWindow::receivedOk(bool own)
     }
 }
 
-void TradeWindow::tradeItem(Item *item, int quantity)
-{
-    Net::getTradeHandler()->addItem(item, quantity);
-}
-
 void TradeWindow::valueChanged(const gcn::SelectionEvent &event)
 {
-    const Item *item;
-
     /* If an item is selected in one container, make sure no item is selected
      * in the other container.
      */
     if (event.getSource() == mMyItemContainer &&
-            (item = mMyItemContainer->getSelectedItem()))
+            mMyItemContainer->getSelectedItem())
         mPartnerItemContainer->selectNone();
-    else if ((item = mPartnerItemContainer->getSelectedItem()))
+    else if (mPartnerItemContainer->getSelectedItem())
         mMyItemContainer->selectNone();
 }
 
diff --git a/src/gui/tradewindow.h b/src/gui/tradewindow.h
index d54ec35e..85b2f97d 100644
--- a/src/gui/tradewindow.h
+++ b/src/gui/tradewindow.h
@@ -81,11 +81,6 @@ class TradeWindow : public Window, gcn::ActionListener, gcn::SelectionListener
         void receivedOk(bool own);
 
         /**
-         * Send trade packet.
-         */
-        void tradeItem(Item *item, int quantity);
-
-        /**
          * Updates the labels and makes sure only one item is selected in
          * either my inventory or partner inventory.
          */
diff --git a/src/inventory.cpp b/src/inventory.cpp
index 6aa30c00..51eb24e1 100644
--- a/src/inventory.cpp
+++ b/src/inventory.cpp
@@ -27,42 +27,33 @@
 #include "net/net.h"
 
 #include <algorithm>
-#include <functional>
 
 static bool slotUsed(const Item *item)
 {
     return item && item->getId() >= 0 && item->getQuantity() > 0;
 }
 
-Inventory::Inventory(Type type, int size):
-    mType(type),
-    mSize(size == -1 ? Net::getInventoryHandler()->getSize(type) : size)
+Inventory::Inventory(Type type, int size)
+    : mType(type)
 {
-    mItems = new Item*[mSize];
-    std::fill_n(mItems, mSize, (Item*) nullptr);
+    mItems.resize(size == -1 ? Net::getInventoryHandler()->getSize(type) : size);
 }
 
-Inventory::~Inventory()
-{
-    for (int i = 0; i < mSize; i++)
-        delete mItems[i];
-
-    delete [] mItems;
-}
+Inventory::~Inventory() = default;
 
 Item *Inventory::getItem(int index) const
 {
-    if (index < 0 || index >= mSize || !mItems[index] || mItems[index]->getQuantity() <= 0)
+    if (index < 0 || index >= getSize() || !mItems[index] || mItems[index]->getQuantity() <= 0)
         return nullptr;
 
-    return mItems[index];
+    return mItems[index].get();
 }
 
 Item *Inventory::findItem(int itemId) const
 {
-    for (int i = 0; i < mSize; i++)
-        if (mItems[i] && mItems[i]->getId() == itemId)
-            return mItems[i];
+    for (auto &item : mItems)
+        if (item && item->getId() == itemId)
+            return item.get();
 
     return nullptr;
 }
@@ -74,24 +65,27 @@ void Inventory::addItem(int id, int quantity)
 
 void Inventory::setItem(int index, int id, int quantity)
 {
-    if (index < 0 || index >= mSize)
+    if (index < 0 || index >= getSize())
     {
         logger->log("Warning: invalid inventory index: %d", index);
         return;
     }
 
-    if (!mItems[index] && id > 0)
+    if (id > 0)
     {
-        Item *item = new Item(id, quantity);
-        item->setInvIndex(index);
-        mItems[index] = item;
-        mUsed++;
-        distributeSlotsChangedEvent();
-    }
-    else if (id > 0)
-    {
-        mItems[index]->setId(id);
-        mItems[index]->setQuantity(quantity);
+        if (!mItems[index])
+        {
+            auto item = std::make_unique<Item>(id, quantity);
+            item->setInvIndex(index);
+            mItems[index] = std::move(item);
+            mUsed++;
+            distributeSlotsChangedEvent();
+        }
+        else
+        {
+            mItems[index]->setId(id);
+            mItems[index]->setQuantity(quantity);
+        }
     }
     else if (mItems[index])
     {
@@ -101,20 +95,14 @@ void Inventory::setItem(int index, int id, int quantity)
 
 void Inventory::clear()
 {
-    for (int i = 0; i < mSize; i++)
-        removeItemAt(i);
-}
-
-void Inventory::removeItem(int id)
-{
-    for (int i = 0; i < mSize; i++)
-        if (mItems[i] && mItems[i]->getId() == id)
-            removeItemAt(i);
+    for (auto &item : mItems)
+        item = nullptr;
+    mUsed = 0;
+    distributeSlotsChangedEvent();
 }
 
 void Inventory::removeItemAt(int index)
 {
-    delete mItems[index];
     mItems[index] = nullptr;
     if (mUsed > 0) {
         mUsed--;
@@ -124,23 +112,26 @@ void Inventory::removeItemAt(int index)
 
 bool Inventory::contains(Item *item) const
 {
-    for (int i = 0; i < mSize; i++)
-        if (mItems[i] && mItems[i]->getId() == item->getId())
-            return true;
-
-    return false;
+    return std::any_of(mItems.begin(),
+                       mItems.end(),
+                       [id = item->getId()](auto &i) {
+        return i->getId() == id;
+    });
 }
 
 int Inventory::getFreeSlot() const
 {
-    Item **i = std::find_if(mItems, mItems + mSize, std::not_fn(slotUsed));
-    return (i == mItems + mSize) ? -1 : (i - mItems);
+    for (int i = 0; i < getSize(); i++)
+        if (!slotUsed(mItems[i].get()))
+            return i;
+
+    return -1;
 }
 
 int Inventory::getLastUsedSlot() const
 {
-    for (int i = mSize - 1; i >= 0; i--)
-        if (slotUsed(mItems[i]))
+    for (int i = getSize() - 1; i >= 0; i--)
+        if (slotUsed(mItems[i].get()))
             return i;
 
     return -1;
diff --git a/src/inventory.h b/src/inventory.h
index 40a9403a..4a0ff6c9 100644
--- a/src/inventory.h
+++ b/src/inventory.h
@@ -22,6 +22,8 @@
 #pragma once
 
 #include <list>
+#include <memory>
+#include <vector>
 
 class Inventory;
 class Item;
@@ -62,7 +64,7 @@ class Inventory
         /**
          * Returns the size that this instance is configured for.
          */
-        int getSize() const { return mSize; }
+        int getSize() const { return mItems.size(); }
 
         /**
          * Returns the item at the specified index.
@@ -88,11 +90,6 @@ class Inventory
         void setItem(int index, int id, int quantity);
 
         /**
-         * Remove a item from the inventory.
-         */
-        void removeItem(int id);
-
-        /**
          * Remove the item at the specified index from the inventory.
          */
         void removeItemAt(int index);
@@ -138,7 +135,6 @@ class Inventory
         void distributeSlotsChangedEvent();
 
         Type mType;
-        Item **mItems;  /**< The holder of items */
-        int mSize;      /**< The max number of inventory items */
-        int mUsed = 0;  /**< THe number of slots in use */
+        std::vector<std::unique_ptr<Item>> mItems;  /**< The holder of items */
+        int mUsed = 0;  /**< The number of slots in use */
 };