Fix for 'Invalid Slot ID' messages, item loss during corpse looting, and possible item loss during LDoN/Adventure merchant purchases

changelog.txt

@@ -1,5 +1,12 @@
 EQEMu Changelog (Started on Sept 24, 2003 15:50)
 -------------------------------------------------------
+== 03/03/2015 ==
+Uleat: Fix for 'Invalid Slot ID' messages. Bag slot count is now enforced during database saves to eliminate existing 'hidden' duplicated items..sorry MQ2 users...
+Uleat: Fix for Item loss during corpse looting and possible item loss when purchasing items from LDoN or Adventure merchants. (cursor-related)
+
+== 02/27/2015 ==
+Uleat: Final 'tweak' for light sources until a valid issue arises. Wearable equipment now matches client behavior. Stackable light sources are bugged in the client..but, the current timer update implementation alleviates this condition.
+
 == 02/26/2015 ==
 Uleat: Updated light source criteria to (hopefully) match what the client uses (still needs tweaking)
 Uleat: Changed 'general' light source checks to accept the last valid light source (client behavior)

common/shareddb.cpp

@@ -151,28 +151,31 @@ bool SharedDatabase::VerifyInventory(uint32 account_id, int16 slot_id, const Ite
 
 bool SharedDatabase::SaveInventory(uint32 char_id, const ItemInst* inst, int16 slot_id) {
 
-	// If we never save tribute slots..how are we to ever benefit from them!!? The client
-	// object is destroyed upon zoning - including its inventory object..and if tributes
-	// don't exist in the database, then they will never be loaded when the new client
-	// object is created in the new zone object... Something to consider...
-	//
-	// (we could add them to the 'NoRent' checks and dispose of after 30 minutes offline)
-
 	//never save tribute slots:
 	if(slot_id >= EmuConstants::TRIBUTE_BEGIN && slot_id <= EmuConstants::TRIBUTE_END)
 		return true;
 
 	if (slot_id >= EmuConstants::SHARED_BANK_BEGIN && slot_id <= EmuConstants::SHARED_BANK_BAGS_END) {
         // Shared bank inventory
-		if (!inst)
+		if (!inst) {
-            return DeleteSharedBankSlot(char_id, slot_id);
+			return DeleteSharedBankSlot(char_id, slot_id);
-		else
+		}
-            return UpdateSharedBankSlot(char_id, inst, slot_id);
+		else {
+			// Needed to clear out bag slots that 'REPLACE' in UpdateSharedBankSlot does not overwrite..otherwise, duplication occurs
+			// (This requires that parent then child items be sent..which should be how they are currently passed)
+			if (Inventory::SupportsContainers(slot_id))
+				DeleteSharedBankSlot(char_id, slot_id);
+			return UpdateSharedBankSlot(char_id, inst, slot_id);
+		}
 	}
 	else if (!inst) { // All other inventory
 		return DeleteInventorySlot(char_id, slot_id);
 	}
 
+	// Needed to clear out bag slots that 'REPLACE' in UpdateInventorySlot does not overwrite..otherwise, duplication occurs
+	// (This requires that parent then child items be sent..which should be how they are currently passed)
+	if (Inventory::SupportsContainers(slot_id))
+		DeleteInventorySlot(char_id, slot_id);
     return UpdateInventorySlot(char_id, inst, slot_id);
 }
 
@@ -209,7 +212,9 @@ bool SharedDatabase::UpdateInventorySlot(uint32 char_id, const ItemInst* inst, i
 
     // Save bag contents, if slot supports bag contents
 	if (inst->IsType(ItemClassContainer) && Inventory::SupportsContainers(slot_id))
-		for (uint8 idx = SUB_BEGIN; idx < EmuConstants::ITEM_CONTAINER_SIZE; idx++) {
+		// Limiting to bag slot count will get rid of 'hidden' duplicated items and 'Invalid Slot ID'
+		// messages through attrition (and the modded code in SaveInventory)
+		for (uint8 idx = SUB_BEGIN; idx < inst->GetItem()->BagSlots && idx < EmuConstants::ITEM_CONTAINER_SIZE; idx++) {
 			const ItemInst* baginst = inst->GetItem(idx);
 			SaveInventory(char_id, baginst, Inventory::CalcSlotId(slot_id, idx));
 		}
@@ -253,7 +258,9 @@ bool SharedDatabase::UpdateSharedBankSlot(uint32 char_id, const ItemInst* inst,
 
     // Save bag contents, if slot supports bag contents
 	if (inst->IsType(ItemClassContainer) && Inventory::SupportsContainers(slot_id)) {
-		for (uint8 idx = SUB_BEGIN; idx < EmuConstants::ITEM_CONTAINER_SIZE; idx++) {
+		// Limiting to bag slot count will get rid of 'hidden' duplicated items and 'Invalid Slot ID'
+		// messages through attrition (and the modded code in SaveInventory)
+		for (uint8 idx = SUB_BEGIN; idx < inst->GetItem()->BagSlots && idx < EmuConstants::ITEM_CONTAINER_SIZE; idx++) {
 			const ItemInst* baginst = inst->GetItem(idx);
 			SaveInventory(char_id, baginst, Inventory::CalcSlotId(slot_id, idx));
 		}

zone/corpse.cpp

@@ -1217,9 +1217,9 @@ void Corpse::LootItem(Client* client, const EQApplicationPacket* app) {
 	linker.SetLinkType(linker.linkItemInst);
 	linker.SetItemInst(inst);
 
-		auto item_link = linker.GenerateLink();
+	auto item_link = linker.GenerateLink();
 
-		client->Message_StringID(MT_LootMessages, LOOTED_MESSAGE, item_link.c_str());
+	client->Message_StringID(MT_LootMessages, LOOTED_MESSAGE, item_link.c_str());
 
 	if (!IsPlayerCorpse()) {
 			Group *g = client->GetGroup();

zone/inventory.cpp

@@ -882,28 +882,64 @@ bool Client::PutItemInInventory(int16 slot_id, const ItemInst& inst, bool client
 void Client::PutLootInInventory(int16 slot_id, const ItemInst &inst, ServerLootItem_Struct** bag_item_data)
 {
 	Log.Out(Logs::Detail, Logs::Inventory, "Putting loot item %s (%d) into slot %d", inst.GetItem()->Name, inst.GetItem()->ID, slot_id);
-	m_inv.PutItem(slot_id, inst);
 
-	SendLootItemInPacket(&inst, slot_id);
+	bool cursor_empty = m_inv.CursorEmpty();
 
 	if (slot_id == MainCursor) {
+		m_inv.PushCursor(inst);
 		auto s = m_inv.cursor_cbegin(), e = m_inv.cursor_cend();
 		database.SaveCursor(this->CharacterID(), s, e);
 	}
 	else {
+		m_inv.PutItem(slot_id, inst);
 		database.SaveInventory(this->CharacterID(), &inst, slot_id);
 	}
 
-	if(bag_item_data) { // bag contents
+	// Subordinate items in cursor buffer must be sent via ItemPacketSummonItem or we just overwrite the visible cursor and desync the client
-		int16 interior_slot;
+	if (slot_id == MainCursor && !cursor_empty) {
-		// our bag went into slot_id, now let's pack the contents in
+		// RoF+ currently has a specialized cursor handler
-		for(int i = SUB_BEGIN; i < EmuConstants::ITEM_CONTAINER_SIZE; i++) {
+		if (GetClientVersion() < ClientVersion::RoF)
-			if(bag_item_data[i] == nullptr)
+			SendItemPacket(slot_id, &inst, ItemPacketSummonItem);
+	}
+	else {
+		SendLootItemInPacket(&inst, slot_id);
+	}
+	
+	if (bag_item_data) {
+		for (int index = 0; index < EmuConstants::ITEM_CONTAINER_SIZE; ++index) {
+			if (bag_item_data[index] == nullptr)
 				continue;
-			const ItemInst *bagitem = database.CreateItem(bag_item_data[i]->item_id, bag_item_data[i]->charges, bag_item_data[i]->aug_1, bag_item_data[i]->aug_2, bag_item_data[i]->aug_3, bag_item_data[i]->aug_4, bag_item_data[i]->aug_5, bag_item_data[i]->aug_6, bag_item_data[i]->attuned);
+
-			interior_slot = Inventory::CalcSlotId(slot_id, i);
+			const ItemInst *bagitem = database.CreateItem(
-			Log.Out(Logs::Detail, Logs::Inventory, "Putting bag loot item %s (%d) into slot %d (bag slot %d)", inst.GetItem()->Name, inst.GetItem()->ID, interior_slot, i);
+				bag_item_data[index]->item_id,
-			PutLootInInventory(interior_slot, *bagitem);
+				bag_item_data[index]->charges,
+				bag_item_data[index]->aug_1,
+				bag_item_data[index]->aug_2,
+				bag_item_data[index]->aug_3,
+				bag_item_data[index]->aug_4,
+				bag_item_data[index]->aug_5,
+				bag_item_data[index]->aug_6,
+				bag_item_data[index]->attuned
+				);
+
+			// Dump bag contents to cursor in the event that owning bag is not the first cursor item
+			// (This assumes that the data passed is correctly associated..no safety checks are implemented)
+			if (slot_id == MainCursor && !cursor_empty) {
+				Log.Out(Logs::Detail, Logs::Inventory,
+					"Putting bag loot item %s (%d) into slot %d (non-empty cursor override)",
+					inst.GetItem()->Name, inst.GetItem()->ID, MainCursor);
+
+				PutLootInInventory(MainCursor, *bagitem);
+			}
+			else {
+				auto bag_slot = Inventory::CalcSlotId(slot_id, index);
+
+				Log.Out(Logs::Detail, Logs::Inventory,
+					"Putting bag loot item %s (%d) into slot %d (bag slot %d)",
+					inst.GetItem()->Name, inst.GetItem()->ID, bag_slot, index);
+
+				PutLootInInventory(bag_slot, *bagitem);
+			}
 			safe_delete(bagitem);
 		}
 	}