mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-11 08:23:45 +00:00
fix(multiplayer): requeue every participant after a proposal is declined
Found by reading the code, not a failing test: no path anywhere transitioned a queue ticket from PROPOSED back to QUEUED after a proposal was declined. A stranded PROPOSED ticket is invisible to the matcher (ListQueuedCandidates only ever reads state='QUEUED'), still counts as that player's one active ticket (blocking a fresh queue_create), and is renewable forever by an ordinary heartbeat -- a player proposed a match with someone who then declines had no way back into matchmaking without realising, on their own, that they needed to manually cancel first. This affects every participant, not just the decliner: an uninvolved player who never even responded was left stuck by someone else's decision. ProposalDeclineRequeueSQL requeues every participant's ticket, including the decliner's own -- nothing yet enforces the decline cooldown task 8.17 documents as a separate, not-yet-built feature, so leaving anyone behind at PROPOSED today isn't "cooldown behaviour", it's just broken. Once that cooldown exists it can exempt the decliner from this immediate requeue; today nothing does. Covered by a real PostgreSQL integration test: after one player declines, both the decliner's and an uninvolved participant's tickets land back at QUEUED with a refreshed expiry, and -- the actual end-to-end regression -- both are visible again to ListQueuedCandidates, the same query the matcher itself uses. Clean across 5 runs, plus the full integration and unit suites.
This commit is contained in:
@@ -495,6 +495,81 @@ func TestPostgreSQLProposalClaimAndResponseAreAtomic(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPostgreSQLProposalDeclineRequeuesEveryParticipant is a real, severe
|
||||
// bug this session found by reading the code, not by a failing test: no
|
||||
// path anywhere transitioned a PROPOSED ticket back to QUEUED after a
|
||||
// decline. A stranded ticket is invisible to the matcher (which only reads
|
||||
// state='QUEUED'), still counts as the player's one active ticket (blocking
|
||||
// a fresh queue_create), and is renewable forever by an ordinary heartbeat
|
||||
// -- a player proposed a match with someone who declines had no way back
|
||||
// into matchmaking without realising they had to manually cancel first.
|
||||
func TestPostgreSQLProposalDeclineRequeuesEveryParticipant(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
applyIntegrationMigrations(t, db)
|
||||
|
||||
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||
ctx := context.Background()
|
||||
for _, player := range []string{"decline-player-a", "decline-player-b"} {
|
||||
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $1)`, player); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
for i, player := range []string{"decline-player-a", "decline-player-b"} {
|
||||
if _, err := db.ExecContext(ctx, `INSERT INTO queue_tickets (ticket_id, player_id, playlist, state, client_build, protocol_version, enqueued_at, expires_at) VALUES ($1, $2, 'casual', 'QUEUED', 'integration-build', 1, $3, $4)`, fmt.Sprintf("decline-ticket-%d", i), player, now, now.Add(time.Minute)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
proposal, err := domain.NewProposal("decline-proposal", domain.Casual, []string{"decline-player-a", "decline-player-b"}, now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := CreateProposal(ctx, db, proposal, map[string]string{"decline-player-a": "decline-ticket-0", "decline-player-b": "decline-ticket-1"}, now); err != nil {
|
||||
t.Fatalf("create proposal: %v", err)
|
||||
}
|
||||
|
||||
// player-a declines; player-b never responded at all -- the bug affects
|
||||
// even a participant who was never asked to do anything wrong.
|
||||
declined, err := RespondToProposal(ctx, db, "decline-player-a", proposal.ProposalID, "decline-response-a-0001", false, 0, now)
|
||||
if err != nil {
|
||||
t.Fatalf("decline: %v", err)
|
||||
}
|
||||
if declined.State != domain.Declined {
|
||||
t.Fatalf("proposal did not close on decline: %+v", declined)
|
||||
}
|
||||
|
||||
var stateA, stateB string
|
||||
var expiresB time.Time
|
||||
if err := db.QueryRow(`SELECT state FROM queue_tickets WHERE ticket_id = 'decline-ticket-0'`).Scan(&stateA); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := db.QueryRow(`SELECT state, expires_at FROM queue_tickets WHERE ticket_id = 'decline-ticket-1'`).Scan(&stateB, &expiresB); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if stateA != "QUEUED" {
|
||||
t.Fatalf("decliner's own ticket state = %s, want QUEUED (no cooldown mechanism exists yet to justify leaving it stuck)", stateA)
|
||||
}
|
||||
if stateB != "QUEUED" {
|
||||
t.Fatalf("uninvolved participant's ticket state = %s, want QUEUED -- they must not be stranded by someone else's decline", stateB)
|
||||
}
|
||||
if !expiresB.After(now) {
|
||||
t.Fatalf("requeued ticket expiry %v was not refreshed forward from %v", expiresB, now)
|
||||
}
|
||||
|
||||
// The real, end-to-end regression: both players can be proposed a NEW
|
||||
// match instead of ListQueuedCandidates silently never seeing them again.
|
||||
candidates, err := ListQueuedCandidates(ctx, db, domain.Casual, now, 10)
|
||||
if err != nil {
|
||||
t.Fatalf("list queued candidates: %v", err)
|
||||
}
|
||||
found := map[string]bool{}
|
||||
for _, candidate := range candidates {
|
||||
found[candidate.PlayerID] = true
|
||||
}
|
||||
if !found["decline-player-a"] || !found["decline-player-b"] {
|
||||
t.Fatalf("requeued players are not visible to the matcher: %+v", candidates)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPostgreSQLProposalCreationRollsBackPartialClaims(t *testing.T) {
|
||||
db := openIntegrationPostgres(t)
|
||||
applyIntegrationMigrations(t, db)
|
||||
|
||||
@@ -69,6 +69,21 @@ const ProposalDeclineSQL = `UPDATE proposals
|
||||
SET state = 'DECLINED', revision = revision + 1
|
||||
WHERE proposal_id = $1 AND state = 'OPEN'`
|
||||
|
||||
// ProposalDeclineRequeueSQL requeues every participant's ticket, including
|
||||
// the decliner's own: nothing yet enforces the decline cooldown §8.17
|
||||
// documents as a separate, not-yet-built feature, so leaving any ticket
|
||||
// behind at PROPOSED here isn't "cooldown behaviour", it's just a stranded
|
||||
// ticket -- invisible to the matcher (which only ever reads state='QUEUED'),
|
||||
// still counted as this player's one active ticket (blocking a fresh
|
||||
// queue_create), and renewable forever by an ordinary heartbeat, so a player
|
||||
// left in this state has no path back into matchmaking without realising
|
||||
// they need to cancel and start over. Once §8.17's cooldown exists, it can
|
||||
// exempt the decliner from this immediate requeue; today nothing does.
|
||||
const ProposalDeclineRequeueSQL = `UPDATE queue_tickets q
|
||||
SET state = 'QUEUED', expires_at = $2, revision = revision + 1
|
||||
FROM proposal_participants pp
|
||||
WHERE pp.proposal_id = $1 AND q.ticket_id = pp.ticket_id AND q.player_id = pp.player_id AND q.state = 'PROPOSED'`
|
||||
|
||||
const ProposalRevisionBumpSQL = `UPDATE proposals
|
||||
SET revision = revision + 1
|
||||
WHERE proposal_id = $1 AND state = 'OPEN'`
|
||||
@@ -217,6 +232,10 @@ func RespondToProposal(ctx context.Context, db *sql.DB, playerID, proposalID, id
|
||||
_, err = tx.ExecContext(ctx, ProposalAcceptSQL, proposalID)
|
||||
} else {
|
||||
_, err = tx.ExecContext(ctx, ProposalDeclineSQL, proposalID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
_, err = tx.ExecContext(ctx, ProposalDeclineRequeueSQL, proposalID, now.Add(domain.QueueExpiryWindow))
|
||||
}
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -16,6 +16,7 @@ func TestProposalRecoverySQLBindsParticipantAndExpiresAtReadBoundary(t *testing.
|
||||
ProposalParticipantLockSQL: {"proposal_id = $1", "player_id = $2", "FOR UPDATE"},
|
||||
ProposalParticipantRespondSQL: {"response = 'PENDING'", "responded_at"},
|
||||
ProposalRevisionBumpSQL: {"revision = revision + 1", "state = 'OPEN'"},
|
||||
ProposalDeclineRequeueSQL: {"SET state = 'QUEUED'", "state = 'PROPOSED'", "proposal_participants"},
|
||||
} {
|
||||
for _, fragment := range fragments {
|
||||
if !contains(query, fragment) {
|
||||
|
||||
Reference in New Issue
Block a user