diff --git a/Source/inv.cpp b/Source/inv.cpp index 398599995..57e7c1271 100644 --- a/Source/inv.cpp +++ b/Source/inv.cpp @@ -1938,28 +1938,28 @@ void ConsumeScroll(Player &player) { const spell_id spellId = player.executedSpell.spellId; - // Try to remove the scroll from selected inventory slot - int itemSlot = player.executedSpell.spellFrom; - int itemIndex = 0; - Item *item; - if (itemSlot <= INVITEM_INV_LAST) { - itemIndex = itemSlot - INVITEM_INV_FIRST; - item = &player.InvList[itemIndex]; - } else { - itemIndex = itemSlot - INVITEM_BELT_FIRST; - item = &player.SpdList[itemIndex]; - } - const auto isCurrentSpell = [spellId](const Item &item) { return item.isScrollOf(spellId) || item.isRuneOf(spellId); }; - if (!item->isEmpty() && isCurrentSpell(*item)) { - if (itemSlot <= INVITEM_INV_LAST) - player.RemoveInvItem(static_cast(itemIndex)); - else + // Try to remove the scroll from selected inventory slot + const int8_t itemSlot = player.executedSpell.spellFrom; + if (itemSlot >= INVITEM_INV_FIRST && itemSlot <= INVITEM_INV_LAST) { + const int itemIndex = itemSlot - INVITEM_INV_FIRST; + const Item *item = &player.InvList[itemIndex]; + if (!item->isEmpty() && isCurrentSpell(*item)) { + player.RemoveInvItem(itemIndex); + return; + } + } else if (itemSlot >= INVITEM_BELT_FIRST && itemSlot <= INVITEM_BELT_LAST) { + const int itemIndex = itemSlot - INVITEM_BELT_FIRST; + const Item *item = &player.SpdList[itemIndex]; + if (!item->isEmpty() && isCurrentSpell(*item)) { player.RemoveSpdBarItem(itemIndex); - return; + return; + } + } else if (itemSlot != 0) { + app_fatal(StrCat("ConsumeScroll: Invalid item index ", itemSlot)); } // Didn't find it at the selected slot, take the first one we find diff --git a/test/inv_test.cpp b/test/inv_test.cpp index 443b5b68d..fe2440f4e 100644 --- a/test/inv_test.cpp +++ b/test/inv_test.cpp @@ -206,13 +206,32 @@ TEST_F(InvTest, RemoveSpdBarItem) } // Test removing a scroll from the inventory -TEST_F(InvTest, RemoveCurrentSpellScroll_inventory) +TEST_F(InvTest, RemoveCurrentSpellScrollFromInventory) { clear_inventory(); // Put a firebolt scroll into the inventory MyPlayer->_pNumInv = 1; MyPlayer->executedSpell.spellId = static_cast(SPL_FIREBOLT); + MyPlayer->executedSpell.spellFrom = INVITEM_INV_FIRST; + MyPlayer->InvList[0]._itype = ItemType::Misc; + MyPlayer->InvList[0]._iMiscId = IMISC_SCROLL; + MyPlayer->InvList[0]._iSpell = SPL_FIREBOLT; + + ConsumeScroll(*MyPlayer); + EXPECT_EQ(MyPlayer->InvGrid[0], 0); + EXPECT_EQ(MyPlayer->_pNumInv, 0); +} + +// Test removing the first matching scroll from inventory +TEST_F(InvTest, RemoveCurrentSpellScrollFromInventoryFirstMatch) +{ + clear_inventory(); + + // Put a firebolt scroll into the inventory + MyPlayer->_pNumInv = 1; + MyPlayer->executedSpell.spellId = static_cast(SPL_FIREBOLT); + MyPlayer->executedSpell.spellFrom = 0; // any matching scroll MyPlayer->InvList[0]._itype = ItemType::Misc; MyPlayer->InvList[0]._iMiscId = IMISC_SCROLL; MyPlayer->InvList[0]._iSpell = SPL_FIREBOLT; @@ -233,6 +252,27 @@ TEST_F(InvTest, RemoveCurrentSpellScroll_belt) } // Put a firebolt scroll into the belt MyPlayer->executedSpell.spellId = static_cast(SPL_FIREBOLT); + MyPlayer->executedSpell.spellFrom = INVITEM_BELT_FIRST + 3; + MyPlayer->SpdList[3]._itype = ItemType::Misc; + MyPlayer->SpdList[3]._iMiscId = IMISC_SCROLL; + MyPlayer->SpdList[3]._iSpell = SPL_FIREBOLT; + + ConsumeScroll(*MyPlayer); + EXPECT_TRUE(MyPlayer->SpdList[3].isEmpty()); +} + +// Test removing the first matching scroll from the belt +TEST_F(InvTest, RemoveCurrentSpellScrollFirstMatchFromBelt) +{ + SNetInitializeProvider(SELCONN_LOOPBACK, nullptr); + + // Clear the belt + for (int i = 0; i < MaxBeltItems; i++) { + MyPlayer->SpdList[i].clear(); + } + // Put a firebolt scroll into the belt + MyPlayer->executedSpell.spellId = static_cast(SPL_FIREBOLT); + MyPlayer->executedSpell.spellFrom = 0; // any matching scroll MyPlayer->SpdList[3]._itype = ItemType::Misc; MyPlayer->SpdList[3]._iMiscId = IMISC_SCROLL; MyPlayer->SpdList[3]._iSpell = SPL_FIREBOLT;