Repository navigation
fix(matchmakingserver): expired invites only unlink their own battleground - #72
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes matchmaking state divergence by ensuring expired battleground invites only unlink the player from the specific battleground whose invite expired (rather than removing them from any battleground of the same queue type, including active matches). The expiry logic is extracted into a tick helper to make it unit-testable, and a regression test is added to verify active-match links survive unrelated invite expiry.
Changes:
- Extracted the invite-expiry loop body into
processExpiredBattlegroundInvitesTickfor testability. - On invite expiry, unlinks only the expired battleground via
removeBattlegroundLinkForPlayerinstead of callingRemovePlayerFromQueue(queueTypeID). - Added a unit test to ensure an active-match battleground link is preserved when another invite expires.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/matchmakingserver/service/battleground_service.go | Refactors expiry processing into a tick method and changes unlink behavior to target only the expired battleground. |
| apps/matchmakingserver/service/battleground_service_test.go | Adds coverage to prevent regressions where expiring an invite incorrectly unlinks an active match of the same queue type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case <-time.After(time.Second): | ||
| bgs, err := s.battlegroundsRepo.GetAllActiveBattlegrounds(ctx) | ||
| if err != nil { | ||
| log.Err(err).Msg("failed to get all active battlegrounds") | ||
| break | ||
| } | ||
| for _, bg := range bgs { | ||
| for _, invites := range bg.InvitedPlayersPerTeam { | ||
| for _, invite := range invites { | ||
| if time.Since(invite.InvitedTime) > time.Minute { | ||
| err = s.battlegroundsRepo.UpdateBattleground( | ||
| ctx, | ||
| bg.InstanceID, | ||
| repo.RealmWithBattlegroupKey{RealmID: bg.RealmID, BattlegroupID: bg.BattleGroupID}, | ||
| func(b *battleground.Battleground) error { | ||
| b.RemovePlayerFromInvite(uint64(invite.GUID.LowGUID), uint32(invite.GUID.RealmID)) | ||
| return nil | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| log.Err(err).Msg("failed to remove invite from Battleground") | ||
| continue | ||
| } | ||
|
|
||
| err = s.RemovePlayerFromQueue(ctx, uint64(invite.GUID.LowGUID), uint32(invite.GUID.RealmID), bg.QueueTypeID) | ||
| if err != nil { | ||
| log.Err(err).Msg("failed to remove invited player from queue") | ||
| continue | ||
| } | ||
|
|
||
| slot := uint8(0) // TODO: provide real slot | ||
| err = s.eventsProducer.InviteExpired(&events.MatchmakingEventPlayersInviteExpiredPayload{ | ||
| RealmID: uint32(invite.GUID.RealmID), | ||
| PlayersGUID: []guid.LowType{invite.GUID.LowGUID}, | ||
| QueueSlotByPlayer: map[guid.LowType]uint8{invite.GUID.LowGUID: slot}, | ||
| }) | ||
| if err != nil { | ||
| log.Err(err).Msg("Failed to add invited player to queue") | ||
| } | ||
| } | ||
| s.processExpiredBattlegroundInvitesTick(ctx) |
|
Done — switched to a ticker with an explicit Stop(). |
…round The invite-expiry loop called RemovePlayerFromQueue with the queue type, which walks every link of the player and removes them from any battleground of that type - including a running match where the player is an active participant. Unlink only the battleground whose invite expired (the invited player left the generic queue at invite time, so that link is the only one to clean).
bdd7af2 to
f4d87db
Compare
|
Ping — still mergeable on master, and the ticker/Stop() change from the review is in. Anything else you'd like here? |
|
Friendly ping — still applies cleanly and running in production on my cluster since July. One heads-up: #100 adds traces and an active-participant guard around the same expiry block, so whichever lands first, I'll rebase the other accordingly. |
The invite-expiry loop calls
RemovePlayerFromQueue(queueTypeID), which walksevery link of the player and removes them from any battleground of that
queue type — including a running match where they are an active participant,
so the service state diverges from the game server's. An invited player
already left the generic queue at invite time, so the only link to clean is
the battleground whose invite expired: unlink exactly that one.
The loop body is extracted into a tick method to make it testable; the test
checks that an active-match link survives the expiry of an invite to another
battleground of the same type.