mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-16 22:42:01 +00:00
fix(server): load authoritative ratings into ranked candidates
The candidate projection selected only from queue_tickets and its scan never set Candidate.Rating, so every PostgreSQL-sourced ranked candidate arrived with Go's zero value. Rating tolerance, selection scoring and team partitioning all read that field, so ranked matchmaking treated a 900-rated player as identical to a 2100-rated one. Unit tests missed it because they construct candidates with ratings already populated. Join the ratings table, defaulting to domain.GlickoInitialRating for a player with no ratings row yet -- a genuinely new profile, matching the column default. Fix the same defect on the Redis path too, which is reached differently: the projection is seeded from the candidate CreateQueueTicket builds, not from the candidate query, and that candidate also left Rating unset. Resolve the rating inside the enqueue transaction so both projections agree on one authoritative value. The rating is never client-supplied. Add a store-backed test with deliberately distant ratings (900 vs 2100) plus an unrated player, asserting both projections and that the spread survives. Verified it fails without the fix.
This commit is contained in:
@@ -1848,3 +1848,71 @@ func TestPostgreSQLMigrationsRollBackAndReapplyCleanly(t *testing.T) {
|
|||||||
t.Fatal("reapply after rollback did not recreate the schema")
|
t.Fatal("reapply after rollback did not recreate the schema")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Ranked matchmaking reads Candidate.Rating for tolerance, selection scoring
|
||||||
|
// and team partitioning. The candidate projection did not join the ratings
|
||||||
|
// table and its scan never set the field, so every PostgreSQL-sourced ranked
|
||||||
|
// candidate arrived with Go's zero value and the matcher treated a 900-rated
|
||||||
|
// player as identical to a 2100-rated one. Unit tests missed this because they
|
||||||
|
// construct candidates with ratings already populated.
|
||||||
|
func TestPostgreSQLQueuedCandidatesCarryAuthoritativeRatings(t *testing.T) {
|
||||||
|
db := openIntegrationPostgres(t)
|
||||||
|
applyIntegrationMigrations(t, db)
|
||||||
|
|
||||||
|
now := time.Now().UTC().Truncate(time.Microsecond)
|
||||||
|
ctx := context.Background()
|
||||||
|
// "rated-low" and "rated-high" are deliberately far apart; "unrated" has no
|
||||||
|
// ratings row at all and must fall back to the new-profile default.
|
||||||
|
seeded := map[string]float64{"rated-low": 900, "rated-high": 2100}
|
||||||
|
for _, playerID := range []string{"rated-low", "rated-high", "unrated"} {
|
||||||
|
if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $2)`, playerID, "steam-"+playerID); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if rating, ok := seeded[playerID]; ok {
|
||||||
|
if _, err := db.ExecContext(ctx, `INSERT INTO ratings (player_id, rating) VALUES ($1, $2)`, playerID, rating); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
spec := domain.QueueSpec{Playlist: domain.Ranked, ClientBuild: "rating-build", ProtocolVersion: 1}
|
||||||
|
ticket, err := CreateQueueTicket(ctx, db, "ticket-"+playerID, playerID, "rating-create-"+playerID+"-01", spec, now)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create queue ticket for %s: %v", playerID, err)
|
||||||
|
}
|
||||||
|
// The Redis projection is seeded from this candidate rather than from
|
||||||
|
// the query below, so it must carry the same rating or the two
|
||||||
|
// projections disagree about who is comparable to whom.
|
||||||
|
want := domain.GlickoInitialRating
|
||||||
|
if rating, ok := seeded[playerID]; ok {
|
||||||
|
want = rating
|
||||||
|
}
|
||||||
|
if ticket.Candidate.Rating != want {
|
||||||
|
t.Fatalf("%s enqueue candidate rating = %v, want %v", playerID, ticket.Candidate.Rating, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
candidates, err := ListQueuedCandidates(ctx, db, domain.Ranked, now, 100)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("list queued candidates: %v", err)
|
||||||
|
}
|
||||||
|
if len(candidates) != 3 {
|
||||||
|
t.Fatalf("expected 3 candidates, got %d", len(candidates))
|
||||||
|
}
|
||||||
|
got := make(map[string]float64, len(candidates))
|
||||||
|
for _, candidate := range candidates {
|
||||||
|
got[candidate.PlayerID] = candidate.Rating
|
||||||
|
}
|
||||||
|
for playerID, want := range map[string]float64{
|
||||||
|
"rated-low": 900, "rated-high": 2100, "unrated": domain.GlickoInitialRating,
|
||||||
|
} {
|
||||||
|
if got[playerID] != want {
|
||||||
|
t.Fatalf("%s durable candidate rating = %v, want %v", playerID, got[playerID], want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The whole point of loading the rating is that the matcher can tell these
|
||||||
|
// players apart. Assert the spread survives into team partitioning rather
|
||||||
|
// than only that the field is non-zero.
|
||||||
|
if got["rated-high"]-got["rated-low"] != 1200 {
|
||||||
|
t.Fatalf("rating spread collapsed: %v", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -58,13 +58,25 @@ ORDER BY ends_at DESC
|
|||||||
LIMIT 1`
|
LIMIT 1`
|
||||||
)
|
)
|
||||||
|
|
||||||
const QueueCandidateProjectionSQL = `SELECT ticket_id, player_id, playlist, client_build,
|
// QueueCandidateProjectionSQL joins the authoritative rating. Without it every
|
||||||
protocol_version, enqueued_at, predicted_rtt
|
// PostgreSQL-sourced candidate carried Go's zero value, and since rating
|
||||||
FROM queue_tickets
|
// tolerance, selection scoring and team partitioning all read that field,
|
||||||
WHERE state = 'QUEUED' AND playlist = $1 AND expires_at > $2
|
// ranked matchmaking treated every player as identically rated. A player with
|
||||||
ORDER BY enqueued_at, ticket_id
|
// no ratings row yet is a genuinely new profile and starts at the Glicko
|
||||||
|
// initial rating, matching domain.GlickoInitialRating and the column default.
|
||||||
|
// The rating is never taken from the client.
|
||||||
|
const QueueCandidateProjectionSQL = `SELECT q.ticket_id, q.player_id, q.playlist, q.client_build,
|
||||||
|
q.protocol_version, q.enqueued_at, q.predicted_rtt, COALESCE(r.rating, $4)
|
||||||
|
FROM queue_tickets q
|
||||||
|
LEFT JOIN ratings r ON r.player_id = q.player_id
|
||||||
|
WHERE q.state = 'QUEUED' AND q.playlist = $1 AND q.expires_at > $2
|
||||||
|
ORDER BY q.enqueued_at, q.ticket_id
|
||||||
LIMIT $3`
|
LIMIT $3`
|
||||||
|
|
||||||
|
// QueueTicketRatingSQL resolves a player's authoritative rating, falling back
|
||||||
|
// to the new-profile default when they have no ratings row yet.
|
||||||
|
const QueueTicketRatingSQL = `SELECT COALESCE((SELECT rating FROM ratings WHERE player_id = $1), $2)`
|
||||||
|
|
||||||
const RankedParticipantSQL = `SELECT player_id, steam_id
|
const RankedParticipantSQL = `SELECT player_id, steam_id
|
||||||
FROM identities
|
FROM identities
|
||||||
WHERE player_id = ANY($1)
|
WHERE player_id = ANY($1)
|
||||||
@@ -106,7 +118,7 @@ func ListQueuedCandidates(ctx context.Context, db *sql.DB, playlist domain.Playl
|
|||||||
if db == nil || (playlist != domain.Casual && playlist != domain.Ranked) || now.IsZero() || limit < 1 || limit > 1000 {
|
if db == nil || (playlist != domain.Casual && playlist != domain.Ranked) || now.IsZero() || limit < 1 || limit > 1000 {
|
||||||
return nil, fmt.Errorf("invalid queued candidate arguments")
|
return nil, fmt.Errorf("invalid queued candidate arguments")
|
||||||
}
|
}
|
||||||
rows, err := db.QueryContext(ctx, QueueCandidateProjectionSQL, string(playlist), now, limit)
|
rows, err := db.QueryContext(ctx, QueueCandidateProjectionSQL, string(playlist), now, limit, domain.GlickoInitialRating)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -116,7 +128,7 @@ func ListQueuedCandidates(ctx context.Context, db *sql.DB, playlist domain.Playl
|
|||||||
var candidate domain.Candidate
|
var candidate domain.Candidate
|
||||||
var playlist string
|
var playlist string
|
||||||
var predictedRTT []byte
|
var predictedRTT []byte
|
||||||
if err := rows.Scan(&candidate.TicketID, &candidate.PlayerID, &playlist, &candidate.ClientBuild, &candidate.ProtocolVersion, &candidate.EnqueuedAt, &predictedRTT); err != nil {
|
if err := rows.Scan(&candidate.TicketID, &candidate.PlayerID, &playlist, &candidate.ClientBuild, &candidate.ProtocolVersion, &candidate.EnqueuedAt, &predictedRTT, &candidate.Rating); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := json.Unmarshal(predictedRTT, &candidate.PredictedRTT); err != nil {
|
if err := json.Unmarshal(predictedRTT, &candidate.PredictedRTT); err != nil {
|
||||||
@@ -138,7 +150,15 @@ func CreateQueueTicket(ctx context.Context, db *sql.DB, ticketID, playerID, idem
|
|||||||
digest := sha256.Sum256([]byte(fmt.Sprintf("%s|%s|%s|%s|%d", ticketID, playerID, spec.Playlist, spec.ClientBuild, spec.ProtocolVersion)))
|
digest := sha256.Sum256([]byte(fmt.Sprintf("%s|%s|%s|%s|%d", ticketID, playerID, spec.Playlist, spec.ClientBuild, spec.ProtocolVersion)))
|
||||||
var ticket domain.QueueTicket
|
var ticket domain.QueueTicket
|
||||||
err := RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error {
|
err := RunSerializable(ctx, db, DefaultSerializableAttempts, func(ctx context.Context, tx *sql.Tx) error {
|
||||||
candidate := domain.Candidate{TicketID: ticketID, PlayerID: playerID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now}
|
// The Redis projection is seeded from this candidate, so it must carry
|
||||||
|
// the same authoritative rating the durable candidate query joins.
|
||||||
|
// Reading it inside the transaction keeps both projections agreeing on
|
||||||
|
// one value rather than one of them defaulting to zero.
|
||||||
|
var rating float64
|
||||||
|
if err := tx.QueryRowContext(ctx, QueueTicketRatingSQL, playerID, domain.GlickoInitialRating).Scan(&rating); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
candidate := domain.Candidate{TicketID: ticketID, PlayerID: playerID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now, Rating: rating}
|
||||||
ticket = domain.QueueTicket{TicketID: ticketID, PlayerID: playerID, Candidate: candidate, Playlist: spec.Playlist, State: domain.Queued, EnqueuedAt: now, ExpiresAt: now.Add(domain.QueueExpiryWindow)}
|
ticket = domain.QueueTicket{TicketID: ticketID, PlayerID: playerID, Candidate: candidate, Playlist: spec.Playlist, State: domain.Queued, EnqueuedAt: now, ExpiresAt: now.Add(domain.QueueExpiryWindow)}
|
||||||
stored, err := json.Marshal(queueTicketRecordFromDomain(ticket))
|
stored, err := json.Marshal(queueTicketRecordFromDomain(ticket))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user