Skip to content

LFG: make the dungeon finder work end to end - #82

Open
MadMaxMangos wants to merge 22 commits into
masterfrom
fix/lfg-matchmaker-role-resolution
Open

LFG: make the dungeon finder work end to end#82
MadMaxMangos wants to merge 22 commits into
masterfrom
fix/lfg-matchmaker-role-resolution

Conversation

@MadMaxMangos

Copy link
Copy Markdown
Member

Makes the dungeon finder work end to end. It was broken at every link, and any one of them was fatal on its own.

What was wrong

Link Before
Join eligibility result = ERR_LFG_OK overwrote every check; solo deserters, level-10s and players in arenas were all admitted
Role check → client Unadmitted and still the 3.3.5 body shape
Role check ← client No handler and no registration — dropped at the dispatcher with no log line
Role check state PerformRoleCheck mutated a copy, so no member's answer was ever recorded
Matchmaking Formed zero groups under any input
Proposal → client Unadmitted and 3.3.5-shaped
Proposal ← client No handler, so accept and decline both did nothing
The tick LFGMgr::Update() had no caller anywhere in the tree
Group creation Double Group::Create, group used before Create, solo queuers never added, never registered with ObjectMgr

The matchmaker deserves detail. Role needs were gated on
dungeon->DifficultyID == DUNGEON_DIFFICULTY_NORMAL, comparing the raw client
DifficultyID against the internal 0-based enum. No queueable row in
LfgDungeons.dbc carries DifficultyID 0, so the branch never fired, every entry
reported needing nobody, and RoleMapsAreCompatible then computed
(3-0)+(3-0) = 6 > 3 and rejected every pair — including two solo queuers.

Separately, the role mask is a bitmask, not an enum. The client's LFD frame
has four independent checkboxes, so a player offering tank-or-damage sends
0x0A (observed on the wire in capture-000112 seq 90341). Every consumer
switched on the exact values 0x02/0x04/0x08, so a hybrid counted as zero of
everything: solo hybrids merged into a full-size entry that still reported every
role missing and could neither complete nor merge again, and any premade
containing one failed its role check outright.

Packet derivation

Four packets. The two CMSGs came off the client's own body writers. The two
SMSGs could not: there is no opcode thunk for packets the client never sends,
the wire readers sit in generated code reached indirectly, and the opcode
literal appears nowhere in the image. So field order came from traffic, which
makes it a hypothesis that had to be verified rather than trusted.

Standard applied: each layout must decode multiple real captures of
deliberately different shape to zero leftover bytes, and the writer must
reproduce those captures byte for byte.

Packet Verified against
SMSG_LFG_ROLE_CHECK_UPDATE 0x12BB capture-000075 seq 891708 (35 B, 2 members, partyIndex 0) and capture-000059 seq 719547 (68 B, 5 members, partyIndex 1)
SMSG_LFG_PROPOSAL_UPDATE 0x1E3B capture-000044 seq 1948 (64 B, 5 players) and capture-000059 seq 2063424 (156 B, 25 players)
CMSG_LFG_SET_ROLES 0x08A2 sub_6688D0; fixture capture-000086 seq 16621 and capture-000112 seq 90341
CMSG_LFG_PROPOSAL_RESPONSE 0x1D9D sub_66A29E; fixture capture-000059 seq 2063770

Two checks that were not arranged and are worth more than the fixtures
themselves:

  • The 156-byte proposal is 25 players at 2 tank / 6 healer / 17 dps
    exactly the LFR quota in LfgDungeons.dbc (Count_tank, Count_healer,
    Count_damage), a fact established from the DBC and not from the packet.
  • capture-000059 seq 2063770 is the client's answer to seq 2063424 in the
    same capture. Every echoed field matches — proposal 11132, queue 37743, flags
    3, join time 1409232359, and both GUIDs. The inbound and outbound layouts were
    derived separately and agree, which neither could establish alone.

Three places the reference layout was wrong and the captures said so:
partyIndex is not always 0; the second proposal GUID is not synthesised from
the dungeon entry; and the recipient is not necessarily player 0.

Verification

  • 115/115 tests pass (ctest, Release). Four new fixtures, all built from real
    captured bytes rather than inverses of our own writers.
  • Both the test build and the deployable build config compile clean.
  • Corpus catalogueGenerationId 2BE10C89…88752 throughout.

Known gaps, deliberately not closed here

  • SMSG_LFG_PLAYER_INFO still sends the empty shape, so random dungeons are
    not offered
    . Specific dungeons list and queue normally — the client's own
    filter treats an empty lock list as "nothing locked". Each random record
    carries quest rewards, currencies, call-to-arms entries and an encounter mask,
    none of which the legacy manager models; that is a wave of its own.
  • SMSG_LFG_JOIN_RESULT is not admitted, so a refused join is silent. This
    matters more now that the gate genuinely refuses.
  • Random dungeons expand by Group_ID instead of Random_ID (id 434 yields an
    empty set, id 493 over-selects) and carry a placeholder MapID into the
    proposal.
  • Group difficulty is left unset rather than set wrong: Difficulty(DifficultyID)
    mixes the raw client key with the internal enum, and GetBoundInstances
    indexes m_boundInstances by it unchecked while MAX_DIFFICULTY is 4.
  • The boot/kick vote opcode and CMSG_LFG_TELEPORT remain unregistered.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afb8fb9b33

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

//
// So: mark it failed, tell everyone still listed so their windows close, run the
// per-player teardown, then erase the proposal unconditionally.
if (plrAnswer == LFG_ANSWER_DENY)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject proposal responses from non-members

When a modified client sends CMSG_LFG_PROPOSAL_RESPONSE with an active proposal id that does not belong to them, this path still inserts their GUID into proposal->answers and treats accepted=false as a real decline, so any logged-in player who guesses or observes the global proposal id can cancel another group's proposal and clear the queued members. Check that plrGuid is already present in the proposal's participant/answer maps before honoring either an accept or a decline.

Useful? React with 👍 / 👎.

Comment thread src/game/WorldHandlers/LFGMgr.cpp Outdated
// still LFG_STATE_QUEUED next tick, gets matched again, and fires a fresh proposal
// -- and a new SMSG_LFG_PROPOSAL_UPDATE -- every single tick, forever.
m_queueSet.erase(guid);
m_playerData.erase(guid);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expire unanswered proposals

When any recipient never sends a proposal response (for example they ignore the popup, disconnect, or the client-side timer expires), there is no LFG_TIME_PROPOSAL reaper in LFGMgr::Update, and this line has already removed the only queue data that could put the players back. The remaining members stay in LFG_STATE_PROPOSAL indefinitely, and JoinLFG now refuses that state, so they can be stuck out of dungeon finder until a relog or manual leave path clears them.

Useful? React with 👍 / 👎.

// Add group to our group set and group map, then teleport to the dungeon
ObjectGuid groupGuid = pGroup->GetObjectGuid();
LFGGroupStatus groupStatus(LFG_STATE_IN_DUNGEON, dungeon->ID, proposal->currentRoles, pGroup->GetLeaderGuid());
pGroup->AddMember(it->first, pMember->GetName());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Convert raid-sized proposals before adding members

When the matched dungeon's DBC quota is larger than five (the new role resolver explicitly allows LFR/flexible raid sizes), the newly created group is still a normal party, so Group::IsFull() makes AddMember fail after MAX_GROUP_SIZE; this return value is ignored here. The extra players have already accepted and are sent the success/leave updates, but they are not added to the group or teleported, so raid-finder-sized proposals complete as a partial five-player party.

Useful? React with 👍 / 👎.

@@ -307,28 +320,27 @@ bool LFGMgr::IsProposalSameGroup(LFGProposal const& proposal)
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Abort proposals with offline members

When a queued player logs out before a completed match is proposed, this skip lets proposal construction continue without that member in groups or answers, while currentRoles still counts them as part of the completed composition. The remaining online players can all accept, allOkay sees no pending answer for the offline member, and CreateDungeonGroup builds/teleports a short group while recording status for someone who was never added; remove or fail the stale queue member instead of silently ignoring them here.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

// needed-role arithmetic is still wrong -- LFGMgr.cpp gates role needs on

P2 Badge Enforce group-leave permissions before cancelling parties

Passing isGroup=true for every grouped caller sends non-leaders into LFGMgr::LeaveLFG's group branch, and that branch does not check pGroup->IsLeader(...) before iterating every member and removing the group queue entry. In a queued, role-check, or proposal state, any party member who sends CMSG_LFG_LEAVE can therefore cancel dungeon finder for the whole party; either keep the leader check here or enforce the permission before the group-wide removal.


roleCheck.currentRoles[plrGuid] = roles;

P2 Badge Ignore role replies from non-participants

Because roleCheck is now a reference to the stored map entry, this subscript persists insertions for any current group member who was not part of the original role check. If someone is invited after the role check starts, they can send CMSG_LFG_SET_ROLES and either abort the pending check or get queued without the join-result and dungeon-lock validation that ran for the original members; verify plrGuid already exists in currentRoles before accepting the role update.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return false;
}

LfgDungeonsEntry const* dungeon = sLfgDungeonsStore.LookupEntry(*dungeonList.begin());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Evaluate each selected dungeon's role quota

When a player queues for multiple specific dungeons whose DBC role counts differ, this helper validates the entire selection against only the lowest-sorted dungeon id. The code now explicitly supports TypeID 1 rows that include 0/0/3 scenarios, 0/0/1 solo content, and 1/1/3 dungeons, so a valid role set for one selected dungeon can be rejected just because another selected dungeon happens to sort first; the matchmaker should test/filter the compatible dungeon set instead of letting begin() decide the whole queue entry.

Useful? React with 👍 / 👎.

Comment on lines +686 to +689
if (!pMember || pGroup->IsMember(it->first))
{
continue;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Abort when an accepted proposal member disconnects

If a participant accepts the proposal and then logs out before the final response arrives, allOkay still succeeds because their answer is already AGREE, but this path silently skips the offline member and continues creating/teleporting the dungeon group. That is a separate post-proposal disconnect case from the pre-proposal online check: it still produces a short group while groupStatus records roles for someone who was never added, so the proposal should be cancelled or requeued instead of continuing past a missing accepted member.

Useful? React with 👍 / 👎.

Comment thread src/game/WorldHandlers/LFGMgr.cpp Outdated
// -- and JoinLFG refuses that state, so they could not re-queue until relog.
for (std::vector<uint32>::const_iterator it = expired.begin(); it != expired.end(); ++it)
{
CancelProposal(*it, std::set<ObjectGuid>());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove timed-out players before requeueing survivors

When a proposal expires because one recipient never answered or went offline, passing an empty culprit set makes CancelProposal requeue the entry unchanged, including the still-pending member. Because the role counts remain complete, the next LFG update can immediately form the same proposal again and trap the players who did accept in a repeat timeout loop; build the culprit set from pending/offline answers before cancelling the expired proposal.

Useful? React with 👍 / 👎.

Comment thread src/game/WorldHandlers/LFGMgrQueue.cpp Outdated
// SECOND live entry. If the first proposal then completed, CreateDungeonGroup put
// them in a dungeon group while they were still queued for another.
LFGPlayerStatus const existingStatus = GetPlayerStatus(plr->GetObjectGuid());
if (existingStatus.state == LFG_STATE_PROPOSAL)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject requeues from merged queued members

This guard only rejects LFG_STATE_PROPOSAL, but after a solo player has been absorbed into another queue entry, m_playerData no longer has an entry under their own GUID while m_playerStatusMap still reports LFG_STATE_QUEUED. If that player sends another CMSG_LFG_JOIN to change roles or dungeons, currentInfo misses and the solo branch creates a second live queue entry while the merged entry still lists them, so the duplicate check needs to use FindQueueEntryContaining or reject queued status as well.

Useful? React with 👍 / 👎.

Comment on lines +552 to +553
for (playerGroupMap::const_iterator it = proposal.groups.begin();
it != proposal.groups.end(); ++it)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Send proposals after all participants are recorded

The new proposal packet serializes proposal.groups, but SendDungeonProposal still sends each player's initial proposal inside the loop that is populating groups and answers. For any multi-player match, everyone except the last loop iteration receives an opening proposal that omits later participants, so the popup can show an incomplete group until someone answers; build the full proposal first, then send it to every participant.

Useful? React with 👍 / 👎.

SendLfgUpdate(*bad, GetPlayerStatus(*bad), false);

m_queueSet.erase(*bad);
m_playerData.erase(*bad);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid erasing the queue entry behind CancelProposal

When the declining culprit is also proposal.queueGuid (for example the solo player whose entry absorbed the match, or a debug-solo proposal), this erase destroys the map node that entry still points to, and the function then reads entry->currentRoles.empty() below. That makes an ordinary proposal decline a use-after-free on the world thread; delay erasing proposal.queueGuid until after the survivor check, or stop using the cached pointer after this erase.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

roleCheck.currentRoles[plrGuid] = roles;

P2 Badge Reject role replies from non-participants

Now that roleCheck is a persistent reference and CMSG_LFG_SET_ROLES reaches this path, a party member who was not in the original currentRoles map (for example, someone invited after the role check started, or a crafted client) is inserted by the unchecked currentRoles[plrGuid] = roles below. That lets a player who never passed JoinLFG eligibility/lock checks get copied into queueInfo->currentRoles and proceed to the proposal, or abort the original party by overfilling the quota; check that plrGuid already exists in the role check before accepting the reply.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +908 to +910
if (*bad == proposal.queueGuid)
{
continue; // handled below, after `entry` is finished with

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-key survivors when the queue owner declines

When the player whose GUID is proposal.queueGuid declines while other members remain in entry (for example, a solo entry that absorbed another solo or party), this continue leaves m_playerData[proposal.queueGuid] and m_queueSet keyed by a player who was just removed from currentRoles. If that player queues or leaves again, FindQueueEntryContaining hits their own key first and JoinLFG can overwrite the survivor entry, silently dropping the players who were returned to the queue; move the survivor entry to a remaining participant/group key before requeueing it.

Useful? React with 👍 / 👎.

// in LeaveLFG, not a reason to cancel the wrong thing.
Group* pGroup = plr->GetGroup();

sLFGMgr.LeaveLFG(plr, pGroup != nullptr);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require party leadership before leaving as a group

With this call, any grouped player who sends CMSG_LFG_LEAVE takes the isGroup path, and LeaveLFG then iterates every member and erases the group's queue/role-check state without checking IsLeader. A non-leader using a modified client can therefore cancel the whole party's dungeon-finder queue or active role check; keep the grouped path limited to the party leader, or only remove the caller when they are not allowed to act for the group.

Useful? React with 👍 / 👎.

Comment on lines +975 to +977
if (ans->second != LFG_ANSWER_AGREE || !sObjectAccessor.FindPlayer(ans->first))
{
silent.insert(ans->first);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove timed-out premades together

When a proposal expires because one member of a premade party never answers or goes offline, this builds the culprit set with only that player's GUID, unlike the decline path which expands to every member in the same proposal.groups entry. Fresh evidence is that the current timeout code calls CancelProposal directly with silent, so the responding party members are requeued without their party mate and can later be pulled into a dungeon as if only part of the premade had queued.

Useful? React with 👍 / 👎.

MadMaxMangos and others added 22 commits August 5, 2026 21:56
The dungeon finder's matching spine could not form a group under any input.
Five independent defects, each sufficient on its own:

Role needs were gated on `dungeon->DifficultyID == DUNGEON_DIFFICULTY_NORMAL`,
comparing a RAW client DifficultyID against the internal 0-based enum. No
queueable row in LfgDungeons.dbc carries DifficultyID 0, so the branch never
fired and every entry reported needing nobody. RoleMapsAreCompatible then
computed (3-0)+(3-0) = 6 > 3 and refused every pair, including two solos.

Take the composition from the dungeon's own row -- Count_tank, Count_healer,
Count_damage -- which removes the difficulty translation from this path and
covers the 108 of 247 queueable TypeID 1 rows that are not 1/1/3 five-mans
(scenarios 0/0/3, solo content 0/0/1, raid finder 2/6/17, flex 0/0/25).

The role mask is a BITMASK, not an enum. The client's LFD frame has four
independent checkboxes, so a player offering tank-or-damage sends 0x0A --
observed on the wire in capture-000112 seq 90341. Every consumer switched on
the exact values 0x02/0x04/0x08, so a hybrid counted as zero of everything:
solo hybrids merged into a full-size entry that still reported every role
missing and could neither complete nor merge again, and a premade containing
one hybrid failed its role check outright. Resolve the mask by backtracking
assignment instead; greedy mis-assigns, because handing the tank slot to a
tank-or-healer player can strand a tank-only specialist.

`neededTanks = 1 - tankCount` in a uint8 wrapped to 255 for a two-tank party,
which the old arithmetic then read as -254 and passed, merging parties that
could never complete. The resolver counts down from the quota and cannot
underflow.

Completion was only ever tested inside MergeGroups, so a premade of exactly
five with a correct composition -- the commonest premade case -- was never
merged with anything and never proposed. Test it wherever an entry becomes
eligible, and dequeue on proposal: without that the entry stays QUEUED, gets
matched again next tick and fires a fresh proposal every tick forever.

Also fixed, all reachable the moment Update() ticks:

- RemoveOldRoleChecks erased inside a `++it` loop over an unordered_map. It is
  the first thing Update() calls.
- LFG_TIME_ROLECHECK was 45*IN_MILLISECONDS added to a seconds-domain time_t,
  expiring role checks after 12.5 hours instead of 45 seconds.
- MergeGroups erased the absorbed entry from m_playerData but not m_queueSet,
  leaving a stale queue entry that could give one player two live proposals.
- Both matching loops iterated m_queueSet while merges erased from it.
- The role check was stored before the loop that fills currentRoles ran, so it
  listed nobody and PerformRoleCheck saw "everyone" answer on the first reply.
- PerformRoleCheck mutated a COPY of the stored role check, so no member's
  answer was ever recorded and a party of two or more could never finish.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reply half of the LFG role check had no handler and no registration, so
the client's answer was dropped at the dispatcher without a log line and a
party entered LFG_STATE_ROLECHECK and stayed there permanently.

Body derived from the client's own writer sub_6688D0, reached as vtable slot 1
behind the opcode thunk sub_6615FE (which writes 2210):

    sub_40F075(pkt, *(uint32*)(this + 16));   // WriteUInt32 -- role mask
    sub_40F018(pkt, *(uint8 *)(this + 20));   // WriteUInt8  -- role check counter

Flat -- no bit packing and no GUID, so nothing to XOR or reorder. All 99
build-18414 packets in the corpus are exactly 5 bytes, which agrees.

Note the Lua SetLFGRoles() does not send this; it only mutates local state.
The packet is emitted by CompleteLFGRoleCheck, i.e. on confirmation.

Fixture uses real captured bodies, not inverses of our own reader:
  capture-000086 seq 16621  08 00 00 00 00  damage only
  capture-000112 seq 90341  0A 00 00 00 00  TANK|DAMAGE

The second is why the mask must be treated as a bitmask: it is one player
offering either role, and it is what the previous exact-value matching threw
away. Corpus catalogueGenerationId 2BE10C89...88752.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LFGProposal had no member initialisers, and groupRawGuid/groupLeaderGuid are
the only two scalars SendDungeonProposal does not always assign -- it sets them
solely on the premade path. Yet it READS groupRawGuid to decide whether to set
it, and CreateDungeonGroup branches on it to choose between reusing an existing
group and making a new one. An all-solo proposal therefore picked its branch
from whatever was on the stack. LFGPlayers had the same problem for joinedTime
and the three needed* counts, which decide both completion and what the queue
advertises.

SendLfgProposalUpdate dereferenced three find() results without checking end().
It is reachable, not theoretical: SendDungeonProposal skips offline players
when filling `groups` and `answers` but still lists them in `currentRoles`, so
a player who queues, logs out and logs back in arrives with no entry of their
own and crashes the session the moment another member answers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The role check prompt never appeared on the client however correct the server
state was: the outbound packet was still the 3.3.5 shape -- uint32 state, flat
counts, raw uint64 GUIDs -- and shared no field order with 18414. It was also
unadmitted, so it never left the server at all.

Derivation. The CMSG technique this campaign runs on (opcode thunk -> vtable
slot 1 -> body writer) does not apply to SMSG: there is no opcode thunk because
the client never sends these. Working from the other end instead:

- GetLFGRoleUpdate binds to sub_98C59C -> sub_98C4D2, which reads a global
  block and yields inProgress = (state == 2), a slot count, a member count, a
  category derived from slot[0] & 0xFFFFF, and a battleground GUID.
- The applier at 0x98953C fills that block from a parsed struct, giving the
  field set and types: state at +0x10, slot vector at +0x14/+0x18, member
  vector at +0x24/+0x28 with stride 0x18, GUID at +0x38.
- The wire reader itself sits in a third layer of generated code reached
  indirectly, with the opcode nowhere in the image as a literal, so field ORDER
  had to come from traffic rather than from a reader.

So the order is a hypothesis verified against real bytes, not read off a
writer. It decodes two captures of deliberately different shape to zero
leftover, and the writer added here reproduces both byte for byte:

  capture-000075 seq 891708, 35 B: partyIndex 0, 2 members, dungeon type 1
  capture-000059 seq 719547, 68 B: partyIndex 1, 5 members, dungeon type 6

Corpus catalogueGenerationId 2BE10C89...88752.

Two things the reference layout this was checked against gets wrong: partyIndex
is not always zero -- the second capture carries 1 -- and the leader's entry
must come first, which both captures confirm by carrying the LEADER bit on
member 0 while later members are still zero.

Also fixes an unchecked find() in the sender: a role check whose leader had
already left dereferenced end().

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The last packet standing between a formed group and a client that can see it.
The body was the 3.3.5 shape -- flat fields and a per-player run of single
bytes -- and shared no field order with 18414. It was also unadmitted.

Derived the same way as the role check: the client's wire reader sits in
generated code reached indirectly with the opcode nowhere in the image as a
literal, so field order comes from traffic rather than from a reader, and is
therefore a hypothesis that had to be verified rather than trusted.

Verified byte-exact against two captures chosen to differ as much as the corpus
allows, both decoding to zero leftover, and the writer reproduces both:

  capture-000044 seq 1948,    64 B:  5 players, roles 0x03/0x04/0x08 x3
  capture-000059 seq 2063424, 156 B: 25 players, 2 tank / 6 healer / 17 dps

The raid case is an independent check on the decode rather than more of the
same: 2/6/17 is exactly what LfgDungeons.dbc carries in Count_tank,
Count_healer and Count_damage for LFR rows -- a fact established from the DBC,
not from this packet. A wrong layout would have to be wrong in a way that
happens to reproduce the shipped data.

Corpus catalogueGenerationId 2BE10C89...88752.

Three corrections to the reference layout the hypothesis came from:

- It builds the second GUID as `dungeonEntry | (0x1F45 << 48)`. Real traffic
  carries neither: the top five bytes are constant 1F 44 00 00 11 across both
  captures while the low three vary, i.e. a genuine instance-side GUID with a
  counter, unrelated to the dungeon entry. We do not model that object, so it
  is sent as zero -- a legal encoding, since every mask bit then reads false
  and WriteByteSeq emits nothing for a zero byte.
- Roles must pass through verbatim. Observed values include 0x32 and 0x09, so
  bits above DAMAGE are real; masking to the four known role bits would corrupt
  them.
- The recipient is not necessarily player 0. In the raid capture the "is this
  you" bit sits on entry 6.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two pieces, and together they close the loop: a proposal can now be answered,
and the matchmaker can now run at all.

CMSG_LFG_PROPOSAL_RESPONSE had no handler and no registration, so accept and
decline both did nothing -- the reply was dropped at the dispatcher. Body
derived from the client's own writer sub_66A29E (vtable slot 1 behind the
opcode thunk sub_6622E8, which writes 7581), with GUID A at this+24..31 and
GUID B at this+48..55:

  uint32 proposalId, clientQueueId, flags, joinTime
  bits   accept, mask A6 A0 A2 A4 B6 B7 A3 B4 A7 B1 A5 B0 A1 B2 B3 B5
  Flush
  bytes  A3 A6 A4 A1 B7 B0 A7 B6 A5 B3 B1 B5 B4 A0 A2 B2, XOR 1 when present

Fixture is capture-000059 seq 2063770, and it is worth more than a size check:
it is the client's answer to seq 2063424 in the SAME capture, the 156-byte
SMSG_LFG_PROPOSAL_UPDATE derived in the previous commit. Every echoed field
matches -- proposal 11132, queue 37743, flags 3, join time 1409232359, and both
GUIDs. The inbound and outbound layouts were derived separately and agree,
which neither could establish on its own.

Nothing in the body is authority. The server answers on behalf of the CALLER
and keys on its own proposal id, so a client returning someone else's guidA
cannot answer for them.

The WUPDATE_LFGMGR timer was configured at startup but never consumed, so
LFGMgr::Update had no caller anywhere: a player could join the queue and
nothing ever looked at it again. This is deliberately the last change of the
sequence rather than the first -- the reaper Update calls first erased while
iterating, the matchmaker it calls next could not form a group under any input,
and the proposal it can now send chose a branch from two uninitialised members.
Ticking it before those were fixed would have crashed the world thread.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit gave LFGMgr::Update a caller for the first time, so this
path went from dead code to live code. These defects were all latent behind
that; none of them is theoretical now.

CreateDungeonGroup, rewritten. Four independent defects on one path:

- The leader search looped over every role-flagged member calling Group::Create
  with no break, so two merged premades carrying two LEADER bits ran Create
  twice on one object. Each call does its own GenerateGroupLowGuid and its own
  INSERT INTO groups in its own transaction, orphaning the first group id and
  stranding that id's group_member rows.
- If a leader bit was set but every leader-flagged player was offline, Create
  never ran while AddMember still did, building a group with id 0 and an empty
  leader guid and inserting it into m_groupSet.
- The existing-group branch called no AddMember at all. One premade plus solo
  queuers is the commonest LFD composition, and those solos were dequeued, told
  a group had been found, and never put in one.
- Nothing called sObjectMgr.AddGroup, so GetGroupById could not find the group,
  it leaked at shutdown, and the boot path called RemoveGroup on a group that
  had never been added.

It also no longer calls SetDungeonDifficulty(Difficulty(dungeon->DifficultyID)).
That mixes the raw client key with the internal 0-based enum, making every
normal five-man heroic, and GetBoundInstances indexes m_boundInstances by it
unchecked while MAX_DIFFICULTY is 4 -- raw ids on LFR, scenario and flex rows
reach 14. Leaving the existing difficulty is wrong-but-safe; setting a wrong one
is neither.

IsProposalSameGroup skipped ungrouped players entirely, so a two-man party
matched with three solos returned true. The proposal was then treated as a
premade and reused the party's group without adding the solos. It also returned
true when nobody was grouped at all.

ProposalUpdate now returns immediately on a decline. Falling through carried two
bugs at once: ProposalDeclined can erase the proposal from m_proposalMap,
leaving the code below iterating and writing through a dangling pointer; and
when it does not erase, it removes the decliner from `answers`, so four accepts
plus one decline in a five-man read as unanimous, built a FOUR-man group and
teleported it in.

GetDungeonFinderRewards was dereferenced unconditionally. dungeonfinder_rewards
ships 66 rows covering levels 15-80, so every level 81-90 character -- every
MoP-relevant one -- crashed the world server on a tracked boss kill.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both reachable today -- CMSG_LFG_JOIN and CMSG_LFG_LEAVE are already live.

GetJoinResult ended its solo branch with an unconditional `result = ERR_LFG_OK`,
discarding every check above it. A solo player with Dungeon Deserter, on LFG
cooldown, in a battleground or in an arena was always admitted. The level 15
minimum was worse: that test existed only inside the group branch, so a solo
player below 15 was never checked at all.

In the group branch, `result` was assigned per member including an else-OK, so
only the LAST iterated member's verdict survived -- a party containing one
deserter was admitted whenever the last member happened to be clean.

`LfgJoinResult result;` was also read uninitialised when a group had members but
every getSource() returned null.

HandleLfgLeaveOpcode tested `pGroup && pGroup->IsLeader(...)`, so a non-leader
went down the SOLO branch. That branch erases m_playerData[playerGuid], and for
a grouped queuer no such entry exists: the party's real entry, keyed by the
group guid, stayed in the queue untouched while the client was told it had
left. Whether a non-leader may cancel for the party is a permission question,
and it belongs in LeaveLFG rather than being answered by cancelling the wrong
thing. This one was mine, from the commit that first wired the opcode.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cross-model review (Devin SWE-1.7 Max) returned BLOCK on seven findings. All
seven were real. Two risks I flagged came back clean: sending the proposal's
second GUID as zero is safe (the client echoes it and never surfaces it), and
the TryFormGroup/FindQueueMatches snapshot iteration is sound.

The resolver could hang the world thread. My own comment claimed "at most 5
players and 3 roles, bounded by 3^5" -- but this is not a five-man-only path.
Raid finder rows ask for 2/6/17 and flexible raid for 0/0/25, and I had derived
a 25-player LFR proposal in the same session that produced that comment.

Measured on the exact algorithm in isolation, 26 hybrid players competing for
25 slots:

  naive     127,337,429 calls   545.014 ms
  memoised          934 calls     0.054 ms

545 ms of world-thread time, growing exponentially with player count. Keying
dead ends on (index, remaining quota) bounds the search by
(players+1) x (tank+1) x (healer+1) x (damage+1). Every fit/no-fit result is
identical between the two, so this changes cost and not semantics.

Declines. Returning early fixed the use-after-free but not the rest. It left
the other members holding a proposal window that never closed, and the stale
proposal stayed in m_proposalMap on the non-premade path -- where a later
accept could still complete it short, which is the bug the early return was
supposed to prevent. A decline now marks the proposal failed, sends
SMSG_LFG_PROPOSAL_UPDATE to everyone still listed so their windows close, runs
the per-player teardown, and erases the proposal unconditionally.
ProposalDeclined no longer erases it or prunes members out of the maps -- the
caller owns the proposal, and pruning was what let survivors read as unanimous.

CreateDungeonGroup leaked a Group on the unknown-dungeon return: the lookup sat
after creation, so it returned having already new'd a Group, run Create (a
group id plus an INSERT INTO groups) and registered it with ObjectMgr. The
lookup now happens first.

Detaching a player from a two-man group makes Group::RemoveMember call Disband,
which neither unregisters nor deletes the object. Both detach sites now use
Player::RemoveFromGroup, the codebase's own helper for this, which handles
RemoveGroup and delete.

Re-queuing during a live proposal produced a second queue entry: the duplicate
cleanup in JoinLFG is guarded on existing queue data, and TryFormGroup erases
that the moment a proposal is sent. A player sitting on an open proposal is now
refused.

Both new parsers now require the body to be fully consumed. Unread tail data is
the cheapest signal that a body was read wrongly and must not be swallowed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-caught while anticipating the re-review, and it is a regression from the
previous commit rather than a pre-existing bug.

ProposalDeclined calls LeaveLFG for the DECLINER alone. The other members were
left at LFG_STATE_PROPOSAL: their queue entry was already erased when the
proposal fired, nothing else resets them, and the re-queue guard added in the
same commit now refuses a player in that state. So one decline would have
locked every other member out of the dungeon finder until they relogged --
turning a fix for a duplicate-entry bug into a worse denial.

Membership is snapshotted before ProposalDeclined runs, because that path calls
LeaveLFG and can mutate the maps being walked.

Everyone leaving LFG on a decline is deliberate. Retail requeues the
non-decliners, but their queue data is gone by this point and rebuilding it is
separate work. Leaving cleanly is correct-but-less, and it is visible to the
player rather than silent.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
From the focused re-review. Clearing server-side state and closing the proposal
window is not enough on its own: without an explicit LEAVE the client keeps
showing itself queued for a queue that no longer exists.

Sent before the status entry is erased, since the update is built from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lure

The client states the three outcomes of a failed proposal plainly, and the
previous behaviour matched none of them:

  ERR_LFG_PROPOSAL_FAILED          "Someone has declined the invite. You have
                                    been returned to the front of the queue."
  ERR_LFG_PROPOSAL_DECLINED_SELF   "You have been removed from the queue
                                    because you did not accept the invitation."
  ERR_LFG_PROPOSAL_DECLINED_PARTY  "...because someone in your party did not
                                    accept the invitation."

So the decliner leaves, their premade leaves with them, and EVERYONE ELSE goes
back in the queue. The previous commit ejected all of them, which I had
described as correct-but-less; it is simply wrong.

That required a change of shape. TryFormGroup used to erase the queue entry the
moment a proposal fired, leaving nothing to put people back into. It now removes
the entry from the match set and marks it LFG_STATE_PROPOSAL while keeping the
data, so a cancellation can restore it. CancelProposal implements the three
outcomes; the decline path routes through it.

That also fixes, from the PR review:

- No proposal timeout existed. A recipient who ignored the popup, disconnected,
  or whose client-side timer lapsed left everyone else pinned at
  LFG_STATE_PROPOSAL for ever, and JoinLFG refuses that state, so they could not
  re-queue until relog. RemoveOldProposals now reaps them through the same
  cancellation path, which requeues the survivors.

- Any logged-in player could cancel someone else's proposal. m_proposalId is a
  plain incrementing counter, so an id is trivially guessable, and writing to
  proposal->answers INSERTED the caller -- a `false` answer from a stranger
  cancelled a group they had nothing to do with. Only participants may answer.

- A queued player who logged out was skipped when filling `groups` and `answers`
  while still counted in `currentRoles`, so the online members could all accept,
  allOkay saw no pending answer for the absent one, and a SHORT group was built
  and teleported. Offline members are now dropped from the entry before a
  proposal is sent, and the entry goes back to looking.

- A raid-sized dungeon built a normal party. Group::IsFull caps at
  MAX_GROUP_SIZE and AddMember just returns false past it, so a raid finder
  proposal (2/6/17 = 25) completed as a five-man while the other twenty were
  told a group had been found, never added and never teleported. Groups whose
  dungeon quota exceeds a party are converted to raid before members are added,
  and the AddMember return is no longer discarded.

Not fixed here, and worth stating: retail also displays those three messages,
but they are delivered through SMSG_DISPLAY_GAME_ERROR, which has no sender
anywhere in this tree and whose body is not yet derived. The behaviour is right;
the notification text is still missing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r people

Mirrors `.debug bg`, which lets a battleground start 1v0. The dungeon finder has
the same problem and worse: a normal five-man will not form until 1 tank,
1 healer and 3 damage are all present, so on a test realm with two accounts the
proposal, group-creation and teleport paths are simply unreachable -- correct
behaviour that cannot be exercised.

  .debug dungeon         a game master's queue entry completes on its own
  .debug dungeon group   as above, and it also absorbs whoever else is waiting,
                         whatever roles they picked
  .debug dungeon off     back to normal matchmaking

Bare `.debug dungeon` toggles off when a mode is already active, matching how
`.debug bg` behaves with no arguments.

While any mode is active a game master leads the resulting dungeon group
regardless of who holds the LEADER bit, so the operator keeps control of the
group under test.

Every relaxation is gated on the queue entry actually CONTAINING a game master:

- TryFormGroup waives the needed-role test only for such an entry.
- RoleMapsAreCompatible waives only the role composition in group mode, and only
  when a GM is on one side. The size cap and the duplicate-membership check both
  still apply, so a five-man still caps at five and nobody can end up in two
  entries.

Scoping it this way matters. Relaxing the matchmaker globally would change how
ordinary players match each other while the operator is testing, which makes a
debug switch untrustworthy -- you can no longer tell whether what you observed
was the system working or the switch lying.

Game master is account security, not `.gm on`: the operator should not have to
make themselves untargetable to test the dungeon finder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both flagged functions were already large on master (ProposalUpdate 100 lines,
SendDungeonProposal 95); my changes grew them to 143 and 106. Fair feedback, so
fixed rather than waived.

The decline branch is lifted out of ProposalUpdate into DeclineProposal. It was
a self-contained forty lines that answered one question -- who is responsible
for this cancellation -- and reads better named than inline. ProposalUpdate is
back to 113 lines.

SendDungeonProposal now takes the queue guid instead of recovering it by
scanning m_playerData for an entry whose value has the same ADDRESS as the
LFGPlayers* it was handed. The caller already knows the key. Beyond the
complexity, identifying a map entry by the address of its value is the sort of
thing that quietly stops working the first time anyone copies the struct, and
LFGPlayers is copied in several places already.

No behaviour change; 115/115 still pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by driving two clients: two solo players queued for the same dungeon, both
damage. One saw a full panel -- 0/1 tank, 0/1 healer, 2/3 damage, average wait,
a live time-in-queue. The other saw a stub: no role counts, no average wait, a
placeholder "< 1 minute", and most of the minimap eye's tooltip missing.

The 2/3 proves the merge itself was fine. Both packets that describe a queue
were reporting it to the wrong identity.

SMSG_LFG_QUEUE_STATUS stamped every recipient with the merged entry's KEY, which
is whichever entry did the absorbing. The absorbed player joined under their own
guid -- that is what SMSG_LFG_UPDATE_STATUS sent them as requesterGuid -- so a
status arriving under a stranger's identity does not match the queue their
client is tracking and is ignored. The absorbing player never saw this, because
for them the merged key IS their own guid, which is exactly why this looked like
"one client works and the other does not".

SMSG_LFG_UPDATE_STATUS had the mirror image. GetStatusPacketData looked the
player up by queue guid alone, and a merged solo queuer has no entry of their
own -- MergeGroups folds them into the absorber and erases theirs. The lookup
missed, the caller got a default-constructed struct, and the update went out
with zero roles, zero needed counts and a zero join time. It now falls back to
whichever entry actually lists the player.

Neither of these is visible from the corpus: it proves what a retail server
sent, not that ours addressed the right person.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reported from a live client: on the absorbed player, Leave Queue did nothing.

It was worse than nothing. LeaveLFG erased m_playerData and m_queueSet by the
player's OWN guid, but a solo queuer who has already been merged has no data
under that key -- MergeGroups folds them into the absorbing entry and erases
theirs. So the erase was a no-op while the client was still sent
LFG_UPDATE_LEAVE: the UI cleared and the server kept them queued inside the
merged entry, where a later proposal would have pulled them into a dungeon they
had left.

This is the third bug from one root cause, after the queue-status and
update-status identity bugs in the previous commit. Anything keyed on a
player's own guid silently misses them once they have been merged. So the
lookup is now a named helper, FindQueueEntryContaining, and both the leave path
and GetStatusPacketData go through it rather than each open-coding the scan.

Removing a player also recomputes the entry's needed roles -- the survivors need
one more of whatever the leaver was covering -- and drops the entry entirely
when the last member leaves.

The group leave path had the same hazard per member and now uses the same
helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found live: after one successful proposal the player could not queue again --
five CMSG_LFG_JOIN attempts in ninety seconds, all refused.

My regression, from the commit that made TryFormGroup keep the queue entry alive
so a declined or timed-out proposal could put the survivors back. That commit
handled both failure paths and neither success path: on success nothing erased
the entry, so it sat in m_playerData for ever with currentState
LFG_STATE_PROPOSAL, every member's stored status stayed LFG_STATE_PROPOSAL, and
the re-queue guard added in the same commit refuses exactly that state. Enter a
dungeon once, never queue again until relog.

The success path now tears the entry down and moves each member to
LFG_STATE_IN_DUNGEON, including any whose teleport was denied -- they must not
be left reading LFG_STATE_PROPOSAL either.

The guard itself was also too trusting. LFG_STATE_PROPOSAL is written in several
places and cleared in fewer, so any path that forgets to reset it locks the
player out of the dungeon finder entirely. It now asks m_proposalMap whether a
proposal is actually awaiting this player's answer, which cannot go stale: if no
live proposal lists them, there is nothing to protect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ects

From the second round of PR review.

BLOCKING -- CancelProposal was a use-after-free on the world thread. It caches
`entry = GetPlayerOrPartyData(proposal.queueGuid)`, then erases m_playerData for
each culprit, then reads entry->currentRoles.empty(). A culprit is very often
the entry key itself: the solo player whose entry did the absorbing, or the
single queuer in a `.debug dungeon` proposal. Erasing m_playerData[queueGuid]
destroyed the node `entry` pointed into and the survivor check read it. So an
ordinary decline -- on exactly the path currently being live-tested -- was
undefined behaviour. The queue entry is now left alone inside that loop and
handled after `entry` is finished with.

The timeout reaper cancelled with an EMPTY culprit set, which requeued the entry
unchanged, including the member who never answered. The role counts were still
complete, so the next tick re-formed the same proposal and timed out again,
trapping everyone who did accept in a permanent loop. Whoever failed to answer,
or went offline, is now the culprit -- exactly as a decliner is.

A merged solo queuer re-joining created a SECOND live entry. The duplicate
cleanup keys on m_playerData under the player's own guid, which an absorbed
player does not have, so the cleanup was skipped and the solo branch built a
fresh entry while the merged one still listed them. It now resolves through
FindQueueEntryContaining and removes them from whatever entry actually holds
them.

SendDungeonProposal sent each player's opening proposal from INSIDE the loop
that fills `groups` and `answers`. The packet serialises those maps, so every
recipient but the last saw a proposal missing the members added after them --
the ready popup showed an incomplete group until somebody answered. Built
first, sent second.

A member who accepted and then logged out before the final answer still passed
allOkay, and skipping them built a short group and teleported it while
groupStatus recorded a role for someone never added. That now cancels, with the
absent member as the culprit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by instrumenting a live queue. The diagnostic said it outright:

  LFG JoinLFG: solo entry for Humanwarrior stores dungeons={6}
  LFG SendDungeonProposal: entry dungeons={6} -> chose 6 (entry 0x0100000C)
  LFG TeleportToDungeon: Humanwarrior DENIED, dungeon 12 map 349, player error 6

The queue entry was correct throughout -- the player asked for dungeon 6 and we
chose dungeon 6. GetDungeonEntry(6) still returned 0x0100000C, which is id 12.

LfgDungeonsEntryfmt began with 'i' rather than 'n'. DBC_FF_IND is 'n' and marks
the INDEX column; DBC_FF_INT is 'i' and is just a uint32. With no index column
declared, DBCFileLoader::GetFormatRecordSize returns index_pos -1 and
AutoProduceData falls into its fallback branch, numbering indexTable 0..
recordCount-1 in FILE ORDER. Every LookupEntry on this store was therefore
positional.

Reading the shipped DBC confirms the observed values exactly:

  row 6  -> id 12 "Stormwind Stockade"  (so LookupEntry(6) gave Stockade)
  row 12 -> id 26 "Maraudon ...", MapID 349

which is the whole chain: LookupEntry(6) yields id 12, CreateDungeonGroup passes
that 12 to TeleportToDungeon, LookupEntry(12) yields map 349. A player queueing
Deadmines was offered Stockade and then denied a teleport to Maraudon.

The second consequence is quieter and larger. Without an index column
GetNumRows() reports recordCount (343) instead of maxId+1 (776), so every loop
of the form `for (id = 0; id < GetNumRows(); ++id) LookupEntry(id)` -- three of
them in the LFG code -- walked less than half the id space, and mis-identified
every row it did visit. That is why a level-appropriate character came back with
a 329-entry locked-dungeon list.

Of the 129 format strings in DBCfmt.h, 86 start with 'n', 29 with 'd', 10 with
'x' and only 4 with 'i'. Two of those four (AreaTable, ScalingStatValues) carry
an 'n' later in the string and are properly indexed. Of the remaining two,
ChatChannels is genuinely unindexed and its only consumer knows it -- it linear
scans comparing ch->ID, with a comment saying so. LfgDungeons was the only store
looked up by ID while indexed by row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ED correctly

The packet log printed "OPCODE: UNKNOWN (0x1E3B)" during a live test.
SMSG_LFG_PROPOSAL_UPDATE and SMSG_LFG_ROLE_CHECK_UPDATE transmit correctly --
they are admitted by IsEnterWorldConverted, which is the real send gate -- but
neither had a DefS row, so the logger could not name them. DefS is logging
metadata only in this tree and does not affect delivery.

SMSG_LFG_TELEPORT_DENIED wrote a uint32. Every 18414 capture of it in the corpus
is exactly 1 byte: capture-000044 seq 70879 and 219256, capture-000465 seq
283035, capture-000628 seq 31349, capture-000873 seq 154730. Now a uint8.

It stays UNADMITTED on purpose. The size is settled but the value space is not
-- the captured body carries 0x10 (16) while our LFGTeleportError enum stops at
8, so our codes are provably not the client's. A correctly sized packet with a
wrong code shows the player a confidently wrong reason, which is worse than the
current silence. This is why an LFG teleport failure currently produces no
on-screen message at all: the packet is built, logged, and dropped at the gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rything

Reported from a live client: every dungeon in the game appeared in the finder,
so a player could queue for content they cannot enter.

The cause is ours. FrameXML's LFGList_DefaultFilterFunction shows a dungeon when
`not LFGLockList[dungeonID]`, and LFGLockList is built from the lock array in
SMSG_LFG_PLAYER_INFO -- which we sent EMPTY. An empty array does not mean "no
locks are known", it means "nothing is locked", so the client correctly
concluded every dungeon was available. The eligibility filter was not degraded;
it was absent, because we never gave it anything to filter on.

Layout derived from a real reply rather than a fork: capture-000006 seq 1953,
6068 bytes to a max-level character, decoding as lockCount 206, hasPlayerGuid 0,
randomDungeonCount 35.

  bits   WriteBits(lockCount, 20)
         WriteBit(hasPlayerGuid)
         WriteBits(randomDungeonCount, 17)
         FlushBits                       -> 38 bits, 5 bytes
  ...random dungeon reward records, variable length...
  tail   lockCount x 16 bytes, flat and unpacked:
             uint32 dungeonEntry   (TypeID << 24) | id
             uint32 lockStatus
             uint32 subReason1
             uint32 subReason2

That the array sits at the TAIL is what makes this shippable now. With zero
random records the header and the array are adjacent, and the client installs
the list and raises LFG_LOCK_INFO_RECEIVED whether or not random rows follow --
so the reward plumbing this manager cannot express is not needed to make the
filter work. The random count stays 0 and is a separate piece of work.

No translation is needed in either direction, which is worth stating because it
looks too convenient: FindRandomDungeonsNotForPlayer already returns a map keyed
by LfgDungeonsEntry::Entry(), and that IS the wire's dungeonEntry field; its
LFGForbiddenTypes values are the client's LFG_INSTANCE_INVALID_CODES verbatim.
The reference packet's own distribution confirms the codes line up -- 167 of its
206 records carry 3, LEVEL_TOO_HIGH, which is exactly what a max-level character
sees for low-level content.

subReason1 and subReason2 stay zero. They carry the required and current item
level for the gear-score reasons, where the client formats them as
"Requires: %2$d. Currently %3$d."; all 206 records of the reference capture have
them zero, and this manager does not compute a gear score for the lock list.

Sent only in reply to CMSG_LFG_LOCK_INFO_REQUEST, never pushed at login. The two
pair seven-for-seven in capture-000006, and the client asks at world-enter --
CMSG_LFG_GET_STATUS then CMSG_LFG_LOCK_INFO_REQUEST at adjacent sequence
numbers. Our handler was already registered and already replied in the right
place; only the content was missing.

Fixture uses the reference packet's own first five lock records byte for byte,
including a TypeID 2 raid entry so the array is not assumed to be dungeons-only,
plus its exact 5-byte header to pin the 20/1/17 field widths and their order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebasing this branch onto merged master brought master's difficulty translation
into CreateDungeonGroup, and that block carries its own lookups of `dungeon` and
`groupGuid`. This branch already has both, earlier and deliberately:

  * `dungeon` is looked up at the top of the function, BEFORE anything is
    created, because the original order returned on an unknown id having already
    new'd a Group, run Create (a group id plus an INSERT) and registered it with
    ObjectMgr -- leaking the object and stranding its rows.
  * `groupGuid` is taken once as `ObjectGuid const`.

Keeping both copies was a redefinition and did not compile. The earlier ones are
the right ones to keep, so the later pair is removed and the reason each exists
is noted where it would otherwise look redundant.

Build clean; ctest 110/110.
@MadMaxMangos
MadMaxMangos force-pushed the fix/lfg-matchmaker-role-resolution branch from ba6aeff to 4e5e4c3 Compare August 5, 2026 21:04
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant