fix: improve skill management in InventoryComponent to ensure correct client updates

dGame/dComponents/InventoryComponent.cpp

@@ -727,10 +727,6 @@ void InventoryComponent::Serialize(RakNet::BitStream& outBitStream, const bool b
 		for (const auto& pair : m_Equipped) {
 			const auto item = pair.second;
 
-			if (bIsInitialUpdate) {
-				AddItemSkills(item.lot);
-			}
-
 			outBitStream.Write(item.id);
 			outBitStream.Write(item.lot);
 
@@ -989,6 +985,9 @@ void InventoryComponent::UnequipScripts(Item* unequippedItem) {
 }
 
 void InventoryComponent::HandlePossession(Item* item) {
+	auto* characterComponent = m_Parent->GetComponent<CharacterComponent>();
+	if (!characterComponent) return;
+
 	auto* possessorComponent = m_Parent->GetComponent<PossessorComponent>();
 	if (!possessorComponent) return;
 
@@ -1004,31 +1003,52 @@ void InventoryComponent::HandlePossession(Item* item) {
 		return;
 	}
 
-	// Set the mount item ID so that we know what we're handling
+	GameMessages::SendSetStunned(m_Parent->GetObjectID(), eStateChangeType::PUSH, m_Parent->GetSystemAddress(), LWOOBJID_EMPTY, true, false, true, false, false, false, false, true, true, true, true, true, true, true, true, true);
+
+	// Set the mount Item ID so that we know what were handling
 	possessorComponent->SetMountItemID(item->GetId());
 	GameMessages::SendSetMountInventoryID(m_Parent, item->GetId(), UNASSIGNED_SYSTEM_ADDRESS);
 
-	// Create the mount entity
+	// Create entity to mount
+	auto startRotation = m_Parent->GetRotation();
+
 	EntityInfo info{};
 	info.lot = item->GetLot();
 	info.pos = m_Parent->GetPosition();
-	info.rot = m_Parent->GetRotation();
+	info.rot = startRotation;
 	info.spawnerID = m_Parent->GetObjectID();
 
 	auto* mount = Game::entityManager->CreateEntity(info, nullptr, m_Parent);
 
+	// Check to see if the mount is a vehicle, if so, flip it
+	auto* vehicleComponent = mount->GetComponent<HavokVehiclePhysicsComponent>();
+	if (vehicleComponent) characterComponent->SetIsRacing(true);
+
+	// Setup the destroyable stats
+	auto* destroyableComponent = mount->GetComponent<DestroyableComponent>();
+	if (destroyableComponent) {
+		destroyableComponent->SetIsImmune(true);
+	}
+
+	// Mount it
 	auto* possessableComponent = mount->GetComponent<PossessableComponent>();
 	if (possessableComponent) {
 		possessableComponent->SetIsItemSpawned(true);
 		possessableComponent->SetPossessor(m_Parent->GetObjectID());
+		// Possess it
+		possessorComponent->SetPossessable(mount->GetObjectID());
+		possessorComponent->SetPossessableType(possessableComponent->GetPossessionType());
 	}
 
-	auto* destroyableComponent = mount->GetComponent<DestroyableComponent>();
-	if (destroyableComponent) destroyableComponent->SetIsImmune(true);
+	GameMessages::SendSetJetPackMode(m_Parent, false);
 
+	// Make it go to the client
 	Game::entityManager->ConstructEntity(mount);
-	possessorComponent->Mount(mount);
+	// Update the possessor
+	Game::entityManager->SerializeEntity(m_Parent);
 
+	// have to unlock the input so it vehicle can be driven
+	if (vehicleComponent) GameMessages::SendVehicleUnlockInput(mount->GetObjectID(), false, m_Parent->GetSystemAddress());
 	GameMessages::SendMarkInventoryItemAsActive(m_Parent->GetObjectID(), true, eUnequippableActiveType::MOUNT, item->GetId(), m_Parent->GetSystemAddress());
 }
 
@@ -1156,14 +1176,12 @@ LOT InventoryComponent::GetConsumable() const {
 void InventoryComponent::AddItemSkills(const LOT lot) {
 	const auto info = Inventory::FindItemComponent(lot);
 
-	const auto slot = FindBehaviorSlot(static_cast<eItemType>(info.itemType));
+	const auto slot = FindBehaviorSlot(info.equipLocation);
 
 	if (slot == BehaviorSlot::Invalid) {
 		return;
 	}
 
-	const auto index = m_Skills.find(slot);
-
 	const auto skill = FindSkill(lot);
 
 	SetSkill(slot, skill);
@@ -1191,7 +1209,7 @@ void InventoryComponent::FixInvisibleItems() {
 void InventoryComponent::RemoveItemSkills(const LOT lot) {
 	const auto info = Inventory::FindItemComponent(lot);
 
-	const auto slot = FindBehaviorSlot(static_cast<eItemType>(info.itemType));
+	const auto slot = FindBehaviorSlot(info.equipLocation);
 
 	if (slot == BehaviorSlot::Invalid) {
 		return;
@@ -1203,15 +1221,31 @@ void InventoryComponent::RemoveItemSkills(const LOT lot) {
 		return;
 	}
 
-	const auto old = index->second;
+	const auto skillId = FindSkill(lot);
 
-	GameMessages::SendRemoveSkill(m_Parent, old);
+	// Only act on this slot if it still holds the skill from this item.
+	// Another item may have overwritten the slot since this one was equipped.
+	if (index->second != skillId) {
+		return;
+	}
 
 	m_Skills.erase(slot);
 
+	// Find another slot that still holds this skillID (if any).
+	const auto surviving = std::ranges::find_if(m_Skills, [skillId](const auto& pair) {
+		return pair.second == skillId;
+	});
+
+	// The client stores one acquiredSkillsInfo entry per skillID, tagged with the slotID
+	// it was originally added with. Always send RemoveSkill to clear that entry, then
+	// re-add with the surviving slot so the client shows it in the correct place.
+	GameMessages::SendRemoveSkill(m_Parent, skillId);
+	if (surviving != m_Skills.end()) {
+		GameMessages::SendAddSkill(m_Parent, skillId, surviving->first);
+	}
+
 	if (slot == BehaviorSlot::Primary) {
 		m_Skills.insert_or_assign(BehaviorSlot::Primary, 1);
-
 		GameMessages::SendAddSkill(m_Parent, 1, BehaviorSlot::Primary);
 	}
 }
@@ -1303,23 +1337,17 @@ void InventoryComponent::RemoveDatabasePet(LWOOBJID id) {
 	m_Pets.erase(id);
 }
 
-BehaviorSlot InventoryComponent::FindBehaviorSlot(const eItemType type) {
-	switch (type) {
-	case eItemType::HAT:
-		return BehaviorSlot::Head;
-	case eItemType::NECK:
-		return BehaviorSlot::Neck;
-	case eItemType::LEFT_HAND:
-		return BehaviorSlot::Offhand;
-	case eItemType::RIGHT_HAND:
-		return BehaviorSlot::Primary;
-	case eItemType::CONSUMABLE:
-		return BehaviorSlot::Consumable;
-	default:
-		return BehaviorSlot::Invalid;
-	}
+BehaviorSlot InventoryComponent::FindBehaviorSlot(const std::string& equipLocation) {
+	// Skill slot is determined by equipLocation, not itemType.
+	// Mapping confirmed against live captures and client data (issue #1339).
+	if (equipLocation == "special_r") return BehaviorSlot::Primary;
+	if (equipLocation == "hair")      return BehaviorSlot::Head;
+	if (equipLocation == "special_l") return BehaviorSlot::Offhand;
+	if (equipLocation == "clavicle")  return BehaviorSlot::Neck;
+	return BehaviorSlot::Invalid;
 }
 
+
 bool InventoryComponent::IsTransferInventory(eInventoryType type, bool includeVault) {
 	return type == VENDOR_BUYBACK || (includeVault && (type == VAULT_ITEMS || type == VAULT_MODELS)) || type == TEMP_ITEMS || type == TEMP_MODELS || type == MODELS_IN_BBB;
 }
@@ -1660,10 +1688,28 @@ bool InventoryComponent::SetSkill(BehaviorSlot slot, uint32_t skillId) {
 	const auto index = m_Skills.find(slot);
 	if (index != m_Skills.end()) {
 		const auto old = index->second;
-		GameMessages::SendRemoveSkill(m_Parent, old);
+		// Only remove the old skill from the client if no other slot still holds it.
+		// The client's acquiredSkillsInfo is keyed by skillID (one entry per skill),
+		// so RemoveSkill clears it globally — sending it while another slot still uses
+		// the same skillID would break that slot on the client.
+		const auto usedElsewhere = std::ranges::any_of(m_Skills, [&](const auto& pair) {
+			return pair.first != slot && pair.second == old;
+		});
+		if (!usedElsewhere) {
+			GameMessages::SendRemoveSkill(m_Parent, old);
+		}
+	}
+
+	// Only send AddSkill if the client doesn't already know about this skillID.
+	// The client early-exits on duplicate AddSkill (same skillID already in
+	// acquiredSkillsInfo) without updating the slot — so only send when it's new.
+	const auto alreadyKnown = std::ranges::any_of(m_Skills, [&](const auto& pair) {
+		return pair.first != slot && pair.second == skillId;
+	});
+	if (!alreadyKnown) {
+		GameMessages::SendAddSkill(m_Parent, skillId, slot);
 	}
 
-	GameMessages::SendAddSkill(m_Parent, skillId, slot);
 	m_Skills.insert_or_assign(slot, skillId);
 	return true;
 }

dGame/dComponents/InventoryComponent.h

@@ -367,11 +367,10 @@ public:
 	void RemoveDatabasePet(LWOOBJID id);
 
 	/**
-	 * Returns the current behavior slot active for the passed item type
-	 * @param type the item type to find the behavior slot for
-	 * @return the current behavior slot active for the passed item type
+	 * Returns the behavior slot for the given equipLocation string.
+	 * This is the authoritative mapping used for skill slot assignment.
 	 */
-	static BehaviorSlot FindBehaviorSlot(eItemType type);
+	static BehaviorSlot FindBehaviorSlot(const std::string& equipLocation);
 
 	/**
 	 * Checks if the inventory type is a temp inventory
@@ -403,6 +402,8 @@ public:
 
 	std::map<BehaviorSlot, uint32_t> GetSkills() { return m_Skills; };
 
+	void ClearSkills() { m_Skills.clear(); };
+
 	bool SetSkill(int slot, uint32_t skillId);
 	bool SetSkill(BehaviorSlot slot, uint32_t skillId);
 

dGame/dComponents/PossessableComponent.cpp

@@ -10,7 +10,7 @@ PossessableComponent::PossessableComponent(Entity* parent, const int32_t compone
 	m_AnimationFlag = static_cast<eAnimationFlags>(item.animationFlag);
 
 	// Get the possession Type from the CDClient
-	auto query = CDClientDatabase::CreatePreppedStmt("SELECT possessionType, depossessOnHit, skillSet FROM PossessableComponent WHERE id = ?;");
+	auto query = CDClientDatabase::CreatePreppedStmt("SELECT possessionType, depossessOnHit FROM PossessableComponent WHERE id = ?;");
 
 	query.bind(1, static_cast<int>(componentID));
 
@@ -20,7 +20,6 @@ PossessableComponent::PossessableComponent(Entity* parent, const int32_t compone
 	if (!result.eof()) {
 		m_PossessionType = static_cast<ePossessionType>(result.getIntField("possessionType", 1)); // Default to Attached Visible
 		m_DepossessOnHit = static_cast<bool>(result.getIntField("depossessOnHit", 0));
-		m_SkillSet = result.getIntField("skillSet", 0);
 	} else {
 		m_PossessionType = ePossessionType::ATTACHED_VISIBLE;
 		m_DepossessOnHit = false;

dGame/dComponents/PossessableComponent.h

@@ -55,12 +55,6 @@ public:
 	 */
 	bool GetDepossessOnHit() const { return m_DepossessOnHit; };
 
-	/**
-	 * Returns the skill set ID for this possessable (0 = no skill set)
-	 * @return the skill set ID
-	 */
-	int32_t GetSkillSet() const { return m_SkillSet; };
-
 	/**
 	 * Forcibly depossess the Entity
 	 */
@@ -124,9 +118,4 @@ private:
 	 *
 	 */
 	bool m_ItemSpawned = false;
-
-	/**
-	 * @brief Skill set ID from PossessableComponent CDClient table (0 = none)
-	 */
-	int32_t m_SkillSet = 0;
 };

dGame/dComponents/PossessorComponent.cpp

@@ -1,10 +1,11 @@
 #include "PossessorComponent.h"
 #include "PossessableComponent.h"
 #include "CharacterComponent.h"
-#include "HavokVehiclePhysicsComponent.h"
 #include "EntityManager.h"
 #include "GameMessages.h"
 #include "eUnequippableActiveType.h"
+#include "eControlScheme.h"
+#include "eStateChangeType.h"
 
 PossessorComponent::PossessorComponent(Entity* parent, const int32_t componentID) : Component(parent, componentID) {
 	m_Possessable = LWOOBJID_EMPTY;
@@ -41,27 +42,21 @@ void PossessorComponent::Mount(Entity* mount) {
 	// Don't do anything if we are busy dismounting
 	if (GetIsDismounting() || !mount) return;
 
+	GameMessages::SendSetMountInventoryID(m_Parent, mount->GetObjectID(), UNASSIGNED_SYSTEM_ADDRESS);
 	auto* possessableComponent = mount->GetComponent<PossessableComponent>();
 	if (possessableComponent) {
 		possessableComponent->SetPossessor(m_Parent->GetObjectID());
 		SetPossessable(mount->GetObjectID());
 		SetPossessableType(possessableComponent->GetPossessionType());
-		if (possessableComponent->GetSkillSet() != 0) {
-			GameMessages::UseSkillSet useSkillSet;
-			useSkillSet.target = m_Parent->GetObjectID();
-			useSkillSet.possessedId = mount->GetObjectID();
-			useSkillSet.setId = possessableComponent->GetSkillSet();
-			useSkillSet.Send(m_Parent->GetSystemAddress());
-		}
 	}
 
+	auto characterComponent = m_Parent->GetComponent<CharacterComponent>();
+	if (characterComponent) characterComponent->SetIsRacing(true);
+
 	// GM's to send
 	GameMessages::SendSetJetPackMode(m_Parent, false);
-	if (mount->GetComponent<HavokVehiclePhysicsComponent>()) {
-		auto characterComponent = m_Parent->GetComponent<CharacterComponent>();
-		if (characterComponent) characterComponent->SetIsRacing(true);
-		GameMessages::SendVehicleUnlockInput(mount->GetObjectID(), false, m_Parent->GetSystemAddress());
-	}
+	GameMessages::SendVehicleUnlockInput(mount->GetObjectID(), false, m_Parent->GetSystemAddress());
+	GameMessages::SendSetStunned(m_Parent->GetObjectID(), eStateChangeType::PUSH, m_Parent->GetSystemAddress(), LWOOBJID_EMPTY, true, false, true, false, false, false, false, true, true, true, true, true, true, true, true, true);
 
 	Game::entityManager->SerializeEntity(m_Parent);
 	Game::entityManager->SerializeEntity(mount);
@@ -77,21 +72,13 @@ void PossessorComponent::Dismount(Entity* mount, bool forceDismount) {
 		if (possessableComponent) {
 			possessableComponent->SetPossessor(LWOOBJID_EMPTY);
 			if (forceDismount) possessableComponent->ForceDepossess();
-			if (possessableComponent->GetSkillSet() != 0) {
-				GameMessages::UseSkillSet useSkillSet;
-				useSkillSet.target = m_Parent->GetObjectID();
-				useSkillSet.possessedId = mount->GetObjectID();
-				useSkillSet.setId = possessableComponent->GetSkillSet();
-				useSkillSet.bRemove = true;
-				useSkillSet.Send(m_Parent->GetSystemAddress());
-			}
 		}
 		Game::entityManager->SerializeEntity(m_Parent);
 		Game::entityManager->SerializeEntity(mount);
 
-		if (mount->GetComponent<HavokVehiclePhysicsComponent>()) {
-			auto characterComponent = m_Parent->GetComponent<CharacterComponent>();
-			if (characterComponent) characterComponent->SetIsRacing(false);
-		}
+		auto characterComponent = m_Parent->GetComponent<CharacterComponent>();
+		if (characterComponent) characterComponent->SetIsRacing(false);
 	}
+	// Make sure we don't have wacky controls
+	GameMessages::SendSetPlayerControlScheme(m_Parent, eControlScheme::SCHEME_A);
 }

dGame/dGameMessages/GameMessageHandler.cpp

@@ -159,6 +159,10 @@ void GameMessageHandler::HandleMessage(RakNet::BitStream& inStream, const System
 
 		InventoryComponent* inv = entity->GetComponent<InventoryComponent>();
 		if (inv) {
+			// Clear server-side skill state so AddItemSkills sends fresh AddSkill
+			// packets to the now-ready client. Skills sent during entity construction
+			// (Serialize) arrive before LWOSkillComponent is initialized and are dropped.
+			inv->ClearSkills();
 			auto items = inv->GetEquippedItems();
 			for (auto pair : items) {
 				const auto item = pair.second;

dGame/dGameMessages/GameMessages.cpp

@@ -3946,19 +3946,11 @@ void GameMessages::SendSetMountInventoryID(Entity* entity, const LWOOBJID& objec
 	CMSGHEADER;
 	bitStream.Write(entity->GetObjectID());
 	bitStream.Write(MessageType::Game::SET_MOUNT_INVENTORY_ID);
-	bitStream.Write(objectID != LWOOBJID_EMPTY);
-	if (objectID != LWOOBJID_EMPTY) bitStream.Write(objectID);
+	bitStream.Write(objectID);
 
 	SEND_PACKET_BROADCAST;
 }
 
-void GameMessages::UseSkillSet::Serialize(RakNet::BitStream& bitStream) const {
-	bitStream.Write(bRemove);
-	bitStream.Write(possessedId != LWOOBJID_EMPTY);
-	if (possessedId != LWOOBJID_EMPTY) bitStream.Write(possessedId);
-	bitStream.Write(setId != -1);
-	if (setId != -1) bitStream.Write(setId);
-}
 
 void GameMessages::HandleDismountComplete(RakNet::BitStream& inStream, Entity* entity, const SystemAddress& sysAddr) {
 	// Get the objectID from the bitstream
@@ -3994,6 +3986,9 @@ void GameMessages::HandleDismountComplete(RakNet::BitStream& inStream, Entity* e
 
 			// Update the entity that was possessing
 			Game::entityManager->SerializeEntity(entity);
+
+			// We aren't mounted so remove the stun
+			GameMessages::SendSetStunned(entity->GetObjectID(), eStateChangeType::POP, UNASSIGNED_SYSTEM_ADDRESS, LWOOBJID_EMPTY, true, false, true, false, false, false, false, true, true, true, true, true, true, true, true, true);
 		}
 	}
 }
@@ -4001,14 +3996,10 @@ void GameMessages::HandleDismountComplete(RakNet::BitStream& inStream, Entity* e
 
 void GameMessages::HandleAcknowledgePossession(RakNet::BitStream& inStream, Entity* entity, const SystemAddress& sysAddr) {
 	Game::entityManager->SerializeEntity(entity);
-	bool hasObjectId{};
-	inStream.Read(hasObjectId);
-	if (hasObjectId) {
-		LWOOBJID objectId{};
-		inStream.Read(objectId);
-		auto* mount = Game::entityManager->GetEntity(objectId);
-		if (mount) Game::entityManager->SerializeEntity(mount);
-	}
+	LWOOBJID objectId{};
+	inStream.Read(objectId);
+	auto* mount = Game::entityManager->GetEntity(objectId);
+	if (mount) Game::entityManager->SerializeEntity(mount);
 }
 
 //Racing

dGame/dGameMessages/GameMessages.h

@@ -961,14 +961,5 @@ namespace GameMessages {
 
 		LWOOBJID childID{};
 	};
-
-	struct UseSkillSet : public GameMsg {
-		UseSkillSet() : GameMsg(MessageType::Game::USE_SKILL_SET) {}
-		void Serialize(RakNet::BitStream& bitStream) const override;
-
-		bool bRemove{};
-		LWOOBJID possessedId{ LWOOBJID_EMPTY };
-		int32_t setId{ -1 };
-	};
 };
 #endif // GAMEMESSAGES_H