Extract logic from CheckInvCut/AutoPlaceItemInInventory (#7494)

* Remove unnecessary namespace

* Extract FindSlotUnderCursor

* Split logic for finding a space in the inventory to dedicated functions

Source/control.cpp

@@ -473,7 +473,7 @@ std::string TextCmdArenaPot(const std::string_view parameter)
 		GenerateNewSeed(item);
 		item.updateRequiredStatsCacheForPlayer(myPlayer);
 
-		if (!AutoPlaceItemInBelt(myPlayer, item, true, true) && !AutoPlaceItemInInventory(myPlayer, item, true, true)) {
+		if (!AutoPlaceItemInBelt(myPlayer, item, true, true) && !AutoPlaceItemInInventory(myPlayer, item, true)) {
 			break; // inventory is full
 		}
 	}

Source/engine/point.hpp

@@ -41,6 +41,13 @@ struct PointOf {
 	{
 	}
 
+	template <typename PointCoordT>
+	DVL_ALWAYS_INLINE explicit constexpr PointOf(DisplacementOf<PointCoordT> other)
+	    : x(other.deltaX)
+	    , y(other.deltaY)
+	{
+	}
+
 	template <typename PointCoordT>
 	DVL_ALWAYS_INLINE constexpr bool operator==(const PointOf<PointCoordT> &other) const
 	{

Source/inv.cpp

@@ -378,7 +378,7 @@ void ChangeTwoHandItem(Player &player)
 		if (player.InvBody[INVLOC_HAND_RIGHT]._itype == ItemType::Shield) {
 			locationToUnequip = INVLOC_HAND_RIGHT;
 		}
-		if (!AutoPlaceItemInInventory(player, player.InvBody[locationToUnequip], true)) {
+		if (!AutoPlaceItemInInventory(player, player.InvBody[locationToUnequip])) {
 			return;
 		}
 
@@ -603,13 +603,33 @@ void CheckInvPaste(Player &player, Point cursorPosition)
 	}
 }
 
-namespace {
 inv_body_loc MapSlotToInvBodyLoc(inv_xy_slot slot)
 {
 	assert(slot <= SLOTXY_CHEST);
 	return static_cast<inv_body_loc>(slot);
 }
-} // namespace
+
+std::optional<inv_xy_slot> FindSlotUnderCursor(Point cursorPosition)
+{
+
+	Point testPosition = static_cast<Point>(cursorPosition - GetRightPanel().position);
+	for (std::underlying_type_t<inv_xy_slot> r = SLOTXY_EQUIPPED_FIRST; r != SLOTXY_BELT_FIRST; r++) {
+		// check which body/inventory rectangle the mouse is in, if any
+		if (InvRect[r].contains(testPosition)) {
+			return static_cast<inv_xy_slot>(r);
+		}
+	}
+
+	testPosition = static_cast<Point>(cursorPosition - GetMainPanel().position);
+	for (std::underlying_type_t<inv_xy_slot> r = SLOTXY_BELT_FIRST; r != NUM_XY_SLOTS; r++) {
+		// check which belt rectangle the mouse is in, if any
+		if (InvRect[r].contains(testPosition)) {
+			return static_cast<inv_xy_slot>(r);
+		}
+	}
+
+	return {};
+}
 
 void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool dropItem)
 {
@@ -619,26 +639,15 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
 
 	CloseGoldDrop();
 
-	uint32_t r = 0;
-	for (; r < NUM_XY_SLOTS; r++) {
-		int xo = GetRightPanel().position.x;
-		int yo = GetRightPanel().position.y;
-		if (r >= SLOTXY_BELT_FIRST) {
-			xo = GetMainPanel().position.x;
-			yo = GetMainPanel().position.y;
-		}
-
-		// check which inventory rectangle the mouse is in, if any
-		if (InvRect[r].contains(cursorPosition - Displacement(xo, yo))) {
-			break;
-		}
-	}
+	std::optional<inv_xy_slot> maybeSlot = FindSlotUnderCursor(cursorPosition);
 
-	if (r == NUM_XY_SLOTS) {
+	if (!maybeSlot) {
 		// not on an inventory slot rectangle
 		return;
 	}
 
+	inv_xy_slot r = *maybeSlot;
+
 	Item &holdItem = player.HoldItem;
 	holdItem.clear();
 
@@ -647,12 +656,12 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
 	bool automaticallyUnequip = false;
 
 	if (r >= SLOTXY_HEAD && r <= SLOTXY_CHEST) {
-		inv_body_loc invloc = MapSlotToInvBodyLoc(static_cast<inv_xy_slot>(r));
+		inv_body_loc invloc = MapSlotToInvBodyLoc(r);
 		if (!player.InvBody[invloc].isEmpty()) {
 			holdItem = player.InvBody[invloc];
 			if (automaticMove) {
 				automaticallyUnequip = true;
-				automaticallyMoved = automaticallyEquipped = AutoPlaceItemInInventory(player, holdItem, true);
+				automaticallyMoved = automaticallyEquipped = AutoPlaceItemInInventory(player, holdItem);
 			}
 
 			if (!automaticMove || automaticallyMoved) {
@@ -716,11 +725,11 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
 						} else {
 							// Both hands are holding items, we must unequip the right hand item and check that there's
 							// space for the left before trying to auto-equip
-							if (!AutoPlaceItemInInventory(player, player.InvBody[INVLOC_HAND_RIGHT], true)) {
+							if (!AutoPlaceItemInInventory(player, player.InvBody[INVLOC_HAND_RIGHT])) {
 								// No space to move right hand item to inventory, abort.
 								break;
 							}
-							if (!AutoPlaceItemInInventory(player, player.InvBody[INVLOC_HAND_LEFT], false)) {
+							if (!CanFitItemInInventory(player, player.InvBody[INVLOC_HAND_LEFT])) {
 								// No space for left item. Move back right item to right hand and abort.
 								player.InvBody[INVLOC_HAND_RIGHT] = player.InvList[player._pNumInv - 1];
 								player.RemoveInvItem(player._pNumInv - 1, false);
@@ -737,7 +746,7 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
 					// Empty the identified InvBody slot (invloc) and hand over to AutoEquip
 					if (invloc != NUM_INVLOC) {
 						if (!player.InvBody[invloc].isEmpty()) {
-							if (AutoPlaceItemInInventory(player, player.InvBody[invloc], true)) {
+							if (AutoPlaceItemInInventory(player, player.InvBody[invloc])) {
 								player.InvBody[invloc].clear();
 							}
 						}
@@ -757,7 +766,7 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
 		if (!beltItem.isEmpty()) {
 			holdItem = beltItem;
 			if (automaticMove) {
-				automaticallyMoved = AutoPlaceItemInInventory(player, holdItem, true);
+				automaticallyMoved = AutoPlaceItemInInventory(player, holdItem);
 			}
 
 			if (!automaticMove || automaticallyMoved) {
@@ -1239,19 +1248,16 @@ bool AutoEquipEnabled(const Player &player, const Item &item)
 
 namespace {
 /**
- * @brief Checks whether the given item can be placed on the specified player's inventory slot.
- * If 'persistItem' is 'True', the item is also placed in the inventory slot.
+ * @brief Checks whether an item of the given size can be placed on the specified player's inventory slot.
  * @param player The player whose inventory will be checked.
  * @param slotIndex The 0-based index of the slot to put the item on.
- * @param item The item to be checked.
- * @param persistItem Pass 'True' to actually place the item in the inventory slot. The default is 'False'.
+ * @param itemSize The size of the item to be checked.
  * @return 'True' in case the item can be placed on the specified player's inventory slot and 'False' otherwise.
  */
-bool AutoPlaceItemInInventorySlot(Player &player, int slotIndex, const Item &item, bool persistItem, bool sendNetworkMessage)
+bool CheckItemFitsInInventorySlot(const Player &player, int slotIndex, const Size &itemSize)
 {
 	int yy = (slotIndex > 0) ? (10 * (slotIndex / 10)) : 0;
 
-	Size itemSize = GetInventorySize(item);
 	for (int j = 0; j < itemSize.height; j++) {
 		if (yy >= InventoryGridCells) {
 			return false;
@@ -1265,70 +1271,82 @@ bool AutoPlaceItemInInventorySlot(Player &player, int slotIndex, const Item &ite
 		}
 		yy += 10;
 	}
-
-	if (persistItem) {
-		player.InvList[player._pNumInv] = item;
-		player._pNumInv++;
-
-		AddItemToInvGrid(player, slotIndex, player._pNumInv, itemSize, sendNetworkMessage);
-		player.CalcScrolls();
-	}
-
 	return true;
 }
-} // namespace
 
-bool AutoPlaceItemInInventory(Player &player, const Item &item, bool persistItem, bool sendNetworkMessage)
+std::optional<int> FindSlotForItem(const Player &player, const Size &itemSize)
 {
-	Size itemSize = GetInventorySize(item);
-
 	if (itemSize.height == 1) {
 		for (int i = 30; i <= 39; i++) {
-			if (AutoPlaceItemInInventorySlot(player, i, item, persistItem, sendNetworkMessage))
-				return true;
+			if (CheckItemFitsInInventorySlot(player, i, itemSize))
+				return i;
 		}
 		for (int x = 9; x >= 0; x--) {
 			for (int y = 2; y >= 0; y--) {
-				if (AutoPlaceItemInInventorySlot(player, 10 * y + x, item, persistItem, sendNetworkMessage))
-					return true;
+				if (CheckItemFitsInInventorySlot(player, 10 * y + x, itemSize))
+					return 10 * y + x;
 			}
 		}
-		return false;
+		return {};
 	}
 
 	if (itemSize.height == 2) {
 		for (int x = 10 - itemSize.width; x >= 0; x--) {
 			for (int y = 0; y < 3; y++) {
-				if (AutoPlaceItemInInventorySlot(player, 10 * y + x, item, persistItem, sendNetworkMessage))
-					return true;
+				if (CheckItemFitsInInventorySlot(player, 10 * y + x, itemSize))
+					return 10 * y + x;
 			}
 		}
-		return false;
+		return {};
 	}
 
 	if (itemSize == Size { 1, 3 }) {
 		for (int i = 0; i < 20; i++) {
-			if (AutoPlaceItemInInventorySlot(player, i, item, persistItem, sendNetworkMessage))
-				return true;
+			if (CheckItemFitsInInventorySlot(player, i, itemSize))
+				return i;
 		}
-		return false;
+		return {};
 	}
 
 	if (itemSize == Size { 2, 3 }) {
 		for (int i = 0; i < 9; i++) {
-			if (AutoPlaceItemInInventorySlot(player, i, item, persistItem, sendNetworkMessage))
-				return true;
+			if (CheckItemFitsInInventorySlot(player, i, itemSize))
+				return i;
 		}
 
 		for (int i = 10; i < 19; i++) {
-			if (AutoPlaceItemInInventorySlot(player, i, item, persistItem, sendNetworkMessage))
-				return true;
+			if (CheckItemFitsInInventorySlot(player, i, itemSize))
+				return i;
 		}
-		return false;
+		return {};
 	}
 
 	app_fatal(StrCat("Unknown item size: ", itemSize.width, "x", itemSize.height));
 }
+} // namespace
+
+bool CanFitItemInInventory(const Player &player, const Item &item)
+{
+	return static_cast<bool>(FindSlotForItem(player, GetInventorySize(item)));
+}
+
+bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetworkMessage)
+{
+	Size itemSize = GetInventorySize(item);
+	std::optional<int> targetSlot = FindSlotForItem(player, itemSize);
+
+	if (targetSlot) {
+		player.InvList[player._pNumInv] = item;
+		player._pNumInv++;
+
+		AddItemToInvGrid(player, *targetSlot, player._pNumInv, itemSize, sendNetworkMessage);
+		player.CalcScrolls();
+
+		return true;
+	}
+
+	return false;
+}
 
 std::vector<int> SortItemsBySize(Player &player)
 {
@@ -1379,7 +1397,7 @@ void ReorganizeInventory(Player &player)
 	bool reorganizationFailed = false;
 	for (int index : sortedIndices) {
 		Item &item = tempStorage[index];
-		if (!AutoPlaceItemInInventory(player, item, true, false)) {
+		if (!AutoPlaceItemInInventory(player, item, false)) {
 			reorganizationFailed = true;
 			break;
 		}
@@ -1682,7 +1700,7 @@ void AutoGetItem(Player &player, Item *itemPointer, int ii)
 			done = AutoPlaceItemInBelt(player, item, true, &player == MyPlayer);
 		}
 		if (!done) {
-			done = AutoPlaceItemInInventory(player, item, true, &player == MyPlayer);
+			done = AutoPlaceItemInInventory(player, item, &player == MyPlayer);
 		}
 	}
 
@@ -2147,7 +2165,7 @@ void CloseStash()
 			NetSendCmdPItem(true, CMD_PUTITEM, *itemTile, myPlayer.HoldItem);
 		} else {
 			if (!AutoPlaceItemInBelt(myPlayer, myPlayer.HoldItem, true, true)
-			    && !AutoPlaceItemInInventory(myPlayer, myPlayer.HoldItem, true, true)
+			    && !AutoPlaceItemInInventory(myPlayer, myPlayer.HoldItem, true)
 			    && !AutoPlaceItemInStash(myPlayer, myPlayer.HoldItem, true)) {
 				// This can fail for max gold, arena potions and a stash that has been arranged
 				// to not have room for the item all 3 cases are extremely unlikely

Source/inv.h

@@ -142,15 +142,21 @@ bool AutoEquip(Player &player, const Item &item, bool persistItem = true, bool s
 
 /**
  * @brief Checks whether the given item can be placed on the specified player's inventory.
- * If 'persistItem' is 'True', the item is also placed in the inventory.
  * @param player The player whose inventory will be checked.
  * @param item The item to be checked.
- * @param persistItem Pass 'True' to actually place the item in the inventory. The default is 'False'.
+ * @return 'True' in case the item can be placed on the player's inventory and 'False' otherwise.
+ */
+bool CanFitItemInInventory(const Player &player, const Item &item);
+
+/**
+ * @brief Attempts to place the given item in the specified player's inventory.
+ * @param player The player whose inventory will be used.
+ * @param item The item to be placed.
  * @param sendNetworkMessage Set to true if you want a network message to be generated if the item is persisted.
  * Should only be set if a local player is placing an item in a play session (not when creating a new game)
- * @return 'True' in case the item can be placed on the player's inventory and 'False' otherwise.
+ * @return 'True' if the item was placed on the player's inventory and 'False' otherwise.
  */
-bool AutoPlaceItemInInventory(Player &player, const Item &item, bool persistItem = false, bool sendNetworkMessage = false);
+bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetworkMessage = false);
 
 /**
  * @brief Checks whether the given item can be placed on the specified player's belt. Returns 'True' when the item can be placed

Source/items.cpp

@@ -2967,7 +2967,7 @@ void CreateStartingItem(Player &player, _item_indexes itemData)
 	InitializeItem(item, itemData);
 	GenerateNewSeed(item);
 	item.updateRequiredStatsCacheForPlayer(player);
-	AutoEquip(player, item) || AutoPlaceItemInBelt(player, item, true) || AutoPlaceItemInInventory(player, item, true);
+	AutoEquip(player, item) || AutoPlaceItemInBelt(player, item, true) || AutoPlaceItemInInventory(player, item);
 }
 } // namespace
 

Source/player.cpp

@@ -2729,7 +2729,7 @@ void StripTopGold(Player &player)
 		return;
 	if (AutoEquip(player, player.HoldItem, false))
 		return;
-	if (AutoPlaceItemInInventory(player, player.HoldItem))
+	if (CanFitItemInInventory(player, player.HoldItem))
 		return;
 	if (AutoPlaceItemInBelt(player, player.HoldItem))
 		return;

Source/qol/autopickup.cpp

@@ -47,7 +47,7 @@ bool DoPickup(Item item)
 		return true;
 
 	if (item._itype == ItemType::Misc
-	    && (AutoPlaceItemInInventory(*MyPlayer, item) || AutoPlaceItemInBelt(*MyPlayer, item))) {
+	    && (CanFitItemInInventory(*MyPlayer, item) || AutoPlaceItemInBelt(*MyPlayer, item))) {
 		switch (item._iMiscId) {
 		case IMISC_HEAL:
 			return *sgOptions.Gameplay.numHealPotionPickup > NumMiscItemsInInv(item._iMiscId);

Source/qol/stash.cpp

@@ -293,7 +293,7 @@ void TransferItemToInventory(Player &player, uint16_t itemId)
 		return;
 	}
 
-	if (!AutoPlaceItemInInventory(player, item, true)) {
+	if (!AutoPlaceItemInInventory(player, item)) {
 		player.SaySpecific(HeroSpeech::IHaveNoRoom);
 		return;
 	}

Source/stores.cpp

@@ -345,7 +345,11 @@ bool StoreAutoPlace(Item &item, bool persistItem)
 		return true;
 	}
 
-	return AutoPlaceItemInInventory(player, item, persistItem, true);
+	if (persistItem) {
+		return AutoPlaceItemInInventory(player, item, true);
+	}
+
+	return CanFitItemInInventory(player, item);
 }
 
 void ScrollVendorStore(Item *itemData, int storeLimit, int idx, int selling = true)