Browse Source

Refactor inventory/belt item presence and removal

Introduces new functions to check for presence of and remove player items.
These functions do not rely on or expose item indices.

They're implemented as free functions instead of Player methods due to
complicated include dependencies between `player.h`, `inv.h`, and
`inv_iterators.h`. We should probably look into cleaning this up at some
point.
pull/4663/head
Gleb Mazovetskiy 4 years ago committed by Anders Jenbo
parent
commit
628406a44c
  1. 32
      Source/inv.cpp
  2. 149
      Source/inv.h
  3. 1
      Source/inv_iterators.hpp
  4. 4
      Source/monster.cpp
  5. 2
      Source/objects.cpp
  6. 23
      Source/player.cpp
  7. 8
      Source/player.h
  8. 2
      Source/spells.cpp
  9. 32
      Source/towners.cpp
  10. 8
      test/inv_test.cpp

32
Source/inv.cpp

@ -862,17 +862,17 @@ void TryCombineNaKrulNotes(Player &player, Item &noteItem)
return;
}
for (auto note : notes) {
if (idx != note && !player.HasItem(note)) {
for (_item_indexes note : notes) {
if (idx != note && !HasInventoryItemWithId(player, note)) {
return; // the player doesn't have all notes
}
}
MyPlayer->Say(HeroSpeech::JustWhatIWasLookingFor, 10);
for (auto note : notes) {
for (_item_indexes note : notes) {
if (idx != note) {
player.TryRemoveInvItemById(note);
RemoveInventoryItemById(player, note);
}
}
@ -1943,30 +1943,6 @@ int8_t CheckInvHLight()
return rv;
}
void RemoveScroll(Player &player)
{
const spell_id spellId = player._pSpell;
const auto isCurrentSpell = [spellId](const Item &item) {
return item.isScrollOf(spellId);
};
{
const InventoryPlayerItemsRange items { player };
const auto scrollIt = std::find_if(items.begin(), items.end(), isCurrentSpell);
if (scrollIt != items.end()) {
player.RemoveInvItem(static_cast<int>(scrollIt.Index()));
return;
}
}
{
const BeltPlayerItemsRange items { player };
const auto scrollIt = std::find_if(items.begin(), items.end(), isCurrentSpell);
if (scrollIt != items.end()) {
player.RemoveSpdBarItem(static_cast<int>(scrollIt.Index()));
return;
}
}
}
bool UseScroll(const spell_id spell)
{
if (pcurs != CURSOR_HAND)

149
Source/inv.h

@ -8,6 +8,7 @@
#include <cstdint>
#include "engine/point.hpp"
#include "inv_iterators.hpp"
#include "items.h"
#include "palette.h"
#include "player.h"
@ -219,7 +220,6 @@ int InvPutItem(Player &player, Point position, Item &item);
int SyncPutItem(Player &player, Point position, int idx, uint16_t icreateinfo, int iseed, int Id, int dur, int mdur, int ch, int mch, int ivalue, uint32_t ibuff, int toHit, int maxDam, int minStr, int minMag, int minDex, int ac);
int SyncDropItem(Point position, int idx, uint16_t icreateinfo, int iseed, int id, int dur, int mdur, int ch, int mch, int ivalue, uint32_t ibuff, int toHit, int maxDam, int minStr, int minMag, int minDex, int ac);
int8_t CheckInvHLight();
void RemoveScroll(Player &player);
bool UseScroll(spell_id spell);
void UseStaffCharge(Player &player);
bool UseStaff(spell_id spell);
@ -235,6 +235,153 @@ int CalculateGold(Player &player);
*/
Size GetInventorySize(const Item &item);
/**
* @brief Checks whether the player has an inventory item matching the predicate.
*/
template <typename Predicate>
bool HasInventoryItem(Player &player, Predicate &&predicate)
{
const InventoryPlayerItemsRange items { player };
return std::find_if(items.begin(), items.end(), std::forward<Predicate>(predicate)) != items.end();
}
/**
* @brief Checks whether the player has a belt item matching the predicate.
*/
template <typename Predicate>
bool HasBeltItem(Player &player, Predicate &&predicate)
{
const BeltPlayerItemsRange items { player };
return std::find_if(items.begin(), items.end(), std::forward<Predicate>(predicate)) != items.end();
}
/**
* @brief Checks whether the player has an inventory or a belt item matching the predicate.
*/
template <typename Predicate>
bool HasInventoryOrBeltItem(Player &player, Predicate &&predicate)
{
return HasInventoryItem(player, predicate) || HasBeltItem(player, predicate);
}
/**
* @brief Checks whether the player has an inventory item with the given ID (IDidx).
*/
inline bool HasInventoryItemWithId(Player &player, _item_indexes id)
{
return HasInventoryItem(player, [id](const Item &item) {
return item.IDidx == id;
});
}
/**
* @brief Checks whether the player has a belt item with the given ID (IDidx).
*/
inline bool HasBeltItemWithId(Player &player, _item_indexes id)
{
return HasBeltItem(player, [id](const Item &item) {
return item.IDidx == id;
});
}
/**
* @brief Checks whether the player has an inventory or a belt item with the given ID (IDidx).
*/
inline bool HasInventoryOrBeltItemWithId(Player &player, _item_indexes id)
{
return HasInventoryItemWithId(player, id) || HasBeltItemWithId(player, id);
}
/**
* @brief Removes the first inventory item matching the predicate.
*
* @return Whether an item was found and removed.
*/
template <typename Predicate>
bool RemoveInventoryItem(Player &player, Predicate &&predicate)
{
const InventoryPlayerItemsRange items { player };
const auto it = std::find_if(items.begin(), items.end(), std::forward<Predicate>(predicate));
if (it == items.end())
return false;
player.RemoveInvItem(static_cast<int>(it.Index()));
return true;
}
/**
* @brief Removes the first belt item matching the predicate.
*
* @return Whether an item was found and removed.
*/
template <typename Predicate>
bool RemoveBeltItem(Player &player, Predicate &&predicate)
{
const BeltPlayerItemsRange items { player };
const auto it = std::find_if(items.begin(), items.end(), std::forward<Predicate>(predicate));
if (it == items.end())
return false;
player.RemoveSpdBarItem(static_cast<int>(it.Index()));
return true;
}
/**
* @brief Removes the first inventory or belt item matching the predicate.
*
* @return Whether an item was found and removed.
*/
template <typename Predicate>
bool RemoveInventoryOrBeltItem(Player &player, Predicate &&predicate)
{
return RemoveInventoryItem(player, predicate) || RemoveBeltItem(player, predicate);
}
/**
* @brief Removes the first inventory item with the given id (IDidx).
*
* @return Whether an item was found and removed.
*/
inline bool RemoveInventoryItemById(Player &player, _item_indexes id)
{
return RemoveInventoryItem(player, [id](const Item &item) {
return item.IDidx == id;
});
}
/**
* @brief Removes the first belt item with the given id (IDidx).
*
* @return Whether an item was found and removed.
*/
inline bool RemoveBeltItemById(Player &player, _item_indexes id)
{
return RemoveInventoryItem(player, [id](const Item &item) {
return item.IDidx == id;
});
}
/**
* @brief Removes the first inventory or belt item with the given id (IDidx).
*
* @return Whether an item was found and removed.
*/
inline bool RemoveInventoryOrBeltItemById(Player &player, _item_indexes id)
{
return RemoveInventoryItemById(player, id) || RemoveBeltItemById(player, id);
}
/**
* @brief Removes the first inventory or belt scroll with the player's current spell.
*
* @return Whether a scroll was found and removed.
*/
inline bool RemoveCurrentSpellScroll(Player &player)
{
const spell_id spellId = player._pSpell;
return RemoveInventoryOrBeltItem(player, [spellId](const Item &item) {
return item.isScrollOf(spellId);
});
}
/* data */
} // namespace devilution

1
Source/inv_iterators.hpp

@ -5,7 +5,6 @@
#include <utility>
#include <vector>
#include "inv.h"
#include "items.h"
#include "player.h"

4
Source/monster.cpp

@ -4877,14 +4877,14 @@ void TalktoMonster(Monster &monster)
}
if (Quests[Q_LTBANNER].IsAvailable() && Quests[Q_LTBANNER]._qvar1 == 2) {
if (player.TryRemoveInvItemById(IDI_BANNER)) {
if (RemoveInventoryItemById(player, IDI_BANNER)) {
Quests[Q_LTBANNER]._qactive = QUEST_DONE;
monster.mtalkmsg = TEXT_BANNER12;
monster._mgoal = MGOAL_INQUIRING;
}
}
if (Quests[Q_VEIL].IsAvailable() && monster.mtalkmsg >= TEXT_VEIL9) {
if (player.TryRemoveInvItemById(IDI_GLDNELIX)) {
if (RemoveInventoryItemById(player, IDI_GLDNELIX)) {
monster.mtalkmsg = TEXT_VEIL11;
monster._mgoal = MGOAL_INQUIRING;
}

2
Source/objects.cpp

@ -2633,7 +2633,7 @@ void OperatePedistal(int pnum, int i)
return;
}
if (Objects[i]._oVar6 == 3 || !Players[pnum].TryRemoveInvItemById(IDI_BLDSTONE)) {
if (Objects[i]._oVar6 == 3 || !RemoveInventoryItemById(Players[pnum], IDI_BLDSTONE)) {
return;
}

23
Source/player.cpp

@ -1886,19 +1886,6 @@ void Player::CalcScrolls()
EnsureValidReadiedSpell(*this);
}
bool Player::HasItem(int item, int *idx) const
{
for (int i = 0; i < _pNumInv; i++) {
if (InvList[i].IDidx == item) {
if (idx != nullptr)
*idx = i;
return true;
}
}
return false;
}
void Player::RemoveInvItem(int iv, bool calcScrolls)
{
// Iterate through invGrid and remove every reference to item
@ -1931,16 +1918,6 @@ void Player::RemoveInvItem(int iv, bool calcScrolls)
}
}
bool Player::TryRemoveInvItemById(int item)
{
int idx;
if (HasItem(item, &idx)) {
RemoveInvItem(idx);
return true;
}
return false;
}
void Player::RemoveSpdBarItem(int iv)
{
SpdList[iv].clear();

8
Source/player.h

@ -359,8 +359,6 @@ struct Player {
&& _pDexterity >= item._iMinDex;
}
bool HasItem(int item, int *idx = nullptr) const;
/**
* @brief Remove an item from player inventory
* @param iv invList index of item to be removed
@ -368,12 +366,6 @@ struct Player {
*/
void RemoveInvItem(int iv, bool calcScrolls = true);
/**
* @brief Remove an item from player inventory and return true if the player has the item, return false otherwise
* @param item IDidx of item to be removed
*/
bool TryRemoveInvItemById(int item);
void RemoveSpdBarItem(int iv);
/**

2
Source/spells.cpp

@ -181,7 +181,7 @@ void UseMana(int id, spell_id sn)
case RSPLTYPE_INVALID:
break;
case RSPLTYPE_SCROLL:
RemoveScroll(myPlayer);
RemoveCurrentSpellScroll(myPlayer);
break;
case RSPLTYPE_CHARGES:
UseStaffCharge(myPlayer);

32
Source/towners.cpp

@ -328,7 +328,7 @@ void TalkToBarOwner(Player &player, Towner &barOwner)
return;
}
if (bannerQuest._qvar2 == 1 && player.TryRemoveInvItemById(IDI_BANNER)) {
if (bannerQuest._qvar2 == 1 && RemoveInventoryItemById(player, IDI_BANNER)) {
bannerQuest._qactive = QUEST_DONE;
bannerQuest._qvar1 = 3;
SpawnUnique(UITEM_HARCREST, barOwner.position + Direction::SouthWest);
@ -375,7 +375,7 @@ void TalkToBlackSmith(Player &player, Towner &blackSmith)
return;
}
if (Quests[Q_ROCK]._qvar2 == 1 && player.TryRemoveInvItemById(IDI_ROCK)) {
if (Quests[Q_ROCK]._qvar2 == 1 && RemoveInventoryItemById(player, IDI_ROCK)) {
Quests[Q_ROCK]._qactive = QUEST_DONE;
SpawnUnique(UITEM_INFRARING, blackSmith.position + Direction::SouthWest);
InitQTextMsg(TEXT_INFRA7);
@ -395,7 +395,7 @@ void TalkToBlackSmith(Player &player, Towner &blackSmith)
return;
}
if (Quests[Q_ANVIL]._qvar2 == 1 && player.TryRemoveInvItemById(IDI_ANVIL)) {
if (Quests[Q_ANVIL]._qvar2 == 1 && RemoveInventoryItemById(player, IDI_ANVIL)) {
Quests[Q_ANVIL]._qactive = QUEST_DONE;
SpawnUnique(UITEM_GRISWOLD, blackSmith.position + Direction::SouthWest);
InitQTextMsg(TEXT_ANVIL7);
@ -411,7 +411,7 @@ void TalkToBlackSmith(Player &player, Towner &blackSmith)
void TalkToWitch(Player &player, Towner & /*witch*/)
{
if (Quests[Q_MUSHROOM]._qactive != QUEST_NOTAVAIL) {
if (Quests[Q_MUSHROOM]._qactive == QUEST_INIT && player.TryRemoveInvItemById(IDI_FUNGALTM)) {
if (Quests[Q_MUSHROOM]._qactive == QUEST_INIT && RemoveInventoryItemById(player, IDI_FUNGALTM)) {
Quests[Q_MUSHROOM]._qactive = QUEST_ACTIVE;
Quests[Q_MUSHROOM]._qlog = true;
Quests[Q_MUSHROOM]._qvar1 = QS_TOMEGIVEN;
@ -420,7 +420,7 @@ void TalkToWitch(Player &player, Towner & /*witch*/)
}
if (Quests[Q_MUSHROOM]._qactive == QUEST_ACTIVE) {
if (Quests[Q_MUSHROOM]._qvar1 >= QS_TOMEGIVEN && Quests[Q_MUSHROOM]._qvar1 < QS_MUSHGIVEN) {
if (player.TryRemoveInvItemById(IDI_MUSHROOM)) {
if (RemoveInventoryItemById(player, IDI_MUSHROOM)) {
Quests[Q_MUSHROOM]._qvar1 = QS_MUSHGIVEN;
QuestDialogTable[TOWN_HEALER][Q_MUSHROOM] = TEXT_MUSH3;
QuestDialogTable[TOWN_WITCH][Q_MUSHROOM] = TEXT_NONE;
@ -435,12 +435,12 @@ void TalkToWitch(Player &player, Towner & /*witch*/)
}
}
if (Quests[Q_MUSHROOM]._qvar1 >= QS_MUSHGIVEN) {
if (player.HasItem(IDI_BRAIN)) {
if (HasInventoryItemWithId(player, IDI_BRAIN)) {
Quests[Q_MUSHROOM]._qmsg = TEXT_MUSH11;
InitQTextMsg(TEXT_MUSH11);
return;
}
if (player.HasItem(IDI_SPECELIX)) {
if (HasInventoryItemWithId(player, IDI_SPECELIX)) {
InitQTextMsg(TEXT_MUSH12);
Quests[Q_MUSHROOM]._qactive = QUEST_DONE;
AllItemsList[IDI_SPECELIX].iUsable = true; /// BUGFIX: This will cause the elixir to be usable in the next game
@ -456,7 +456,7 @@ void TalkToWitch(Player &player, Towner & /*witch*/)
void TalkToBarmaid(Player &player, Towner & /*barmaid*/)
{
if (!player._pLvlVisited[21] && player.HasItem(IDI_MAPOFDOOM)) {
if (!player._pLvlVisited[21] && HasInventoryItemWithId(player, IDI_MAPOFDOOM)) {
Quests[Q_GRAVE]._qactive = QUEST_ACTIVE;
Quests[Q_GRAVE]._qlog = true;
Quests[Q_GRAVE]._qmsg = TEXT_GRAVE8;
@ -492,7 +492,7 @@ void TalkToHealer(Player &player, Towner &healer)
}
}
if (Quests[Q_MUSHROOM]._qactive == QUEST_ACTIVE) {
if (Quests[Q_MUSHROOM]._qvar1 >= QS_MUSHGIVEN && Quests[Q_MUSHROOM]._qvar1 < QS_BRAINGIVEN && player.TryRemoveInvItemById(IDI_BRAIN)) {
if (Quests[Q_MUSHROOM]._qvar1 >= QS_MUSHGIVEN && Quests[Q_MUSHROOM]._qvar1 < QS_BRAINGIVEN && RemoveInventoryItemById(player, IDI_BRAIN)) {
SpawnQuestItem(IDI_SPECELIX, healer.position + Displacement { 0, 1 }, 0, 0);
InitQTextMsg(TEXT_MUSH4);
Quests[Q_MUSHROOM]._qvar1 = QS_BRAINGIVEN;
@ -515,7 +515,7 @@ void TalkToStoryteller(Player &player, Towner & /*storyteller*/)
{
auto &betrayerQuest = Quests[Q_BETRAYER];
if (!gbIsMultiplayer) {
if (betrayerQuest._qactive == QUEST_INIT && player.TryRemoveInvItemById(IDI_LAZSTAFF)) {
if (betrayerQuest._qactive == QUEST_INIT && RemoveInventoryItemById(player, IDI_LAZSTAFF)) {
InitQTextMsg(TEXT_VILE1);
betrayerQuest._qlog = true;
betrayerQuest._qactive = QUEST_ACTIVE;
@ -582,7 +582,7 @@ void TalkToFarmer(Player &player, Towner &farmer)
switch (quest._qactive) {
case QUEST_NOTAVAIL:
case QUEST_INIT:
if (player.HasItem(IDI_RUNEBOMB)) {
if (HasInventoryItemWithId(player, IDI_RUNEBOMB)) {
InitQTextMsg(TEXT_FARMER2);
quest._qactive = QUEST_ACTIVE;
quest._qvar1 = 1;
@ -615,7 +615,7 @@ void TalkToFarmer(Player &player, Towner &farmer)
NetSendCmdQuest(true, quest);
break;
case QUEST_ACTIVE:
InitQTextMsg(player.HasItem(IDI_RUNEBOMB) ? TEXT_FARMER2 : TEXT_FARMER3);
InitQTextMsg(HasInventoryItemWithId(player, IDI_RUNEBOMB) ? TEXT_FARMER2 : TEXT_FARMER3);
break;
case QUEST_DONE:
InitQTextMsg(TEXT_FARMER4);
@ -635,14 +635,14 @@ void TalkToFarmer(Player &player, Towner &farmer)
void TalkToCowFarmer(Player &player, Towner &cowFarmer)
{
if (player.TryRemoveInvItemById(IDI_GREYSUIT)) {
if (RemoveInventoryItemById(player, IDI_GREYSUIT)) {
InitQTextMsg(TEXT_JERSEY7);
return;
}
auto &quest = Quests[Q_JERSEY];
if (player.TryRemoveInvItemById(IDI_BROWNSUIT)) {
if (RemoveInventoryItemById(player, IDI_BROWNSUIT)) {
SpawnUnique(UITEM_BOVINE, cowFarmer.position + Direction::SouthEast);
InitQTextMsg(TEXT_JERSEY8);
quest._qactive = QUEST_DONE;
@ -652,7 +652,7 @@ void TalkToCowFarmer(Player &player, Towner &cowFarmer)
return;
}
if (player.HasItem(IDI_RUNEBOMB)) {
if (HasInventoryItemWithId(player, IDI_RUNEBOMB)) {
InitQTextMsg(TEXT_JERSEY5);
quest._qactive = QUEST_ACTIVE;
quest._qvar1 = 1;
@ -721,7 +721,7 @@ void TalkToGirl(Player &player, Towner &girl)
{
auto &quest = Quests[Q_GIRL];
if (quest._qactive != QUEST_DONE && player.TryRemoveInvItemById(IDI_THEODORE)) {
if (quest._qactive != QUEST_DONE && RemoveInventoryItemById(player, IDI_THEODORE)) {
InitQTextMsg(TEXT_GIRL4);
CreateAmulet(girl.position, 13, true, false);
quest._qlog = false;

8
test/inv_test.cpp

@ -192,7 +192,7 @@ TEST(Inv, RemoveSpdBarItem)
}
// Test removing a scroll from the inventory
TEST(Inv, RemoveScroll_inventory)
TEST(Inv, RemoveCurrentSpellScroll_inventory)
{
clear_inventory();
@ -203,13 +203,13 @@ TEST(Inv, RemoveScroll_inventory)
MyPlayer->InvList[0]._iMiscId = IMISC_SCROLL;
MyPlayer->InvList[0]._iSpell = SPL_FIREBOLT;
RemoveScroll(*MyPlayer);
RemoveCurrentSpellScroll(*MyPlayer);
EXPECT_EQ(MyPlayer->InvGrid[0], 0);
EXPECT_EQ(MyPlayer->_pNumInv, 0);
}
// Test removing a scroll from the belt
TEST(Inv, RemoveScroll_belt)
TEST(Inv, RemoveCurrentSpellScroll_belt)
{
// Clear the belt
for (int i = 0; i < MAXBELTITEMS; i++) {
@ -221,7 +221,7 @@ TEST(Inv, RemoveScroll_belt)
MyPlayer->SpdList[3]._iMiscId = IMISC_SCROLL;
MyPlayer->SpdList[3]._iSpell = SPL_FIREBOLT;
RemoveScroll(*MyPlayer);
RemoveCurrentSpellScroll(*MyPlayer);
EXPECT_TRUE(MyPlayer->SpdList[3].isEmpty());
}

Loading…
Cancel
Save