Repository navigation
[Feat] LFG premade party queueing - #332
Merged
Merged
Conversation
LFGRoleCheck::state stays LFG_ROLECHECK_INITIALITING across every member's selection, so BuildRoleCheckPacket's old isBeginning derivation (state == INITIALITING) set the flag on every intermediate update. The client treats a set flag as "the check just started": it prints ERR_LFG_ROLE_CHECK_INITIATED, plays sound 17318, and replays LFG_ROLE_CHECK_ROLE_CHOSEN for every member who already answered. Sending it more than once produced escalating chat spam and repeated sounds for a real party (solo queueing never has a role check, so this was never exercised before). Add LFGRoleCheck::beginningSent, latched the first time PerformRoleCheck sends a packet for a given check, and thread an explicit isBeginning parameter through BuildRoleCheckPacket so the caller decides instead of the packet re-deriving it from state. RemoveOldRoleChecks's timeout packet always passes false -- a timeout is never a beginning. Also fold in the roleChosen fix this exposes: the old guard (state != LFG_ROLECHECK_DEFAULT) was true on the very first call too, since JoinLFG seeds state to INITIALITING before calling in, so SMSG_LFG_ROLE_CHOSEN fired for the leader's own selection and duplicated the isBeginning replay's chat line. roleChosen is now !isBeginning && !plrGuid.IsEmpty() -- still fires on a genuine decline (roles == 0 takes the LFG_ROLECHECK_NO_ROLE branch, not the abort one).
BuildRoleChosen wrote Accepted = (roleMask > 0). A PLAYER_ROLE_LEADER-only mask (0x01) has no TANK/HEALER/DAMAGE bit but is still > 0, so Accepted came out true with RolesDesired & 0x0E == 0. The client's LFG_ROLE_CHECK_ROLE_CHOSEN handler builds a role list from roles & 2, roles & 4, roles & 8 and asserts it is non-nil (LFGFrame.lua:166) -- a leader-only mask crashes the default UI with a Lua error. Gate Accepted on ROLE_MASK_ANY (TANK|HEALER|DAMAGE) instead of a bare nonzero check. A genuine decline (roleMask == 0) still yields Accepted = 0, unchanged.
LFD_IsEmpowered (UIParent.lua:3649) only greys the queue/dequeue button; the C function behind it, JoinLFG (0x140700860), has no leadership check at all -- a modified client can queue as a non-leader today and m3's leader-only gate is both too strict (any LFD party member may queue or dequeue, not just the leader) and unenforced against a client that skips the grey-out. Add LFGEmpowerment.h: a header-only, game-free mirror of the client's six predicates (solo / party leader / raid leader / LFD party member / LFD raid assistant), so the decision table has unit coverage without linking `game`. LFGMgr::BuildEmpowermentState/IsEmpowered adapt a live Player's group into that state; HandleLfgJoinOpcode and HandleLfgLeaveOpcode both gate on IsEmpowered and drop the packet silently on failure -- there is no "leader only" string anywhere in GlobalStrings.lua, so retail's refusal is silent and ours must be too. Group::IsAssistant(ObjectGuid) already exists (Group.h:405), so the raid branch needed no new accessor.
CreateDungeonGroup always built a fresh Group, picking a leader by leader-bit-then-lowest-guid and calling RemoveMember on every proposal player's old group -- including a real premade's. For a 3-person premade that meant: RemoveMember sent the leader a solo SMSG_GROUP_LIST and homebound them if they stood in an instance, and removing the second member (count now <= 2) took Group::RemoveMember's own Disband(true) branch, disbanding a group the caller still expected to delete manually. Add LFGMgr::PickHostGroup, backed by a new game-free LFGProposalLogic::PickHostGroup(playerToGroup): the pre-existing party contributing the most members to the proposal, ties broken on the lowest guid, 0 when nobody in the proposal has a party. CreateDungeonGroup now looks that group up via ObjectMgr first; when found, its Group object and leader are left untouched and only the outsiders (solo queuers, or a smaller second party) are detached from their own group and added to it. The "build from scratch" path only runs when nobody in the proposal has a pre-existing party. Add LFGMgr::DetachFromGroup, which decides disband ownership from GetMembersCount() *before* calling RemoveMember rather than from the post-remove return value, and only tears down the group when it actually disbanded. Group::Disband never unregisters from ObjectMgr or deletes itself, so this single RemoveGroup+delete is the correct cleanup, not a double-free of a self-disbanded group -- confirmed by reading Disband end to end (Group.cpp:914-1009). The AddMember failure path no longer disbands the (possibly real, kept) group; it returns false and lets the normal proposal-failure path re-queue whoever is left. Members already detached before the failure are left solo -- an accepted, documented limitation, and strictly better than destroying a real party. IsSuccessfulProposalMove needed no code change: its VALUE-arm match on proposal.groups already covers a preserved host group receiving new members via AddMember. Added a comment recording why, so it does not get "simplified" away later.
SMSG_LFG_UPDATE_STATUS's IsParty flag is not cosmetic: the client discards the whole update -- LFG_UPDATE never fires -- when IsParty disagrees with its own current group state (spec 7c, section 1.5b). Two call sites derived it from a snapshot instead of live membership: - SendDungeonProposal's LFG_UPDATE_PROPOSAL_BEGIN send used "were you in a party when the proposal was built" (proposal.groups) instead of "are you in a party right now". At that specific call site the two happen to agree today, but the shape invites drift, so it now reads the player's live group membership like every other correctly derived site in this file. - The LFG_UPDATE_GROUP_FOUND send in ProposalUpdate ran *after* CreateDungeonGroup had already formed the group, so the same proposal.groups snapshot was stale for every former solo queuer -- they are now genuinely in the group, but the update still reported IsParty = false for them. This is state-machine step 20 (section 6.2): IsParty must be TRUE for everyone once formation has happened, or a solo queuer's Dungeon Finder UI never clears the "queued" state after a mixed premade+solo proposal succeeds. RemoveProposal's two isParty sites are unchanged: that path never forms a group, so its proposal.groups snapshot still equals live membership. Every other SendLfgUpdate call site in these two files already derives isParty correctly, either from live lookup or by construction (iterating a real Group's own members).
|
✅ Build Mangos THREE Server 22.03.465 completed (commit 7403ebcd98 by @r-log) |
Merged
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
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.
Continues the Dungeon Finder work from #331. Everything so far assumed a solo queuer; this makes an existing party able to queue together.
Keep the party that already exists.
CreateDungeonGroupalways built a freshGroupand calledRemoveMemberon every proposal player's old group — including a real premade's. That sent its leader a soloSMSG_GROUP_LIST, homebound them if they stood in an instance, and on the second removal tookGroup::RemoveMember's ownDisband(true)branch, disbanding a group the caller still meant to delete itself. The pre-existing party contributing the most members now hosts the dungeon group, ties broken on the lowest guid; only outsiders are moved into it. Build-from-scratch remains for an all-solo proposal. The original leader keeps leadership, verified in-game.Announce a role check once.
LFGRoleCheck::statestaysINITIALITINGacross every member's selection, so the oldisBeginningderivation set the flag on each intermediate update. The client reads a set flag as "the check just started" —ERR_LFG_ROLE_CHECK_INITIATED, sound 17318, and a replayedROLE_CHOSENfor everyone who already answered — so a real party got escalating chat spam and repeated sounds. Solo queueing never runs a role check, which is why this only surfaces now.Leader-only masks are not a choice.
AcceptedwasroleMask > 0, but aPLAYER_ROLE_LEADER-only mask (0x01) carries no TANK/HEALER/DAMAGE bit.LFGFrame.lua:166builds a role list from bits 2/4/8 and asserts it non-nil, so that combination threw a Lua error in the default UI. Gated onTANK|HEALER|DAMAGE; a genuine decline still reportsAccepted = 0.Empowerment is now enforced.
LFD_IsEmpowered(UIParent.lua:3649) only greys the button —JoinLFG(0x140700860) has no leadership check — so m3's leader-only gate was simultaneously too strict (any LFD party member may queue or dequeue) and unenforced against a client that skips the grey-out.LFGEmpowerment.hmirrors the client's six predicates. Refusal is silent becauseGlobalStrings.luahas no "leader only" string.IsPartymust be live. The client discards a wholeSMSG_LFG_UPDATE_STATUSwhoseIsPartydisagrees with its current group state, freezing the UI with no error. Two sites read a pre-formation snapshot; both now read live membership.122 LFG unit tests pass. Verified in-game with a 3-person premade plus solo queuers.
This change is