From dd24e201653114c79043d692496dc0d1db3cab98 Mon Sep 17 00:00:00 2001
From: Aaron Kimbrell <aronwk.aaron@gmail.com>
Date: Tue, 9 Jun 2026 16:03:15 -0500
Subject: [PATCH] fix: improve skill management in InventoryComponent to ensure
 correct client updates

---
 dGame/dComponents/InventoryComponent.cpp   | 48 ++++++++++++++++++----
 dGame/dComponents/InventoryComponent.h     |  2 +
 dGame/dGameMessages/GameMessageHandler.cpp |  4 ++
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/dGame/dComponents/InventoryComponent.cpp b/dGame/dComponents/InventoryComponent.cpp
index 9b009b23..f2b277df 100644
--- a/dGame/dComponents/InventoryComponent.cpp
+++ b/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);
 
@@ -1225,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);
 	}
 }
@@ -1676,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;
 }
diff --git a/dGame/dComponents/InventoryComponent.h b/dGame/dComponents/InventoryComponent.h
index 49ba7cdb..230d88d9 100644
--- a/dGame/dComponents/InventoryComponent.h
+++ b/dGame/dComponents/InventoryComponent.h
@@ -402,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);
 
diff --git a/dGame/dGameMessages/GameMessageHandler.cpp b/dGame/dGameMessages/GameMessageHandler.cpp
index e7aa97f9..1918ca2e 100644
--- a/dGame/dGameMessages/GameMessageHandler.cpp
+++ b/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;