From 71012e3ce3e4a276b3fe9ded20481187c5daceb1 Mon Sep 17 00:00:00 2001 From: Andrew <47818697+Nyeriah@users.noreply.github.com> Date: Sun, 13 Sep 2026 14:36:33 -0300 Subject: [PATCH] fix(Core/Groups): identify the finished roll in SMSG_LOOT_ROLL_WON (#27631) Co-authored-by: Cyberium --- e2e/README.md | 2 +- .../social/loot/loot_roll_timeout_e2e_test.go | 228 ++++++++++++++++++ src/server/game/Groups/Group.cpp | 4 +- 3 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 e2e/suites/social/loot/loot_roll_timeout_e2e_test.go diff --git a/e2e/README.md b/e2e/README.md index 892fc83a98..cde1f08681 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -151,7 +151,7 @@ go test -tags=e2e ./local/... -count=1 -v -timeout 30m -parallel 1 | spells/cast | Charge on dummy; fail path; stance; Raise Dead + ghoul | P1 | covered (`TestAC_27061_*`) | #27061 | | spells/effects | Charge / grounding totem / Sweeping Strikes Execute; forced cast summons at the forced caster, not at the unit that forced it | P1 | covered (`TestAC_26997_*`, `TestEffects_ForceCastDestination`); dummy-summon `blocked-harness` (engineering dummy lifetime) | #26774 #26997 #27621 | | social/group | form / leave / leader / loot method / disband | P2 | covered | — | -| social/loot | need/greed / master loot; below-half kill | P1 | covered (`TestAC_26862_*`); chest mid-roll `blocked-harness` (GO 194821 UseGameObject); pass-on-loot delete `blocked-harness` (item-survive after ALL_PASSED) | #26894 #26862 #22000 | +| social/loot | need/greed / master loot; below-half kill; a roll that ends on the timer names the roll in its terminating packet, and the member who never voted gets one PASS | P1 | covered (`TestAC_26862_*`, `TestAC_27299_*`); chest mid-roll `blocked-harness` (GO 194821 UseGameObject); pass-on-loot delete `blocked-harness` (item-survive after ALL_PASSED) | #26894 #26862 #27299 #22000 | | social/trade | item+gold accept; cancel; walk-OOR TARGET_TO_FAR | P1 | covered | #25723 | | quests/lifecycle | STAY_ALIVE fail on death; status after save/relog | P1 | covered (`TestAC_26549_*`) | #26549 | | quests/escort | find spawned unit; follow-NPC despawns on logout | P2 | covered (`TestAC_24450_*`) | #24450 | diff --git a/e2e/suites/social/loot/loot_roll_timeout_e2e_test.go b/e2e/suites/social/loot/loot_roll_timeout_e2e_test.go new file mode 100644 index 0000000000..b4084c5036 --- /dev/null +++ b/e2e/suites/social/loot/loot_roll_timeout_e2e_test.go @@ -0,0 +1,228 @@ +//go:build e2e + +package loot_test + +import ( + "fmt" + "sync" + "testing" + "time" + + _ "github.com/go-sql-driver/mysql" + + "github.com/azerothcore/AzerothGhost/client" + "github.com/azerothcore/AzerothGhost/e2e/e2eharness" + "github.com/azerothcore/azerothcore-wotlk/e2e/internal/meta" +) + +// rollPacketLog collects every SMSG_LOOT_ROLL a bot receives. +type rollPacketLog struct { + mu sync.Mutex + events []client.LootRollEvent +} + +func (r *rollPacketLog) add(e client.LootRollEvent) { + r.mu.Lock() + r.events = append(r.events, e) + r.mu.Unlock() +} + +func (r *rollPacketLog) snapshot() []client.LootRollEvent { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]client.LootRollEvent, len(r.events)) + copy(out, r.events) + return out +} + +func (r *rollPacketLog) describe() string { + s := "" + for _, e := range r.snapshot() { + s += fmt.Sprintf("\n rollID=0x%X target=0x%X item=%d num=%d type=%d autoPass=%d", + e.ItemGUID, e.TargetGUID, e.ItemID, e.RollNumber, e.RollType, e.AutoPass) + } + if s == "" { + return " (none)" + } + return s +} + +// rollStillOpen reports whether the bot still holds rollID as a pending roll. +// A roll is dropped only when a terminating packet matches it by roll id and +// slot, which is the identity rule the client uses to cancel its roll frame. +func rollStillOpen(bot *e2eharness.ScenarioBot, rollID uint64) bool { + for _, r := range bot.World.ActiveLootRolls() { + if r.ItemGUID == rollID { + return true + } + } + return false +} + +// countPasses returns how many PASS lines target got for rollID. +func countPasses(events []client.LootRollEvent, target, rollID uint64) int { + n := 0 + for _, e := range events { + if e.TargetGUID == target && e.RollType == client.RollPass && e.ItemGUID == rollID { + n++ + } + } + return n +} + +// TestAC_27299_TimedOutRollTerminates covers a group loot roll that ends on the +// timer rather than on a vote. A member who never answers must be told it +// passed, and the packet that ends the roll must name the roll, otherwise the +// client cannot cancel the roll frame and the window stays on screen. +// +// Issue: https://github.com/azerothcore/azerothcore-wotlk/issues/27299 +func TestAC_27299_TimedOutRollTerminates(t *testing.T) { + meta.Begin(t, meta.TestMeta{Tags: []string{"long", "loot", "multi_bot", "issue", "serial"}, Issue: 27299, Runtime: "long", Category: "social/loot"}) + + bots := e2eharness.NewScenario(t, e2eharness.ScenarioOpts{ + Prefix: "LootTmo", + Bots: []e2eharness.BotSpec{ + {Role: "leader", Class: e2eharness.ClassWarrior, Level: 80, LearnAllClass: true}, + {Role: "silent", Class: e2eharness.ClassWarrior, Level: 80, LearnAllClass: true}, + }, + }) + leader := e2eharness.ByRole(t, bots, "leader") + silent := e2eharness.ByRole(t, bots, "silent") + e2eharness.FormPartyAtPad(t, e2eharness.PackagePad(t), leader, silent) + + // AC rejects a threshold below Uncommon; greys never start a group roll. + leader.SetLootMethod(t, client.LootMethodGroupLoot, 0, e2eharness.LootThresholdUncommon) + leader.WaitLootMethod(t, client.LootMethodGroupLoot, 10*time.Second) + silent.CombatReady(t) + + // startRoll kills a fresh fixture corpse and returns the roll the silent bot + // was offered, with a collector for every roll packet it subsequently sees. + startRoll := func(t *testing.T) (client.LootStartRoll, uint64, *rollPacketLog, bool) { + t.Helper() + corpse := leader.SpawnKillLootable(t, e2eharness.CreatureGroupLootFixture, 45*time.Second) + + leader.World.ClearActiveLootRolls() + silent.World.ClearActiveLootRolls() + + waitRoll, cancelRoll := silent.ArmLootStartRoll() + t.Cleanup(cancelRoll) + + items, opened := leader.TryOpenLoot(t, corpse, 8*time.Second) + if !opened { + _ = leader.World.SetTarget(corpse) + items, opened = leader.TryOpenLoot(t, corpse, 8*time.Second) + } + if !opened { + e2eharness.Preconditionf(t, "group-loot fixture corpse not lootable guid=0x%X", corpse) + return client.LootStartRoll{}, 0, nil, false + } + + roll, ok := waitRoll(0, 12*time.Second) + if !ok { + leader.LootRelease(t, corpse) + e2eharness.Preconditionf(t, "no SMSG_LOOT_START_ROLL on fixture %d (items=%d)", e2eharness.CreatureGroupLootFixture, len(items)) + return client.LootStartRoll{}, 0, nil, false + } + + seen := &rollPacketLog{} + t.Cleanup(silent.World.AddLootRollHook(seen.add)) + t.Logf("roll armed item=%d rollID=0x%X slot=%d countdown=%dms silent=0x%X", + roll.ItemID, roll.ItemGUID, roll.ItemSlot, roll.CountdownMS, silent.GUID) + return roll, corpse, seen, true + } + + // The reported case: someone else needs and wins while one member never + // answers the popup. + t.Run("NeedWinNamesTheRoll", func(t *testing.T) { + roll, corpse, seen, ok := startRoll(t) + if !ok { + return + } + + wonCh, allCh, cancelOut := silent.ArmLootRollOutcome(roll.ItemID) + t.Cleanup(cancelOut) + + leader.RollNeed(t, roll) + // silent deliberately never votes; the server timer must resolve the roll. + + var won client.LootRollWon + select { + case won = <-wonCh: + case all := <-allCh: + e2eharness.Preconditionf(t, "expected a NEED winner, roll ended as ALL_PASSED item=%d", all.ItemID) + return + case <-time.After(100 * time.Second): + e2eharness.Assertf(t, "roll never resolved item=%d rollID=0x%X; SMSG_LOOT_ROLL seen:%s", + roll.ItemID, roll.ItemGUID, seen.describe()) + return + } + + // Primary oracle: the packet that ends the roll names the roll, so the + // client can cancel the frame it opened from SMSG_LOOT_START_ROLL. + if won.ItemGUID != roll.ItemGUID { + e2eharness.Assertf(t, "SMSG_LOOT_ROLL_WON rollID=0x%X does not match SMSG_LOOT_START_ROLL rollID=0x%X; the client cannot close the roll window", + won.ItemGUID, roll.ItemGUID) + } + if rollStillOpen(silent, roll.ItemGUID) { + e2eharness.Assertf(t, "roll 0x%X still pending after LOOT_ROLL_WON; nothing terminated it for the member who never voted", roll.ItemGUID) + } + + // The member who never voted is told it passed, exactly once, and not as + // an automatic "you cannot loot that item" pass. + events := seen.snapshot() + switch n := countPasses(events, silent.GUID, roll.ItemGUID); { + case n == 0: + e2eharness.Assertf(t, "no timeout PASS for the member who never voted (target=0x%X rollID=0x%X); SMSG_LOOT_ROLL seen:%s", + silent.GUID, roll.ItemGUID, seen.describe()) + case n > 1: + e2eharness.Assertf(t, "member who never voted got %d PASS lines for one roll, want 1;%s", n, seen.describe()) + } + for _, e := range events { + if e.TargetGUID == silent.GUID && e.RollType == client.RollPass && e.ItemGUID == roll.ItemGUID && e.AutoPass != 0 { + e2eharness.Assertf(t, "timeout PASS sent with autoPass=%d; the client prints the \"cannot loot that item\" line instead of a plain pass", e.AutoPass) + } + } + + t.Logf("PASS timed-out NEED win: winner=0x%X rollID=0x%X matches start-roll, roll closed", won.WinnerGUID, won.ItemGUID) + leader.LootRelease(t, corpse) + }) + + // Nobody answers, so the roll ends as all-passed instead of with a winner. + t.Run("AllTimeoutNamesTheRoll", func(t *testing.T) { + roll, corpse, seen, ok := startRoll(t) + if !ok { + return + } + + wonCh, allCh, cancelOut := silent.ArmLootRollOutcome(roll.ItemID) + t.Cleanup(cancelOut) + + var all client.LootAllPassed + select { + case all = <-allCh: + case won := <-wonCh: + e2eharness.Preconditionf(t, "expected ALL_PASSED with nobody voting, got a winner 0x%X", won.WinnerGUID) + return + case <-time.After(100 * time.Second): + e2eharness.Assertf(t, "roll never resolved item=%d rollID=0x%X; SMSG_LOOT_ROLL seen:%s", + roll.ItemID, roll.ItemGUID, seen.describe()) + return + } + + if all.ItemGUID != roll.ItemGUID { + e2eharness.Assertf(t, "SMSG_LOOT_ALL_PASSED rollID=0x%X does not match SMSG_LOOT_START_ROLL rollID=0x%X", + all.ItemGUID, roll.ItemGUID) + } + if rollStillOpen(silent, roll.ItemGUID) { + e2eharness.Assertf(t, "roll 0x%X still pending after LOOT_ALL_PASSED", roll.ItemGUID) + } + if n := countPasses(seen.snapshot(), silent.GUID, roll.ItemGUID); n != 1 { + e2eharness.Assertf(t, "member who never voted got %d PASS lines for one roll, want 1;%s", n, seen.describe()) + } + + t.Logf("PASS full timeout: ALL_PASSED rollID=0x%X matches start-roll, roll closed", all.ItemGUID) + leader.LootRelease(t, corpse) + }) + + leader.AssertWorldAlive(t) +} diff --git a/src/server/game/Groups/Group.cpp b/src/server/game/Groups/Group.cpp index b84a10e240..bb8879089a 100644 --- a/src/server/game/Groups/Group.cpp +++ b/src/server/game/Groups/Group.cpp @@ -1642,7 +1642,7 @@ void Group::CountTheRoll(Rolls::iterator rollI) if (maxguid) // pussywizard: added condition { - SendLootRollWon(ObjectGuid::Empty, maxguid, maxresul, ROLL_NEED, *roll); + SendLootRollWon(roll->itemGUID, maxguid, maxresul, ROLL_NEED, *roll); player = ObjectAccessor::FindPlayer(maxguid); if (player) @@ -1723,7 +1723,7 @@ void Group::CountTheRoll(Rolls::iterator rollI) if (maxguid) // pussywizard: added condition { - SendLootRollWon(ObjectGuid::Empty, maxguid, maxresul, rollvote, *roll); + SendLootRollWon(roll->itemGUID, maxguid, maxresul, rollvote, *roll); player = ObjectAccessor::FindPlayer(maxguid); if (player)