From ed8abb6b714bf9ae7cbd0293ce744dfeb88e73c6 Mon Sep 17 00:00:00 2001 From: Marco Date: Thu, 21 Nov 2024 15:17:27 -0300 Subject: [PATCH 01/10] fix: initialize totalCost correctly and refactor blessing purchase logic (#3142) This fixes the initialization of the `totalCost` variable and refactors the blessing purchase logic. Improvements include replacing string concatenation with `string.format` for more efficient formatting, as well as adjustments for better code clarity and readability. The code is now more organized, and the calculation error has been fixed. --- data/libs/systems/blessing.lua | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/data/libs/systems/blessing.lua b/data/libs/systems/blessing.lua index fee0e6fe8e8..f5403fbb07b 100644 --- a/data/libs/systems/blessing.lua +++ b/data/libs/systems/blessing.lua @@ -258,8 +258,9 @@ Blessings.BuyAllBlesses = function(player) local hasToF = Blessings.Config.HasToF and player:hasBlessing(1) or true local missingBless = player:getBlessings(nil, donthavefilter) local missingBlessAmt = #missingBless + (hasToF and 0 or 1) - local totalCost - for i, bless in ipairs(missingBless) do + local totalCost = 0 + + for _, bless in ipairs(missingBless) do totalCost = totalCost + Blessings.getBlessingCost(player:getLevel(), true, bless.id >= 7) end @@ -274,19 +275,19 @@ Blessings.BuyAllBlesses = function(player) end if player:removeMoneyBank(totalCost) then - metrics.addCounter("balance_decrease", remainsPrice, { + metrics.addCounter("balance_decrease", totalCost, { player = player:getName(), context = "blessings", }) - for i, v in ipairs(missingBless) do - player:addBlessing(v.id, 1) + for _, bless in ipairs(missingBless) do + player:addBlessing(bless.id, 1) end - player:sendCancelMessage("You received the remaining " .. missingBlessAmt .. " blesses for a total of " .. totalCost .. " gold.") + player:sendCancelMessage(string.format("You received the remaining %d blesses for a total of %d gold.", missingBlessAmt, totalCost)) player:getPosition():sendMagicEffect(CONST_ME_HOLYAREA) else - player:sendCancelMessage("You don't have enough money. You need " .. totalCost .. " to buy all blesses.", cid) + player:sendCancelMessage(string.format("You don't have enough money. You need %d to buy all blesses.", totalCost)) player:getPosition():sendMagicEffect(CONST_ME_POFF) end From 6253d64f70b5109c4895479a42734a3f7b2f8d10 Mon Sep 17 00:00:00 2001 From: Marco Date: Thu, 21 Nov 2024 16:05:39 -0300 Subject: [PATCH 02/10] fix: prevent teleportation (#3143) This fixes an issue where the player was teleported even when a Lua script was blocking the action. --- src/game/movement/teleport.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/game/movement/teleport.cpp b/src/game/movement/teleport.cpp index 368bcf6e476..d11552220a3 100644 --- a/src/game/movement/teleport.cpp +++ b/src/game/movement/teleport.cpp @@ -80,29 +80,27 @@ void Teleport::addThing(int32_t, const std::shared_ptr &thing) { // Prevent infinity loop if (checkInfinityLoop(destTile)) { const Position &pos = getPosition(); - g_logger().warn("[Teleport:addThing] - " - "Infinity loop teleport at position: {}", - pos.toString()); + g_logger().warn("[Teleport:addThing] - Infinity loop teleport at position: {}", pos.toString()); return; } const MagicEffectClasses effect = Item::items[id].magicEffect; - if (const std::shared_ptr &creature = thing->getCreature()) { + if (const auto &creature = thing->getCreature()) { Position origPos = creature->getPosition(); g_game().internalCreatureTurn(creature, origPos.x > destPos.x ? DIRECTION_WEST : DIRECTION_EAST); - g_dispatcher().addWalkEvent([=] { - g_game().map.moveCreature(creature, destTile); - if (effect != CONST_ME_NONE) { - g_game().addMagicEffect(origPos, effect); - g_game().addMagicEffect(destTile->getPosition(), effect); - } - }); + g_game().map.moveCreature(creature, destTile); + + if (effect != CONST_ME_NONE) { + g_game().addMagicEffect(origPos, effect); + g_game().addMagicEffect(destTile->getPosition(), effect); + } } else if (const auto &item = thing->getItem()) { if (effect != CONST_ME_NONE) { g_game().addMagicEffect(destTile->getPosition(), effect); g_game().addMagicEffect(item->getPosition(), effect); } + g_game().internalMoveItem(getTile(), destTile, INDEX_WHEREEVER, item, item->getItemCount(), nullptr); } } From db2611b1f2a97caeaf96ff722fc2dc059d4f7d06 Mon Sep 17 00:00:00 2001 From: Marco Date: Thu, 21 Nov 2024 16:12:29 -0300 Subject: [PATCH 03/10] fix: item usage mechanics to obtain Phantasmal Jade (#3112) This fixes the item usage mechanics for obtaining the Phantasmal Jade mount. It ensures that the correct number of items are used, and the mount is granted when all requirements are met. Additionally, it automates the registration of item IDs to simplify the code and reduce redundancy. Close #3100 --- .../soul_war/action-reward_soul_war.lua | 32 ------------- .../items/usable_phantasmal_jade_items.lua | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+), 32 deletions(-) create mode 100644 data/scripts/actions/items/usable_phantasmal_jade_items.lua diff --git a/data-otservbr-global/scripts/quests/soul_war/action-reward_soul_war.lua b/data-otservbr-global/scripts/quests/soul_war/action-reward_soul_war.lua index fae3fc59794..652a74af84b 100644 --- a/data-otservbr-global/scripts/quests/soul_war/action-reward_soul_war.lua +++ b/data-otservbr-global/scripts/quests/soul_war/action-reward_soul_war.lua @@ -26,35 +26,3 @@ end rewardSoulWar:position({ x = 33620, y = 31400, z = 10 }) rewardSoulWar:register() - -local phantasmalJadeMount = Action() - -function phantasmalJadeMount.onUse(player, item, fromPosition, target, toPosition, isHotkey) - local soulWarQuest = player:soulWarQuestKV() - if soulWarQuest:get("panthasmal-jade-mount") then - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You already have Phantasmal Jade mount!") - return true - end - - if table.contains({ 34072, 34073, 34074 }, item.itemid) then - if player:getItemCount(34072) >= 4 and player:getItemCount(34073) == 1 and player:getItemCount(34074) == 1 then - player:removeItem(34072, 4) - player:removeItem(34073, 1) - player:removeItem(34074, 1) - player:addMount(167) - player:addAchievement("You got Horse Power") - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "Congratulations! You won Phantasmal Jade mount.") - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "Congratulations! You won You got Horse Power achievement.") - player:getPosition():sendMagicEffect(CONST_ME_HOLYDAMAGE) - soulWarQuest:set("panthasmal-jade-mount", true) - else - player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You don't have the necessary items!") - player:getPosition():sendMagicEffect(CONST_ME_POFF) - end - end - - return true -end - -phantasmalJadeMount:id(34072, 34073, 34074) -phantasmalJadeMount:register() diff --git a/data/scripts/actions/items/usable_phantasmal_jade_items.lua b/data/scripts/actions/items/usable_phantasmal_jade_items.lua new file mode 100644 index 00000000000..8075125df25 --- /dev/null +++ b/data/scripts/actions/items/usable_phantasmal_jade_items.lua @@ -0,0 +1,45 @@ +local config = { + requiredItems = { + [34072] = { key = "spectral-horseshoes", count = 4 }, + [34073] = { key = "spectral-saddle", count = 1 }, + [34074] = { key = "spectral-horse-tac", count = 1 }, + }, + + mountId = 167, +} + +local usablePhantasmalJadeItems = Action() + +function usablePhantasmalJadeItems.onUse(player, item, fromPosition, target, toPosition, isHotkey) + local itemInfo = config.requiredItems[item:getId()] + if not itemInfo then + return true + end + + if player:hasMount(config.mountId) then + return true + end + + local currentCount = (player:kv():get(itemInfo.key) or 0) + 1 + player:kv():set(itemInfo.key, currentCount) + player:getPosition():sendMagicEffect(CONST_ME_HOLYDAMAGE) + item:remove(1) + + for _, info in pairs(config.requiredItems) do + if (player:kv():get(info.key) or 0) < info.count then + return true + end + end + + player:addMount(config.mountId) + player:addAchievement("Natural Born Cowboy") + player:addAchievement("You got Horse Power") + player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "Phantasmal jade is now yours!") + return true +end + +for itemId, _ in pairs(config.requiredItems) do + usablePhantasmalJadeItems:id(itemId) +end + +usablePhantasmalJadeItems:register() From 34b6fa8e08ee0bb397fe7583b985ea4455e5e345 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Fri, 22 Nov 2024 11:53:38 -0300 Subject: [PATCH 04/10] fix: boss lever check god access (#3141) Check god access instead of normal, to ensure that tutors also count towards the cooldown --- data/libs/functions/boss_lever.lua | 2 +- src/utils/pugicast.hpp | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/data/libs/functions/boss_lever.lua b/data/libs/functions/boss_lever.lua index 40bc8e0b4b0..ae8a89d0e54 100644 --- a/data/libs/functions/boss_lever.lua +++ b/data/libs/functions/boss_lever.lua @@ -177,7 +177,7 @@ function BossLever:onUse(player) return true end - local isAccountNormal = creature:getAccountType() == ACCOUNT_TYPE_NORMAL + local isAccountNormal = creature:getAccountType() < ACCOUNT_TYPE_GAMEMASTER if isAccountNormal and creature:getLevel() < self.requiredLevel then local message = "All players need to be level " .. self.requiredLevel .. " or higher." creature:sendTextMessage(MESSAGE_EVENT_ADVANCE, message) diff --git a/src/utils/pugicast.hpp b/src/utils/pugicast.hpp index b59e95d37ab..91043596605 100644 --- a/src/utils/pugicast.hpp +++ b/src/utils/pugicast.hpp @@ -38,14 +38,12 @@ namespace pugi { // If the string could not be parsed as the specified type if (errorCode == std::errc::invalid_argument) { // Throw an exception indicating that the argument is invalid - logError(fmt::format("Invalid argument {}", str)); - throw std::invalid_argument("Invalid argument: " + std::string(str)); + logError(fmt::format("[{}] Invalid argument {}", __FUNCTION__, str)); } // If the parsed value is out of range for the specified type else if (errorCode == std::errc::result_out_of_range) { // Throw an exception indicating that the result is out of range - logError(fmt::format("Result out of range: {}", str)); - throw std::out_of_range("Result out of range: " + std::string(str)); + logError(fmt::format("[{}] Result out of range: {}", __FUNCTION__, str)); } // Return a default value if no exception is thrown From 419ce43b5627dab9c832072ba39ed8a39ea47990 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Sat, 23 Nov 2024 15:46:45 -0300 Subject: [PATCH 05/10] 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 06/10] 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 07/10] 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 08/10] 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 From 42bd15bac180466751ff5dcf9cd8e66edc60a1aa Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Tue, 3 Dec 2024 00:46:02 -0300 Subject: [PATCH 09/10] fix: docker gha build workflow (#3167) --- .github/workflows/build-docker.yml | 16 ---------------- docker/Dockerfile.arm | 4 ++-- docker/Dockerfile.x86 | 4 ++-- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/.github/workflows/build-docker.yml b/.github/workflows/build-docker.yml index d99ba82878c..7dd7b657708 100644 --- a/.github/workflows/build-docker.yml +++ b/.github/workflows/build-docker.yml @@ -47,14 +47,6 @@ jobs: with: install: true - - name: Cache Docker layers - uses: actions/cache@main - with: - path: /tmp/.buildx-cache - key: ${{ runner.os }}-x86-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-x86- - - name: Login to GitHub Container Registry uses: docker/login-action@v2 with: @@ -109,14 +101,6 @@ jobs: with: install: true - - name: Cache Docker layers - uses: actions/cache@main - with: - path: /tmp/.buildx-cache - key: ${{ runner.os }}-arm-${{ github.sha }} - restore-keys: | - ${{ runner.os }}-arm- - - name: Build uses: docker/build-push-action@v3.2.0 with: diff --git a/docker/Dockerfile.arm b/docker/Dockerfile.arm index ef159ba142f..2fca1b525a0 100644 --- a/docker/Dockerfile.arm +++ b/docker/Dockerfile.arm @@ -1,5 +1,5 @@ # Stage 1: Download all dependencies -FROM ubuntu:23.04 AS dependencies +FROM ubuntu:24.04 AS dependencies RUN apt-get update && apt-get install -y --no-install-recommends cmake git \ unzip build-essential ca-certificates curl zip unzip tar \ @@ -30,7 +30,7 @@ WORKDIR /srv RUN export VCPKG_ROOT=/opt/vcpkg/ && VCPKG_FORCE_SYSTEM_BINARIES=1 cmake --preset linux-release && cmake --build --preset linux-release # Stage 3: load data and execute -FROM ubuntu:23.04 +FROM ubuntu:24.04 VOLUME [ "/data" ] diff --git a/docker/Dockerfile.x86 b/docker/Dockerfile.x86 index 3b035ecfd37..9e75c1078dd 100644 --- a/docker/Dockerfile.x86 +++ b/docker/Dockerfile.x86 @@ -1,5 +1,5 @@ # Stage 1: Download all dependencies -FROM ubuntu:23.04 AS dependencies +FROM ubuntu:24.04 AS dependencies RUN apt-get update && apt-get install -y --no-install-recommends cmake git \ unzip build-essential ca-certificates curl zip unzip tar \ @@ -30,7 +30,7 @@ WORKDIR /srv RUN export VCPKG_ROOT=/opt/vcpkg/ && cmake --preset linux-release && cmake --build --preset linux-release # Stage 3: load data and execute -FROM ubuntu:23.04 +FROM ubuntu:24.04 VOLUME [ "/data" ] COPY --from=build /srv/build/linux-release/bin/canary /bin/canary From b54a712ffa6c9ae2f0250b74d2f65ed51e53ba6b Mon Sep 17 00:00:00 2001 From: Renato Machado Date: Thu, 5 Dec 2024 20:02:07 -0300 Subject: [PATCH 10/10] fix: check creatures crash (#3168) --- src/game/game.cpp | 30 ++++++++++++++++-------------- src/game/scheduling/task.hpp | 1 + 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/game/game.cpp b/src/game/game.cpp index e69654a47ad..7a492348649 100644 --- a/src/game/game.cpp +++ b/src/game/game.cpp @@ -72,7 +72,7 @@ #include -std::vector> checkCreatureLists[EVENT_CREATURECOUNT]; +std::vector> checkCreatureLists[EVENT_CREATURECOUNT]; namespace InternalGame { void sendBlockEffect(BlockType_t blockType, CombatType_t combatType, const Position &targetPos, const std::shared_ptr &source) { @@ -6461,16 +6461,15 @@ void Game::addCreatureCheck(const std::shared_ptr &creature) { creature->creatureCheck.store(true); - if (creature->inCheckCreaturesVector.load()) { + if (creature->inCheckCreaturesVector.exchange(true)) { // already in a vector return; } - creature->inCheckCreaturesVector.store(true); - - creature->safeCall([this, creature] { - checkCreatureLists[uniform_random(0, EVENT_CREATURECOUNT - 1)].emplace_back(creature); - }); + g_dispatcher().addEvent([this, index = uniform_random(0, EVENT_CREATURECOUNT - 1), creature] { + checkCreatureLists[index].emplace_back(creature); + }, + "Game::addCreatureCheck"); } void Game::removeCreatureCheck(const std::shared_ptr &creature) { @@ -6484,15 +6483,18 @@ void Game::checkCreatures() { metrics::method_latency measure(__METRICS_METHOD_NAME__); static size_t index = 0; - std::erase_if(checkCreatureLists[index], [this](const std::shared_ptr creature) { - if (creature->creatureCheck && creature->isAlive()) { - creature->onThink(EVENT_CREATURE_THINK_INTERVAL); - creature->onAttacking(EVENT_CREATURE_THINK_INTERVAL); - creature->executeConditions(EVENT_CREATURE_THINK_INTERVAL); - return false; + std::erase_if(checkCreatureLists[index], [this](const std::weak_ptr &weak) { + if (const auto creature = weak.lock()) { + if (creature->creatureCheck && creature->isAlive()) { + creature->onThink(EVENT_CREATURE_THINK_INTERVAL); + creature->onAttacking(EVENT_CREATURE_THINK_INTERVAL); + creature->executeConditions(EVENT_CREATURE_THINK_INTERVAL); + return false; + } + + creature->inCheckCreaturesVector = false; } - creature->inCheckCreaturesVector = false; return true; }); diff --git a/src/game/scheduling/task.hpp b/src/game/scheduling/task.hpp index c01dbe2f676..6cd9eef342d 100644 --- a/src/game/scheduling/task.hpp +++ b/src/game/scheduling/task.hpp @@ -75,6 +75,7 @@ class Task { "Game::createInfluencedMonsters", "Game::updateCreatureWalk", "Game::updateForgeableMonsters", + "Game::addCreatureCheck", "GlobalEvents::think", "LuaEnvironment::executeTimerEvent", "Modules::executeOnRecvbyte",