Rebase of #7876 - Shift item order when selling to keep consistent window order (#7886)

* Shift item order when selling instead of using last item to fill the slot

Player has items i1, i2, i3, picked up in that order.

Previous Behavior:
1. Sell window at vendor shows i1, i2, i3.
2. Player sells i1
3. i3 is last in list and replaces i1
4. Sell window now shows i3, i2.

New Behavior:
1. Sell window at vendor shows i1, i2, i3.
2. Player sells i1
3. i2/i3 are shifted in index position down to fill the gap left by previous index.
4. Sell window now shows i2, i3.

* Remove whitespace

Source/inv.cpp

@@ -148,7 +148,7 @@ void AddItemToInvGrid(Player &player, int invGridIndex, int invListIndex, Size i
 			if (x == 0 && y == itemSize.height - 1)
 				player.InvGrid[rowGridIndex + x] = invListIndex;
 			else
-				player.InvGrid[rowGridIndex + x] = -invListIndex;
+				player.InvGrid[rowGridIndex + x] = -invListIndex; // use negative index to denote it's occupied but it's not the top-left cell.
 		}
 	}
 

Source/player.cpp

@@ -1558,16 +1558,18 @@ void Player::RemoveInvItem(int iv, bool calcScrolls)
 
 	_pNumInv--;
 
-	// If the item at the end of inventory array isn't the one we removed, we need to swap its position in the array with the removed item
+	// If the item at the end of inventory array isn't the one removed, shift all following items back one index to retain inventory order.
 	if (_pNumInv > 0 && _pNumInv != iv) {
-		InvList[iv] = InvList[_pNumInv].pop();
+		for (size_t newIndex = iv; newIndex < _pNumInv; newIndex++) {
+			InvList[newIndex] = InvList[newIndex + 1].pop();
+		}
 
 		for (int8_t &itemIndex : InvGrid) {
-			if (itemIndex == _pNumInv + 1) {
-				itemIndex = iv + 1;
+			if (itemIndex > iv + 1) { // if item was shifted, decrease the index so it's paired with the correct item.
+				itemIndex--;
 			}
-			if (itemIndex == -(_pNumInv + 1)) {
-				itemIndex = -(iv + 1);
+			if (itemIndex < -(iv + 1)) {
+				itemIndex++; // since occupied cells are negative, increment the index to keep it same as as top-left cell for item, only negative.
 			}
 		}
 	}

test/inv_test.cpp

@@ -180,15 +180,15 @@ TEST_F(InvTest, RemoveInvItem)
 	EXPECT_EQ(MyPlayer->_pNumInv, 0);
 }
 
-// Test removing an item from inventory with other items in it.
-TEST_F(InvTest, RemoveInvItem_other_item)
+// Test removing an item from middle of inventory list.
+TEST_F(InvTest, RemoveInvItem_shiftsListFromMiddle)
 {
 	SNetInitializeProvider(SELCONN_LOOPBACK, nullptr);
 
 	clear_inventory();
-	// Put a two-slot misc item and a ring into the inventory:
-	// | (item) | (item) | (ring) | ...
-	MyPlayer->_pNumInv = 2;
+	// Put a two-slot misc item and a ring into the inventory, followed by another two-slot misc item:
+	// | (item) | (item) | (ring) | (item) | (item) | ...
+	MyPlayer->_pNumInv = 3;
 	MyPlayer->InvGrid[0] = 1;
 	MyPlayer->InvGrid[1] = -1;
 	MyPlayer->InvList[0]._itype = ItemType::Misc;
@@ -196,12 +196,54 @@ TEST_F(InvTest, RemoveInvItem_other_item)
 	MyPlayer->InvGrid[2] = 2;
 	MyPlayer->InvList[1]._itype = ItemType::Ring;
 
+	MyPlayer->InvGrid[3] = 3;
+	MyPlayer->InvGrid[4] = -3;
+	MyPlayer->InvList[2]._itype = ItemType::Misc;
+
+	MyPlayer->RemoveInvItem(1);
+	EXPECT_EQ(MyPlayer->InvGrid[0], 1);
+	EXPECT_EQ(MyPlayer->InvGrid[1], -1);
+	EXPECT_EQ(MyPlayer->InvGrid[2], 0);
+	EXPECT_EQ(MyPlayer->InvGrid[3], 2);
+	EXPECT_EQ(MyPlayer->InvGrid[4], -2);
+
+	EXPECT_EQ(MyPlayer->InvList[0]._itype, ItemType::Misc);
+	EXPECT_EQ(MyPlayer->InvList[1]._itype, ItemType::Misc);
+
+	EXPECT_EQ(MyPlayer->_pNumInv, 2);
+}
+
+// Test removing an item from front of inventory list.
+TEST_F(InvTest, RemoveInvItem_shiftsListFromFront)
+{
+	SNetInitializeProvider(SELCONN_LOOPBACK, nullptr);
+
+	clear_inventory();
+	// Put a two-slot misc item and a ring into the inventory, followed by another two-slot misc item:
+	// | (item) | (item) | (ring) | (item) | (item) | ...
+	MyPlayer->_pNumInv = 3;
+	MyPlayer->InvGrid[0] = 1;
+	MyPlayer->InvGrid[1] = -1;
+	MyPlayer->InvList[0]._itype = ItemType::Misc;
+
+	MyPlayer->InvGrid[2] = 2;
+	MyPlayer->InvList[1]._itype = ItemType::Ring;
+
+	MyPlayer->InvGrid[3] = 3;
+	MyPlayer->InvGrid[4] = -3;
+	MyPlayer->InvList[2]._itype = ItemType::Misc;
+
 	MyPlayer->RemoveInvItem(0);
 	EXPECT_EQ(MyPlayer->InvGrid[0], 0);
 	EXPECT_EQ(MyPlayer->InvGrid[1], 0);
 	EXPECT_EQ(MyPlayer->InvGrid[2], 1);
+	EXPECT_EQ(MyPlayer->InvGrid[3], 2);
+	EXPECT_EQ(MyPlayer->InvGrid[4], -2);
+
 	EXPECT_EQ(MyPlayer->InvList[0]._itype, ItemType::Ring);
-	EXPECT_EQ(MyPlayer->_pNumInv, 1);
+	EXPECT_EQ(MyPlayer->InvList[1]._itype, ItemType::Misc);
+
+	EXPECT_EQ(MyPlayer->_pNumInv, 2);
 }
 
 // Test removing an item from the belt