mirror
/
devilutionX
mirror of https://github.com/diasurgical/devilutionX.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Fix torn note inventory combine semantics 

Keep Na-Krul note auto-combine limited to player-facing inventory inserts so internal inventory reflow does not trigger partial side effects.\n\nTrack the inserted note through inventory compaction when combining, and add inventory regression tests for duplicate notes and sort behavior.
pull/8504/head
morfidon 5 days ago
parent
0f62a8c388
commit
99a4f8b5ec
3 changed files with 205 additions and 83 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 143
      Source/inv.cpp
  2. 39
      Source/inv.h
  3. 106
      test/inv_test.cpp

143
Source/inv.cpp
Unescape View File

@ -140,8 +140,73 @@ namespace {
OptionalOwnedClxSpriteList pInvCels;
/**
* @brief Adds an item to a player's InvGrid array
bool IsTornNaKrulNote(_item_indexes id)
{
return IsAnyOf(id, IDI_NOTE1, IDI_NOTE2, IDI_NOTE3);
}
bool PlayerHasAllTornNaKrulNotes(const Player &player)
{
return HasInventoryItemWithId(player, IDI_NOTE1)
&& HasInventoryItemWithId(player, IDI_NOTE2)
&& HasInventoryItemWithId(player, IDI_NOTE3);
}
void ConvertItemToFullNote(Item &item)
{
item = {};
GetItemAttrs(item, IDI_FULLNOTE, 16);
SetupItem(item);
}
int TryCombineInsertedNaKrulNote(Player &player, int insertedInvIndex)
{
if (insertedInvIndex < 0 || insertedInvIndex >= player._pNumInv) {
return insertedInvIndex;
}
const _item_indexes insertedId = player.InvList[insertedInvIndex].IDidx;
if (!IsTornNaKrulNote(insertedId) || !PlayerHasAllTornNaKrulNotes(player)) {
return insertedInvIndex;
}
player.Say(HeroSpeech::JustWhatIWasLookingFor, 10);
std::array<int, 2> removedNoteIndices {};
size_t removeCount = 0;
for (const _item_indexes note : { IDI_NOTE1, IDI_NOTE2, IDI_NOTE3 }) {
if (note == insertedId) {
continue;
}
for (int i = 0; i < player._pNumInv; i++) {
if (player.InvList[i].IDidx == note) {
removedNoteIndices[removeCount++] = i;
break;
}
}
}
if (removeCount != removedNoteIndices.size()) {
return insertedInvIndex;
}
std::sort(removedNoteIndices.begin(), removedNoteIndices.end(), std::greater<int>());
for (const int removedNoteIndex : removedNoteIndices) {
player.RemoveInvItem(removedNoteIndex, false);
if (removedNoteIndex < insertedInvIndex) {
insertedInvIndex--;
}
}
Item &combinedNote = player.InvList[insertedInvIndex];
ConvertItemToFullNote(combinedNote);
combinedNote.updateRequiredStatsCacheForPlayer(player);
return insertedInvIndex;
}
/**
* @brief Adds an item to a player's InvGrid array
* @param player The player reference
* @param invGridIndex Item's position in InvGrid (this should be the item's topleft grid tile)
* @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)
@ -498,7 +563,7 @@ bool ChangeInvItem(Player &player, int slot, Size itemSize)
if (prevItemId == 0) {
player.InvList[player._pNumInv] = player.HoldItem.pop();
player._pNumInv++;
prevItemId = CheckSpecialInventoryItem(player, player._pNumInv - 1) + 1;
prevItemId = TryCombineInsertedNaKrulNote(player, player._pNumInv - 1) + 1;
} else {
const int invIndex = prevItemId - 1;
if (player.HoldItem._itype == ItemType::Gold)
@ -512,7 +577,7 @@ bool ChangeInvItem(Player &player, int slot, Size itemSize)
if (itemIndex == -prevItemId)
itemIndex = 0;
}
prevItemId = CheckSpecialInventoryItem(player, invIndex) + 1;
prevItemId = TryCombineInsertedNaKrulNote(player, invIndex) + 1;
}
itemSize = GetInventorySize(player.InvList[prevItemId - 1]);
@ -960,31 +1025,25 @@ void CheckInvCut(Player &player, Point cursorPosition, bool automaticMove, bool
void TryCombineNaKrulNotes(Player &player, Item &noteItem)
{
const int idx = noteItem.IDidx;
const _item_indexes notes[] = { IDI_NOTE1, IDI_NOTE2, IDI_NOTE3 };
if (IsNoneOf(idx, IDI_NOTE1, IDI_NOTE2, IDI_NOTE3)) {
const _item_indexes idx = noteItem.IDidx;
if (!IsTornNaKrulNote(idx)) {
return;
}
for (const _item_indexes note : notes) {
if (idx != note && !HasInventoryItemWithId(player, note)) {
return; // the player doesn't have all notes
}
if (!PlayerHasAllTornNaKrulNotes(player)) {
return; // the player doesn't have all notes
}
MyPlayer->Say(HeroSpeech::JustWhatIWasLookingFor, 10);
for (const _item_indexes note : notes) {
for (const _item_indexes note : { IDI_NOTE1, IDI_NOTE2, IDI_NOTE3 }) {
if (idx != note) {
RemoveInventoryItemById(player, note);
}
}
const Point position = noteItem.position; // copy the position to restore it after re-initialising the item
noteItem = {};
GetItemAttrs(noteItem, IDI_FULLNOTE, 16);
SetupItem(noteItem);
ConvertItemToFullNote(noteItem);
noteItem.position = position; // this ensures CleanupItem removes the entry in the dropped items lookup table
}
@ -1124,49 +1183,6 @@ int CreateGoldItemInInventorySlot(Player &player, int slotIndex, int value)
} // namespace
int CheckSpecialInventoryItem(Player &player, int invIndex)
{
if (invIndex < 0 || invIndex >= player._pNumInv) {
return invIndex;
}
const _item_indexes currentId = player.InvList[invIndex].IDidx;
const _item_indexes notes[] = { IDI_NOTE1, IDI_NOTE2, IDI_NOTE3 };
if (IsNoneOf(currentId, IDI_NOTE1, IDI_NOTE2, IDI_NOTE3)) {
return invIndex;
}
for (const _item_indexes note : notes) {
if (!HasInventoryItemWithId(player, note)) {
return invIndex;
}
}
player.Say(HeroSpeech::JustWhatIWasLookingFor, 10);
for (const _item_indexes note : notes) {
if (note != currentId) {
RemoveInventoryItemById(player, note);
}
}
for (int i = 0; i < player._pNumInv; i++) {
Item &noteItem = player.InvList[i];
if (noteItem.IDidx != currentId) {
continue;
}
noteItem = {};
GetItemAttrs(noteItem, IDI_FULLNOTE, 16);
SetupItem(noteItem);
noteItem.updateRequiredStatsCacheForPlayer(player);
return i;
}
return invIndex;
}
void InvDrawSlotBack(const Surface &out, Point targetPosition, Size size, item_quality itemQuality)
{
SDL_Rect srcRect = MakeSdlRect(0, 0, size.width, size.height);
@ -1438,7 +1454,7 @@ 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)
bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetworkMessage, InventoryInsertSemantics semantics)
{
const Size itemSize = GetInventorySize(item);
std::optional<int> targetSlot = FindSlotForItem(player, itemSize);
@ -1446,7 +1462,10 @@ bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetwork
if (targetSlot) {
player.InvList[player._pNumInv] = item;
player._pNumInv++;
const int invIndex = CheckSpecialInventoryItem(player, player._pNumInv - 1);
int invIndex = player._pNumInv - 1;
if (semantics == InventoryInsertSemantics::PlayerAction) {
invIndex = TryCombineInsertedNaKrulNote(player, invIndex);
}
AddItemToInvGrid(player, *targetSlot, invIndex + 1, GetInventorySize(player.InvList[invIndex]), sendNetworkMessage);
player.CalcScrolls();
@ -1506,7 +1525,7 @@ void ReorganizeInventory(Player &player)
bool reorganizationFailed = false;
for (const int index : sortedIndices) {
const Item &item = tempStorage[index];
if (!AutoPlaceItemInInventory(player, item, false)) {
if (!AutoPlaceItemInInventory(player, item, false, InventoryInsertSemantics::InternalRebuild)) {
reorganizationFailed = true;
break;
}

39
Source/inv.h
Unescape View File

@ -74,14 +74,19 @@ enum inv_xy_slot : uint8_t {
};
enum item_color : uint8_t {
// clang-format off
ICOL_YELLOW = PAL16_YELLOW + 5,
ICOL_WHITE = PAL16_GRAY + 5,
ICOL_BLUE = PAL16_BLUE + 5,
// clang-format off
ICOL_YELLOW = PAL16_YELLOW + 5,
ICOL_WHITE = PAL16_GRAY + 5,
ICOL_BLUE = PAL16_BLUE + 5,
ICOL_RED = PAL16_RED + 5,
// clang-format on
};
enum class InventoryInsertSemantics {
PlayerAction,
InternalRebuild,
};
extern bool invflag;
extern const Rectangle InvRect[NUM_XY_SLOTS];
@ -148,24 +153,16 @@ bool AutoEquip(Player &player, const Item &item, bool persistItem = true, bool s
*/
bool CanFitItemInInventory(const Player &player, const Item &item);
/**
* @brief Handles special item behavior after an item has been inserted into InvList.
* @param player The player whose inventory was modified.
* @param invIndex Index of the inserted item in InvList.
* @return The item's current InvList index. This can change if handling the item
* removes other inventory entries and the inventory gets compacted.
*/
int CheckSpecialInventoryItem(Player &player, int invIndex);
/**
* @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' if the item was placed on the player's inventory and 'False' otherwise.
/**
* @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)
* @param semantics Distinguishes player-facing item insertion from internal inventory rebuilds.
* @return 'True' if the item was placed on the player's inventory and 'False' otherwise.
*/
bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetworkMessage = false);
bool AutoPlaceItemInInventory(Player &player, const Item &item, bool sendNetworkMessage = false, InventoryInsertSemantics semantics = InventoryInsertSemantics::PlayerAction);
/**
* @brief Checks whether the given item can be placed on the specified player's belt. Returns 'True' when the item can be placed

106
test/inv_test.cpp
Unescape View File

@ -1,3 +1,5 @@
#include <algorithm>
#include <gtest/gtest.h>
#include "cursor.h"
@ -64,6 +66,39 @@ void clear_inventory()
MyPlayer->_pNumInv = 0;
}
void place_inventory_item(int invIndex, int gridIndex, _item_indexes itemId)
{
Item &item = MyPlayer->InvList[invIndex];
InitializeItem(item, itemId);
item.updateRequiredStatsCacheForPlayer(*MyPlayer);
MyPlayer->InvGrid[gridIndex] = invIndex + 1;
MyPlayer->_pNumInv = std::max(MyPlayer->_pNumInv, invIndex + 1);
}
int count_inventory_items_with_id(_item_indexes itemId)
{
int count = 0;
for (int i = 0; i < MyPlayer->_pNumInv; i++) {
if (MyPlayer->InvList[i].IDidx == itemId) {
count++;
}
}
return count;
}
int count_positive_inv_grid_slots()
{
int count = 0;
for (const int8_t cell : MyPlayer->InvGrid) {
if (cell > 0) {
count++;
}
}
return count;
}
// Test that the scroll is used in the inventory in correct conditions
TEST_F(InvTest, UseScroll_from_inventory)
{
@ -385,5 +420,76 @@ TEST_F(InvTest, ItemSizeLastDiabloItem)
EXPECT_EQ(GetInventorySize(testItem), Size(2, 3));
}
TEST_F(InvTest, AutoPlaceItemInInventoryCombinesInsertedNaKrulNote)
{
if (!gbIsHellfire) return;
SNetInitializeProvider(SELCONN_LOOPBACK, nullptr);
clear_inventory();
place_inventory_item(0, 0, IDI_NOTE2);
place_inventory_item(1, 1, IDI_NOTE3);
Item insertedNote {};
InitializeItem(insertedNote, IDI_NOTE1);
insertedNote.updateRequiredStatsCacheForPlayer(*MyPlayer);
ASSERT_TRUE(AutoPlaceItemInInventory(*MyPlayer, insertedNote));
EXPECT_EQ(MyPlayer->_pNumInv, 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_FULLNOTE), 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE1), 0);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE2), 0);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE3), 0);
EXPECT_EQ(MyPlayer->InvGrid[0], 0);
EXPECT_EQ(MyPlayer->InvGrid[1], 0);
EXPECT_EQ(MyPlayer->InvGrid[2], 1);
}
TEST_F(InvTest, AutoPlaceItemInInventoryCombinesInsertedDuplicateNaKrulNote)
{
if (!gbIsHellfire) return;
SNetInitializeProvider(SELCONN_LOOPBACK, nullptr);
clear_inventory();
place_inventory_item(0, 0, IDI_NOTE1);
place_inventory_item(1, 1, IDI_NOTE2);
place_inventory_item(2, 2, IDI_NOTE3);
Item insertedNote {};
InitializeItem(insertedNote, IDI_NOTE1);
insertedNote.updateRequiredStatsCacheForPlayer(*MyPlayer);
ASSERT_TRUE(AutoPlaceItemInInventory(*MyPlayer, insertedNote));
EXPECT_EQ(MyPlayer->_pNumInv, 2);
EXPECT_EQ(count_inventory_items_with_id(IDI_FULLNOTE), 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE1), 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE2), 0);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE3), 0);
EXPECT_EQ(count_positive_inv_grid_slots(), 2);
EXPECT_EQ(MyPlayer->InvGrid[0], 1);
EXPECT_EQ(MyPlayer->InvGrid[3], 2);
EXPECT_EQ(MyPlayer->InvList[0].IDidx, IDI_NOTE1);
EXPECT_EQ(MyPlayer->InvList[1].IDidx, IDI_FULLNOTE);
}
TEST_F(InvTest, ReorganizeInventoryDoesNotCombineNaKrulNotes)
{
if (!gbIsHellfire) return;
SNetInitializeProvider(SELCONN_LOOPBACK, nullptr);
clear_inventory();
place_inventory_item(0, 0, IDI_NOTE1);
place_inventory_item(1, 1, IDI_NOTE2);
place_inventory_item(2, 2, IDI_NOTE3);
ReorganizeInventory(*MyPlayer);
EXPECT_EQ(MyPlayer->_pNumInv, 3);
EXPECT_EQ(count_inventory_items_with_id(IDI_FULLNOTE), 0);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE1), 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE2), 1);
EXPECT_EQ(count_inventory_items_with_id(IDI_NOTE3), 1);
EXPECT_EQ(count_positive_inv_grid_slots(), 3);
}
} // namespace
} // namespace devilution

Write Preview
Loading…
Cancel
Save