From 419ce43b5627dab9c832072ba39ed8a39ea47990 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Sat, 23 Nov 2024 15:46:45 -0300 Subject: [PATCH 1/4] fix: wrong loot items from bosses (#3148) --- .../monster/quests/ferumbras_ascension/bosses/shulgrax.lua | 1 - .../monster/quests/the_secret_library/bosses/ghulosh.lua | 1 - .../monster/quests/the_secret_library/lokathmor.lua | 1 - 3 files changed, 3 deletions(-) diff --git a/data-otservbr-global/monster/quests/ferumbras_ascension/bosses/shulgrax.lua b/data-otservbr-global/monster/quests/ferumbras_ascension/bosses/shulgrax.lua index 1eef0866ceb..2168dcc3bf2 100644 --- a/data-otservbr-global/monster/quests/ferumbras_ascension/bosses/shulgrax.lua +++ b/data-otservbr-global/monster/quests/ferumbras_ascension/bosses/shulgrax.lua @@ -81,7 +81,6 @@ monster.loot = { { id = 6558, chance = 10000 }, -- flask of demonic blood { id = 6558, chance = 10000 }, -- flask of demonic blood { id = 6558, chance = 10000 }, -- flask of demonic blood - { id = 17838, chance = 1800 }, -- unknown item { id = 3019, chance = 1000 }, -- demonbone amulet { id = 3026, chance = 12000, maxCount = 8 }, -- white pearl { id = 3029, chance = 12000, maxCount = 9 }, -- small sapphire diff --git a/data-otservbr-global/monster/quests/the_secret_library/bosses/ghulosh.lua b/data-otservbr-global/monster/quests/the_secret_library/bosses/ghulosh.lua index 60717045935..fe80352ff13 100644 --- a/data-otservbr-global/monster/quests/the_secret_library/bosses/ghulosh.lua +++ b/data-otservbr-global/monster/quests/the_secret_library/bosses/ghulosh.lua @@ -95,7 +95,6 @@ monster.loot = { { name = "butcher's axe", chance = 1000 }, { name = "dreaded cleaver", chance = 1000 }, { name = "mercenary sword", chance = 1000 }, - { id = 28341, chance = 1000 }, -- tessellated wall { name = "slightly rusted shield", chance = 5880 }, { name = "slightly rusted helmet", chance = 35290 }, { name = "epaulette", chance = 500 }, diff --git a/data-otservbr-global/monster/quests/the_secret_library/lokathmor.lua b/data-otservbr-global/monster/quests/the_secret_library/lokathmor.lua index 7df23258767..8ef0357fca8 100644 --- a/data-otservbr-global/monster/quests/the_secret_library/lokathmor.lua +++ b/data-otservbr-global/monster/quests/the_secret_library/lokathmor.lua @@ -100,7 +100,6 @@ monster.loot = { { name = "dreaded cleaver", chance = 30000 }, { name = "slightly rusted shield", chance = 26670 }, { name = "wand of inferno", chance = 30000 }, - { id = 28341, chance = 1000 }, -- tessellated wall { name = "sturdy book", chance = 1000 }, } From a77f3c3d2839159a00f114f85b3ad0ccb1935123 Mon Sep 17 00:00:00 2001 From: Pedro Cruz Date: Sun, 24 Nov 2024 00:58:58 -0300 Subject: [PATCH 2/4] fix: crash on try gem grade upgrade greater than 3 (#3151) Fixes the crash when trying to upgrade gem to a grade greater than 3 --- src/creatures/players/wheel/player_wheel.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/creatures/players/wheel/player_wheel.cpp b/src/creatures/players/wheel/player_wheel.cpp index 7d39720dc4d..165a43f0687 100644 --- a/src/creatures/players/wheel/player_wheel.cpp +++ b/src/creatures/players/wheel/player_wheel.cpp @@ -1448,8 +1448,13 @@ void PlayerWheel::improveGemGrade(WheelFragmentType_t fragmentType, uint8_t pos) return; } + if (value == 0 && quantity == 0) { + g_logger().error("[{}] Player {} trying to upgrade gem to grade greater than 3", std::source_location::current().function_name(), m_player.getName()); + return; + } + if (!m_player.hasItemCountById(fragmentId, quantity, true)) { - g_logger().error("[{}] Player {} does not have the required {} fragments with id {}", __FUNCTION__, m_player.getName(), quantity, fragmentId); + g_logger().error("[{}] Player {} does not have the required {} fragments with id {}", std::source_location::current().function_name(), m_player.getName(), quantity, fragmentId); return; } @@ -1477,7 +1482,7 @@ std::tuple PlayerWheel::getLesserGradeCost(uint8_t grade) const { case 3: return std::make_tuple(30000000, 30); default: - throw std::invalid_argument("Invalid level for Lesser Fragment."); + return {}; } } @@ -1490,7 +1495,7 @@ std::tuple PlayerWheel::getGreaterGradeCost(uint8_t grade) const { case 3: return std::make_tuple(75000000, 30); default: - throw std::invalid_argument("Invalid level for Greater Fragment."); + return {}; } } From 6a6a6883ec3dec5124a4214f6ab06bcbefb234d7 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Wed, 27 Nov 2024 11:16:52 -0300 Subject: [PATCH 3/4] perf: optimize getInbox usage and shared pointer handling in loops (#3150) This addresses performance issues by optimizing the use of shared pointers and removing redundant calls. Specifically, the `getInbox` call was removed from extensive loops to prevent unnecessary reference count increments, which were causing CPU overhead. Additionally, shared pointers used in recursive or iteration-heavy functions, such as the `ContainerIterator`, were changed to `const&` where applicable, reducing the impact of reference counting on performance. --- src/creatures/players/player.cpp | 8 +++-- src/game/game.cpp | 34 ++++++++++++-------- src/io/functions/iologindata_load_player.cpp | 12 +++++-- src/io/iomarket.cpp | 6 ++-- src/items/containers/container.cpp | 19 ++++++++--- src/items/containers/mailbox/mailbox.cpp | 4 ++- src/map/house/house.cpp | 5 +-- 7 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp index 3ef1d64e982..4335db2bd53 100644 --- a/src/creatures/players/player.cpp +++ b/src/creatures/players/player.cpp @@ -5039,10 +5039,14 @@ ItemsTierCountList Player::getDepotInboxItemsId() const { ItemsTierCountList itemMap; const auto &inboxPtr = getInbox(); - const auto &container = inboxPtr->getContainer(); + const auto &container = inboxPtr ? inboxPtr->getContainer() : nullptr; if (container) { for (ContainerIterator it = container->iterator(); it.hasNext(); it.advance()) { const auto &item = *it; + if (!item) { + continue; + } + (itemMap[item->getID()])[item->getTier()] += Item::countByType(item, -1); } } @@ -9073,7 +9077,7 @@ void Player::forgeFuseItems(ForgeAction_t actionType, uint16_t firstItemId, uint returnValue = g_game().internalAddItem(static_self_cast(), exaltationContainer, INDEX_WHEREEVER); if (returnValue != RETURNVALUE_NOERROR) { - g_logger().error("Failed to add exaltation chest to player with name {}", fmt::underlying(ITEM_EXALTATION_CHEST), getName()); + g_logger().error("Failed to add exaltation chest to player with name {}", getName()); sendCancelMessage(getReturnMessage(returnValue)); sendForgeError(RETURNVALUE_CONTACTADMINISTRATOR); return; diff --git a/src/game/game.cpp b/src/game/game.cpp index 068ecad4a50..e69654a47ad 100644 --- a/src/game/game.cpp +++ b/src/game/game.cpp @@ -9025,7 +9025,7 @@ void Game::playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t ite return; } - std::shared_ptr depotLocker = player->getDepotLocker(player->getLastDepotId()); + const std::shared_ptr &depotLocker = player->getDepotLocker(player->getLastDepotId()); if (depotLocker == nullptr) { offerStatus << "Depot locker is nullptr for player " << player->getName(); return; @@ -9108,6 +9108,7 @@ void Game::playerCancelMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 return; } + const auto &playerInbox = player->getInbox(); if (offer.type == MARKETACTION_BUY) { player->setBankBalance(player->getBankBalance() + offer.price * offer.amount); g_metrics().addCounter("balance_decrease", offer.price * offer.amount, { { "player", player->getName() }, { "context", "market_purchase" } }); @@ -9124,10 +9125,11 @@ void Game::playerCancelMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 player->getAccount()->addCoins(CoinType::Transferable, offer.amount, ""); } else if (it.stackable) { uint16_t tmpAmount = offer.amount; + while (tmpAmount > 0) { int32_t stackCount = std::min(it.stackSize, tmpAmount); const auto &item = Item::CreateItem(it.id, stackCount); - if (internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { break; } @@ -9147,7 +9149,7 @@ void Game::playerCancelMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 for (uint16_t i = 0; i < offer.amount; ++i) { const auto &item = Item::CreateItem(it.id, subType); - if (internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { break; } @@ -9205,35 +9207,41 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 return; } + const auto &playerInbox = player->getInbox(); + uint64_t totalPrice = offer.price * amount; // The player has an offer to by something and someone is going to sell to item type // so the market action is 'buy' as who created the offer is buying. if (offer.type == MARKETACTION_BUY) { - std::shared_ptr depotLocker = player->getDepotLocker(player->getLastDepotId()); + const std::shared_ptr &depotLocker = player->getDepotLocker(player->getLastDepotId()); if (depotLocker == nullptr) { offerStatus << "Depot locker is nullptr"; return; } - std::shared_ptr buyerPlayer = getPlayerByGUID(offer.playerId, true); + const std::shared_ptr &buyerPlayer = getPlayerByGUID(offer.playerId, true); if (!buyerPlayer) { offerStatus << "Failed to load buyer player " << player->getName(); return; } - if (!buyerPlayer->getAccount()) { + const auto &buyerPlayerAccount = buyerPlayer->getAccount(); + if (!buyerPlayerAccount) { player->sendTextMessage(MESSAGE_MARKET, "Cannot accept offer."); return; } - if (player == buyerPlayer || player->getAccount() == buyerPlayer->getAccount()) { + const auto &playerAccount = player->getAccount(); + if (player == buyerPlayer || playerAccount == buyerPlayerAccount) { player->sendTextMessage(MESSAGE_MARKET, "You cannot accept your own offer."); return; } + const auto &buyerPlayerInbox = buyerPlayer->getInbox(); + if (it.id == ITEM_STORE_COIN) { - auto [transferableCoins, error] = player->getAccount()->getCoins(CoinType::Transferable); + auto [transferableCoins, error] = playerAccount->getCoins(CoinType::Transferable); if (error != AccountErrors_t::Ok) { offerStatus << "Failed to load transferable coins for player " << player->getName(); @@ -9245,7 +9253,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 return; } - player->getAccount()->removeCoins( + playerAccount->removeCoins( CoinType::Transferable, amount, "Sold on Market" @@ -9280,7 +9288,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 while (tmpAmount > 0) { uint16_t stackCount = std::min(it.stackSize, tmpAmount); const auto &item = Item::CreateItem(it.id, stackCount); - if (internalAddItem(buyerPlayer->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (internalAddItem(buyerPlayerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { offerStatus << "Failed to add player inbox stackable item for buy offer for player " << player->getName(); break; @@ -9302,7 +9310,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 for (uint16_t i = 0; i < amount; ++i) { const auto &item = Item::CreateItem(it.id, subType); - if (internalAddItem(buyerPlayer->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (internalAddItem(buyerPlayerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { offerStatus << "Failed to add player inbox item for buy offer for player " << player->getName(); break; @@ -9353,7 +9361,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 const auto &item = Item::CreateItem(it.id, stackCount); if ( // Init-statement - auto ret = internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT); + auto ret = internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT); // Condition ret != RETURNVALUE_NOERROR ) { @@ -9381,7 +9389,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16 const auto &item = Item::CreateItem(it.id, subType); if ( // Init-statement - auto ret = internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT); + auto ret = internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT); // Condition ret != RETURNVALUE_NOERROR ) { diff --git a/src/io/functions/iologindata_load_player.cpp b/src/io/functions/iologindata_load_player.cpp index 7cb08a7b9a6..e724d0ceda9 100644 --- a/src/io/functions/iologindata_load_player.cpp +++ b/src/io/functions/iologindata_load_player.cpp @@ -679,8 +679,14 @@ void IOLoginDataLoad::loadPlayerInboxItems(const std::shared_ptr &player ItemsMap inboxItems; loadItems(inboxItems, result, player); - for (auto it = inboxItems.rbegin(), end = inboxItems.rend(); it != end; ++it) { - const std::pair, int32_t> &pair = it->second; + const auto &playerInbox = player->getInbox(); + if (!playerInbox) { + g_logger().warn("[{}] - Player inbox nullptr", __FUNCTION__); + return; + } + + for (const auto &it : std::ranges::reverse_view(inboxItems)) { + const std::pair, int32_t> &pair = it.second; const auto &item = pair.first; if (!item) { continue; @@ -688,7 +694,7 @@ void IOLoginDataLoad::loadPlayerInboxItems(const std::shared_ptr &player int32_t pid = pair.second; if (pid >= 0 && pid < 100) { - player->getInbox()->internalAddThing(item); + playerInbox->internalAddThing(item); item->startDecaying(); } else { auto inboxIt = inboxItems.find(pid); diff --git a/src/io/iomarket.cpp b/src/io/iomarket.cpp index ff2f9595d46..fe91b3f6d94 100644 --- a/src/io/iomarket.cpp +++ b/src/io/iomarket.cpp @@ -175,12 +175,14 @@ void IOMarket::processExpiredOffers(const DBResult_ptr &result, bool) { continue; } + const auto &playerInbox = player->getInbox(); + if (itemType.stackable) { uint16_t tmpAmount = amount; while (tmpAmount > 0) { uint16_t stackCount = std::min(100, tmpAmount); const auto &item = Item::CreateItem(itemType.id, stackCount); - if (g_game().internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (g_game().internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { g_logger().error("[{}] Ocurred an error to add item with id {} to player {}", __FUNCTION__, itemType.id, player->getName()); break; @@ -202,7 +204,7 @@ void IOMarket::processExpiredOffers(const DBResult_ptr &result, bool) { for (uint16_t i = 0; i < amount; ++i) { const auto &item = Item::CreateItem(itemType.id, subType); - if (g_game().internalAddItem(player->getInbox(), item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { + if (g_game().internalAddItem(playerInbox, item, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { break; } diff --git a/src/items/containers/container.cpp b/src/items/containers/container.cpp index 70ad1cbabbf..77a37942e37 100644 --- a/src/items/containers/container.cpp +++ b/src/items/containers/container.cpp @@ -78,9 +78,9 @@ std::shared_ptr Container::createBrowseField(const std::shared_ptrgetTile(); + const auto &tile = parent->getTile(); if (tile) { auto browseField = g_game().browseFields.find(tile); if (browseField != g_game().browseFields.end()) { @@ -385,7 +385,12 @@ uint32_t Container::getContainerHoldingCount() { bool Container::isHoldingItem(const std::shared_ptr &item) { for (ContainerIterator it = iterator(); it.hasNext(); it.advance()) { - if (*it == item) { + const auto &compareItem = *it; + if (!compareItem || !item) { + continue; + } + + if (compareItem == item) { return true; } } @@ -395,6 +400,10 @@ bool Container::isHoldingItem(const std::shared_ptr &item) { bool Container::isHoldingItemWithId(const uint16_t id) { for (ContainerIterator it = iterator(); it.hasNext(); it.advance()) { const auto &item = *it; + if (!item) { + continue; + } + if (item && item->getID() == id) { return true; } @@ -1024,9 +1033,9 @@ void ContainerIterator::advance() { return; } - auto currentItem = container->itemlist[top.index]; + const auto ¤tItem = container->itemlist[top.index]; if (currentItem) { - auto subContainer = currentItem->getContainer(); + const auto &subContainer = currentItem->getContainer(); if (subContainer && !subContainer->itemlist.empty()) { size_t newDepth = top.depth + 1; if (newDepth <= maxTraversalDepth) { diff --git a/src/items/containers/mailbox/mailbox.cpp b/src/items/containers/mailbox/mailbox.cpp index 9abe6d0f6a3..27562e92d7e 100644 --- a/src/items/containers/mailbox/mailbox.cpp +++ b/src/items/containers/mailbox/mailbox.cpp @@ -98,7 +98,9 @@ bool Mailbox::sendItem(const std::shared_ptr &item) const { text = item->getAttribute(ItemAttribute_t::TEXT); } if (player && item) { - if (g_game().internalMoveItem(item->getParent(), player->getInbox(), INDEX_WHEREEVER, item, item->getItemCount(), nullptr, FLAG_NOLIMIT) == RETURNVALUE_NOERROR) { + const auto &playerInbox = player->getInbox(); + const auto &itemParent = item->getParent(); + if (g_game().internalMoveItem(itemParent, playerInbox, INDEX_WHEREEVER, item, item->getItemCount(), nullptr, FLAG_NOLIMIT) == RETURNVALUE_NOERROR) { const auto &newItem = g_game().transformItem(item, item->getID() + 1); if (newItem && newItem->getID() == ITEM_LETTER_STAMPED && !writer.empty()) { newItem->setAttribute(ItemAttribute_t::WRITER, writer); diff --git a/src/map/house/house.cpp b/src/map/house/house.cpp index 4d6cd59df3b..6d93561172b 100644 --- a/src/map/house/house.cpp +++ b/src/map/house/house.cpp @@ -849,7 +849,7 @@ void Houses::payHouses(RentPeriod_t rentPeriod) const { if (house->getPayRentWarnings() < 7) { const int32_t daysLeft = 7 - house->getPayRentWarnings(); - std::shared_ptr letter = Item::CreateItem(ITEM_LETTER_STAMPED); + const std::shared_ptr &letter = Item::CreateItem(ITEM_LETTER_STAMPED); std::string period; switch (rentPeriod) { @@ -876,7 +876,8 @@ void Houses::payHouses(RentPeriod_t rentPeriod) const { std::ostringstream ss; ss << "Warning! \nThe " << period << " rent of " << house->getRent() << " gold for your house \"" << house->getName() << "\" is payable. Have it within " << daysLeft << " days or you will lose this house."; letter->setAttribute(ItemAttribute_t::TEXT, ss.str()); - g_game().internalAddItem(player->getInbox(), letter, INDEX_WHEREEVER, FLAG_NOLIMIT); + const auto &playerInbox = player->getInbox(); + g_game().internalAddItem(playerInbox, letter, INDEX_WHEREEVER, FLAG_NOLIMIT); house->setPayRentWarnings(house->getPayRentWarnings() + 1); } else { house->setOwner(0, true, player); From 30d209a56caf37d38f956c46ca5b9d3504d18a46 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 2 Dec 2024 23:06:43 -0300 Subject: [PATCH 4/4] fix: console error related to wrong "monster" to "self" (#3166) --- data-otservbr-global/scripts/lib/monster_functions.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data-otservbr-global/scripts/lib/monster_functions.lua b/data-otservbr-global/scripts/lib/monster_functions.lua index cdb56c5e177..0b362de27a9 100644 --- a/data-otservbr-global/scripts/lib/monster_functions.lua +++ b/data-otservbr-global/scripts/lib/monster_functions.lua @@ -1,7 +1,7 @@ function Monster:handleCobraOnSpawn() if Game.getStorageValue(Global.Storage.CobraFlask) >= os.time() then - monster:setHealth(monster:getMaxHealth() * 0.75) - monster:getPosition():sendMagicEffect(CONST_ME_GREEN_RINGS) + self:setHealth(self:getMaxHealth() * 0.75) + self:getPosition():sendMagicEffect(CONST_ME_GREEN_RINGS) else Game.setStorageValue(Global.Storage.CobraFlask, -1) end