TrinityCore/TrinityCore — pull request #7747 by Elron103 (2012-09-13 01:34:45+00:00)
Title: Core/Mail: Fix merging of items taken from mail leading to invalid entries in item_instance

## Description

At the moment taking items from mail can cause invalid entries in item_instance table under specific circumstances.

Everything works fine if you take an item from mail and it creates a new stack in your inventory. But an invalid entry in item_instance table is left behind, if you take an item from mail, and that item merges with another stack of the same item in your inventory.

## Fix description

https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/Handlers/MailHandler.cpp#L448
"Item* it = player->GetMItem(itemId);"

This gives us a pointer to the item, which is to be taken from mail to characters inventory. This item pointer has the item state ITEM_NEW.

Because of this item state, the code for storing items, assumes, that the item never existed in database. The code for storing that new stacked item tries to delete the item, but it fails, since the item will not be removed from database, if it is set from ITEM_NEW to ITEM_REMOVED. See following code:

https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/Entities/Item/Item.cpp#L678
"// pretend the item never existed"

But in fact, our item existed in database. If you take a look at that item state handling there, you will notice, that we are only allowed to set ITEM_UNCHANGED, if we currently have ITEM_NEW.

The state ITEM_UNCHANGED works fine for both cases (either creating a new item or stacking with another item).

On one hand, ITEM_UNCHANGED can be set to ITEM_REMOVED successfully by the storing item code here:

https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/Entities/Player/Player.cpp#L12226

By setting this to ITEM_REMOVED, it will be removed on next inventory save, which even seems to be called by taking items from the mail. So stacking two items from mail works smoothly again.

ITEM_UNCHANGED also works for storing new items from mail, since the state will be changed back to ITEM_NEW anyways:

https://github.com/TrinityCore/TrinityCore/blob/master/src/server/game/Entities/Player/Player.cpp#L12540

These are the reasons, why it has to be set to state ITEM_UNCHANGED in order to work as intended.

## Revisions

Trinity: https://github.com/TrinityCore/TrinityCore/commit/936c62ef4cfe85b04f689d08696a29a792815c2a
Database: TDB rev. 48