From ba6228ef09759d16a7d30fc63428a8948539689d Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Fri, 2 Oct 2026 10:26:20 -0500 Subject: [PATCH] fix(inventory): MoveItemInInventory moves only where the item belongs, as the client LWOInventoryComponent_Common::msgMoveItemInInventory (0x00d20590) looks the item up in the inventory the message names and moves it only to the inventory its LOT belongs in (GetInventoryTypeForLOT 0x00c3c4f0), or for the vaults (VAULT_ITEMS, VAULT_MODELS) the LOT's vault type (GetValueTypeForLOT 0x00c3c4a0). An item in the target slot swaps into the moved item's old slot and inventory, and must belong there. The server moved an item into any inventory destInvType named and left the swapped item in the destination. Live clients never set destInvType (0 of 61 in the captures). Co-Authored-By: Claude Opus 5.5 --- dGame/dComponents/InventoryComponent.cpp | 61 ++++++--- dGame/dComponents/InventoryComponent.h | 15 +-- .../dComponentsTests/CMakeLists.txt | 1 + .../dComponentsTests/InventoryMoveTests.cpp | 125 ++++++++++++++++++ 4 files changed, 179 insertions(+), 23 deletions(-) create mode 100644 tests/dGameTests/dComponentsTests/InventoryMoveTests.cpp diff --git a/dGame/dComponents/InventoryComponent.cpp b/dGame/dComponents/InventoryComponent.cpp index 644d50fa3..cf6a3d917 100644 --- a/dGame/dComponents/InventoryComponent.cpp +++ b/dGame/dComponents/InventoryComponent.cpp @@ -551,16 +551,6 @@ void InventoryComponent::MoveItemToInventory(Item* item, const eInventoryType in } } -void InventoryComponent::MoveStack(Item* item, const eInventoryType inventory, const uint32_t slot) { - if (inventory != INVALID && item->GetInventory()->GetType() != inventory) { - auto* newInventory = GetInventory(inventory); - - item->SetInventory(newInventory); - } - - item->SetSlot(slot); -} - Item* InventoryComponent::FindItemById(const LWOOBJID id) const { for (const auto& inventory : m_Inventories) { auto* item = inventory.second->FindItemById(id); @@ -2263,14 +2253,55 @@ void InventoryComponent::OnRemoveItemFromInventory(const GameMessages::RemoveIte } } -void InventoryComponent::OnMoveItemInInventory(const GameMessages::MoveItemInInventory& msg) { - auto* item = FindItemById(msg.iObjID); +eInventoryType InventoryComponent::GetInventoryTypeForMove(const LOT lot, const eInventoryType inventory) { + // LWOInventoryComponent_Common::GetInventoryTypeForLOT (0x00c3c4f0) and, for the vaults (sub_00bd2410: VAULT_ITEMS + // and VAULT_MODELS), GetValueTypeForLOT (0x00c3c4a0): tables by item type; types out of range are ITEMS / VAULT_ITEMS + const auto itemType = static_cast(Inventory::FindItemComponent(lot).itemType); + const bool vault = inventory == VAULT_ITEMS || inventory == VAULT_MODELS; + switch (itemType) { + case eItemType::BRICK: return vault ? INVALID : BRICKS; + case eItemType::BEHAVIOR: return vault ? INVALID : BEHAVIORS; + case eItemType::PROPERTY: return vault ? INVALID : PROPERTY_DEEDS; + case eItemType::QUEST_OBJECT: return vault ? INVALID : QUEST; + case eItemType::MODEL: + case eItemType::PET_INVENTORY_ITEM: + case eItemType::LOOT_MODEL: + case eItemType::VEHICLE: + case eItemType::MOUNT: + return vault ? VAULT_MODELS : MODELS; + default: return vault ? VAULT_ITEMS : ITEMS; + } +} - if (!item) { - return; +void InventoryComponent::OnMoveItemInInventory(const GameMessages::MoveItemInInventory& msg) { + // As LWOInventoryComponent_Common::msgMoveItemInInventory (0x00d20590): the item is looked up in its inventory, and + // moves only to the inventory its LOT belongs in (or that inventory's vault) + const auto sourceType = static_cast(msg.inventoryType); + const auto source = m_Inventories.find(sourceType); + if (source == m_Inventories.end()) return; + + auto* const item = source->second->FindItemById(msg.iObjID); + if (!item) return; + + const auto destinationType = msg.destInvType != INVALID ? static_cast(msg.destInvType) : sourceType; + if (GetInventoryTypeForMove(item->GetLot(), destinationType) != destinationType) return; + const auto destination = m_Inventories.find(destinationType); + if (destination == m_Inventories.end()) return; + + // The item in the slot moved to swaps places with it: back to the moved item's slot and inventory, so it must + // belong there + const auto slot = static_cast(msg.slot); + auto* const swapped = destination->second->FindItemBySlot(slot); + if (swapped && swapped != item && GetInventoryTypeForMove(swapped->GetLot(), sourceType) != sourceType) return; + + if (destinationType != sourceType) { + item->SetInventory(destination->second); + item->SetSlot(slot); // the swapped item takes the moved item's old slot + if (swapped && swapped != item) swapped->SetInventory(source->second); + } else { + item->SetSlot(slot); } - MoveStack(item, static_cast(msg.destInvType), msg.slot); Game::entityManager->SerializeEntity(m_Parent); } diff --git a/dGame/dComponents/InventoryComponent.h b/dGame/dComponents/InventoryComponent.h index c6074cde8..07543c2fa 100644 --- a/dGame/dComponents/InventoryComponent.h +++ b/dGame/dComponents/InventoryComponent.h @@ -217,14 +217,6 @@ public: */ void MoveItemToInventory(Item* item, eInventoryType inventory, uint32_t count, bool showFlyingLot = true, bool isModMoveAndEquip = false, bool ignoreEquipped = false, int32_t preferredSlot = -1); - /** - * Moves a stack of items to an inventory - * @param item the item to move - * @param inventory the inventory to move the item to - * @param slot the slot in the inventory to move the item to - */ - void MoveStack(Item* item, eInventoryType inventory, uint32_t slot = 0); - /** * Returns an item in the inventory by object ID * @param id the id of the item to find @@ -483,6 +475,13 @@ public: */ bool CanDelete(const Item& item) const; void OnMoveItemInInventory(const GameMessages::MoveItemInInventory& msg); + + /** + * The inventory an item of this LOT may be moved to within inventory (its own inventory, or for a vault the + * vault type), as the client finds it (GetInventoryTypeForLOT 0x00c3c4f0 / GetValueTypeForLOT 0x00c3c4a0). + * INVALID when the LOT has no vault. + */ + static eInventoryType GetInventoryTypeForMove(LOT lot, eInventoryType inventory); void OnMoveItemBetweenInventoryTypes(const GameMessages::MoveItemBetweenInventoryTypes& msg); // Answers the client at sysAddr with ResponseMoveItemBetweenInventoryTypes. void OnRequestMoveItemBetweenInventoryTypes(const GameMessages::RequestMoveItemBetweenInventoryTypes& msg, const SystemAddress& sysAddr); diff --git a/tests/dGameTests/dComponentsTests/CMakeLists.txt b/tests/dGameTests/dComponentsTests/CMakeLists.txt index 6bc704a03..2045ca40e 100644 --- a/tests/dGameTests/dComponentsTests/CMakeLists.txt +++ b/tests/dGameTests/dComponentsTests/CMakeLists.txt @@ -21,6 +21,7 @@ set(DCOMPONENTS_TESTS "ReplicaConstructionTests.cpp" "ReplicaDecoderTests.cpp" "CapturePropertyTests.cpp" + "InventoryMoveTests.cpp" "ItemRemovalTests.cpp" "UseItemResultTests.cpp" "LootMetricsTests.cpp" diff --git a/tests/dGameTests/dComponentsTests/InventoryMoveTests.cpp b/tests/dGameTests/dComponentsTests/InventoryMoveTests.cpp new file mode 100644 index 000000000..8c9fa8da6 --- /dev/null +++ b/tests/dGameTests/dComponentsTests/InventoryMoveTests.cpp @@ -0,0 +1,125 @@ +#include "GameDependencies.h" + +#include "CDClientManager.h" +#include "CDComponentsRegistryTable.h" +#include "CDItemComponentTable.h" +#include "Entity.h" +#include "InventoryComponent.h" +#include "InventoryMessages.h" +#include "Item.h" +#include "eItemType.h" +#include "eReplicaComponentType.h" + +#include + +// Moving within the inventories as LWOInventoryComponent_Common::msgMoveItemInInventory (0x00d20590) does. +class InventoryMoveTests : public GameDependenciesTest { +protected: + static constexpr LOT HAT = 8000; + static constexpr LOT MODEL = 8001; + static constexpr LOT BRICK = 8002; + + std::unique_ptr entity; + InventoryComponent* inventory{}; + LWOOBJID nextId = 0x8000; + + void SetUp() override { + SetUpDependencies(); + RegisterLot(HAT, eItemType::HAT); + RegisterLot(MODEL, eItemType::MODEL); + RegisterLot(BRICK, eItemType::BRICK); + CDClientManager::GetEntriesMutable().insert_or_assign(static_cast(info.lot), 0); + entity = std::make_unique(1, info); + inventory = entity->AddComponent(-1); + } + + void TearDown() override { + entity.reset(); + TearDownDependencies(); + } + + static void RegisterLot(const LOT lot, const eItemType type) { + const auto componentID = static_cast(96000 + lot); + auto& registry = CDClientManager::GetEntriesMutable(); + registry.insert_or_assign(static_cast(lot), componentID); + registry.insert_or_assign(static_cast(eReplicaComponentType::ITEM) << 32 | static_cast(lot), componentID); + CDItemComponent component{}; + component.id = componentID; + component.itemType = static_cast(type); + CDClientManager::GetEntriesMutable().insert_or_assign(componentID, component); + } + + Item* Give(const LOT lot, const eInventoryType type, const uint32_t slot) { + return new Item(nextId++, lot, inventory->GetInventory(type), slot, 1, false, {}, LWOOBJID_EMPTY, LWOOBJID_EMPTY, eLootSourceType::NONE); + } + + void Move(const Item& item, const eInventoryType from, const int32_t to, const int32_t slot) { + GameMessages::MoveItemInInventory msg; + msg.iObjID = item.GetId(); + msg.inventoryType = from; + msg.destInvType = to; + msg.slot = slot; + inventory->OnMoveItemInInventory(msg); + } +}; + +TEST_F(InventoryMoveTests, TheInventoryALotMovesTo) { + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(HAT, eInventoryType::ITEMS), eInventoryType::ITEMS); + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(HAT, eInventoryType::VAULT_MODELS), eInventoryType::VAULT_ITEMS); + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(MODEL, eInventoryType::ITEMS), eInventoryType::MODELS); + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(MODEL, eInventoryType::VAULT_ITEMS), eInventoryType::VAULT_MODELS); + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(BRICK, eInventoryType::BRICKS), eInventoryType::BRICKS); + EXPECT_EQ(InventoryComponent::GetInventoryTypeForMove(BRICK, eInventoryType::VAULT_ITEMS), eInventoryType::INVALID); +} + +TEST_F(InventoryMoveTests, MovingWithinTheInventory) { + auto* const hat = Give(HAT, eInventoryType::ITEMS, 0); + auto* const other = Give(HAT, eInventoryType::ITEMS, 3); + + Move(*hat, eInventoryType::ITEMS, eInventoryType::INVALID, 3); + EXPECT_EQ(hat->GetSlot(), 3u); + EXPECT_EQ(other->GetSlot(), 0u); +} + +TEST_F(InventoryMoveTests, OnlyToTheInventoryTheLotBelongsIn) { + auto* const hat = Give(HAT, eInventoryType::ITEMS, 0); + + Move(*hat, eInventoryType::ITEMS, eInventoryType::MODELS, 5); + Move(*hat, eInventoryType::ITEMS, eInventoryType::VAULT_MODELS, 5); + EXPECT_EQ(hat->GetInventory()->GetType(), eInventoryType::ITEMS); + EXPECT_EQ(hat->GetSlot(), 0u); + + // Looked up only in the inventory named + inventory->GetInventory(eInventoryType::MODELS); + Move(*hat, eInventoryType::MODELS, eInventoryType::ITEMS, 5); + EXPECT_EQ(hat->GetSlot(), 0u); + + inventory->GetInventory(eInventoryType::VAULT_ITEMS); + Move(*hat, eInventoryType::ITEMS, eInventoryType::VAULT_ITEMS, 5); + EXPECT_EQ(hat->GetInventory()->GetType(), eInventoryType::VAULT_ITEMS); + EXPECT_EQ(hat->GetSlot(), 5u); +} + +TEST_F(InventoryMoveTests, TheSwappedItemGoesBackToTheSourceInventory) { + auto* const hat = Give(HAT, eInventoryType::ITEMS, 2); + auto* const vaulted = Give(HAT, eInventoryType::VAULT_ITEMS, 7); + + Move(*hat, eInventoryType::ITEMS, eInventoryType::VAULT_ITEMS, 7); + EXPECT_EQ(hat->GetInventory()->GetType(), eInventoryType::VAULT_ITEMS); + EXPECT_EQ(hat->GetSlot(), 7u); + EXPECT_EQ(vaulted->GetInventory()->GetType(), eInventoryType::ITEMS); + EXPECT_EQ(vaulted->GetSlot(), 2u); +} + +TEST_F(InventoryMoveTests, NoSwapWithAnItemThatDoesNotBelongInTheSource) { + auto* const hat = Give(HAT, eInventoryType::VAULT_ITEMS, 2); + auto* const model = Give(MODEL, eInventoryType::VAULT_ITEMS, 7); + inventory->GetInventory(eInventoryType::ITEMS); + + // The vault's model at slot 7 can't go to ITEMS + auto* const itemsHat = Give(HAT, eInventoryType::ITEMS, 4); + Move(*itemsHat, eInventoryType::ITEMS, eInventoryType::VAULT_ITEMS, 7); + EXPECT_EQ(itemsHat->GetInventory()->GetType(), eInventoryType::ITEMS); + EXPECT_EQ(model->GetInventory()->GetType(), eInventoryType::VAULT_ITEMS); + EXPECT_EQ(hat->GetSlot(), 2u); +}