Skip to content

Commit a705e7c

Browse files
jwkeatingjwkeating
committed
Fix memory stomp due to inconsistent inventory layout and missing error check
1 parent 9f9489b commit a705e7c

5 files changed

Lines changed: 96 additions & 72 deletions

File tree

‎Source/inv.cpp‎

Lines changed: 83 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ bool invflag;
6767
*
6868
* 01 02
6969
*
70-
* 07 08 09 10 11 12 13 14 15 16
71-
* 17 18 19 20 21 22 23 24 25 26
72-
* 27 28 29 30 31 32 33 34 35 36
7370
* 37 38 39 40 41 42 43 44 45 46
71+
* 27 28 29 30 31 32 33 34 35 36
72+
* 17 18 19 20 21 22 23 24 25 26
73+
* 07 08 09 10 11 12 13 14 15 16
7474
*
7575
* 47 48 49 50 51 52 53 54
7676
* @endcode
@@ -85,36 +85,6 @@ const Rectangle InvRect[] = {
8585
{ { 17, 75 }, { 58, 86 } }, // left hand
8686
{ { 248, 75 }, { 58, 87 } }, // right hand
8787
{ { 132, 75 }, { 58, 87 } }, // chest
88-
{ { 17, 222 }, { 29, 29 } }, // inv row 1
89-
{ { 46, 222 }, { 29, 29 } }, // inv row 1
90-
{ { 75, 222 }, { 29, 29 } }, // inv row 1
91-
{ { 104, 222 }, { 29, 29 } }, // inv row 1
92-
{ { 133, 222 }, { 29, 29 } }, // inv row 1
93-
{ { 162, 222 }, { 29, 29 } }, // inv row 1
94-
{ { 191, 222 }, { 29, 29 } }, // inv row 1
95-
{ { 220, 222 }, { 29, 29 } }, // inv row 1
96-
{ { 249, 222 }, { 29, 29 } }, // inv row 1
97-
{ { 278, 222 }, { 29, 29 } }, // inv row 1
98-
{ { 17, 251 }, { 29, 29 } }, // inv row 2
99-
{ { 46, 251 }, { 29, 29 } }, // inv row 2
100-
{ { 75, 251 }, { 29, 29 } }, // inv row 2
101-
{ { 104, 251 }, { 29, 29 } }, // inv row 2
102-
{ { 133, 251 }, { 29, 29 } }, // inv row 2
103-
{ { 162, 251 }, { 29, 29 } }, // inv row 2
104-
{ { 191, 251 }, { 29, 29 } }, // inv row 2
105-
{ { 220, 251 }, { 29, 29 } }, // inv row 2
106-
{ { 249, 251 }, { 29, 29 } }, // inv row 2
107-
{ { 278, 251 }, { 29, 29 } }, // inv row 2
108-
{ { 17, 280 }, { 29, 29 } }, // inv row 3
109-
{ { 46, 280 }, { 29, 29 } }, // inv row 3
110-
{ { 75, 280 }, { 29, 29 } }, // inv row 3
111-
{ { 104, 280 }, { 29, 29 } }, // inv row 3
112-
{ { 133, 280 }, { 29, 29 } }, // inv row 3
113-
{ { 162, 280 }, { 29, 29 } }, // inv row 3
114-
{ { 191, 280 }, { 29, 29 } }, // inv row 3
115-
{ { 220, 280 }, { 29, 29 } }, // inv row 3
116-
{ { 249, 280 }, { 29, 29 } }, // inv row 3
117-
{ { 278, 280 }, { 29, 29 } }, // inv row 3
11888
{ { 17, 309 }, { 29, 29 } }, // inv row 4
11989
{ { 46, 309 }, { 29, 29 } }, // inv row 4
12090
{ { 75, 309 }, { 29, 29 } }, // inv row 4
@@ -125,6 +95,36 @@ const Rectangle InvRect[] = {
12595
{ { 220, 309 }, { 29, 29 } }, // inv row 4
12696
{ { 249, 309 }, { 29, 29 } }, // inv row 4
12797
{ { 278, 309 }, { 29, 29 } }, // inv row 4
98+
{ { 17, 280 }, { 29, 29 } }, // inv row 3
99+
{ { 46, 280 }, { 29, 29 } }, // inv row 3
100+
{ { 75, 280 }, { 29, 29 } }, // inv row 3
101+
{ { 104, 280 }, { 29, 29 } }, // inv row 3
102+
{ { 133, 280 }, { 29, 29 } }, // inv row 3
103+
{ { 162, 280 }, { 29, 29 } }, // inv row 3
104+
{ { 191, 280 }, { 29, 29 } }, // inv row 3
105+
{ { 220, 280 }, { 29, 29 } }, // inv row 3
106+
{ { 249, 280 }, { 29, 29 } }, // inv row 3
107+
{ { 278, 280 }, { 29, 29 } }, // inv row 3
108+
{ { 17, 251 }, { 29, 29 } }, // inv row 2
109+
{ { 46, 251 }, { 29, 29 } }, // inv row 2
110+
{ { 75, 251 }, { 29, 29 } }, // inv row 2
111+
{ { 104, 251 }, { 29, 29 } }, // inv row 2
112+
{ { 133, 251 }, { 29, 29 } }, // inv row 2
113+
{ { 162, 251 }, { 29, 29 } }, // inv row 2
114+
{ { 191, 251 }, { 29, 29 } }, // inv row 2
115+
{ { 220, 251 }, { 29, 29 } }, // inv row 2
116+
{ { 249, 251 }, { 29, 29 } }, // inv row 2
117+
{ { 278, 251 }, { 29, 29 } }, // inv row 2
118+
{ { 17, 222 }, { 29, 29 } }, // inv row 1
119+
{ { 46, 222 }, { 29, 29 } }, // inv row 1
120+
{ { 75, 222 }, { 29, 29 } }, // inv row 1
121+
{ { 104, 222 }, { 29, 29 } }, // inv row 1
122+
{ { 133, 222 }, { 29, 29 } }, // inv row 1
123+
{ { 162, 222 }, { 29, 29 } }, // inv row 1
124+
{ { 191, 222 }, { 29, 29 } }, // inv row 1
125+
{ { 220, 222 }, { 29, 29 } }, // inv row 1
126+
{ { 249, 222 }, { 29, 29 } }, // inv row 1
127+
{ { 278, 222 }, { 29, 29 } }, // inv row 1
128128
{ { 205, 5 }, { 29, 29 } }, // belt
129129
{ { 234, 5 }, { 29, 29 } }, // belt
130130
{ { 263, 5 }, { 29, 29 } }, // belt
@@ -143,7 +143,7 @@ OptionalOwnedClxSpriteList pInvCels;
143143
/**
144144
* @brief Adds an item to a player's InvGrid array
145145
* @param player The player reference
146-
* @param invGridIndex Item's position in InvGrid (this should be the item's topleft grid tile)
146+
* @param invGridIndex Item's position in InvGrid (this should be the item's bottom left grid tile)
147147
* @param invListIndex The item's InvList index (it's expected this already has +1 added to it since InvGrid can't store a 0 index)
148148
* @param itemSize Size of item
149149
*/
@@ -153,10 +153,10 @@ void AddItemToInvGrid(Player &player, int invGridIndex, int invListIndex, Size i
153153
for (int y = 0; y < itemSize.height; y++) {
154154
const int rowGridIndex = invGridIndex + pitch * y;
155155
for (int x = 0; x < itemSize.width; x++) {
156-
if (x == 0 && y == itemSize.height - 1)
156+
if (x == 0 && y == 0)
157157
player.InvGrid[rowGridIndex + x] = invListIndex;
158158
else
159-
player.InvGrid[rowGridIndex + x] = -invListIndex; // use negative index to denote it's occupied but it's not the top-left cell.
159+
player.InvGrid[rowGridIndex + x] = -invListIndex; // use negative index to denote it's occupied but it's not the bottom-left cell.
160160
}
161161
}
162162

@@ -307,23 +307,23 @@ int FindTargetSlotUnderItemCursor(Point cursorPosition, Size itemSize)
307307
}
308308
for (int r = SLOTXY_INV_FIRST; r <= SLOTXY_INV_LAST; r++) {
309309
if (InvRect[r].contains(cursorPosition + panelOffset)) {
310-
// When trying to paste into the inventory we need to determine the top left cell of the nearest area that could fit the item, not the slot under the center/hot pixel.
310+
// When trying to paste into the inventory we need to determine the bottom left cell of the nearest area that could fit the item, not the slot under the center/hot pixel.
311311
if (itemSize.height <= 1 && itemSize.width <= 1) {
312-
// top left cell of a 1x1 item is the same cell as the hot pixel, no work to do
312+
// bottom left cell of a 1x1 item is the same cell as the hot pixel, no work to do
313313
return r;
314314
}
315-
// Otherwise work out how far the central cell is from the top-left cell
315+
// Otherwise work out how far the central cell is from the bottom left cell
316316
Displacement hotPixelCellOffset = { (itemSize.width - 1) / 2, (itemSize.height - 1) / 2 };
317317
// For even dimension items we need to work out if the cursor is in the left/right (or top/bottom) half of the central cell and adjust the offset so the item lands in the area most covered by the cursor.
318318
if (itemSize.width % 2 == 0 && InvRect[r].contains(cursorPosition + panelOffset + Displacement { INV_SLOT_HALF_SIZE_PX, 0 })) {
319319
// hot pixel was in the left half of the cell, so we want to increase the offset to preference the column to the left
320320
hotPixelCellOffset.deltaX++;
321321
}
322-
if (itemSize.height % 2 == 0 && InvRect[r].contains(cursorPosition + panelOffset + Displacement { 0, INV_SLOT_HALF_SIZE_PX })) {
323-
// hot pixel was in the top half of the cell, so we want to increase the offset to preference the row above
322+
if (itemSize.height % 2 == 0 && !InvRect[r].contains(cursorPosition + panelOffset + Displacement { 0, INV_SLOT_HALF_SIZE_PX })) {
323+
// hot pixel was in the bottom half of the cell, so we want to decrease the offset to preference the row below
324324
hotPixelCellOffset.deltaY++;
325325
}
326-
// Then work out the top left cell of the nearest area that could fit this item (as pasting on the edge of the inventory would otherwise put it out of bounds)
326+
// Then work out the bottom left cell of the nearest area that could fit this item (as pasting on the edge of the inventory would otherwise put it out of bounds)
327327
const int hotPixelCell = r - SLOTXY_INV_FIRST;
328328
const int targetRow = std::clamp((hotPixelCell / InventorySizeInSlots.width) - hotPixelCellOffset.deltaY, 0, InventorySizeInSlots.height - itemSize.height);
329329
const int targetColumn = std::clamp((hotPixelCell % InventorySizeInSlots.width) - hotPixelCellOffset.deltaX, 0, InventorySizeInSlots.width - itemSize.width);
@@ -687,12 +687,12 @@ bool CheckItemFitsInInventorySlot(const Player &player, int slotIndex, const Siz
687687
std::optional<int> FindSlotForItem(const Player &player, const Size &itemSize, int itemIndexToIgnore = -1)
688688
{
689689
if (itemSize.height == 1) {
690-
for (int i = 30; i <= 39; i++) {
690+
for (int i = 0; i <= 9; i++) {
691691
if (CheckItemFitsInInventorySlot(player, i, itemSize, itemIndexToIgnore))
692692
return i;
693693
}
694694
for (int x = 9; x >= 0; x--) {
695-
for (int y = 2; y >= 0; y--) {
695+
for (int y = 1; y <= 3; y++) {
696696
if (CheckItemFitsInInventorySlot(player, 10 * y + x, itemSize, itemIndexToIgnore))
697697
return 10 * y + x;
698698
}
@@ -702,7 +702,7 @@ std::optional<int> FindSlotForItem(const Player &player, const Size &itemSize, i
702702

703703
if (itemSize.height == 2) {
704704
for (int x = 10 - itemSize.width; x >= 0; x--) {
705-
for (int y = 0; y < 3; y++) {
705+
for (int y = 2; y >= 0; y--) {
706706
if (CheckItemFitsInInventorySlot(player, 10 * y + x, itemSize, itemIndexToIgnore))
707707
return 10 * y + x;
708708
}
@@ -711,20 +711,24 @@ std::optional<int> FindSlotForItem(const Player &player, const Size &itemSize, i
711711
}
712712

713713
if (itemSize == Size { 1, 3 }) {
714-
for (int i = 0; i < 20; i++) {
714+
for (int i = 10; i < 20; i++) {
715+
if (CheckItemFitsInInventorySlot(player, i, itemSize, itemIndexToIgnore))
716+
return i;
717+
}
718+
for (int i = 0; i < 10; i++) {
715719
if (CheckItemFitsInInventorySlot(player, i, itemSize, itemIndexToIgnore))
716720
return i;
717721
}
718722
return {};
719723
}
720724

721725
if (itemSize == Size { 2, 3 }) {
722-
for (int i = 0; i < 9; i++) {
726+
for (int i = 10; i < 19; i++) {
723727
if (CheckItemFitsInInventorySlot(player, i, itemSize, itemIndexToIgnore))
724728
return i;
725729
}
726730

727-
for (int i = 10; i < 19; i++) {
731+
for (int i = 0; i < 9; i++) {
728732
if (CheckItemFitsInInventorySlot(player, i, itemSize, itemIndexToIgnore))
729733
return i;
730734
}
@@ -1520,14 +1524,14 @@ int AddGoldToInventory(Player &player, int value)
15201524
SetPlrHandGoldCurs(goldItem);
15211525
}
15221526

1523-
// Last row right to left
1524-
for (int i = 39; i >= 30 && value > 0; i--) {
1527+
// Bottom row right to left
1528+
for (int i = 9; i >= 0 && value > 0; i--) {
15251529
value = CreateGoldItemInInventorySlot(player, i, value);
15261530
}
15271531

15281532
// Remaining inventory in columns, bottom to top, right to left
15291533
for (int x = 9; x >= 0 && value > 0; x--) {
1530-
for (int y = 2; y >= 0 && value > 0; y--) {
1534+
for (int y = 1; y <= 3 && value > 0; y++) {
15311535
value = CreateGoldItemInInventorySlot(player, 10 * y + x, value);
15321536
}
15331537
}
@@ -1565,29 +1569,39 @@ void inv_update_rem_item(Player &player, inv_body_loc iv)
15651569
CalcPlrInv(player, player._pmode != PM_DEATH);
15661570
}
15671571

1568-
void CheckInvSwap(Player &player, const Item &item, int invGridIndex)
1572+
bool CheckInvSwap(Player &player, const Item &item, int invGridIndex)
15691573
{
15701574
Size itemSize = GetInventorySize(item);
15711575

15721576
const int pitch = 10;
1573-
const int invListIndex = [&]() -> int {
1574-
for (int y = 0; y < itemSize.height; y++) {
1575-
const int rowGridIndex = invGridIndex + pitch * y;
1576-
for (int x = 0; x < itemSize.width; x++) {
1577-
const int gridIndex = rowGridIndex + x;
1578-
if (player.InvGrid[gridIndex] != 0)
1579-
return std::abs(player.InvGrid[gridIndex]);
1577+
if (invGridIndex + itemSize.width - 1 + pitch * (itemSize.height - 1) >= InventoryGridCells)
1578+
return false;
1579+
1580+
int invListIndex = 0;
1581+
for (int y = 0; y < itemSize.height; y++) {
1582+
int rowGridIndex = invGridIndex + pitch * y;
1583+
for (int x = 0; x < itemSize.width; x++) {
1584+
int gridIndex = rowGridIndex + x;
1585+
if (player.InvGrid[gridIndex] != 0) {
1586+
int existingItem = std::abs(player.InvGrid[gridIndex]);
1587+
if (!invListIndex) {
1588+
invListIndex = existingItem;
1589+
} else if (invListIndex != existingItem) {
1590+
// More than one item exists where we want to place the new item
1591+
return false;
1592+
}
15801593
}
15811594
}
1582-
player._pNumInv++;
1583-
return player._pNumInv;
1584-
}();
1595+
}
15851596

1586-
if (invListIndex < player._pNumInv) {
1587-
for (int8_t &itemIndex : player.InvGrid) {
1588-
if (itemIndex == invListIndex)
1589-
itemIndex = 0;
1590-
if (itemIndex == -invListIndex)
1597+
if (!invListIndex) {
1598+
// The inventory is empty where we want to place the new item. Allocate a new spot.
1599+
player._pNumInv++;
1600+
invListIndex = player._pNumInv;
1601+
} else {
1602+
// Remove the old location of the existing item
1603+
for (int8_t& itemIndex : player.InvGrid) {
1604+
if (std::abs(itemIndex) == invListIndex)
15911605
itemIndex = 0;
15921606
}
15931607
}
@@ -1597,14 +1611,15 @@ void CheckInvSwap(Player &player, const Item &item, int invGridIndex)
15971611
for (int y = 0; y < itemSize.height; y++) {
15981612
const int rowGridIndex = invGridIndex + pitch * y;
15991613
for (int x = 0; x < itemSize.width; x++) {
1600-
if (x == 0 && y == itemSize.height - 1)
1614+
if (x == 0 && y == 0)
16011615
player.InvGrid[rowGridIndex + x] = invListIndex;
16021616
else
16031617
player.InvGrid[rowGridIndex + x] = -invListIndex;
16041618
}
16051619
}
16061620

16071621
CalcPlrInv(player, true);
1622+
return true;
16081623
}
16091624

16101625
void CheckInvRemove(Player &player, int invGridIndex)

‎Source/inv.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ int AddGoldToInventory(Player &player, int value);
188188
bool GoldAutoPlace(Player &player, Item &goldStack);
189189
void CheckInvSwap(Player &player, inv_body_loc bLoc);
190190
void inv_update_rem_item(Player &player, inv_body_loc iv);
191-
void CheckInvSwap(Player &player, const Item &item, int invGridIndex);
191+
bool CheckInvSwap(Player &player, const Item &item, int invGridIndex);
192192
void CheckInvRemove(Player &player, int invGridIndex);
193193
void TransferItemToStash(Player &player, int location);
194194
void CheckInvItem(bool isShiftHeld = false, bool isCtrlHeld = false);

‎Source/items.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3037,7 +3037,7 @@ void CreatePlrItems(Player &player)
30373037
MakeGoldStack(goldItem, loadout.gold);
30383038

30393039
player._pNumInv++;
3040-
player.InvGrid[30] = player._pNumInv;
3040+
player.InvGrid[0] = player._pNumInv;
30413041

30423042
player._pGold = goldItem._ivalue;
30433043
}

‎Source/msg.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2172,7 +2172,10 @@ size_t OnChangeInventoryItems(const TCmdChItem &message, Player &player)
21722172
} else if (&player != MyPlayer && IsItemAvailable(static_cast<_item_indexes>(Swap16LE(message.def.wIndx)))) {
21732173
Item item {};
21742174
RecreateItem(player, message, item);
2175-
CheckInvSwap(player, item, message.bLoc);
2175+
if (!CheckInvSwap(player, item, message.bLoc)) {
2176+
// item is larger than 1x1, and it occupies invalid inventory slots
2177+
return sizeof(message);
2178+
}
21762179
}
21772180

21782181
return sizeof(message);

‎Source/player.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ struct Player {
220220

221221
int lightId;
222222

223-
int _pNumInv;
223+
int _pNumInv; // Number of items in InvList[]
224224
int _pStrength;
225225
int _pBaseStr;
226226
int _pMagic;
@@ -294,6 +294,12 @@ struct Player {
294294
int8_t _pHFrames;
295295
int8_t _pDFrames;
296296
int8_t _pBFrames;
297+
298+
// Player inventory grid is indexed as InvGrid[x + 10*y] where x in [0,9], y in [0,3].
299+
// [0] is bottom left, [9] is bottom right, [30] is upper left, [39] is upper right.
300+
// InvGrid[p] == 0 means no item in grid cell p. InvGrid[p] != 0 means InvList[abs(InvGrid[p]) - 1] is the item at grid cell p.
301+
// Items that take 1x1 grid cells use positive index InvGrid[p] > 0. For larger items like a staff which takes 2x3 grid cells,
302+
// InvGrid[p] > 0 is used for the bottom left grid cell of the item, and InvGrid[p] < 0 is used for the other grid cells of the item.
297303
int8_t InvGrid[InventoryGridCells];
298304

299305
uint8_t plrlevel;

0 commit comments

Comments
 (0)