Repository navigation
Conversation
Previously, the AOE loot merge blindly copied every item from every
nearby corpse into the main loot window and then called loot->clear()
on all source corpses unconditionally. This caused two distinct bugs
when players were in a group:
1. Wrong player receives items
Every LootItem carries ownership metadata that the merge ignored:
- allowedGUIDs: non-empty after a Need/Greed roll; only those GUIDs
may take the item.
- Loot::roundRobinPlayer: in ROUND_ROBIN, GROUP_LOOT and
NEED_BEFORE_GREED modes, under-threshold items belong to the
player whose turn it is in the rotation.
- Creature::GetLootRecipient() / GetLootRecipientGroup(): identify
who killed the creature (solo vs. group tap).
Because none of this was checked, player A could merge and take
items that were round-robin assigned to player B.
2. Other group members permanently lost their loot
Calling loot->clear() + AllLootRemovedFromCorpse() + removing
UNIT_DYNFLAG_LOOTABLE on a source corpse destroyed all items it
contained, including items that belonged to other group members who
had not yet looted.
Changes in this commit
----------------------
* Add isEligibleForPlayer lambda
Checks item ownership in the correct priority order:
1. freeforall items -> always eligible (any player may take a copy).
2. allowedGUIDs non-empty -> eligible only if the current player's
GUID is in the set (post-roll assignment).
3. No allowedGUIDs (round-robin / standard):
- Solo kill: eligible only for GetLootRecipient().
- Group kill: eligible only if the player's group matches
GetLootRecipientGroup(), and (for ROUND_ROBIN / GROUP_LOOT /
NEED_BEFORE_GREED) only when roundRobinPlayer is empty or equals
the current player's GUID.
- FREE_FOR_ALL / MASTER_LOOT group methods: always eligible.
* Surgical item removal instead of loot->clear()
For each item in a source corpse the loop now:
- Skips items where isEligibleForPlayer returns false and sets
itemsRemainingForOthers = true, leaving those items untouched.
- For eligible items: copies to itemsToAdd, sets srcItem.is_looted =
true, and decrements loot->unlootedCount to keep the loot state
consistent.
Source gold is zeroed immediately after being merged so it cannot be
double-counted if the corpse is opened again.
* Conditional corpse removal
AllLootRemovedFromCorpse() and UNIT_DYNFLAG_LOOTABLE removal are now
only called when both conditions are true:
- itemsRemainingForOthers is false (no items were skipped for other
players), AND
- loot->isLooted() returns true (gold == 0 && unlootedCount == 0).
Corpses that still contain items for other group members remain fully
lootable.
* Fix hardcoded maxCorpses
The inner loop used a hardcoded limit of 10, silently ignoring the
AOELoot.MaxCorpses config option (default 20, max 50). The limit now
reads from config, consistent with every other tunable in the module.
* Add questItem.is_looted guard
Quest items that were already individually looted are now skipped
before the active-quest and cap checks, avoiding unnecessary work.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Closes azerothcore#63. When a group kills one pack of mobs and opens the loot window before resolving all Need/Greed rolls, then kills a second pack and opens its loot, the AOE merge intercepted the second CMSG_LOOT packet and processed the first pack's corpses while rolls were still in flight. Items with an active roll have is_blocked = true and allowedGUIDs still empty (the winner has not been assigned yet). isEligibleForPlayer fell through to the group loot method check and treated these items as eligible because roundRobinPlayer was also empty for above-threshold items. The merge then copied them into the second loot window and marked them as is_looted = true in the source, leaving the roll system holding a reference to an item that no longer existed — causing it to silently disappear for all players. Fix: return false immediately in isEligibleForPlayer when is_blocked is true. Blocked items are left untouched on the source corpse. Once the roll resolves and the loot system populates allowedGUIDs with the winner's GUID, any subsequent AOE loot triggered by the winner will correctly pick up the item via the allowedGUIDs branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pangolp
marked this pull request as draft
June 3, 2026 17:58
The previous commit used Loot::roundRobinPlayer to decide whether a
player was eligible to merge items from a nearby corpse. This field is
set lazily — it is populated the first time Player::SendLoot is called
on a corpse, not when the creature dies. For any corpse that has not
yet been opened, roundRobinPlayer is an empty GUID, so the condition
!loot->roundRobinPlayer evaluated to true for every player, effectively
granting full access to every nearby corpse regardless of group loot
rules. The result was that one player received all items from all
nearby corpses while other group members could loot nothing.
Root cause: wrong ownership signal.
Loot::roundRobinPlayer — lazy, empty until first SendLoot call.
Creature::GetLootRecipient() — set at creature death by SetLootRecipient,
always reflects the player who held the tap at the moment the loot was
generated. This is the correct signal for pre-open corpse ownership.
Fix: move the per-corpse ownership check into the nearbyCorpses filter.
For GROUP_LOOT, ROUND_ROBIN, and NEED_BEFORE_GREED:
- A nearby corpse is included in the merge only if
GetLootRecipient() == player (this player tapped that creature).
- Corpses tapped by other group members are filtered out and remain
lootable for their rightful owner.
For FREE_FOR_ALL and MASTER_LOOT the filter is unchanged: any group
member may include any nearby corpse.
isEligibleForPlayer is simplified accordingly:
- The roundRobinPlayer switch is removed; corpse-level filtering
already guarantees the player is the correct recipient before any
per-item check runs.
- For group kills, returning recipientGroup == player->GetGroup() is
now sufficient for standard items.
- is_blocked and allowedGUIDs checks are retained as before.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Quest items have per-player ownership semantics that the merge pipeline cannot replicate correctly, causing two distinct bugs confirmed in in-game testing: 1. Double delivery to the same player When a nearby corpse's quest item was delivered via AddItem (direct to inventory), it was NOT marked as is_looted in the source quest_items entry. If the player later clicked the same corpse directly, FillQuestLoot saw the item still available and showed it again in the loot window, allowing a second take. A player looting two nearby wolves could end up with three copies of the same quest item: one from direct delivery plus one from each loot window. 2. Other group members losing access to quest items When a corpse's regular items were fully consumed by the merge (unlootedCount reaching zero), isLooted() returned true and the lootable flag was removed — even though quest items were still present in quest_items. Other group members who needed the same quest item from that corpse could no longer open it. Root cause: quest items do not participate in unlootedCount. The isLooted() check (gold == 0 && unlootedCount == 0) is blind to them, so a corpse can be mistakenly declared fully looted while quest items remain. Fix: remove the questItemsToAdd collection and direct delivery loop entirely. Quest items are left untouched on each source corpse and obtained the standard way — each player clicks the corpse they are eligible to loot and takes the quest item from the loot window. This is consistent with vanilla WoW group behavior and avoids all per-player tracking edge cases. Regular items and gold continue to be merged as before. Also removes the now-unused ObjectMgr.h include. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In-game testing confirmed that the previous GetLootRecipient() filter did not solve the unfair distribution in group context. The root cause: GetLootRecipient() returns who tapped (first-hit) the creature, not who is assigned items by the round-robin system. If one player consistently attacks first, every nearby corpse has GetLootRecipient() == that player, so the filter passes all of them and the merge gives that player every item — quest items included — while other group members receive nothing. The round-robin assignment lives in Loot::roundRobinPlayer, which is populated lazily on the first SendLoot call for a given corpse. It is empty for any corpse that has not yet been opened, making it impossible to determine the correct round-robin owner at AOE merge time without replicating the group's internal rotation state. Fix: for GROUP_LOOT, ROUND_ROBIN, and NEED_BEFORE_GREED, exclude ALL group-tagged nearby corpses from the merge. Players in these loot modes must open each corpse individually; the server's existing round-robin logic then assigns under-threshold items to the correct player and starts rolls for above-threshold items, exactly as vanilla WoW intends. AOE merge continues to work without restriction for: - Solo kills (no loot recipient group on the corpse) - FREE_FOR_ALL groups (any member can take any item) - MASTER_LOOT groups (master looter collects items for distribution) Also updates isEligibleForPlayer comments to reflect the narrower set of corpses that now reach per-item evaluation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the CMSG_LOOT packet interception approach (ServerScript /
CanPacketReceive) with the new OnPlayerCreatureLootOpened hook added
to the core (feat/player-creature-loot-opened-hook).
Why the old approach was fundamentally broken for groups
-------------------------------------------------------
The old code intercepted CMSG_LOOT before the server processed the
loot, then tried to determine item ownership by reading roundRobinPlayer
or GetLootRecipient(). Both signals were unreliable:
- roundRobinPlayer is SET in Loot::FillLoot at creature death (not
lazily), but the module read it before GroupLoot/NeedBeforeGreed
roll setup ran in Player::SendLoot, so above-threshold items were
not yet finalized.
- GetLootRecipient() reflects who tapped the creature (first hit),
which is the round-robin player for that kill — correct in theory,
but the filter logic had bugs when both players tapped the same mob.
The attempt to merge items manually into the main loot window also
bypassed the client/server loot protocol, causing double deliveries
and phantom items.
How the new approach works
--------------------------
OnPlayerCreatureLootOpened fires inside Player::SendLoot AFTER:
1. roundRobinPlayer is confirmed set (assigned at creature death).
2. FillNotNormalLootFor has run for all nearby group members.
3. GroupLoot / NeedBeforeGreed / MasterLoot roll setup is complete.
4. The loot window packet has been sent to the client.
The hook handler finds nearby lootable corpses and calls
Player::AutoTakeCreatureLoot for each one. AutoTakeCreatureLoot uses
the same StoreLootItem path the client would use, so all loot rules
(round-robin, allowedGUIDs, is_blocked, quest items) are enforced by
the existing server code — the module no longer reimplements any of it.
isAllowedToLoot is the sole filter: if the server considers the player
entitled to open a corpse, the module auto-takes the eligible items.
This correctly handles GROUP_LOOT, ROUND_ROBIN, NEED_BEFORE_GREED,
FREE_FOR_ALL, MASTER_LOOT, and solo kills without special-casing each.
Changes
-------
- AOELootPlayer now subscribes to PLAYERHOOK_ON_CREATURE_LOOT_OPENED
in addition to PLAYERHOOK_ON_LOGIN.
- AOELootServer (ServerScript / CanPacketReceive) is removed entirely.
- AOELootConfig struct removed (config is read directly; the struct was
never loaded in the hot path).
- MAX_LOOT_ITEMS constant removed (no longer merging into a window).
- Unused includes (Group.h, LootMgr.h, ScriptedGossip.h, <vector>,
<algorithm>, <limits>) removed from the header.
Depends on core PR: feat/player-creature-loot-opened-hook
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
isAllowedToLoot passes for a player who has quest items available on another player's round-robin corpse (FillNotNormalLootFor populates PlayerQuestItems for all nearby group members at creature death). This caused AutoTakeCreatureLoot to be called for corpses that do not belong to the current player, where it would then take the round-robin player's regular items via StoreLootItem without restriction — StoreLootItem does not block under-threshold round-robin items in GROUP_LOOT mode because that enforcement is normally done client-side via LOOT_SLOT_TYPE_LOCKED. The result: Player 1 would silently receive grey items belonging to Player 2, Player 2's corpse would be marked fully looted and disappear, and Player 1 could re-open their own already-visited corpse to take any remaining items still accessible. Fix: after isAllowedToLoot passes, add a secondary check on roundRobinPlayer for GROUP_LOOT, ROUND_ROBIN, and NEED_BEFORE_GREED. roundRobinPlayer is set in Loot::FillLoot at creature death via the chain: Unit::Kill → Group::UpdateLooterGuid → SetLootRecipient → FillLoot:569. It correctly reflects the group's rotation before any player opens a loot window, making it a reliable ownership signal at the time the hook fires. Corpses where roundRobinPlayer is set to another player are excluded from the auto-take pass and remain fully intact for their rightful owner. FREE_FOR_ALL and MASTER_LOOT are unaffected: for those methods the inner check does not apply and all nearby eligible corpses are included. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rpse When Player A manually opens a corpse whose roundRobinPlayer is Player B, the hook fires after SMSG_LOOT_RESPONSE is sent — Player A already sees the items in their client loot window. Although StoreLootItem was correctly preventing Player A from auto-taking B's grey items (roundRobinPlayer filter in the nearby-corpse loop), Player A could still take Player B's quest items from the loot window directly, because quest items are accessible to any group member who has the quest (isAllowedToLoot passes via hasItemFor, and StoreLootItem does not restrict quest items by round-robin in GROUP_LOOT mode). Fix: at the start of OnPlayerCreatureLootOpened, before building the nearby corpse list, detect whether the main creature's roundRobinPlayer belongs to a different connected group member on the same map. If so, call AutoTakeCreatureLoot for that owner immediately and synchronously. Because the hook runs in the same server tick as the packet handler — before the client's follow-up CMSG_AUTOSTORE_LOOT_ITEM can arrive — the owner's items are consumed first. When Player A then tries to take those slots, StoreLootItem finds them already marked is_looted and rejects the take. Player A's loot window updates via the removal notifications sent during the owner's auto-take. The owner must be on the same map to receive the items; if offline or on a different map the corpse remains accessible under normal loot rules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tecture - Rewrite README (EN + ES): document how items now land directly in bags via AutoTakeCreatureLoot instead of a merged loot window; add dedicated "Group loot behavior" section explaining round-robin enforcement and the cross-member corpse protection; update configuration table to include the new AOELoot.MaxCorpses option. - Update conf/mod_aoe_loot.conf.dist: fix stale option descriptions, add AOELoot.MaxCorpses entry (default 20, range 1-50), and expand the AOELoot.Group description with the full group loot behavior note.
pangolp
marked this pull request as ready for review
June 4, 2026 15:55
pangolp
marked this pull request as draft
June 4, 2026 15:55
pangolp
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes Proposed:
AOELootServer::CanPacketReceive(rawCMSG_LOOTpacket interception) with theOnPlayerCreatureLootOpenedhook, which fires after the server has already processed loot assignment and sent the loot window to the client. This is the correct, stable integration point for AOE looting.AutoTakeCreatureLootinto the module itself (previously depended on a pending core PR that added it to the core).StoreLootItem.AOELoot.MaxCorpsesconfig option (default: 20, range: 1–50) to bound server load per loot trigger.AOELootConfigsingleton,AOELootServerclass, and dead helper methods.Issues Addressed:
SOURCE:
Player::StoreLootItemenforces all server-side loot validation; loot slot locking for round-robin items is enforced client-side viaLOOT_SLOT_TYPE_LOCKED, so consuming items server-side for the rightful owner before the opener's client sendsCMSG_AUTOSTORE_LOOT_ITEMis the correct way to protect ownership.Loot::roundRobinPlayeris set inLoot::FillLootat creature death (viaGroup::UpdateLooterGuid) and reliably identifies the assigned looter for each corpse.Tests Performed:
.aoeloot on/.aoeloot offcommands work as before.AOELoot.MaxCorpsescap respected during large pulls.How to Test the Changes:
-DMODULES=static).conf/mod_aoe_loot.conf.distto your server's config directory. EnsureAOELoot.Enable = 1.AOELoot.Rangeare automatically looted without opening each one individually.mod-aoe-loot.