diff --git a/src/server/scripts/Commands/cs_mail.cpp b/src/server/scripts/Commands/cs_mail.cpp
index 6f622e2b30..3fcf3a2c83 100644
--- a/src/server/scripts/Commands/cs_mail.cpp
+++ b/src/server/scripts/Commands/cs_mail.cpp
@@ -243,37 +243,119 @@ public:
Player* player = target.GetConnectedPlayer();
- // Run the same script hook as the client return handler, failing early before any deletions
+ CharacterDatabaseTransaction trans = CharacterDatabase.BeginTransaction();
+
+ // Collect attachments before the script-hook check and any deletion, so a hook
+ // veto can still abort the command with nothing changed.
+ // mail_items is flushed to the DB on every item take, so it is authoritative even
+ // for online players; mails the session never loaded (expired at login, delivered
+ // mid-login, inserted externally) are returned with their items too. Same query
+ // shape as CHAR_SEL_MAILITEMS (LEFT JOIN to handle dangling mail_items) and same
+ // logic as Player::_LoadMailedItem. Items the session already has loaded are
+ // reused and stay owned by the session; the bool marks objects loaded (and owned)
+ // here.
+ std::vector> attachments;
+
+ QueryResult itemResult = CharacterDatabase.Query(
+ "SELECT creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments,"
+ " randomPropertyId, durability, playedTime, text, mi.item_guid, itemEntry, ii.owner_guid"
+ " FROM mail_items mi LEFT JOIN item_instance ii ON mi.item_guid = ii.guid"
+ " WHERE mi.mail_id = {}", mailId);
+
+ if (itemResult)
+ {
+ do
+ {
+ Field* itemFields = itemResult->Fetch();
+ uint32 itemGuid = itemFields[11].Get();
+ uint32 itemEntry = itemFields[12].Get();
+
+ // Prefer the item object the session already has loaded over creating a duplicate
+ if (Item* item = player ? player->GetMItem(itemGuid) : nullptr)
+ {
+ attachments.emplace_back(item, false);
+ continue;
+ }
+
+ // Handle dangling mail_items (missing item_instance)
+ if (!itemEntry)
+ {
+ LOG_ERROR("misc", "cs_mail: Mail #{} has dangling mail_items row for item_guid {}. Cleaning up.", mailId, itemGuid);
+
+ CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
+ delStmt->SetData(0, itemGuid);
+ trans->Append(delStmt);
+ continue;
+ }
+
+ ItemTemplate const* proto = sObjectMgr->GetItemTemplate(itemEntry);
+ if (!proto)
+ {
+ LOG_ERROR("misc", "cs_mail: Mail #{} has unknown item (entry: {}, guid: {}). Cleaning up.", mailId, itemEntry, itemGuid);
+
+ CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
+ delStmt->SetData(0, itemGuid);
+ trans->Append(delStmt);
+
+ // The mail is going away, so drop the unloadable item_instance row too
+ Item::DeleteFromDB(trans, itemGuid);
+ continue;
+ }
+
+ Item* item = NewItemOrBag(proto);
+ ObjectGuid ownerGuid = itemFields[13].Get()
+ ? ObjectGuid::Create(itemFields[13].Get())
+ : ObjectGuid::Empty;
+
+ if (!item->LoadFromDB(itemGuid, ownerGuid, itemFields, itemEntry))
+ {
+ LOG_ERROR("misc", "cs_mail: Item (GUID: {}) in mail #{} failed to load. Cleaning up.", itemGuid, mailId);
+
+ CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
+ delStmt->SetData(0, itemGuid);
+ trans->Append(delStmt);
+
+ // Queue the row cleanup on the shared transaction instead of executing it
+ // immediately; SaveToDB in ITEM_REMOVED state also frees the object
+ item->FSetState(ITEM_REMOVED);
+ item->SaveToDB(trans);
+ continue;
+ }
+
+ attachments.emplace_back(item, true);
+ } while (itemResult->NextRow());
+ }
+
+ // Run the same script hook as the client return handler for online targets,
+ // covering DB-loaded attachments as well, before any deletion
if (player)
{
- Mail* m = player->GetMail(mailId);
- if (m)
- {
- ObjectGuid senderGuid = ObjectGuid(HighGuid::Player, sender);
+ ObjectGuid senderGuid = ObjectGuid(HighGuid::Player, sender);
+ bool blocked = false;
- if (m->HasItems())
- {
- for (auto const& itemInfo : m->items)
- {
- Item* item = player->GetMItem(itemInfo.item_guid);
- if (item && !sScriptMgr->OnPlayerCanSendMail(player, senderGuid, ObjectGuid::Empty, subject, body, money, 0, item))
- {
- handler->SendErrorMessage(LANG_MAIL_RETURN_HOOK_BLOCKED);
- return true;
- }
- }
- }
- else if (!sScriptMgr->OnPlayerCanSendMail(player, senderGuid, ObjectGuid::Empty, subject, body, money, 0, nullptr))
- {
- handler->SendErrorMessage(LANG_MAIL_RETURN_HOOK_BLOCKED);
- return true;
- }
+ if (attachments.empty())
+ blocked = !sScriptMgr->OnPlayerCanSendMail(player, senderGuid, ObjectGuid::Empty, subject, body, money, 0, nullptr);
+
+ for (auto const& [item, loadedFromDb] : attachments)
+ {
+ if (blocked)
+ break;
+
+ blocked = !sScriptMgr->OnPlayerCanSendMail(player, senderGuid, ObjectGuid::Empty, subject, body, money, 0, item);
+ }
+
+ if (blocked)
+ {
+ for (auto const& [item, loadedFromDb] : attachments)
+ if (loadedFromDb)
+ delete item;
+
+ handler->SendErrorMessage(LANG_MAIL_RETURN_HOOK_BLOCKED);
+ return true;
}
}
// Same logic as WorldSession::HandleReturnToSender
- CharacterDatabaseTransaction trans = CharacterDatabase.BeginTransaction();
-
CharacterDatabasePreparedStatement* stmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_MAIL_BY_ID);
stmt->SetData(0, mailId);
trans->Append(stmt);
@@ -288,87 +370,19 @@ public:
if (player)
{
- // Online: same logic as WorldSession::HandleReturnToSender
- // Get pointer before RemoveMail (which removes from deque but does not delete the object)
+ // Drop the mail from the session's loaded state if present.
+ // RemoveMail only erases from the deque, it does not delete the object.
Mail* m = player->GetMail(mailId);
-
player->RemoveMail(mailId);
-
- if (m && m->HasItems())
- {
- for (auto const& itemInfo : m->items)
- {
- if (Item* item = player->GetMItem(itemInfo.item_guid))
- draft.AddItem(item);
-
- player->RemoveMItem(itemInfo.item_guid);
- }
- }
-
delete m;
}
- else
+
+ for (auto const& [item, loadedFromDb] : attachments)
{
- // Offline: load Item* objects from DB using same query shape as CHAR_SEL_MAILITEMS
- // (LEFT JOIN to handle dangling mail_items) and same logic as Player::_LoadMailedItem
- QueryResult itemResult = CharacterDatabase.Query(
- "SELECT creatorGuid, giftCreatorGuid, count, duration, charges, flags, enchantments,"
- " randomPropertyId, durability, playedTime, text, mi.item_guid, itemEntry, ii.owner_guid"
- " FROM mail_items mi LEFT JOIN item_instance ii ON mi.item_guid = ii.guid"
- " WHERE mi.mail_id = {}", mailId);
+ if (!loadedFromDb)
+ player->RemoveMItem(item->GetGUID().GetCounter());
- if (itemResult)
- {
- do
- {
- Field* itemFields = itemResult->Fetch();
- uint32 itemGuid = itemFields[11].Get();
- uint32 itemEntry = itemFields[12].Get();
-
- // Handle dangling mail_items (missing item_instance)
- if (!itemEntry)
- {
- LOG_ERROR("misc", "cs_mail: Mail #{} has dangling mail_items row for item_guid {}. Cleaning up.", mailId, itemGuid);
-
- CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
- delStmt->SetData(0, itemGuid);
- trans->Append(delStmt);
- continue;
- }
-
- ItemTemplate const* proto = sObjectMgr->GetItemTemplate(itemEntry);
- if (!proto)
- {
- LOG_ERROR("misc", "cs_mail: Mail #{} has unknown item (entry: {}, guid: {}). Cleaning up.", mailId, itemEntry, itemGuid);
-
- CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
- delStmt->SetData(0, itemGuid);
- trans->Append(delStmt);
- continue;
- }
-
- Item* item = NewItemOrBag(proto);
- ObjectGuid ownerGuid = itemFields[13].Get()
- ? ObjectGuid::Create(itemFields[13].Get())
- : ObjectGuid::Empty;
-
- if (!item->LoadFromDB(itemGuid, ownerGuid, itemFields, itemEntry))
- {
- LOG_ERROR("misc", "cs_mail: Item (GUID: {}) in mail #{} failed to load. Cleaning up.", itemGuid, mailId);
-
- CharacterDatabasePreparedStatement* delStmt = CharacterDatabase.GetPreparedStatement(CHAR_DEL_INVALID_MAIL_ITEM);
- delStmt->SetData(0, itemGuid);
- trans->Append(delStmt);
-
- item->FSetState(ITEM_REMOVED);
- CharacterDatabaseTransaction nullTrans = CharacterDatabaseTransaction(nullptr);
- item->SaveToDB(nullTrans);
- return true;
- }
-
- draft.AddItem(item);
- } while (itemResult->NextRow());
- }
+ draft.AddItem(item);
}
uint32 accountId = sCharacterCache->GetCharacterAccountIdByGuid(ObjectGuid(HighGuid::Player, receiver));