View Single Post
  #1  
Old 08-10-2012, 06:29 AM
Uleat's Avatar
Uleat
Developer
 
Join Date: Apr 2012
Location: North Carolina
Posts: 2,815
Default COMMITTED: CSD Bugged Corpse Patch

After some very trying times with the 'RespawnFromHover' option, I finally came up with something that should address
the client lock-out associated with it and not cause any CSD's.

I've tested this particular patch numerous times and found no discontinuity in memory vs. database vs. corpse data.

[CSD Bugged Corpse Patch]
Code:
Index: inventory.cpp
===================================================================
--- inventory.cpp	(revision 2175)
+++ inventory.cpp	(working copy)
@@ -397,9 +397,11 @@
 		LogFile->write(EQEMuLog::Debug, "DeleteItemInInventory(%i, %i, %s)", slot_id, quantity, (client_update) ? "true":"false");
 	#endif
 
-	if(!m_inv[slot_id]) {
+	// Added 'IsSlotValid(slot_id)' check to both segments of client packet processing.
+	// Cursor queue slots were slipping through and crashing client
+	if(!m_inv[slot_id]) { 
 		// Make sure the client deletes anything in this slot to match the server.
-		if(client_update) {
+		if(client_update && IsValidSlot(slot_id)) {
 			EQApplicationPacket* outapp;
 			outapp = new EQApplicationPacket(OP_DeleteItem, sizeof(DeleteItem_Struct));
 			DeleteItem_Struct* delitem	= (DeleteItem_Struct*)outapp->pBuffer;
@@ -427,7 +429,7 @@
 		    database.SaveInventory(character_id, inst, slot_id);
 	}
 
-	if(client_update) {
+	if(client_update && IsValidSlot(slot_id)) { 
 		EQApplicationPacket* outapp;
 		if(inst) {
 			if(!inst->IsStackable() && !isDeleted) 
@@ -437,13 +439,13 @@
 				// Stackable, arrows, etc ? Delete one from the stack
 				outapp = new EQApplicationPacket(OP_DeleteItem, sizeof(MoveItem_Struct));
 
-			DeleteItem_Struct* delitem	= (DeleteItem_Struct*)outapp->pBuffer;
-			delitem->from_slot			= slot_id;
-			delitem->to_slot			= 0xFFFFFFFF;
-			delitem->number_in_stack	= 0xFFFFFFFF;
-			for(int loop=0;loop<quantity;loop++)
-				QueuePacket(outapp);
-			safe_delete(outapp);
+				DeleteItem_Struct* delitem	= (DeleteItem_Struct*)outapp->pBuffer;
+				delitem->from_slot			= slot_id;
+				delitem->to_slot			= 0xFFFFFFFF;
+				delitem->number_in_stack	= 0xFFFFFFFF;
+				for(int loop=0;loop<quantity;loop++)
+					QueuePacket(outapp);
+				safe_delete(outapp);
 		}
 		else {
 			outapp = new EQApplicationPacket(OP_MoveItem, sizeof(MoveItem_Struct));
Index: PlayerCorpse.cpp
===================================================================
--- PlayerCorpse.cpp	(revision 2175)
+++ PlayerCorpse.cpp	(working copy)
@@ -352,13 +352,20 @@
 		pp->silver = 0;
 		pp->gold = 0;
 		pp->platinum = 0;
+
+		// need to tell client that cash has changed... issue looks like a money duplication error,
+		// but server value is actually correct. (think it is 'RespawnFromHover' related.)
 	
 		// get their tints
 		memcpy(item_tint, &client->GetPP().item_tint, sizeof(item_tint));
 	
 		// solar: TODO soulbound items need not be added to corpse, but they need
 		// to go into the regular slots on the player, out of bags
-	
+		
+		// personal and cursor bag slots (251-340) are moved to corpse..should be deleting db entries
+		// too. reworked code to return and merge a list for the delete query builder instead of adding
+		// MoveItemToCorpse code to each loop.
+
 		// worn + inventory + cursor
         std::list<uint32> removed_list;
         bool cursor = false;
@@ -367,23 +374,26 @@
 			item = client->GetInv().GetItem(i);
 			if((item && (!client->IsBecomeNPC())) || (item && client->IsBecomeNPC() && !item->GetItem()->NoRent))
 			{
-				MoveItemToCorpse(client, item, i);
-                removed_list.push_back(i);
+				removed_list.merge(MoveItemToCorpse(client, item, i));   
 			}
 		}
 
 		// cursor queue
-		iter_queue it;
-		for(it=client->GetInv().cursor_begin(),i=8000; it!=client->GetInv().cursor_end(); it++,i++) {
-			item = *it;
-			if((item && (!client->IsBecomeNPC())) || (item && client->IsBecomeNPC() && !item->GetItem()->NoRent))
-			{
-				MoveItemToCorpse(client, item, i);
-                cursor = true;
+		if (!RuleB(Character, RespawnFromHover)) {
+			// bumped starting assignment to 8001 because any in-memory 'slot 8000' item was moved above as 'slot 30'
+			// this was mainly for client profile state reflection..should match db player inventory entries now.
+			iter_queue it;
+			for(it=client->GetInv().cursor_begin(),i=8001; it!=client->GetInv().cursor_end(); it++,i++) {
+				item = *it;
+				if((item && (!client->IsBecomeNPC())) || (item && client->IsBecomeNPC() && !item->GetItem()->NoRent))
+				{
+					removed_list.merge(MoveItemToCorpse(client, item, i));
+			        cursor = true;
+				}
 			}
 		}
 		
-        if(removed_list.size() != 0) {
+		if(removed_list.size() != 0) {
             std::stringstream ss("");
             ss << "DELETE FROM inventory WHERE charid=" << client->CharacterID();
             ss << " AND (";
@@ -401,14 +411,33 @@
             ss << ")";
             database.RunQuery(ss.str().c_str(), ss.str().length());
         }
-        
+
+		// some of this code is redundent now..hover rule check in cursor queue should eliminate some code's activation.
         if(cursor) {
-            std::list<ItemInst*>::const_iterator start = client->GetInv().cursor_begin();
-            std::list<ItemInst*>::const_iterator finish = client->GetInv().cursor_end();
-            database.SaveCursor(client->CharacterID(), 
-                start, finish);
+			while(!client->GetInv().CursorEmpty()) {
+				if (RuleB(Character, RespawnFromHover)) {
+					client->Message(0, "Attempting to delete %s from cursor.", client->GetInv().GetItem(SLOT_CURSOR)->GetItem()->Name);
+					client->DeleteItemInInventory(SLOT_CURSOR, 0, true, false);
+				}
+				else {
+					client->DeleteItemInInventory(SLOT_CURSOR, 0, false, false); // no need to update client without hover
+				}
+			}
+			// you shouldn't see these messages unless 'RespawnFromHover' rule is changed between cursor queue process and here...
+			if (RuleB(Character, RespawnFromHover)) {
+				client->Message(13, "Warning: Your cursor may contain duplicate items not found on the server!");
+				client->Message(0, "Place these items into an empty inventory slot to resolve this problem.");
+			}            
         }
+		else {
+			if(RuleB(Character, RespawnFromHover) && !client->GetInv().CursorEmpty()) {
+				std::list<ItemInst*>::const_iterator start = client->GetInv().cursor_begin();
+				std::list<ItemInst*>::const_iterator finish = client->GetInv().cursor_end();
+				database.SaveCursor(client->CharacterID(), start, finish);
+			}
+		}
 
+		// client->CalcBonuses(); // will only affect offline profile viewing for dead characters..otherwise unneeded overhead
 		client->Save();
 	} //end "not leaving naked corpses"
 	
@@ -417,28 +446,35 @@
 }
 
 // solar: helper function for client corpse constructor
-void Corpse::MoveItemToCorpse(Client *client, ItemInst *item, sint16 equipslot)
+std::list<uint32> Corpse::MoveItemToCorpse(Client *client, ItemInst *item, sint16 equipslot)
 {
 	int bagindex;
 	sint16 interior_slot;
 	ItemInst *interior_item;
+	std::list<uint32> returnlist;
 
 	AddItem(item->GetItem()->ID, item->GetCharges(), equipslot, item->GetAugmentItemID(0), item->GetAugmentItemID(1), item->GetAugmentItemID(2), item->GetAugmentItemID(3), item->GetAugmentItemID(4));
-	if(item->IsType(ItemClassContainer))
+	returnlist.push_back(equipslot);
+
+	// Qualified bag slot iterations. processing bag slots that don't exist is probably not a good idea.
+	if(item->IsType(ItemClassContainer) && ((equipslot >= 22 && equipslot <=30))) // Limit the bag check to inventory and cursor slots.
 	{
 		for(bagindex = 0; bagindex <= 10; bagindex++)
 		{
+			// For empty bags in cursor queue, slot was being resolved as SLOT_INVALID (-1)
 			interior_slot = Inventory::CalcSlotId(equipslot, bagindex);
 			interior_item = client->GetInv().GetItem(interior_slot);
 
 			if(interior_item)
 			{
 				AddItem(interior_item->GetItem()->ID, interior_item->GetCharges(), interior_slot, interior_item->GetAugmentItemID(0), interior_item->GetAugmentItemID(1), interior_item->GetAugmentItemID(2), interior_item->GetAugmentItemID(3), interior_item->GetAugmentItemID(4));
+				returnlist.push_back(Inventory::CalcSlotId(equipslot, bagindex));
 				client->DeleteItemInInventory(interior_slot, 0, true, false);
 			}
 		}
 	}
 	client->DeleteItemInInventory(equipslot, 0, true, false);
+	return returnlist;
 }
 
 // To be called from LoadFromDBData
@@ -981,19 +1017,43 @@
 		ItemList::iterator cur,end;
 		cur = itemlist.begin();
 		end = itemlist.end();
+
+		// Observed some odd behavior concerning the last corpse slot as coded (SoF client in this case.)
+		// Clicking index 29 (zero-based) with an item in index 28 results in retrieving index 28 item.
+		// Clicking index 29 with no item in index 28 results in the closing of the corpse loot window.
+
+		int corpselootlimit = 30; // 30 is the original value // con check value in QueryLoot needs to reflect this value
+		/* need actual corpse limit values per client (or client range)..if always 30, then these con checks are unneeded
+		// enumeration shouldn't be needed unless someone finds a use for this info elsewhere
+		if (client->GetClientVersion()>=EQClientVoA)
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClientHoT)
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClientUnderfoot)
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClientSoD)
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClientSoF) // SoF has 32 visible slots..change untested
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClientTitanium)
+			corpselootlimit=30;
+		else if (client->GetClientVersion()>=EQClient62)
+			corpselootlimit=30;
+		else
+			corpselootlimit=30; // */
+
 		for(; cur != end; cur++) {
 			ServerLootItem_Struct* item_data = *cur;
 			item_data->lootslot = 0xFFFF;
+			
+			// Dont display the item if it's in a bag
 
-			// Dont display the item if it's in a bag
-			if(!IsPlayerCorpse() || item_data->equipSlot <= 30 || tCanLoot>=3)
+			// added cursor queue slots to corpse item visibility list. Nothing else should be making it to corpse.
+			if(!IsPlayerCorpse() || item_data->equipSlot <= 30 || tCanLoot>=3 ||
+				(item_data->equipSlot >= 8000 && item_data->equipSlot <= 8999))
 			{
-				if (i >= 30)
+				if (i < corpselootlimit) // < 30 (0 - 29) 
 				{
-						Message(13, "Warning: Too many items to display. Loot some then re-loot the corpse to see the rest");
-				}
-				else
-				{
 					item = database.GetItem(item_data->item_id);
 					if (client && item)
 					{
@@ -1006,9 +1066,24 @@
 						item_data->lootslot = i;
 					}
 				}
+				else if (i == corpselootlimit) // = 30
+				{
+					client->Message(13, "*** This corpse contains more items than can be displayed! ***");
+					client->Message(0, "Remove items and re-loot corpse to access remaining inventory.");
+				}
 				i++;
 			}
 		}
+		if (i > corpselootlimit) // > 30 (remember 'i' is increased again after the last iteration, so no '=')
+			client->Message(0, "(%s contains %i additional %s.)", GetName(), (i-corpselootlimit), (i-corpselootlimit)==1?"item":"items");
+
+		if (IsPlayerCorpse() && i == 0 && itemlist.size() > 0) { // somehow, corpse contains items, but client doesn't see them...
+			client->Message(13, "This corpse contains items that you do not have permission to access!");
+			client->Message(0, "Contact a GM for assistance to see if item replacement is necessary.");
+			client->Message(0, "BUGGED CORPSE [DBID: %i, Name: %s, Item Count: %i]", GetDBID(), GetName(), itemlist.size());
+			// if needed/wanted - create log dump->iterate corpse list..need pointer to log file
+			// could add code to check for owning client and give list of bugged items on corpse
+		}
 	}
 	
 	// Disgrace: Client seems to require that we send the packet back...
Index: PlayerCorpse.h
===================================================================
--- PlayerCorpse.h	(revision 2175)
+++ PlayerCorpse.h	(working copy)
@@ -111,7 +111,7 @@
 	inline int GetRezzExp() { return rezzexp; }
 
 protected:
-	void MoveItemToCorpse(Client *client, ItemInst *item, sint16 equipslot);
+	std::list<uint32> MoveItemToCorpse(Client *client, ItemInst *item, sint16 equipslot);
 
 private:
 	bool		p_PlayerCorpse;

Change to Client:eleteItemInInventory:
Added IsValidSlot() checks to client updates. When invalid slots were being sent, the client was crashing.

Change to Corpse::Corpse(Client):
Added discrimantor for 'RespawnFromHover' in the cursor queue processor. This matches the client behavior in this mode
and still allows for full cursor removal with RespawnFromHover = false. The money issue was not addressed in this patch
and I will leave that to someone else.

Change to Corpse::MoveItemToCorpse:
Changed the procedure to a return function of std::list<uint32> and qualified the bag slot check to only check bags
in the 251 to 340 range (inventory slots 22 to 30.) Otherwise, cursor queue bags slots were being processed for items..and
we know those don't exist...

Change to Corpse::MakeLootRequestPacket:
Added cursor queue slot range to packet builder. Should alleviate the cursor queue items that are hidden on a corpse and
making it bugged. Original coding allowed these items to be placed on a corpse, but not be seen by a client when they
looted the corpse. (CSD Support Patch contains a new #corpse sub-command that allows inspection of a corpse's
contents. Based on the iterations in Corpse::Corpse(Client...), there shouldn't be any additional ranges on a corpse.)


There is some outdated code that I left in place because this patch currently works and I didn't want to spend additional
time with it. It shouldn't affect server operation and worked as intended prior to adding the cursor queue
RespawnFromHover code.

I consider this patch full BETA, but it should be tested out on test servers before full implementation due to the nature
of the affected code.
__________________
Uleat of Bertoxxulous

Compilin' Dirty

Last edited by cavedude; 09-13-2012 at 01:49 PM..
Reply With Quote