Fix crash in picking up an item that no longer exists

Adds ActorSpriteListener to manage ActorSprite destruction as
recommended by Jaxad0127. This likewise includes the changes made by
Bertram.

Reviewed-by: Jaxad0127, Bertram.

Resolves: Manasource Mantis #160

9 files changed, 108 insertions, 8 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 2df9ef49..a3312b62 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -406,6 +406,7 @@ SET(SRCS
     actor.h
     actorsprite.cpp
     actorsprite.h
+    actorspritelistener.h
     actorspritemanager.cpp
     actorspritemanager.h
     animatedsprite.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 2b18b693..28672571 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -305,6 +305,7 @@ mana_SOURCES = gui/widgets/avatarlistbox.cpp \
 	      actor.h \
 	      actorsprite.cpp \
 	      actorsprite.h \
+	      actorspritelistener.h \
 	      actorspritemanager.cpp \
 	      actorspritemanager.h \
 	      animatedsprite.cpp \
diff --git a/src/actorsprite.cpp b/src/actorsprite.cpp
index 13b7abf2..6a1dcbc7 100644
--- a/src/actorsprite.cpp
+++ b/src/actorsprite.cpp
@@ -19,6 +19,7 @@
  */
 
 #include "actorsprite.h"
+#include "actorspritelistener.h"
 
 #include "client.h"
 #include "effectmanager.h"
@@ -64,6 +65,11 @@ ActorSprite::~ActorSprite()
 
     if (player_node && player_node->getTarget() == this)
         player_node->setTarget(NULL);
+
+    // Notify listeners of the destruction.
+    for (ActorSpriteListenerIterator iter = mActorSpriteListeners.begin(),
+            end = mActorSpriteListeners.end(); iter != end; ++iter)
+        (*iter)->actorSpriteDestroyed(*this);
 }
 
 bool ActorSprite::draw(Graphics *graphics, int offsetX, int offsetY) const
@@ -358,6 +364,16 @@ void ActorSprite::unload()
     loaded = false;
 }
 
+void ActorSprite::addActorSpriteListener(ActorSpriteListener *listener)
+{
+    mActorSpriteListeners.push_front(listener);
+}
+
+void ActorSprite::removeActorSpriteListener(ActorSpriteListener *listener)
+{
+    mActorSpriteListeners.remove(listener);
+}
+
 static const char *cursorType(int type)
 {
     switch (type)
diff --git a/src/actorsprite.h b/src/actorsprite.h
index e218ef74..7cc91c53 100644
--- a/src/actorsprite.h
+++ b/src/actorsprite.h
@@ -29,9 +29,11 @@
 #include <SDL_types.h>
 
 #include <set>
+#include <list>
 
 class SimpleAnimation;
 class StatusEffect;
+class ActorSpriteListener;
 
 class ActorSprite : public CompoundSprite, public Actor
 {
@@ -160,6 +162,16 @@ public:
 
     static void unload();
 
+    /**
+     * Add an ActorSprite listener.
+     */
+    void addActorSpriteListener(ActorSpriteListener *listener);
+
+    /**
+     * Remove an ActorSprite listener.
+     */
+    void removeActorSpriteListener(ActorSpriteListener *listener);
+
 protected:
     /**
      * Trigger visual effect, with components
@@ -227,6 +239,10 @@ private:
 
     /** Target cursor being used */
     SimpleAnimation *mUsedTargetCursor;
+
+    typedef std::list<ActorSpriteListener*> ActorSpriteListeners;
+    typedef ActorSpriteListeners::iterator ActorSpriteListenerIterator;
+    ActorSpriteListeners mActorSpriteListeners;
 };
 
 #endif // ACTORSPRITE_H
diff --git a/src/actorspritelistener.h b/src/actorspritelistener.h
new file mode 100644
index 00000000..994494f4
--- /dev/null
+++ b/src/actorspritelistener.h
@@ -0,0 +1,42 @@
+/*
+ *  The Mana Client
+ *  Copyright (C) 2010  The Mana Developers
+ *
+ *  This file is part of The Mana Client.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  any later version.
+ *
+ *  This program 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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef ACTORSPRITELISTENER_H
+#define ACTORSPRITELISTENER_H
+
+class ActorSprite;
+
+class ActorSpriteListener
+{
+    public:
+        /**
+         * Destructor.
+         */
+        virtual ~ActorSpriteListener() {}
+
+        /**
+         * Called when the ActorSprite has been destroyed. The listener will
+         * have to be registered first.
+         * @param actorSprite the ActorSprite being destroyed.
+         */
+        virtual void actorSpriteDestroyed(const ActorSprite &actorSprite) = 0;
+};
+
+#endif // ACTORSPRITELISTENER_H
diff --git a/src/localplayer.cpp b/src/localplayer.cpp
index 96062867..15047f5a 100644
--- a/src/localplayer.cpp
+++ b/src/localplayer.cpp
@@ -686,6 +686,9 @@ void LocalPlayer::setInvItem(int index, int id, int amount)
 
 void LocalPlayer::pickUp(FloorItem *item)
 {
+    if (!item)
+        return;
+
     int dx = item->getTileX() - (int) getPosition().x / 32;
     int dy = item->getTileY() - (int) getPosition().y / 32;
 
@@ -700,16 +703,24 @@ void LocalPlayer::pickUp(FloorItem *item)
         {
             setDestination(item->getPixelX() + 16, item->getPixelY() + 16);
             mPickUpTarget = item;
+            mPickUpTarget->addActorSpriteListener(this);
         }
         else
         {
             setDestination(item->getTileX(), item->getTileY());
             mPickUpTarget = item;
+            mPickUpTarget->addActorSpriteListener(this);
             stopAttack();
         }
     }
 }
 
+void LocalPlayer::actorSpriteDestroyed(const ActorSprite &actorSprite)
+{
+    if (mPickUpTarget == &actorSprite)
+        mPickUpTarget = 0;
+}
+
 Being *LocalPlayer::getTarget() const
 {
     return mTarget;
diff --git a/src/localplayer.h b/src/localplayer.h
index a7ed33f8..403b4530 100644
--- a/src/localplayer.h
+++ b/src/localplayer.h
@@ -23,6 +23,7 @@
 #define LOCALPLAYER_H
 
 #include "being.h"
+#include "actorspritelistener.h"
 
 #include "gui/userpalette.h"
 
@@ -109,7 +110,7 @@ enum
 /**
  * The local player character.
  */
-class LocalPlayer : public Being
+class LocalPlayer : public Being, public ActorSpriteListener
 {
     public:
         /**
@@ -164,6 +165,12 @@ class LocalPlayer : public Being
         void pickUp(FloorItem *item);
 
         /**
+         * Called when an ActorSprite has been destroyed.
+         * @param actorSprite the ActorSprite being destroyed.
+         */
+        void actorSpriteDestroyed(const ActorSprite &actorSprite);
+
+        /**
          * Sets the attack range.
          */
         void setAttackRange(int range) { mAttackRange = range; }
diff --git a/src/net/manaserv/playerhandler.cpp b/src/net/manaserv/playerhandler.cpp
index 60fa5b29..c2803ce1 100644
--- a/src/net/manaserv/playerhandler.cpp
+++ b/src/net/manaserv/playerhandler.cpp
@@ -329,11 +329,14 @@ void PlayerHandler::increaseSkill(int skillId)
 
 void PlayerHandler::pickUp(FloorItem *floorItem)
 {
-    int id = floorItem->getId();
-    MessageOut msg(PGMSG_PICKUP);
-    msg.writeInt16(id >> 16);
-    msg.writeInt16(id & 0xFFFF);
-    gameServerConnection->send(msg);
+    if (floorItem)
+    {
+        int id = floorItem->getId();
+        MessageOut msg(PGMSG_PICKUP);
+        msg.writeInt16(id >> 16);
+        msg.writeInt16(id & 0xFFFF);
+        gameServerConnection->send(msg);
+    }
 }
 
 void PlayerHandler::setDirection(char direction)
diff --git a/src/net/tmwa/playerhandler.cpp b/src/net/tmwa/playerhandler.cpp
index 26c7e922..735da082 100644
--- a/src/net/tmwa/playerhandler.cpp
+++ b/src/net/tmwa/playerhandler.cpp
@@ -582,8 +582,11 @@ void PlayerHandler::increaseSkill(int skillId)
 
 void PlayerHandler::pickUp(FloorItem *floorItem)
 {
-    MessageOut outMsg(CMSG_ITEM_PICKUP);
-    outMsg.writeInt32(floorItem->getId());
+    if (floorItem)
+    {
+        MessageOut outMsg(CMSG_ITEM_PICKUP);
+        outMsg.writeInt32(floorItem->getId());
+    }
 }
 
 void PlayerHandler::setDirection(char direction)