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 <noreply@anthropic.com>

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<eItemType>(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<eInventoryType>(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<eInventoryType>(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<uint32_t>(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<eInventoryType>(msg.destInvType), msg.slot);
 	Game::entityManager->SerializeEntity(m_Parent);
 }
 

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);

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"

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 <gtest/gtest.h>
+
+// 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> entity;
+	InventoryComponent* inventory{};
+	LWOOBJID nextId = 0x8000;
+
+	void SetUp() override {
+		SetUpDependencies();
+		RegisterLot(HAT, eItemType::HAT);
+		RegisterLot(MODEL, eItemType::MODEL);
+		RegisterLot(BRICK, eItemType::BRICK);
+		CDClientManager::GetEntriesMutable<CDComponentsRegistryTable>().insert_or_assign(static_cast<uint64_t>(info.lot), 0);
+		entity = std::make_unique<Entity>(1, info);
+		inventory = entity->AddComponent<InventoryComponent>(-1);
+	}
+
+	void TearDown() override {
+		entity.reset();
+		TearDownDependencies();
+	}
+
+	static void RegisterLot(const LOT lot, const eItemType type) {
+		const auto componentID = static_cast<uint32_t>(96000 + lot);
+		auto& registry = CDClientManager::GetEntriesMutable<CDComponentsRegistryTable>();
+		registry.insert_or_assign(static_cast<uint64_t>(lot), componentID);
+		registry.insert_or_assign(static_cast<uint64_t>(eReplicaComponentType::ITEM) << 32 | static_cast<uint64_t>(lot), componentID);
+		CDItemComponent component{};
+		component.id = componentID;
+		component.itemType = static_cast<uint32_t>(type);
+		CDClientManager::GetEntriesMutable<CDItemComponentTable>().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);
+}