From b7849fe18a589ec796d6dd29d6a6a2e0edc5a490 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Mon, 30 Sep 2024 03:59:32 -0300 Subject: [PATCH] enhance: optimize functions for use custom ContainerIterator --- src/creatures/players/player.cpp | 106 +++++----- src/game/game.cpp | 191 +++++++++++------- src/items/containers/container.cpp | 16 ++ src/items/containers/container.hpp | 3 + .../unit/items/containers/container_test.cpp | 29 +-- 5 files changed, 197 insertions(+), 148 deletions(-) diff --git a/src/creatures/players/player.cpp b/src/creatures/players/player.cpp index 6f5ed27d079..485d986abd2 100644 --- a/src/creatures/players/player.cpp +++ b/src/creatures/players/player.cpp @@ -3433,28 +3433,22 @@ std::shared_ptr Player::queryDestination(int32_t &index, const std::sh *destItem = nullptr; std::shared_ptr item = thing->getItem(); - if (item == nullptr) { + if (!item) { return getPlayer(); } - bool autoStack = !((flags & FLAG_IGNOREAUTOSTACK) == FLAG_IGNOREAUTOSTACK); + bool autoStack = !(flags & FLAG_IGNOREAUTOSTACK); bool isStackable = item->isStackable(); - std::vector> containers; - + // First, check the player's inventory slots for (uint32_t slotIndex = CONST_SLOT_FIRST; slotIndex <= CONST_SLOT_AMMO; ++slotIndex) { std::shared_ptr inventoryItem = inventory[slotIndex]; if (inventoryItem) { - if (inventoryItem == tradeItem) { - continue; - } - - if (inventoryItem == item) { + if (inventoryItem == tradeItem || inventoryItem == item) { continue; } if (autoStack && isStackable) { - // try find an already existing item to stack with if (queryAdd(slotIndex, item, item->getItemCount(), 0) == RETURNVALUE_NOERROR) { if (inventoryItem->equals(item) && inventoryItem->getItemCount() < inventoryItem->getStackSize()) { index = slotIndex; @@ -3462,77 +3456,73 @@ std::shared_ptr Player::queryDestination(int32_t &index, const std::sh return getPlayer(); } } - - if (std::shared_ptr subContainer = inventoryItem->getContainer()) { - containers.push_back(subContainer); - } - } else if (std::shared_ptr subContainer = inventoryItem->getContainer()) { - containers.push_back(subContainer); } - } else if (queryAdd(slotIndex, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { // empty slot + } else if (queryAdd(slotIndex, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { // Empty slot index = slotIndex; *destItem = nullptr; return getPlayer(); } } - size_t i = 0; - while (i < containers.size()) { - std::shared_ptr tmpContainer = containers[i++]; - if (!autoStack || !isStackable) { - // we need to find first empty container as fast as we can for non-stackable items - uint32_t n = tmpContainer->capacity() - tmpContainer->size(); - while (n) { - if (tmpContainer->queryAdd(tmpContainer->capacity() - n, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { - index = tmpContainer->capacity() - n; - *destItem = nullptr; - return tmpContainer; - } - - n--; - } - - for (const std::shared_ptr &tmpContainerItem : tmpContainer->getItemList()) { - if (std::shared_ptr subContainer = tmpContainerItem->getContainer()) { - containers.push_back(subContainer); - } + // Collect all top-level containers from the player's inventory + std::vector> containers; + for (uint32_t slotIndex = CONST_SLOT_FIRST; slotIndex <= CONST_SLOT_AMMO; ++slotIndex) { + std::shared_ptr inventoryItem = inventory[slotIndex]; + if (inventoryItem) { + if (auto subContainer = inventoryItem->getContainer()) { + containers.push_back(subContainer); } - - continue; } + } - uint32_t n = 0; - - for (const std::shared_ptr &tmpItem : tmpContainer->getItemList()) { - if (tmpItem == tradeItem) { - continue; - } + // Iterate over all items in the player's containers using ContainerIterator + for (const auto& container : containers) { + ContainerIterator it(container, static_cast(g_configManager().getNumber(MAX_CONTAINER_DEPTH, __FUNCTION__))); + while (it.hasNext()) { + auto currentItem = *it; - if (tmpItem == item) { + if (!currentItem || currentItem == tradeItem || currentItem == item) { + it.advance(); continue; } - // try find an already existing item to stack with - if (tmpItem->equals(item) && tmpItem->getItemCount() < tmpItem->getStackSize()) { - index = n; - *destItem = tmpItem; - return tmpContainer; + // Check for stacking + if (autoStack && isStackable) { + if (currentItem->equals(item) && currentItem->getItemCount() < currentItem->getStackSize()) { + index = it.getCurrentIndex(); + *destItem = currentItem; + return it.getCurrentContainer(); + } } - if (std::shared_ptr subContainer = tmpItem->getContainer()) { - containers.push_back(subContainer); + // If the item is a container, check if it has space + if (auto currentContainer = currentItem->getContainer()) { + if (currentContainer->size() < currentContainer->capacity()) { + uint32_t n = currentContainer->size(); + if (currentContainer->queryAdd(n, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { + index = n; + *destItem = nullptr; + return currentContainer; + } + } } - n++; + it.advance(); } - if (n < tmpContainer->capacity() && tmpContainer->queryAdd(n, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { - index = n; - *destItem = nullptr; - return tmpContainer; + // Check for an empty slot in the top-level container + if (container->size() < container->capacity()) { + uint32_t n = container->size(); + if (container->queryAdd(n, item, item->getItemCount(), flags) == RETURNVALUE_NOERROR) { + index = n; + *destItem = nullptr; + return container; + } } } + // Default to backpack slot if no suitable place is found + *destItem = inventory[CONST_SLOT_BACKPACK]; return getPlayer(); } diff --git a/src/game/game.cpp b/src/game/game.cpp index 381792c3092..91693efe814 100644 --- a/src/game/game.cpp +++ b/src/game/game.cpp @@ -2356,7 +2356,8 @@ ReturnValue Game::internalRemoveItem(std::shared_ptr item, int32_t count / std::tuple Game::addItemBatch(const std::shared_ptr &toCylinder, const std::vector> &items, uint32_t flags /* = 0 */, bool dropOnMap /* = true */, uint32_t autoContainerId /* = 0 */) { uint32_t totalAdded = 0; uint32_t containersCreated = 0; - ReturnValue ret = RETURNVALUE_NOTPOSSIBLE; + ReturnValue ret = RETURNVALUE_NOERROR; + if (dropOnMap) { for (const auto &item : items) { auto returnError = internalAddItem(toCylinder->getTile(), item, INDEX_WHEREEVER, FLAG_NOLIMIT); @@ -2365,90 +2366,129 @@ std::tuple Game::addItemBatch(const std::shared containersCreated++; } totalAdded += item->getItemCount(); + } else { + // Handle error if needed + ret = returnError; + break; } - - ret = returnError; } - return std::make_tuple(ret, totalAdded, containersCreated); } + // When not dropping on map metrics::method_latency measure(__METHOD_NAME__); const auto player = toCylinder->getPlayer(); - bool dropping = false; - auto setupDestination = [&]() -> std::shared_ptr { - if (autoContainerId == 0) { - return toCylinder; - } - auto autoContainer = Item::CreateItem(autoContainerId); - if (!autoContainer) { - g_logger().error("[{}] Failed to create auto container", __FUNCTION__); - return toCylinder; - } - if (internalAddItem(toCylinder, autoContainer, CONST_SLOT_WHEREEVER, flags) != RETURNVALUE_NOERROR) { - if (internalAddItem(toCylinder->getTile(), autoContainer, INDEX_WHEREEVER, FLAG_NOLIMIT) != RETURNVALUE_NOERROR) { - g_logger().error("[{}] Failed to add auto container", __FUNCTION__); - return toCylinder; + if (!player) { + ret = RETURNVALUE_NOTPOSSIBLE; + return std::make_tuple(ret, totalAdded, containersCreated); + } + + // Function to find an available container with free slots + auto findAvailableContainer = [&](std::shared_ptr item) -> std::shared_ptr { + // First, try the player's main inventory slots + for (uint32_t slotIndex = CONST_SLOT_FIRST; slotIndex <= CONST_SLOT_AMMO; ++slotIndex) { + std::shared_ptr inventoryItem = player->inventory[slotIndex]; + if (inventoryItem) { + if (auto container = inventoryItem->getContainer()) { + if (container->size() < container->capacity()) { + return container; + } + } } } - auto container = autoContainer->getContainer(); - if (!container) { - g_logger().error("[{}] Failed to get auto container", __FUNCTION__); - return toCylinder; - } - containersCreated++; - return container; - }; - auto destination = setupDestination(); - for (const auto &item : items) { - auto container = destination->getContainer(); - if (container && container->getFreeSlots() == 0) { - destination = setupDestination(); - } - if (!dropping) { - uint32_t remainderCount = 0; - bool addedToAutoContainer = false; - // First, try adding to the autoContainer, if it is set - if (autoContainerId != 0) { - ret = internalAddItem(destination, item, CONST_SLOT_WHEREEVER, flags, false, remainderCount); - if (ret == RETURNVALUE_NOERROR) { - addedToAutoContainer = true; + // Use ContainerIterator to search nested containers + std::vector> containers; + for (uint32_t slotIndex = CONST_SLOT_FIRST; slotIndex <= CONST_SLOT_AMMO; ++slotIndex) { + std::shared_ptr inventoryItem = player->inventory[slotIndex]; + if (inventoryItem) { + if (auto subContainer = inventoryItem->getContainer()) { + containers.push_back(subContainer); } } - // If it failed to add to the autoContainer, or it's not set, use the current logic - if (!addedToAutoContainer) { - ret = internalCollectManagedItems(player, item, g_game().getObjectCategory(item), false); - // If it can't place in the player's backpacks, add normally - if (ret != RETURNVALUE_NOERROR) { - ret = internalAddItem(destination, item, CONST_SLOT_WHEREEVER, flags, false, remainderCount); + } + + for (const auto& container : containers) { + ContainerIterator it(container, static_cast(g_configManager().getNumber(MAX_CONTAINER_DEPTH, __FUNCTION__))); + while (it.hasNext()) { + auto currentItem = *it; + + if (!currentItem) { + it.advance(); + continue; } - } - if (remainderCount != 0) { - std::shared_ptr remainderItem = Item::CreateItem(item->getID(), remainderCount); - ReturnValue remaindRet = internalAddItem(destination->getTile(), remainderItem, INDEX_WHEREEVER, FLAG_NOLIMIT); - if (player && remaindRet != RETURNVALUE_NOERROR) { - player->sendLootStats(item, static_cast(item->getItemCount())); + // If the item is a container, check if it has space + if (auto currentContainer = currentItem->getContainer()) { + if (currentContainer->size() < currentContainer->capacity()) { + return currentContainer; + } } + + it.advance(); } } - if (dropping || (ret != RETURNVALUE_NOERROR && dropOnMap)) { - dropping = true; - ret = internalAddItem(destination->getTile(), item, INDEX_WHEREEVER, FLAG_NOLIMIT); - } + return nullptr; + }; + + // Collect remainder items to handle them after main loop + std::vector> remainderItems; - if (player && ret == RETURNVALUE_NOERROR) { - player->sendForgingData(); + for (const auto& item : items) { + // Try to find a container with space + auto destinationContainer = findAvailableContainer(item); + if (!destinationContainer) { + // No available container, try to create one if autoContainerId is provided + if (autoContainerId != 0) { + auto autoContainerItem = Item::CreateItem(autoContainerId); + if (autoContainerItem && internalAddItem(player, autoContainerItem, CONST_SLOT_WHEREEVER, flags) == RETURNVALUE_NOERROR) { + destinationContainer = autoContainerItem->getContainer(); + if (destinationContainer) { + containersCreated++; + } + } + } } - if (ret != RETURNVALUE_NOERROR) { + + if (!destinationContainer) { + ret = RETURNVALUE_CONTAINERNOTENOUGHROOM; break; - } else { + } + + // Try to add item to the destination container + uint32_t remainderCount = 0; + ret = internalAddItem(destinationContainer, item, CONST_SLOT_WHEREEVER, flags, false, remainderCount); + if (ret == RETURNVALUE_NOERROR) { totalAdded += item->getItemCount(); + } else if (remainderCount > 0) { + // Couldn't add full item, collect remainder + auto remainderItem = Item::CreateItem(item->getID(), remainderCount); + remainderItems.push_back(remainderItem); + totalAdded += (item->getItemCount() - remainderCount); + } else { + // Adding item failed + ret = RETURNVALUE_CONTAINERNOTENOUGHROOM; + break; } } + // Handle remainder items + if (!remainderItems.empty()) { + for (const auto& remainderItem : remainderItems) { + ret = internalAddItem(player->getTile(), remainderItem, INDEX_WHEREEVER, FLAG_NOLIMIT); + if (ret != RETURNVALUE_NOERROR) { + break; + } + totalAdded += remainderItem->getItemCount(); + } + } + + // Send necessary updates to the player + if (player && totalAdded > 0) { + player->sendForgingData(); + } + return std::make_tuple(ret, totalAdded, containersCreated); } @@ -2457,28 +2497,25 @@ std::tuple Game::createItemBatch(const std::sha std::vector> items; for (const auto &[itemId, count, subType] : itemCounts) { const auto &itemType = Item::items[itemId]; - if (itemType.id <= 0) { - continue; - } - if (count == 0) { + if (itemType.id <= 0 || count == 0) { continue; } - uint32_t countPerItem = itemType.stackable ? itemType.stackSize : 1; - for (uint32_t i = 0; i < count; ++i) { - std::shared_ptr item; - if (itemType.isWrappable()) { - countPerItem = 1; - item = Item::CreateItem(ITEM_DECORATION_KIT, subType); - item->setAttribute(ItemAttribute_t::DESCRIPTION, "Unwrap this item in your own house to create a <" + itemType.name + ">."); - item->setCustomAttribute("unWrapId", static_cast(itemId)); - } else { - item = Item::CreateItem(itemId, itemType.stackable ? std::min(countPerItem, count - i) : subType); + if (itemType.stackable) { + uint32_t totalCount = count; + while (totalCount > 0) { + uint32_t stackCount = std::min(totalCount, itemType.stackSize); + auto item = Item::CreateItem(itemId, stackCount); + items.push_back(item); + totalCount -= stackCount; + } + } else { + items.reserve(items.size() + count); + for (uint32_t i = 0; i < count; ++i) { + auto item = Item::CreateItem(itemId, subType); + items.push_back(item); } - items.push_back(item); - i += countPerItem - 1; } } - return addItemBatch(toCylinder, items, flags, dropOnMap, autoContainerId); } diff --git a/src/items/containers/container.cpp b/src/items/containers/container.cpp index 887bbb55f31..d93bffabe93 100644 --- a/src/items/containers/container.cpp +++ b/src/items/containers/container.cpp @@ -1053,3 +1053,19 @@ std::shared_ptr ContainerIterator::operator*() const { bool ContainerIterator::hasReachedMaxDepth() const { return m_maxDepthReached; } + +std::shared_ptr ContainerIterator::getCurrentContainer() const { + if (states.empty()) { + return nullptr; + } + const auto& top = states.back(); + return top.container.lock(); +} + +size_t ContainerIterator::getCurrentIndex() const { + if (states.empty()) { + return 0; + } + const auto& top = states.back(); + return top.index; +} diff --git a/src/items/containers/container.hpp b/src/items/containers/container.hpp index 751bbc50490..9d564a4b47b 100644 --- a/src/items/containers/container.hpp +++ b/src/items/containers/container.hpp @@ -63,6 +63,9 @@ class ContainerIterator { bool hasReachedMaxDepth() const; + std::shared_ptr getCurrentContainer() const; + size_t getCurrentIndex() const; + private: /** * @brief Represents the state of the iterator at a given point in time. diff --git a/tests/unit/items/containers/container_test.cpp b/tests/unit/items/containers/container_test.cpp index 801fc5f9e1b..3847ade0e11 100644 --- a/tests/unit/items/containers/container_test.cpp +++ b/tests/unit/items/containers/container_test.cpp @@ -13,22 +13,25 @@ suite<"ContainerIteratorTest"> containerIteratorTest = [] { DI::setTestContainer(&InMemoryLogger::install(injector)); auto& logger = dynamic_cast(injector.create()); - auto createNestedContainers = [](size_t depth, size_t itemsPerContainer) -> std::shared_ptr { - if (depth == 0) { - return std::make_shared(ITEM_SHOPPING_BAG); - } + std::function(size_t, size_t)> createNestedContainers = + [&](size_t depth, size_t itemsPerContainer) -> std::shared_ptr { + if (depth == 0) { + return std::make_shared(ITEM_SHOPPING_BAG); + } - auto container = std::make_shared(ITEM_SHOPPING_BAG); - for (size_t i = 0; i < itemsPerContainer; ++i) { - auto item = Item::CreateItem(ITEM_GOLD_COIN, 1); - container->addItem(item); - } + auto container = std::make_shared(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); + // Recursive call + auto nestedContainer = createNestedContainers(depth - 1, itemsPerContainer); + container->addItem(nestedContainer); + + return container; + }; - return container; - }; test("ContainerIterator performance com contêineres profundamente aninhados") = [&]() { size_t depth = 100;