Skip to content

feat: mail AoE-looted items when bags are full - #69

Merged
Helias merged 1 commit into
azerothcore:masterfrom
EricksOliveira:featLootMail
Sep 25, 2026
Merged

Helias merged 1 commit into
azerothcore:masterfrom
EricksOliveira:featLootMail

Conversation

@EricksOliveira

@EricksOliveira EricksOliveira commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Changes Proposed:

  • Adds an opt-in AOELoot.MailEnable option, off by default. When it's on and a player takes an item from a loot window the module opened, and the only thing stopping the pickup is full bags, the item goes out as a Postmaster "Recovered Item" mail and the row leaves the window just like a normal pickup. Anything else is still handled by the core exactly as before.
  • Only rows the player takes from a module-opened creature loot window are mailed. Chests, gathering nodes, containers and windows the core opened itself (after .aoeloot off, or in a group with AOELoot.Group = 0) never mail. Quest items the module auto-collects from nearby corpses aren't mailed either and stay on their corpse. The window is tracked on the player (Player::CustomData) and not in a static map, because the loot pickup packet runs in map threads.
  • Only "inventory full" gets mailed. Items with a carry limit (unique, MaxCount, limit category) stay with the core's error, since mail doesn't count toward those limits and extra copies would get stuck in the mailbox (HasCarryLimit).
  • A mailed item shows the module's "Your items has been mailed to you." message and no red "Inventory is full" error, since it was delivered. It doesn't count for loot achievements or fire OnPlayerLootItem, the same as the core's mail path for roll winners.
  • BoP items from a corpse with several eligible looters keep the 2 hour group trade window, same logic as the core's SendRollWonItemViaMail. The mailed item is created with the player as owner and then taken out of the player's inventory save queue. Otherwise random-suffix items get deleted from the mail on the next character save (CreateMailItems).
  • The new option is read once on config load and reload, not per packet (AOELootWorld::OnAfterConfigLoad).
  • Documents the option in the .conf.dist and both READMEs.

The part worth a close look is HandleAutostoreLootItem in src/aoe_loot.cpp. Its checks mirror the core's WorldSession::HandleAutostoreLootItemOpcode and Player::StoreLootItem, and any failed check hands the packet back to the core. It only consumes the packet once the item has been mailed. CreateMailItems and SendMailItems next to it handle the mail itself. The rest (aoe_loot.h, the CanPacketReceive opcode dispatch, config, READMEs) is mechanical.

The core's own SendRollWonItemViaMail has the same save-queue bug for random-suffix items won in a roll with full bags. That needs a separate core fix.

Issues Addressed:

SOURCE:

Tests Performed:

  • Builds without errors or warnings (Windows, MSVC, RelWithDebInfo).
  • In-game, solo, full bags: items taken from an AoE loot window arrive by mail and leave the window.
  • In-game after the save-queue fix: works as expected.

How to Test the Changes:

  1. Set AOELoot.MailEnable = 1 and fill your bags.
  2. Kill a few mobs close together and loot one of them. Click an item, or use auto-loot. The item leaves the window and you get the mail message.
  3. Loot a green with a random suffix ("of the Bear"), run .save before opening the mailbox, then relog. The mail still has the item with the same suffix.
  4. Loot a stack that only partly fits: the whole stack is mailed and your bags don't change.
  5. Try a unique item you don't own yet: it isn't mailed and you get the usual error.
  6. In a group: other members see the mailed row disappear from their window, rows with a pending roll aren't mailed, and a mailed BoP item stays tradeable with the other looters for 2 hours.
  7. Set AOELoot.MailEnable = 0: full bags show "Inventory is full" and nothing is mailed.
Self-review record

Self-review — featLootMail → master

Outcome 9 findings over 5 rounds — 7 fixed, 2 dismissed — final round clean

Reviewed b9f71d7 (featLootMail vs upstream/master of azerothcore/mod-aoe-loot, merge-base diff — 5 files, +309 −4)

By Claude Code (Claude Agent SDK), Opus 5.5 (claude-opus-5-5) — self-review v0.13, 2026-09-24

In-game testing solo main path tested by the author on 5d074d4: mailing worked, but a random-suffix item logged _SaveInventory ... bag(255) and slot(0) ... incorrect and was deleted from the mail on the next save — fixed in b545f6e/b9f71d7. Author rebuilt worldserver at b9f71d7 and re-tested in-game: works as expected.

Intent Add an opt-in AOELoot.MailEnable option (default 0): when a player takes an item from a creature loot window the module opened and the only obstacle is full bags, the item is mailed and the row removed as on a normal pickup; every other case keeps core behaviour.

Diff 8e57fe917b8fe38a6c70f6841ac3019fb793d136

Metadata b31d82e839ed3d926abfca280a67193f7e160f8e

Round 1 — 598a938, 5 findings

  1. src/aoe_loot.cpp HandleAutostoreLootItem — carry-limited items held below the limit were mailed on full bags, since mail isn't counted → fixed
  2. src/aoe_loot.cpp row removal — mailed item skips UpdateLootAchievements → dismissed: same as the core's roll-won mail path; OnPlayerLootItem expects an item in bags
  3. READMEs — AOE_ITEM_IN_THE_MAIL still labelled "(reserved)" → fixed
  4. module .editorconfig 80 cols — new C++/conf lines too long → fixed; README lines dismissed: tables can't wrap, READMEs already exceed it
  5. commit subject carried the issue number where squash-merge appends the PR number → fixed

Round 2 — ac8a181, round 1 fixes, 2 findings
6. CreateMailItems — repeated template lookup and unreachable null check → fixed
7. README conf snippet lines over 80 cols → dismissed (as finding 4)

Round 3 — 5d074d4, round 2 fix, clean

Round 4 — b545f6e, in-game bug fix (mailed items created without an owner), 2 findings

  1. CreateMailItems — ownerless item saved with owner_guid = 0; deleting the character orphans the item_instance row → fixed
  2. same — without a player, cross-realm item GUID comes from realm 0's counter → fixed (owner kept, item taken out of the owner's inventory save queue right after creation)

Round 5 — b9f71d7, round 4 fixes, clean

Summary by CodeRabbit

  • New Features
    • Added an optional setting to mail eligible looted items when bags are full. The setting is disabled by default.
    • Unique items and quest items automatically collected from nearby corpses are not mailed.
    • Updated English and Spanish configuration documentation to describe the setting and the mail notification.

Add the opt-in AOELoot.MailEnable option (default 0). When it is on and
a player takes a row from a loot window the module opened, and the only
reason the core would refuse it is a full inventory, the item is mailed
by The Postmaster and the row is removed as on a normal pickup. Every
other case is left to the core handler.

The BoP group-trade window on mailed items mirrors the core's
SendRollWonItemViaMail (AzerothCore commit 4f1745483,
azerothcore/azerothcore-wotlk#26023).

Closes azerothcore#31

Co-authored-by: Andrew <47818697+Nyeriah@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 706cfd0b-5457-4813-9be8-1d8165a45f6a

📥 Commits

Reviewing files that changed from the base of the PR and between 3d1466e and b9f71d7.

📒 Files selected for processing (5)
  • .github/README.md
  • .github/README_ES.md
  • conf/mod_aoe_loot.conf.dist
  • src/aoe_loot.cpp
  • src/aoe_loot.h

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds a disabled-by-default mail option for AoE loot. For tracked AoE loot windows, eligible items are mailed when inventory storage fails because the inventory is full. The documentation describes the option and updates the mail notification entry.

Changes

AoE loot mail delivery

Layer / File(s) Summary
Mail setting and configuration loading
.github/README.md, .github/README_ES.md, conf/mod_aoe_loot.conf.dist, src/aoe_loot.cpp, src/aoe_loot.h
Adds and documents AOELoot.MailEnable, defaulting to disabled. Loads the setting after configuration and registers the world script. Updates the description for database entry 50001.
Track loot windows and route autostore
src/aoe_loot.cpp, src/aoe_loot.h
Tracks individual and merged AoE loot windows. Clears tracked state on release or before opening another window. Routes autostore packets for the handler.
Check eligibility and mail overflow items
src/aoe_loot.cpp
Checks pickup eligibility and item carry limits. If inventory storage fails specifically with EQUIP_ERR_INVENTORY_FULL, creates item stacks, updates the loot row and notifications, and mails the items. Otherwise, core handling continues.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant AOELootServer
  participant InventoryStorage
  participant MailDelivery
  Player->>AOELootServer: Send autostore packet for tracked loot
  AOELootServer->>InventoryStorage: Check pickup eligibility and store item
  InventoryStorage-->>AOELootServer: Return inventory-full result
  AOELootServer->>MailDelivery: Send eligible item stacks
  AOELootServer-->>Player: Update loot row and notify of mail delivery
Loading

Merge Risk: ⚪ Minimal · up to b9f71

The opt-in mail feature appears mergeable after normal checks; no actionable issue remains established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: mailing AoE-looted items when bags are full.
Description check ✅ Passed The description includes all required template sections and provides detailed behavior, exclusions, issue linkage, source, test results, and step-by-step testing instructions.
Linked Issues check ✅ Passed Issue [#31] requires delivery of loot by mail when bag capacity is the only pickup failure. HandleAutostoreLootItem applies this behavior for loot windows opened by the module when `AOELoot.MailEnab…
Out of Scope Changes check ✅ Passed The changes stay within issue [#31]. They add the mailing handler, track module-opened loot windows, preserve relevant core mail behavior, add the opt-in configuration, and document the option. The RE…
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Helias

Helias commented Sep 24, 2026

Copy link
Copy Markdown
Member

did you run the self-review? https://www.azerothcore.org/wiki/agentic-self-review

@EricksOliveira

EricksOliveira commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Yes. I attached it directly to the PR, but I can add it separately if you wish.

@EricksOliveira

Copy link
Copy Markdown
Contributor Author
Self-review record

Self-review — featLootMail → master

Outcome 9 findings over 5 rounds — 7 fixed, 2 dismissed — final round clean

Reviewed b9f71d7 (featLootMail vs upstream/master of azerothcore/mod-aoe-loot, merge-base diff — 5 files, +309 −4)

By Claude Code (Claude Agent SDK), Opus 5.5 (claude-opus-5-5) — self-review v0.13, 2026-09-24

In-game testing solo main path tested by the author on 5d074d4: mailing worked, but a random-suffix item logged _SaveInventory ... bag(255) and slot(0) ... incorrect and was deleted from the mail on the next save — fixed in b545f6e/b9f71d7. Author rebuilt worldserver at b9f71d7 and re-tested in-game: works as expected.

Intent Add an opt-in AOELoot.MailEnable option (default 0): when a player takes an item from a creature loot window the module opened and the only obstacle is full bags, the item is mailed and the row removed as on a normal pickup; every other case keeps core behaviour.

Diff 8e57fe917b8fe38a6c70f6841ac3019fb793d136

Metadata b31d82e839ed3d926abfca280a67193f7e160f8e

Round 1 — 598a938, 5 findings

  1. src/aoe_loot.cpp HandleAutostoreLootItem — carry-limited items held below the limit were mailed on full bags, since mail isn't counted → fixed
  2. src/aoe_loot.cpp row removal — mailed item skips UpdateLootAchievements → dismissed: same as the core's roll-won mail path; OnPlayerLootItem expects an item in bags
  3. READMEs — AOE_ITEM_IN_THE_MAIL still labelled "(reserved)" → fixed
  4. module .editorconfig 80 cols — new C++/conf lines too long → fixed; README lines dismissed: tables can't wrap, READMEs already exceed it
  5. commit subject carried the issue number where squash-merge appends the PR number → fixed

Round 2 — ac8a181, round 1 fixes, 2 findings
6. CreateMailItems — repeated template lookup and unreachable null check → fixed
7. README conf snippet lines over 80 cols → dismissed (as finding 4)

Round 3 — 5d074d4, round 2 fix, clean

Round 4 — b545f6e, in-game bug fix (mailed items created without an owner), 2 findings

  1. CreateMailItems — ownerless item saved with owner_guid = 0; deleting the character orphans the item_instance row → fixed
  2. same — without a player, cross-realm item GUID comes from realm 0's counter → fixed (owner kept, item taken out of the owner's inventory save queue right after creation)

Round 5 — b9f71d7, round 4 fixes, clean

@Helias
Helias merged commit 57279b6 into azerothcore:master Sep 25, 2026
2 checks passed
@Helias

Helias commented Sep 25, 2026

Copy link
Copy Markdown
Member

perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Items are not mailed when inventory is full

2 participants