Merge branch 'main' into feature/support_dat_items

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<Player>(), 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;

src/game/game.cpp

@@ -9030,7 +9030,7 @@ void Game::playerCreateMarketOffer(uint32_t playerId, uint8_t type, uint16_t ite
 			return;
 		}
 
-		std::shared_ptr<DepotLocker> depotLocker = player->getDepotLocker(player->getLastDepotId());
+		const std::shared_ptr<DepotLocker> &depotLocker = player->getDepotLocker(player->getLastDepotId());
 		if (depotLocker == nullptr) {
 			offerStatus << "Depot locker is nullptr for player " << player->getName();
 			return;
@@ -9113,6 +9113,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" } });
@@ -9129,10 +9130,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<int32_t>(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;
 				}
 
@@ -9152,7 +9154,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;
 				}
 
@@ -9210,35 +9212,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> depotLocker = player->getDepotLocker(player->getLastDepotId());
+		const std::shared_ptr<DepotLocker> &depotLocker = player->getDepotLocker(player->getLastDepotId());
 		if (depotLocker == nullptr) {
 			offerStatus << "Depot locker is nullptr";
 			return;
 		}
 
-		std::shared_ptr<Player> buyerPlayer = getPlayerByGUID(offer.playerId, true);
+		const std::shared_ptr<Player> &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();
@@ -9250,7 +9258,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16
 				return;
 			}
 
-			player->getAccount()->removeCoins(
+			playerAccount->removeCoins(
 				CoinType::Transferable,
 				amount,
 				"Sold on Market"
@@ -9285,7 +9293,7 @@ void Game::playerAcceptMarketOffer(uint32_t playerId, uint32_t timestamp, uint16
 			while (tmpAmount > 0) {
 				uint16_t stackCount = std::min<uint16_t>(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;
@@ -9307,7 +9315,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;
@@ -9358,7 +9366,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
 				) {
@@ -9386,7 +9394,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
 				) {

src/io/functions/iologindata_load_player.cpp

@@ -679,16 +679,22 @@ void IOLoginDataLoad::loadPlayerInboxItems(const std::shared_ptr<Player> &player
 		ItemsMap inboxItems;
 		loadItems(inboxItems, result, player);
 
-		for (auto it = inboxItems.rbegin(), end = inboxItems.rend(); it != end; ++it) {
-			const std::pair<std::shared_ptr<Item>, 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<std::shared_ptr<Item>, int32_t> &pair = it.second;
 			const auto &item = pair.first;
 			if (!item) {
 				continue;
 			}
 
 			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);

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<uint16_t>(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;
 					}
 

src/items/containers/container.cpp

@@ -78,9 +78,9 @@ std::shared_ptr<Container> Container::createBrowseField(const std::shared_ptr<Ti
 
 Container::~Container() {
 	if (getID() == ITEM_BROWSEFIELD) {
-		auto parent = getParent();
+		const auto &parent = getParent();
 		if (parent) {
-			auto tile = parent->getTile();
+			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> &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> &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 &currentItem = 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) {

src/items/containers/mailbox/mailbox.cpp

@@ -98,7 +98,9 @@ bool Mailbox::sendItem(const std::shared_ptr<Item> &item) const {
 		text = item->getAttribute<std::string>(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);

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<Item> letter = Item::CreateItem(ITEM_LETTER_STAMPED);
+				const std::shared_ptr<Item> &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);