items disappearing after last charge is used - possible fix, test your items!

**Congdar** · #1 01-08-2009, 09:16 PM

This is working for me using the Titanium client, but I read a post where KLS mentioned that the 0.6.2 client handles some things differently. Looking for testers!

\common\item.h existing:

Code:

	// Remove item from inventory
	void DeleteItem(sint16 slot_id, uint8 quantity=0);

\common\item.h change to:

Code:

	// Remove item from inventory
	bool DeleteItem(sint16 slot_id, uint8 quantity=0, bool isEquipment=false);

\common\Item.cpp replace method Inventory::DeleteItem(sint16 slot_id, uint8 quantity) with:

Code:

// Remove item from inventory (with memory delete)
bool Inventory::DeleteItem(sint16 slot_id, uint8 quantity, bool isEquipment)
{
	// Pop item out of inventory map (or queue)
	ItemInst* item_to_delete = PopItem(slot_id);
	
	// Determine if object should be fully deleted, or
	// just a quantity of charges of the item can be deleted
	if (item_to_delete && (quantity > 0)) {
		item_to_delete->SetCharges(item_to_delete->GetCharges() - quantity);
		if((item_to_delete->GetCharges() > 0) || isEquipment) {
			// Charges still exist!  Put back into inventory
			_PutItem(slot_id, item_to_delete);
			return false;
		}
	}
	
	// Item can now be destroyed
	safe_delete(item_to_delete);
	return true;
}

\zone\inventory.cpp replace method Client::DeleteItemInInventory(sint16 slot_id, sint8 quantity, bool client_update) with:

Code:

// Remove item from inventory
void Client::DeleteItemInInventory(sint16 slot_id, sint8 quantity, bool client_update) {
	#if (EQDEBUG >= 5)
		LogFile->write(EQEMuLog::Debug, "DeleteItemInInventory(%i, %i, %s)", slot_id, quantity, (client_update) ? "true":"false");
	#endif

	bool isDeleted = false;
	const Item_Struct* item = m_inv[slot_id]->GetItem();
	if(
		(m_inv[slot_id]->GetItem()->Click.Type == ET_Expendable) ||
		(m_inv[slot_id]->GetItem()->Click.Type == ET_ClickEffect) ||
		(m_inv[slot_id]->GetItem()->Click.Type == ET_ClickEffect2) ||
		(item->ItemType == ItemTypeFood) ||
		(item->ItemType == ItemTypeDrink) ||
		(item->ItemType == ItemTypeBandage) ||
		(item->ItemType == ItemTypeThrowing) ||
		(item->ItemType == ItemTypeThrowingv2) ||
		(item->ItemType == ItemTypeArrow) ||
		(item->ItemType == ItemTypeFishingBait) ||
		(item->ItemType == ItemTypeAlcohol) ||
		(item->ItemType == ItemTypePoison) ||
		(item->ItemType == ItemTypeStackable)
		) {
			isDeleted = m_inv.DeleteItem(slot_id, quantity);
	}
	else if(m_inv[slot_id]->GetItem()->Click.Type == ET_EquipClick) {
		isDeleted = m_inv.DeleteItem(slot_id, quantity, true);
	}

	const ItemInst* inst=NULL;
	if(slot_id==SLOT_CURSOR) {
		list<ItemInst*>::const_iterator s=m_inv.cursor_begin(),e=m_inv.cursor_end();
		database.SaveCursor(character_id, s, e);
	}
	else {
		// Save change to database
		inst = m_inv[slot_id];
		database.SaveInventory(character_id, inst, slot_id);
	}

	if(client_update) {
		EQApplicationPacket* outapp;
		if(inst) {
			if(!inst->IsStackable() && !isDeleted) 
				// Non stackable item with charges = Item with clicky spell effect ? Delete a charge.
				outapp = new EQApplicationPacket(OP_DeleteCharge, sizeof(MoveItem_Struct));
			else
				// 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);
		}
		else {
			outapp = new EQApplicationPacket(OP_MoveItem, sizeof(MoveItem_Struct));
			MoveItem_Struct* delitem	= (MoveItem_Struct*)outapp->pBuffer;
			delitem->from_slot			= slot_id;
			delitem->to_slot			= 0xFFFFFFFF;
			delitem->number_in_stack	= 0xFFFFFFFF;
			QueuePacket(outapp);
			safe_delete(outapp);
		}
	}
}

**Congdar** · #2 01-10-2009, 06:55 PM

This still needs some tweaking. Scribing spells is broke and it looks like trading is broke too, but hey... your items don't poof when you use the last charge! I've made a fix for the spell scribing and I'm working on the trading issue now.

**cavedude** · #3 01-10-2009, 07:33 PM

Certain items don't deplete when used in tradeskills as well with this patch, one example is: http://lucy.allakhazam.com/item.html?id=27760 I also got a report that inventories didn't seem to be updating at all anymore.

Also, PEQ was getting this crash which is related to the above code:

Code:

#0  Client::DeleteItemInInventory(short, signed char, bool) (this=0x85915e0,
    slot_id=30, quantity=0 '\0', client_update=false) at Item.h:300
#1  0x080e271c in Client::OPMemorizeSpell(EQApplicationPacket const*) (
    this=0x85915e0, app=0x84a0440) at client_process.cpp:1097
#2  0x0819ab52 in Client::Handle_OP_MemorizeSpell(EQApplicationPacket const*) (
    this=0x0, app=0x0) at client_packet.cpp:3053
#3  0x08192570 in Client::HandlePacket(EQApplicationPacket const*) (
    this=0x85915e0, app=0x44112008) at client_packet.cpp:409
#4  0x080dfefd in Client::Process() (this=0x85915e0) at client_process.cpp:604
#5  0x080c2553 in EntityList::MobProcess() (this=0x82e2a20) at entity.cpp:462
#6  0x080eb9a4 in main (argc=0, argv=0xbfffef74) at net.cpp:494

**Congdar** · #4 01-10-2009, 08:37 PM

Thanks for testing this. Your crash looks like the spellbook scribing I mentioned above and I'll take a look into the tradeskill consumption.

**Congdar** · #5 01-11-2009, 11:59 PM

Try this new code out. I've fixed the Spinechill Silk combine and the Spellbook Scribe crash.
If somebody could list some thing to test or items that currently go away when the charges are used but aren't supposed to, that would be great.

Things I've tested with this new update:
Tradeskills:
Created Fish Rolls by combining 1 Fresh Fish and 1 Bat Wings. Stacked supplies correctly subtracted.
Created Spinechill Silk by combining 1 Spinechill Ichor Vial and 2 Spinechill Silk. Unstackable supplies correctly subtracted.

Worn items with charges:
Cleric/Paladin Shield - Aegis of Life 1 charge of Superior Healing. Charge used, Item not deleted.
Journeyman's Boots. Unlimited charges, not deleted.

Inventory items with charges:
Prayers of Life 5 charges of Word of Healing. Charges used, Item deleted.
Crystalized Pumice 5 charges of Nullify Magic. Charges used, Item deleted.
Stack of 10 Cloudy Potions. Stack decremented correctly and last one deleted.
5 Dose Cloudy Potion. Charges used, Item deleted.
Stack of Bandages. Took some damage and used Bind Wound skill. Bandages decremented correctly.

Spell Scribing:
Wizard memorized Gate spell. Spell scroll deleted, no crash.

Merchants:
Purchased stackable items
Purchased unstackable items
Sold stackable items
Sold unstackable items

Bots:
Trade with bots to give them equipment works.
#bot inventory remove used successfully

GM:
#si [itemid] worked.
#nukeitem [itemid] worked.

You will still need the item.h and Item.cpp changes from the first post.

This is the spell scribing fix:
\zone\client_process.cpp void Client::OPMemorizeSpell(const EQApplicationPacket* app)
Replace the case statement for memSpellScribing:

Code:

		case memSpellScribing:	{	// scribing spell to book
			ItemInst* inst = m_inv.PopItem(SLOT_CURSOR);
			
			if(inst && inst->IsType(ItemClassCommon))
			{
				const Item_Struct* item = inst->GetItem();
				
				if(item && item->Scroll.Effect == (sint32)(memspell->spell_id))
				{
					ScribeSpell(memspell->spell_id, memspell->slot);

					// Destroy scroll on cursor
					EQApplicationPacket* outapp = new EQApplicationPacket(OP_MoveItem, sizeof(MoveItem_Struct));
					MoveItem_Struct* spellmoveitem = (MoveItem_Struct*) outapp->pBuffer;
					spellmoveitem->from_slot = SLOT_CURSOR;
					spellmoveitem->to_slot = SLOT_INVALID;
					spellmoveitem->number_in_stack = 0;
					QueuePacket(outapp);
					safe_delete(outapp);

					DeleteItemInInventory(SLOT_CURSOR);
					
				}
				else 
					Message(0,"Scribing spell: inst exists but item does not or spell ids do not match.");
			}
			else
				Message(0,"Scribing a spell without an inst on your cursor?");
			break;

			}

with

Code:

		case memSpellScribing:	{	// scribing spell to book
			const ItemInst* inst = m_inv[SLOT_CURSOR];

			if(inst && inst->IsType(ItemClassCommon))
			{
				const Item_Struct* item = inst->GetItem();
				
				if(item && item->Scroll.Effect == (sint32)(memspell->spell_id))
				{
					ScribeSpell(memspell->spell_id, memspell->slot);
					DeleteItemInInventory(SLOT_CURSOR, 1, true);
				}
				else 
					Message(0,"Scribing spell: inst exists but item does not or spell ids do not match.");
			}
			else
				Message(0,"Scribing a spell without an inst on your cursor?");
			break;
		}

Here's a complete replacement again for the method \zone\inventory.cpp Client::DeleteItemInInventory(sint16 slot_id, sint8 quantity, bool client_update)

Code:

// Remove item from inventory
void Client::DeleteItemInInventory(sint16 slot_id, sint8 quantity, bool client_update) {
	#if (EQDEBUG >= 5)
		LogFile->write(EQEMuLog::Debug, "DeleteItemInInventory(%i, %i, %s)", slot_id, quantity, (client_update) ? "true":"false");
	#endif

	if(!m_inv[slot_id]) {
		return;
	}

	bool isDeleted = false;
	if(m_inv[slot_id]->GetItem()->Click.Type == ET_EquipClick) {
		isDeleted = m_inv.DeleteItem(slot_id, quantity, true);
	}
	else {
		isDeleted = m_inv.DeleteItem(slot_id, quantity);
	}

	const ItemInst* inst=NULL;
	if(slot_id==SLOT_CURSOR) {
		list<ItemInst*>::const_iterator s=m_inv.cursor_begin(),e=m_inv.cursor_end();
		database.SaveCursor(character_id, s, e);
	}
	else {
		// Save change to database
		inst = m_inv[slot_id];
		database.SaveInventory(character_id, inst, slot_id);
	}

	if(client_update) {
		EQApplicationPacket* outapp;
		if(inst) {
			if(!inst->IsStackable() && !isDeleted) 
				// Non stackable item with charges = Item with clicky spell effect ? Delete a charge.
				outapp = new EQApplicationPacket(OP_DeleteCharge, sizeof(MoveItem_Struct));
			else
				// 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);
		}
		else {
			outapp = new EQApplicationPacket(OP_MoveItem, sizeof(MoveItem_Struct));
			MoveItem_Struct* delitem	= (MoveItem_Struct*)outapp->pBuffer;
			delitem->from_slot		= slot_id;
			delitem->to_slot		= 0xFFFFFFFF;
			delitem->number_in_stack	= 0xFFFFFFFF;
			QueuePacket(outapp);
			safe_delete(outapp);
		}
	}
}

**cavedude** · #6 01-13-2009, 12:01 PM

Found a new problem with the above code. Permanent items with charges do not delete off of player corpses. They remain on the player, AND also appear on the corpse. The charges of the items that remain on the player do seem to be depleted, as even unlimited items say the item is out of charges when used. Deleting those items, and looting the ones of their corpse works fine.

Edit: I should mention this bug only effects those who enable player corpses with items!

**Congdar** · #7 01-13-2009, 12:20 PM

Let me see if I understand this. A character dies, the item does correctly go to thier corpse with the correct charges, but a duplicate item with no charges stays with them including items normally with unlimited charges but they can loot multiple dupes off the corpse because '...do not delete off of player corpses'?

Can you tell me which item this is or is it any item with charges? I'll change my server rules to have corpses with items and check it out.

**cavedude** · #8 01-13-2009, 12:30 PM

That's correct. It's only been proven using items with unlimited charges. However, I would assume limited charged items also suffer from the same bug. Here are the items that were used:

http://lucy.allakhazam.com/item.html?id=26606
http://lucy.allakhazam.com/item.html?id=1113
http://lucy.allakhazam.com/item.html?id=28994

**cavedude** · #9 01-13-2009, 01:28 PM

OK, I just tried it out and here is what I came up with:

I died with Aegis of Life (1 charge) being the only worn clicky item in my possession. I used the charge. When I zoned back in, the item was missing from my inventory, however the item on the corpse had gotten its charge back.

I died with Talisman of the Burrower (unlimited charges) being the only worn clicky item in my possession. When I zoned back in, the item was both in my inventory and my corpse (duplicated.) However, both items worked normally, so at least that part of the above report was incorrect.

I died with Aegis of Life (1 charge) and Talisman of the Burrower (unlimited charges) being the only worn clicky items in my possession. I used the charge on Aegis. When I zoned back in, BOTH items were duplicated on my corpse and inventory. Aegis of Life in my inventory still showed as the charge being used, however the one on the corpse once again had the charge replenished.

I hope this helps some!

Also, the Wee Harvester http://lucy.allakhazam.com/item.html?id=13980 is an example of a worn clicky with 20 charges that still deletes when the last charge is used, I remember from Live it should not do so.

**Congdar** · #10 01-15-2009, 10:11 AM

here's the fix for the charged item duping:

in PlayerCorpse.cpp find method: void Corpse::MoveItemToCorpse(Client *client, ItemInst *item, sint16 equipslot)
then find:

Code:

client->DeleteItemInInventory(interior_slot, interior_item->GetCharges(), false);

and delete the last 2 args so it looks like:

Code:

client->DeleteItemInInventory(interior_slot);

and the same for the one a couple of lines below it find:

Code:

client->DeleteItemInInventory(equipslot, item->GetCharges(), false);

change to:

Code:

client->DeleteItemInInventory(equipslot);

**cavedude** · #11 01-16-2009, 12:24 AM

I tested the recent changes briefly on my test server and all looked well. It'll go Live on PEQ with the reboot in the morning. Thanks for perfecting this! If all goes well tomorrow, might as well commit to SVN.

**Congdar** · #12 01-16-2009, 01:21 PM

This fixes the Wee Harvester from poofing when the last charge is used:
Inventory.cpp find method: void Client::DeleteItemInInventory(sint16 slot_id, sint8 quantity, bool client_update) then find:

Code:

	if(m_inv[slot_id]->GetItem()->Click.Type == ET_EquipClick) {

and replace with:

Code:

	if((m_inv[slot_id]->GetItem()->Click.Type == ET_EquipClick) || (m_inv[slot_id]->IsWeapon())) {

and this fixes the items that are getting recharged when looting from your corpse:
PlayerCorpse.cpp find method: void Corpse::LootItem(Client* client, const EQApplicationPacket* app) then find:

Code:

		if(inst->IsStackable()) {
			if(item_data) 
				//Restore charges from the original item.
				inst->SetCharges(item_data->charges);
			else
				inst->SetCharges(1);
		} else {
			//default changes
			if(item->MaxCharges == -1)
				inst->SetCharges(1);
			else
				inst->SetCharges(item->MaxCharges);
		}

and replace with:

Code:

		if(inst->IsStackable()) {
			if(item_data) 
				//Restore charges from the original item.
				inst->SetCharges(item_data->charges);
			else
				inst->SetCharges(1);
		} else {
			//default changes
			if(item->MaxCharges == -1)
				inst->SetCharges(1);
			else {
				if(item_data) {
					//Restore charges from the original item.
					inst->SetCharges(item_data->charges);
				}
				else {
					inst->SetCharges(item->MaxCharges);
				}
			}
		}

**cavedude** · #13 01-19-2009, 12:31 AM

This has been on PEQ for a bit now with no ill effects reported. It works like a charm.

I say commit it.

**Congdar** · #14 01-19-2009, 01:12 AM

Thanks for testing this on your server Cavedude. Committed in SVN 278.