Simplified and made generic the way the pickup is handled.

I also made the range be taken from the server type
as for the pickup and npc talk ranges.

Last but no least, I fixed the parameters sent with
PGMSG_PICKUP to send the (item) position where to pickup at
as described in the manaserv protocol.

The pickup is still not 100% functional due certainly
to two problems:
1. The client item coordinates might not be the exact same as in the server.
2. The client seems to try to pick up the item a bit too soon,
probably for the reason given in 1.

I'll investigate this in another patch.

Reviewed-by: Thorbjørn Lindeijer, Erik Schilling.

11 files changed, 68 insertions, 32 deletions

diff --git a/src/gui/viewport.cpp b/src/gui/viewport.cpp
index 855a05e4..fd6bc6a3 100644
--- a/src/gui/viewport.cpp
+++ b/src/gui/viewport.cpp
@@ -468,12 +468,17 @@ void Viewport::mousePressed(gcn::MouseEvent &event)
                 // Ignore it if its dead
                 if (mHoverBeing->isAlive())
                 {
-                    if (local_player->withinAttackRange(mHoverBeing) ||
-                        keyboard.isKeyActive(keyboard.KEY_ATTACK))
+                    if (local_player->withinRange(mHoverBeing,
+                                                 local_player->getAttackRange())
+                        || keyboard.isKeyActive(keyboard.KEY_ATTACK))
+                    {
                         local_player->attack(mHoverBeing,
                             !keyboard.isKeyActive(keyboard.KEY_TARGET));
+                    }
                     else
+                    {
                         local_player->setGotoTarget(mHoverBeing);
+                    }
                 }
              }
         // Picks up a item if we clicked on one
diff --git a/src/localplayer.cpp b/src/localplayer.cpp
index efdcaa60..7ee5753f 100644
--- a/src/localplayer.cpp
+++ b/src/localplayer.cpp
@@ -40,6 +40,7 @@
 #include "gui/widgets/chattab.h"
 
 #include "net/chathandler.h"
+#include "net/gamehandler.h"
 #include "net/guildhandler.h"
 #include "net/inventoryhandler.h"
 #include "net/net.h"
@@ -133,20 +134,15 @@ void LocalPlayer::logic()
     {
         if (mTarget->getType() == ActorSprite::NPC)
         {
-            // NPCs are always in range
-            mTarget->setTargetType(TCT_IN_RANGE);
+            mTarget->setTargetType(
+                withinRange(mTarget,
+                            Net::getGameHandler()->getNpcTalkRange()) ?
+                            TCT_IN_RANGE : TCT_NORMAL);
         }
         else
         {
-            // Find whether target is in range
-            const int rangeX = abs(mTarget->getPosition().x - getPosition().x);
-            const int rangeY = abs(mTarget->getPosition().y - getPosition().y);
-
-            const int attackRange = getAttackRange();
-            const TargetCursorType targetType = rangeX > attackRange ||
-                                                rangeY > attackRange ?
-                                                TCT_NORMAL : TCT_IN_RANGE;
-            mTarget->setTargetType(targetType);
+            mTarget->setTargetType(withinRange(mTarget, getAttackRange()) ?
+                                   TCT_IN_RANGE : TCT_NORMAL);
 
             if (!mTarget->isAlive())
                 stopAttack();
@@ -155,6 +151,12 @@ void LocalPlayer::logic()
                 attack(mTarget, true);
         }
     }
+    else if (mPickUpTarget
+        && withinRange(mPickUpTarget, Net::getGameHandler()->getPickupRange()))
+    {
+        Net::getPlayerHandler()->pickUp(mPickUpTarget);
+        mPickUpTarget = 0;
+    }
 
     Being::logic();
 }
@@ -589,17 +591,14 @@ void LocalPlayer::pickUp(FloorItem *item)
     if (!item)
         return;
 
-    int dx = item->getTileX() - getTileX();
-    int dy = item->getTileY() - getTileY();
-
-    if (dx * dx + dy * dy < 4)
+    if (withinRange(item, Net::getGameHandler()->getPickupRange()))
     {
         Net::getPlayerHandler()->pickUp(item);
         // We found it, so set the player direction to it
         // if the player does not move
         if (getDestination() == getPosition())
             lookAt(item->getPosition());
-        mPickUpTarget = NULL;
+        mPickUpTarget = 0;
     }
     else
     {
@@ -694,7 +693,7 @@ void LocalPlayer::setDestination(int x, int y)
             Net::getPlayerHandler()->setDestination(x, y, mDirection);
     }
 
-    mPickUpTarget = NULL;
+    mPickUpTarget = 0;
     mKeepAttacking = false;
 }
 
@@ -937,14 +936,15 @@ void LocalPlayer::setAttackRange(int range)
     }
 }
 
-bool LocalPlayer::withinAttackRange(Being *target)
+bool LocalPlayer::withinRange(Actor *target, int range) const
 {
+    if (!target || range < 0)
+        return false;
+
     const Vector &targetPos = target->getPosition();
     const Vector &pos = getPosition();
     const int dx = abs(targetPos.x - pos.x);
     const int dy = abs(targetPos.y - pos.y);
-    const int range = getAttackRange();
-
     return !(dx > range || dy > range);
 }
 
diff --git a/src/localplayer.h b/src/localplayer.h
index 9e2139c2..8e5795c3 100644
--- a/src/localplayer.h
+++ b/src/localplayer.h
@@ -92,7 +92,7 @@ class LocalPlayer : public Being
         /**
          * Gets the attack range.
          */
-        int getAttackRange()
+        int getAttackRange() const
         { return mAttackRange; }
 
         void attack(Being *target = NULL, bool keep = false);
@@ -147,9 +147,9 @@ class LocalPlayer : public Being
         void setGotoTarget(Being *target);
 
         /**
-         * Returns whether the target is in range to attack
+         * Returns whether the target is in range (in pixels).
          */
-        bool withinAttackRange(Being *target);
+        bool withinRange(Actor *target, int range) const;
 
         void toggleSit();
         void emote(uint8_t emotion);
diff --git a/src/map.h b/src/map.h
index 1067b5b6..0bec3ef4 100644
--- a/src/map.h
+++ b/src/map.h
@@ -40,7 +40,7 @@ class Tileset;
 typedef std::vector<Tileset*> Tilesets;
 typedef std::vector<MapLayer*> Layers;
 
-#define DEFAULT_TILE_LENGTH 32
+const int DEFAULT_TILE_LENGTH = 32;
 
 /**
  * A meta tile stores additional information about a location on a tile map.
diff --git a/src/net/gamehandler.h b/src/net/gamehandler.h
index 599e89b8..40e4b099 100644
--- a/src/net/gamehandler.h
+++ b/src/net/gamehandler.h
@@ -47,6 +47,16 @@ class GameHandler
          * Tells whether the protocol is using the MP status bar
          */
         virtual bool canUseMagicBar() const = 0;
+
+        /**
+         * Tells the range is pixel where the player can pickup items from.
+         */
+        virtual int getPickupRange() const = 0;
+
+        /**
+         * Tells the range is pixel where the player can talk to an NPC from.
+         */
+        virtual int getNpcTalkRange() const = 0;
 };
 
 } // namespace Net
diff --git a/src/net/manaserv/gamehandler.cpp b/src/net/manaserv/gamehandler.cpp
index 9d0bf0d6..89b80e68 100644
--- a/src/net/manaserv/gamehandler.cpp
+++ b/src/net/manaserv/gamehandler.cpp
@@ -27,7 +27,6 @@
 #include "net/manaserv/chathandler.h"
 #include "net/manaserv/connection.h"
 #include "net/manaserv/messageout.h"
-#include "net/manaserv/manaserv_protocol.h"
 
 extern Net::GameHandler *gameHandler;
 
diff --git a/src/net/manaserv/gamehandler.h b/src/net/manaserv/gamehandler.h
index b8dda036..2d33710e 100644
--- a/src/net/manaserv/gamehandler.h
+++ b/src/net/manaserv/gamehandler.h
@@ -27,6 +27,8 @@
 
 #include "net/manaserv/messagehandler.h"
 
+#include "net/manaserv/manaserv_protocol.h"
+
 namespace ManaServ {
 
 class GameHandler : public MessageHandler, public Net::GameHandler
@@ -56,6 +58,10 @@ class GameHandler : public MessageHandler, public Net::GameHandler
 
         /** The ManaServ protocol doesn't use the MP status bar. */
         bool canUseMagicBar() const { return false; }
+
+        int getPickupRange() const { return PICKUP_RANGE; }
+
+        int getNpcTalkRange() const { return NPC_TALK_RANGE; }
 };
 
 } // namespace ManaServ
diff --git a/src/net/manaserv/manaserv_protocol.h b/src/net/manaserv/manaserv_protocol.h
index 2c09aaa4..aa1976f1 100644
--- a/src/net/manaserv/manaserv_protocol.h
+++ b/src/net/manaserv/manaserv_protocol.h
@@ -93,8 +93,8 @@ enum {
     // Game
     GPMSG_PLAYER_MAP_CHANGE        = 0x0100, // S filename, W x, W y
     GPMSG_PLAYER_SERVER_CHANGE     = 0x0101, // B*32 token, S game address, W game port
-    PGMSG_PICKUP                   = 0x0110, // W*2 position
-    PGMSG_DROP                     = 0x0111, // B slot, B amount
+    PGMSG_PICKUP                   = 0x0110, // W * 2 items position
+    PGMSG_DROP                     = 0x0111, // W slot, W amount
     PGMSG_EQUIP                    = 0x0112, // W inventory slot
     PGMSG_UNEQUIP                  = 0x0113, // W item Instance id
     PGMSG_MOVE_ITEM                = 0x0114, // W slot1, W slot2, W amount
@@ -441,6 +441,12 @@ enum BeingGender
     GENDER_UNSPECIFIED
 };
 
+/** The permited range to pick up an item */
+const int PICKUP_RANGE = 32 + 16;
+
+/** The permited range to to talk to a NPC. */
+const int NPC_TALK_RANGE = 32 * 7;
+
 } // namespace ManaServ
 
 #endif // MANASERV_PROTOCOL_H
diff --git a/src/net/manaserv/playerhandler.cpp b/src/net/manaserv/playerhandler.cpp
index 96a4b49a..adb60fb3 100644
--- a/src/net/manaserv/playerhandler.cpp
+++ b/src/net/manaserv/playerhandler.cpp
@@ -348,10 +348,9 @@ void PlayerHandler::pickUp(FloorItem *floorItem)
 {
     if (floorItem)
     {
-        int id = floorItem->getId();
         MessageOut msg(PGMSG_PICKUP);
-        msg.writeInt16(id >> 16);
-        msg.writeInt16(id & 0xFFFF);
+        msg.writeInt16(floorItem->getPixelX());
+        msg.writeInt16(floorItem->getPixelY());
         gameServerConnection->send(msg);
     }
 }
diff --git a/src/net/tmwa/gamehandler.cpp b/src/net/tmwa/gamehandler.cpp
index e4dc62b0..93f0b86f 100644
--- a/src/net/tmwa/gamehandler.cpp
+++ b/src/net/tmwa/gamehandler.cpp
@@ -177,4 +177,11 @@ void GameHandler::setMap(const std::string map)
     mMap = map.substr(0, map.rfind("."));
 }
 
+int GameHandler::getPickupRange() const
+{
+    if (Game *game = Game::instance())
+        return game->getCurrentTileWidth();
+    return DEFAULT_TILE_LENGTH;
+}
+
 } // namespace TmwAthena
diff --git a/src/net/tmwa/gamehandler.h b/src/net/tmwa/gamehandler.h
index 968a80c9..4f0525b9 100644
--- a/src/net/tmwa/gamehandler.h
+++ b/src/net/tmwa/gamehandler.h
@@ -62,6 +62,10 @@ class GameHandler : public MessageHandler, public Net::GameHandler,
         /** The tmwAthena protocol is making use of the MP status bar. */
         bool canUseMagicBar() const { return true; }
 
+        int getPickupRange() const;
+
+        int getNpcTalkRange() const { return DEFAULT_TILE_LENGTH * 30; }
+
     private:
         std::string mMap; ///< Keeps the map filename.
         int mCharID; /// < Saved for map-server switching