fix: some things related to containers and add container test

config.lua.dist

@@ -66,8 +66,6 @@ serverMotd = "Welcome to the OTServBR-Global!"
 statusTimeout = 5 * 1000
 replaceKickOnLogin = true
 maxPacketsPerSecond = 25
-maxItem = 2000
-maxContainer = 100
 maxPlayersOnlinePerAccount = 1
 maxPlayersOutsidePZPerAccount = 1
 
@@ -81,6 +79,12 @@ freeDepotLimit = 2000
 premiumDepotLimit = 10000
 depotBoxes = 20
 
+-- Item and containers limit
+-- NOTE: 'maxContainerDepth' defines the maximum depth to which containers can be nested
+maxItem = 5000
+maxContainer = 500
+maxContainerDepth = 200
+
 -- Augments System (Get more info in: https://github.com/opentibiabr/canary/pull/2602)
 -- NOTE: the following values are for all weapons and equipments that have type of "increase damage", "powerful impact" and "strong impact".
 -- To customize the percentage of a particular item with these augment types, please add to the item "augments" section on items.xml as the example above.

src/config/config_enums.hpp

@@ -142,6 +142,7 @@ enum ConfigKey_t : uint16_t {
 	MAX_ALLOWED_ON_A_DUMMY,
 	MAX_CONTAINER_ITEM,
 	MAX_CONTAINER,
+	MAX_CONTAINER_DEPTH,
 	MAX_DAMAGE_REFLECTION,
 	MAX_ELEMENTAL_RESISTANCE,
 	MAX_MARKET_OFFERS_AT_A_TIME_PER_PLAYER,

src/config/configmanager.cpp

@@ -269,6 +269,7 @@ bool ConfigManager::load() {
 	loadIntConfig(L, MAX_ALLOWED_ON_A_DUMMY, "maxAllowedOnADummy", 1);
 	loadIntConfig(L, MAX_CONTAINER_ITEM, "maxItem", 5000);
 	loadIntConfig(L, MAX_CONTAINER, "maxContainer", 500);
+	loadIntConfig(L, MAX_CONTAINER_DEPTH, "maxContainerDepth", 200);
 	loadIntConfig(L, MAX_DAMAGE_REFLECTION, "maxDamageReflection", 200);
 	loadIntConfig(L, MAX_ELEMENTAL_RESISTANCE, "maxElementalResistance", 200);
 	loadIntConfig(L, MAX_MARKET_OFFERS_AT_A_TIME_PER_PLAYER, "maxMarketOffersAtATimePerPlayer", 100);

src/items/containers/container.cpp

@@ -497,8 +497,8 @@ ReturnValue Container::queryAdd(int32_t addIndex, const std::shared_ptr<Thing> &
 		return RETURNVALUE_CONTAINERNOTENOUGHROOM;
 	}
 
-	if (const auto topParentContainer = getTopParentContainer()) {
-		if (const auto addContainer = item->getContainer()) {
+	if (const auto &topParentContainer = getTopParentContainer()) {
+		if (const auto &addContainer = item->getContainer()) {
 			uint32_t addContainerCount = addContainer->getContainerHoldingCount() + 1;
 			uint32_t maxContainer = static_cast<uint32_t>(g_configManager().getNumber(MAX_CONTAINER, __FUNCTION__));
 			if (addContainerCount + topParentContainer->getContainerHoldingCount() > maxContainer) {
@@ -922,7 +922,7 @@ uint16_t Container::getFreeSlots() {
 }
 
 ContainerIterator Container::iterator() {
-	return { getContainer() };
+	return { getContainer(), static_cast<size_t>(g_configManager().getNumber(MAX_CONTAINER_DEPTH, __FUNCTION__))};
 }
 
 void Container::removeItem(std::shared_ptr<Thing> thing, bool sendUpdateToClient /* = false*/) {
@@ -968,18 +968,24 @@ uint32_t Container::getOwnerId() const {
 ContainerIterator::ContainerIterator(const std::shared_ptr<Container> &container, size_t maxDepth) :
 	maxTraversalDepth(maxDepth) {
 	if (container) {
-		(void)states.emplace(container, 0, 1);
+		states.reserve(maxDepth);
+		visitedContainers.reserve(g_configManager().getNumber(MAX_CONTAINER, __FUNCTION__));
+		(void)states.emplace_back(container, 0, 1);
 		(void)visitedContainers.insert(container);
 	}
 }
 
 bool ContainerIterator::hasNext() const {
 	while (!states.empty()) {
-		auto &top = states.top();
-		if (top.index < top.container->itemlist.size()) {
+		const auto& top = states.back();
+		const auto &container = top.container.lock();
+		if (!container) {
+			// Container has been deleted
+			states.pop_back();
+		} else if (top.index < container->itemlist.size()) {
 			return true;
 		} else {
-			states.pop();
+			states.pop_back();
 		}
 	}
 	return false;
@@ -990,28 +996,39 @@ void ContainerIterator::advance() {
 		return;
 	}
 
-	auto &top = states.top();
-	if (top.index >= top.container->itemlist.size()) {
-		states.pop();
+	auto& top = states.back();
+	const auto &container = top.container.lock();
+	if (!container) {
+		// Container has been deleted
+		states.pop_back();
 		return;
 	}
 
-	auto currentItem = top.container->itemlist[top.index];
+	if (top.index >= container->itemlist.size()) {
+		states.pop_back();
+		return;
+	}
+
+	auto currentItem = container->itemlist[top.index];
 	if (currentItem) {
 		auto subContainer = currentItem->getContainer();
 		if (subContainer && !subContainer->itemlist.empty()) {
 			size_t newDepth = top.depth + 1;
 			if (newDepth <= maxTraversalDepth) {
-				// Check if we have already visited this container to avoid cycles
 				if (visitedContainers.find(subContainer) == visitedContainers.end()) {
-					(void)states.emplace(subContainer, 0, newDepth);
-					(void)visitedContainers.insert(subContainer);
+					states.emplace_back(subContainer, 0, newDepth);
+					visitedContainers.insert(subContainer);
 				} else {
-					// Cycle detection
-					g_logger().error("[{}] Cycle detected in container: {}", __FUNCTION__, subContainer->getName());
+					if (!m_cycleDetected) {
+						//g_logger().debug("[{}] Cycle detected in container: {}", __FUNCTION__, subContainer->getName());
+						m_cycleDetected = true;
+					}
 				}
 			} else {
-				g_logger().error("[{}] Maximum iteration depth reached", __FUNCTION__);
+				if (!m_maxDepthReached) {
+					//g_logger().debug("[{}] Maximum iteration depth reached", __FUNCTION__);
+					m_maxDepthReached = true;
+				}
 			}
 		}
 	}
@@ -1024,9 +1041,15 @@ std::shared_ptr<Item> ContainerIterator::operator*() const {
 		return nullptr;
 	}
 
-	auto &top = states.top();
-	if (top.index < top.container->itemlist.size()) {
-		return top.container->itemlist[top.index];
+	const auto& top = states.back();
+	if (const auto &container = top.container.lock()) {
+		if (top.index < container->itemlist.size()) {
+			return container->itemlist[top.index];
+		}
 	}
 	return nullptr;
 }
+
+bool ContainerIterator::hasReachedMaxDepth() const {
+	return m_maxDepthReached;
+}

src/items/containers/container.hpp

@@ -30,7 +30,7 @@ class ContainerIterator {
 	 * @param container The root container to start iterating from.
 	 * @param maxDepth The maximum depth of nested containers to traverse.
 	 */
-	ContainerIterator(const std::shared_ptr<Container> &container, size_t maxDepth = 1000);
+	ContainerIterator(const std::shared_ptr<Container> &container, size_t maxDepth);
 
 	/**
 	 * @brief Checks if there are more items to iterate over in the container.
@@ -61,6 +61,8 @@ class ContainerIterator {
 	 */
 	std::shared_ptr<Item> operator*() const;
 
+	bool hasReachedMaxDepth() const;
+
 private:
 	/**
 	 * @brief Represents the state of the iterator at a given point in time.
@@ -75,7 +77,7 @@ class ContainerIterator {
 		/**
 		 * @brief The container being iterated over.
 		 */
-		std::shared_ptr<Container> container;
+		std::weak_ptr<Container> container;
 
 		/**
 		 * @brief The current index within the container's item list.
@@ -106,7 +108,7 @@ class ContainerIterator {
 	 * used to traverse containers and their nested sub-containers in a depth-first manner.
 	 * Each element in the stack represents a different level in the container hierarchy.
 	 */
-	mutable std::stack<IteratorState> states;
+	mutable std::vector<IteratorState> states;
 
 	/**
 	 * @brief Set of containers that have already been visited during the iteration.
@@ -119,6 +121,9 @@ class ContainerIterator {
 	mutable std::unordered_set<std::shared_ptr<Container>> visitedContainers;
 	size_t maxTraversalDepth = 0;
 
+	bool m_maxDepthReached = false;
+	bool m_cycleDetected = false;
+
 	friend class Container;
 };
 

src/lua/functions/creatures/player/player_functions.cpp

@@ -1864,6 +1864,8 @@ int PlayerFunctions::luaPlayerAddItem(lua_State* L) {
 			if (!hasTable) {
 				lua_pushnil(L);
 			}
+
+			player->sendCancelMessage(ret);
 			return 1;
 		}
 

tests/unit/CMakeLists.txt

@@ -1,6 +1,7 @@
 setup_test(canary_ut unit)
 
 add_subdirectory(account)
+add_subdirectory(items)
 add_subdirectory(kv)
 add_subdirectory(lib)
 add_subdirectory(security)

tests/unit/items/CMakeLists.txt

@@ -0,0 +1,3 @@
+target_sources(canary_ut PRIVATE
+    containers/container_test.cpp
+)

tests/unit/items/containers/container_test.cpp

@@ -0,0 +1,60 @@
+#include "pch.hpp"
+
+#include <boost/ut.hpp>
+
+#include "lib/logging/in_memory_logger.hpp"
+
+#include "items/containers/container.hpp"
+
+using namespace boost::ut;
+
+suite<"ContainerIteratorTest"> containerIteratorTest = [] {
+    di::extension::injector<> injector{};
+    DI::setTestContainer(&InMemoryLogger::install(injector));
+    auto& logger = dynamic_cast<InMemoryLogger&>(injector.create<Logger&>());
+
+    auto createNestedContainers = [](size_t depth, size_t itemsPerContainer) -> std::shared_ptr<Container> {
+        if (depth == 0) {
+            return std::make_shared<Container>(ITEM_SHOPPING_BAG);
+        }
+
+        auto container = std::make_shared<Container>(ITEM_SHOPPING_BAG);
+        for (size_t i = 0; i < itemsPerContainer; ++i) {
+            auto item = Item::CreateItem(ITEM_GOLD_COIN, 1);
+            container->addItem(item);
+        }
+
+        auto nestedContainer = createNestedContainers(depth - 1, itemsPerContainer);
+        container->addItem(nestedContainer);
+
+        return container;
+    };
+
+    test("ContainerIterator performance com contêineres profundamente aninhados") = [&]() {
+        size_t depth = 100;
+        size_t itemsPerContainer = 10;
+
+        auto rootContainer = createNestedContainers(depth, itemsPerContainer);
+
+        auto startTime = std::chrono::high_resolution_clock::now();
+
+        size_t itemCount = 0;
+        ContainerIterator iterator = rootContainer->iterator();
+        while (iterator.hasNext()) {
+            auto item = *iterator;
+            ++itemCount;
+
+            iterator.advance();
+        }
+
+        auto endTime = std::chrono::high_resolution_clock::now();
+
+        auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(endTime - startTime).count();
+
+        logger.info("Percorridos {} itens em {} milissegundos.", itemCount, duration);
+
+        size_t expectedItemCount = (itemsPerContainer + 1) * depth;
+
+        expect(itemCount == expectedItemCount) << "Contagem de itens esperada: " << expectedItemCount << ", Obtida: " << itemCount;
+    };
+};