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

[Congdar]

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]

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:

		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

		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)

// 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]

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]

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]

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]

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]

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:

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

and delete the last 2 args so it looks like:

client->DeleteItemInInventory(interior_slot);

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

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

change to:

client->DeleteItemInInventory(equipslot);