From 0cfd0e194a9e22476b02d45cb957d53e89beca5d Mon Sep 17 00:00:00 2001
From: ephphatha <ephphatha@thelettereph.com>
Date: Mon, 18 Apr 2022 21:21:10 +1000
Subject: [PATCH] Refactor TryDropItem to reduce use of globals

The way TryInvPut and DropItemBeforeTrig interacted was fragile, don't think this would've worked as expected at the best of times. Looks like it was added as part of controller support so guessing this isn't vanilla behaviour anyway.
---
 Source/controls/plrctrls.cpp | 25 ++++++++++++++-----------
 Source/diablo.cpp            |  3 ++-
 Source/inv.cpp               | 24 +++++-------------------
 Source/inv.h                 | 18 ++++++++++++++++--
 4 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/Source/controls/plrctrls.cpp b/Source/controls/plrctrls.cpp
index 55edf0035..a6b883796 100644
--- a/Source/controls/plrctrls.cpp
+++ b/Source/controls/plrctrls.cpp
@@ -1809,7 +1809,7 @@ void UpdateSpellTarget(spell_id spell)
  */
 bool TryDropItem()
 {
-	auto &myPlayer = Players[MyPlayerId];
+	auto &myPlayer = *MyPlayer;
 
 	if (myPlayer.HoldItem.isEmpty()) {
 		return false;
@@ -1828,18 +1828,21 @@ bool TryDropItem()
 		}
 	}
 
-	cursPosition = myPlayer.position.future + Direction::SouthEast;
-	if (!DropItemBeforeTrig()) {
-		// Try to drop on the other side
-		cursPosition = myPlayer.position.future + Direction::SouthWest;
-		DropItemBeforeTrig();
-	}
-
-	if (pcurs != CURSOR_HAND) {
-		myPlayer.Say(HeroSpeech::WhereWouldIPutThis);
+	Point position = myPlayer.position.future;
+	Direction direction = myPlayer._pdir;
+	if (!CanPut(position, direction)) {
+		direction = Opposite(direction);
+		// if we can't drop in front of the player, can we drop it behind?
+		if (!CanPut(position, direction)) {
+			myPlayer.Say(HeroSpeech::WhereWouldIPutThis);
+			return false;
+		}
 	}
 
-	return pcurs == CURSOR_HAND;
+	NetSendCmdPItem(true, CMD_PUTITEM, position + direction, myPlayer.HoldItem);
+	myPlayer.HoldItem.Clear();
+	NewCursor(CURSOR_HAND);
+	return true;
 }
 
 void PerformSpellAction()
diff --git a/Source/diablo.cpp b/Source/diablo.cpp
index ed44cd3d1..d77672f06 100644
--- a/Source/diablo.cpp
+++ b/Source/diablo.cpp
@@ -343,7 +343,8 @@ void LeftMouseDown(int wParam)
 			} else if (sbookflag && GetRightPanel().Contains(MousePosition)) {
 				CheckSBook();
 			} else if (!MyPlayer->HoldItem.isEmpty()) {
-				if (TryInvPut()) {
+				Point currentPosition = MyPlayer->position.tile;
+				if (CanPut(currentPosition, GetDirection(currentPosition, cursPosition))) {
 					NetSendCmdPItem(true, CMD_PUTITEM, cursPosition, MyPlayer->HoldItem);
 					NewCursor(CURSOR_HAND);
 				}
diff --git a/Source/inv.cpp b/Source/inv.cpp
index deb379581..71ff7f5ac 100644
--- a/Source/inv.cpp
+++ b/Source/inv.cpp
@@ -1736,27 +1736,24 @@ bool CanPut(Point position)
 	return true;
 }
 
-bool TryInvPut()
+bool CanPut(Point position, Direction dir)
 {
 	if (ActiveItemCount >= MAXITEMS)
 		return false;
 
-	auto &myPlayer = Players[MyPlayerId];
-
-	Direction dir = GetDirection(myPlayer.position.tile, cursPosition);
-	if (CanPut(myPlayer.position.tile + dir)) {
+	if (CanPut(position + dir)) {
 		return true;
 	}
 
-	if (CanPut(myPlayer.position.tile + Left(dir))) {
+	if (CanPut(position + Left(dir))) {
 		return true;
 	}
 
-	if (CanPut(myPlayer.position.tile + Right(dir))) {
+	if (CanPut(position + Right(dir))) {
 		return true;
 	}
 
-	return CanPut(myPlayer.position.tile);
+	return CanPut(position);
 }
 
 int InvPutItem(Player &player, Point position, Item &item)
@@ -2163,17 +2160,6 @@ int CalculateGold(Player &player)
 	return gold;
 }
 
-bool DropItemBeforeTrig()
-{
-	if (!TryInvPut()) {
-		return false;
-	}
-
-	NetSendCmdPItem(true, CMD_PUTITEM, cursPosition, Players[MyPlayerId].HoldItem);
-	NewCursor(CURSOR_HAND);
-	return true;
-}
-
 Size GetInventorySize(const Item &item)
 {
 	int itemSizeIndex = item._iCurs + CURSOR_FIRSTITEM;
diff --git a/Source/inv.h b/Source/inv.h
index fe8a615aa..b5da7d856 100644
--- a/Source/inv.h
+++ b/Source/inv.h
@@ -198,8 +198,23 @@ void AutoGetItem(int pnum, Item *item, int ii);
  */
 int FindGetItem(int32_t iseed, _item_indexes idx, uint16_t ci);
 void SyncGetItem(Point position, int32_t iseed, _item_indexes idx, uint16_t ci);
+
+/**
+ * @brief Checks if the tile has room for an item
+ * @param position tile coordinates
+ * @return True if the space is free of obstructions, false if blocked
+ */
 bool CanPut(Point position);
-bool TryInvPut();
+
+/**
+ * @brief Checks for free spaces in the three "adjacent" tiles in the given direction and also the given position.
+ * @see CanPut
+ * @param position base tile coordinates
+ * @param facing direction to check, tiles "left" and "right" of this direction will also be checked.
+ * @return True if any of the four tiles checked can accept an item, false if all blocked
+ */
+bool CanPut(Point position, Direction facing);
+
 int InvPutItem(Player &player, Point position, Item &item);
 int SyncPutItem(Player &player, Point position, int idx, uint16_t icreateinfo, int iseed, int Id, int dur, int mdur, int ch, int mch, int ivalue, uint32_t ibuff, int toHit, int maxDam, int minStr, int minMag, int minDex, int ac);
 int SyncDropItem(Point position, int idx, uint16_t icreateinfo, int iseed, int id, int dur, int mdur, int ch, int mch, int ivalue, uint32_t ibuff, int toHit, int maxDam, int minStr, int minMag, int minDex, int ac);
@@ -212,7 +227,6 @@ Item &GetInventoryItem(Player &player, int location);
 bool UseInvItem(int pnum, int cii);
 void DoTelekinesis();
 int CalculateGold(Player &player);
-bool DropItemBeforeTrig();
 
 /**
  * @brief Gets the size, in inventory cells, of the given item.