From 04004a735f11c1441d77979a689a0e7182ee6675 Mon Sep 17 00:00:00 2001 From: 0xward <0xward.dev@gmail.com> Date: Mon, 3 Aug 2026 07:30:46 +0000 Subject: [PATCH] The season score punished the players who had been here longest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OWNER, Season 2, from the live board: *"pake wallet lainnya kills terhitung tp xp tidak."* His row read 30 kills and 0 XP. Season XP was a difference against a BASELINE — career XP now, minus career XP when the season started. Career XP is stored as a high-water mark, because a New Game legitimately restarts at 0 and must not erase it. Put the two together and a returning player is frozen: stored career xp (Season 1) .... 8581 baseline for Season 2 .......... 8581 this run's xp .................. 585 max(8581, 585) - 8581 .......... 0 They earn nothing for the season until ONE RUN beats their all-time best. A brand-new wallet is baselined at 0 and works perfectly — which is why some accounts looked right, others sat at zero, and the whole thing survived a month. It is also why kills looked fine on the same row: recordRunKills has always accumulated a delta. THE FIX IS THE SHAPE THAT ALREADY WORKED. seasonXpGain remembers what was last reported and adds the increase: never reported before -> 0 anchor, do not pay higher than last time -> the increase LOWER than last time -> the whole figure (a run restarted) That third line is what the baseline could never express. A restart used to read as "no progress, forever"; it now counts in full. MIGRATION NEEDS NO BACKFILL. Every existing doc has no lastRecordedXp, so it lands on the anchor case exactly once and starts earning from the next report. Season totals already banked are added to, not replaced — nobody loses what they have, and nobody is handed their career total as a season score. seasonBaseXp and seasonId are no longer written to the leaderboard doc, and seasonBaseline is deleted rather than left exported. A baseline nothing computes against is a value whose only remaining power is to mislead whoever reads it next. Both writers — updateLeaderboardEntry and recordRunProgress — now accumulate through the same helper and the same lastRecordedXp, so re-reporting an unchanged figure is worth zero and they cannot double-count each other. updateLeaderboardEntry gained a second read; both transactions were re-checked against the read-before-write rule that already cost this file every kill for a month, and the test asserts it for every transaction in the module. Verified: 19/19 test:seasonxp (rewritten, and it fails on the old baseline), 8/8 test:kills, 128/128 test-season-close, tsc clean, build unchanged at 143/242 kB, lint 35. Season 2 is running, so the board changes on deploy: veterans stop reading 0 and start banking what they earn from their next sync. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017A764RdnwpyWnG7uCNhMiQ --- lib/leaderboardService.ts | 46 +++++++---- lib/seasonXp.ts | 99 ++++++++++++------------ scripts/test-season-xp.js | 155 +++++++++++++++++++++----------------- 3 files changed, 168 insertions(+), 132 deletions(-) diff --git a/lib/leaderboardService.ts b/lib/leaderboardService.ts index fabc0eb..4fbc19e 100644 --- a/lib/leaderboardService.ts +++ b/lib/leaderboardService.ts @@ -14,7 +14,7 @@ import { LeaderboardEntry } from './contract' import { currentSeasonId } from './season' // The arithmetic that decides who gets paid lives in its own Firebase-free // module so it can be tested directly — see lib/seasonXp.ts. -import { seasonBaseline } from './seasonXp' +import { seasonXpGain } from './seasonXp' // ─── THE SEASON BOARD, AND WHY IT HAD TO EXIST ─────────────────────────────── // @@ -73,10 +73,16 @@ interface LeaderboardDoc { // reset between deaths within the same continuous play session (a // Revive keeps counting from where it left off) — see recordRunKills. lastRecordedKills?: number - /** Which season `seasonBaseXp` / `seasonBaseKills` were taken for. */ - seasonId?: string - /** Career xp at the moment this season started, for this wallet. */ - seasonBaseXp?: number + /** + * The engine's raw xp as last reported — NOT the career high-water mark + * above. Season XP is the increase between reports, so this is the only + * field that makes a returning player's season score move. See seasonXp.ts. + * + * Replaces `seasonId` + `seasonBaseXp`, which are no longer written: a + * baseline that nothing computes against is a value that can only mislead + * the next reader. Old docs may still carry them; nothing reads them. + */ + lastRecordedXp?: number updatedAt: number } @@ -108,20 +114,24 @@ export async function updateLeaderboardEntry( // reached", so a New Game (which legitimately restarts at 0 XP) can never // erase it — and can never drive season xp negative either. const careerXp = Math.max(data.xp ?? 0, xp) - const base = seasonBaseline(data, careerXp, seasonId) + // The season total ACCUMULATES — see seasonXpGain. Both writers use the + // same `lastRecordedXp`, so whichever runs first books the gain and the + // other adds nothing: re-reporting an unchanged figure is worth zero. + const gain = seasonXpGain(data, xp) + const seasonSnap = await tx.get(seasonPlayerRef(seasonId, normalizedAddr)) + const seasonPrev = (seasonSnap.exists() ? seasonSnap.data() : {}) as { xp?: number } tx.set(ref, { walletAddress: normalizedAddr, username, xp: careerXp, level: Math.max(data.level ?? 1, level), - seasonId, - seasonBaseXp: base, + lastRecordedXp: xp, updatedAt: Date.now(), }, { merge: true }) tx.set(seasonPlayerRef(seasonId, normalizedAddr), { walletAddress: normalizedAddr, username, - xp: Math.max(0, careerXp - base), + xp: (seasonPrev.xp ?? 0) + gain, level: Math.max(data.level ?? 1, level), updatedAt: Date.now(), }, { merge: true }) @@ -238,20 +248,28 @@ export async function recordRunProgress( const normalizedAddr = walletAddress.toLowerCase() const ref = doc(db, 'leaderboard', normalizedAddr) await runTransaction(db, async (tx) => { + // BOTH READS FIRST — Firestore rejects a read after a write, and that + // rule already cost this file every kill for a month. See + // scripts/test-leaderboard-writes.js. + const seasonId = seasonKey() const snap = await tx.get(ref) + const seasonSnap = await tx.get(seasonPlayerRef(seasonId, normalizedAddr)) const data = (snap.exists() ? snap.data() : {}) as Partial + const seasonPrev = (seasonSnap.exists() ? seasonSnap.data() : {}) as { xp?: number } const nextXp = Math.max(data.xp ?? 0, xp) const nextLevel = Math.max(data.level ?? 1, level) - const seasonId = seasonKey() - const base = seasonBaseline(data, nextXp, seasonId) + // Career xp stays a high-water mark, because that is what "career" means + // and a New Game must not erase it. The SEASON no longer derives from it: + // for a returning player the high-water mark never moves, which is + // exactly how a wallet with 30 kills ended up showing 0 season XP. + const gain = seasonXpGain(data, xp) tx.set( ref, { walletAddress: normalizedAddr, xp: nextXp, level: nextLevel, - seasonId, - seasonBaseXp: base, + lastRecordedXp: xp, updatedAt: Date.now(), }, { merge: true } @@ -263,7 +281,7 @@ export async function recordRunProgress( seasonPlayerRef(seasonId, normalizedAddr), { walletAddress: normalizedAddr, - xp: Math.max(0, nextXp - base), + xp: (seasonPrev.xp ?? 0) + gain, level: nextLevel, updatedAt: Date.now(), }, diff --git a/lib/seasonXp.ts b/lib/seasonXp.ts index e2b4322..8d4f286 100644 --- a/lib/seasonXp.ts +++ b/lib/seasonXp.ts @@ -14,60 +14,65 @@ // opened August already 5,926 XP ahead of second place — earned in a month that // was over — and a player joining in September could never have won at all. // -// A season score is therefore a DELTA: career XP now, minus career XP when the -// season started for that wallet. +// A season score is therefore a DELTA. The first version took that delta +// against a BASELINE — career XP now, minus career XP when the season started — +// and it was wrong for exactly the players who had been here longest. +// +// ── HOW THE BASELINE FAILED, AND WHY IT LOOKED FINE ───────────────────────── +// +// OWNER, Season 2, from the live board: *"pake wallet lainnya kills terhitung +// tp xp tidak."* His row read 30 kills and 0 XP. +// +// Career XP is stored as a high-water mark — Math.max(stored, incoming) — so a +// New Game, which legitimately restarts at 0, cannot erase it. Combine the two +// and a returning player is frozen: +// +// stored career xp (from Season 1) .... 8581 +// baseline for Season 2 ............... 8581 +// this run's xp ....................... 585 +// max(8581, 585) - 8581 ............... 0 +// +// They earn nothing for the season until ONE RUN beats their all-time best. A +// brand-new wallet is baselined at 0 and works perfectly, which is why some +// accounts looked right and others sat at zero. +// +// ── WHAT REPLACED IT ──────────────────────────────────────────────────────── +// +// The same shape recordRunKills has used correctly all along: remember what was +// last reported, add the increase. It accumulates what the player actually +// earned, it survives a New Game (a lower figure is a fresh run and counts in +// full), and it never consults a high-water mark. /** The fields the calculation reads off a stored leaderboard doc. */ -export interface SeasonBaselineDoc { - /** Career XP as of the last write. */ - xp?: number - /** Which season `seasonBaseXp` was taken for. */ - seasonId?: string - /** Career XP at the moment that season started, for this wallet. */ - seasonBaseXp?: number +export interface SeasonXpDoc { + /** + * The engine's raw xp figure as last reported, NOT the career high-water + * mark. The two differ the moment a player starts a New Game, and telling + * them apart is the whole fix. + */ + lastRecordedXp?: number } /** - * Where this wallet's season starts. - * - * Three cases, and the middle one is the whole point: + * How much XP to ADD to this season's total for a report of `xp`. * - * 1. Baseline already taken for THIS season -> keep it. Re-deriving it on - * every write would reset the player's season score to zero each time. - * 2. Baseline is from a PREVIOUS season (or absent) -> take `doc.xp`, the - * career total as of the last write, which by definition was last season. - * NOT the incoming figure: that would silently discard whatever the player - * earned before their first sync of the new month. - * 3. No document at all -> the incoming career total. A wallet with no row is - * a new player, whose career total is near zero anyway. + * Three cases, and the first two are the ones the baseline got wrong: * - * At rollout every existing doc lands in case 2 with no `seasonId`, so each - * player is baselined at their current career total and the season starts level - * for everybody. That is the desired migration, and it needs no backfill. - */ -export function seasonBaseline( - doc: SeasonBaselineDoc | null | undefined, - careerXp: number, - seasonId: string, -): number { - const d = doc || {} - if (d.seasonId === seasonId && typeof d.seasonBaseXp === 'number') return d.seasonBaseXp - return typeof d.xp === 'number' ? d.xp : careerXp -} - -/** - * XP earned this season. Never negative. + * 1. Never reported before -> 0. There is nothing to compare against, so the + * player is anchored here and starts earning from their next report. This + * is also the migration: every existing doc lands here once, and the + * season carries on from whatever it had rather than jumping. + * 2. Higher than last time -> the increase. What they earned since. + * 3. LOWER than last time -> the whole figure. p.xp only goes down when a run + * restarts, so the new run's progress counts in full. The old code read + * this as "no progress" forever. * - * The clamp is not defensive noise: "New Game" legitimately restarts a run at 0 - * XP, and while the stored career total is kept as a high-water mark, a doc - * written before that rule existed could still hold a baseline above the - * current total. A negative score would sort a player to the very bottom of a - * board they might be winning. + * Never negative, and never inflated by re-reporting the same number: two + * writers calling this with an unchanged `xp` add nothing the second time. */ -export function seasonXpFrom( - doc: SeasonBaselineDoc | null | undefined, - careerXp: number, - seasonId: string, -): number { - return Math.max(0, careerXp - seasonBaseline(doc, careerXp, seasonId)) +export function seasonXpGain(doc: SeasonXpDoc | null | undefined, xp: number): number { + if (!Number.isFinite(xp) || xp < 0) return 0 + const last = doc?.lastRecordedXp + if (typeof last !== 'number') return 0 + return xp >= last ? xp - last : xp } diff --git a/scripts/test-season-xp.js b/scripts/test-season-xp.js index ff22c86..b99b19d 100644 --- a/scripts/test-season-xp.js +++ b/scripts/test-season-xp.js @@ -1,99 +1,112 @@ #!/usr/bin/env node /** - * test-season-xp.js — the arithmetic that decides who gets paid. + * test-season-xp.js — the number that decides who gets paid. * - * OWNER: *"pertimbangan rank skrg sudah bagus belum? mengingat hanya dari xp?"* + * A season score has to be what the player earned THIS season. The first + * version took it as a difference against a baseline — career XP now, minus + * career XP when the season started — and it was wrong for precisely the + * players who had been here longest. * - * It was not. XP is cumulative and never resets, so the season prize was being - * paid off an all-time board: July's winner opened August already 5,926 XP - * ahead of second place on work that was already finished, and a player joining - * in September could never have won at all — they start at zero against months - * of accumulation. + * OWNER, Season 2, from the live board: *"pake wallet lainnya kills terhitung + * tp xp tidak."* His row read 30 kills and 0 XP. * - * A season score is a DELTA, and this covers the three ways that delta can be - * got wrong, each of which silently pays the wrong person: + * Career XP is stored as a high-water mark, so a New Game (which legitimately + * restarts at 0) cannot erase it. Combine the two: * - * - re-deriving the baseline on every write (score resets to 0 constantly), - * - baselining on the INCOMING figure at rollover (the first session of the - * month vanishes), - * - letting it go negative (a player sorts to the bottom of a board they may - * be winning). + * stored career xp .... 8581 baseline .... 8581 + * this run's xp ....... 585 score ....... max(8581,585) - 8581 = 0 + * + * The player earns nothing until ONE RUN beats their all-time best. A new + * wallet is baselined at 0 and works perfectly, which is why some accounts + * looked right and others sat at zero — and why it survived a whole month. + * + * seasonXpGain is the same shape recordRunKills has used correctly all along: + * remember what was last reported, add the increase. * * node scripts/test-season-xp.js */ const { execFileSync } = require('child_process') -const fs = require('fs'), path = require('path'), os = require('os') +const path = require('path'), os = require('os') const out = path.join(os.tmpdir(), 'ns-sxp-' + process.pid + '.cjs') execFileSync('npx', ['esbuild', 'lib/seasonXp.ts', '--bundle', '--platform=node', '--format=cjs', '--log-level=error', '--alias:@=' + path.resolve('.'), '--outfile=' + out], { stdio: ['ignore', 'ignore', 'inherit'] }) -const S = require(out) +const { seasonXpGain } = require(out) let fails = 0 const ok = (l, c, d) => { console.log((c ? ' ✓ ' : ' ✗ FAIL: ') + l + (d !== undefined ? ' (' + d + ')' : '')); if (!c) fails++ } +const eq = (l, got, want) => ok(l, got === want, `got ${got}, want ${want}`) -const AUG = '202608', JUL = '202607' - -// ── a brand-new player ──────────────────────────────────────────────────── -ok('a wallet with no row starts the season at zero', - S.seasonXpFrom(null, 0, AUG) === 0) -ok('and one with no row but existing career XP is baselined where it stands — ' - + 'joining mid-season must not hand you the whole career as a season score', - S.seasonXpFrom(undefined, 5000, AUG) === 0) - -// ── the rollover, which is the entire point ─────────────────────────────── -// Rondo's real July figure. On 1 August he must be on zero like everyone else. -const julyVeteran = { xp: 8581, seasonId: JUL, seasonBaseXp: 0 } -ok('July\'s winner starts August on ZERO, not on 8,581', - S.seasonXpFrom(julyVeteran, 8581, AUG) === 0, String(S.seasonXpFrom(julyVeteran, 8581, AUG))) -ok('and after one August run, only that run counts', - S.seasonXpFrom(julyVeteran, 8581 + 2000, AUG) === 2000) +// ── THE BUG, AS THE OWNER SAW IT ──────────────────────────────────────────── +// A veteran with 8581 career XP plays a run and reaches 585. Under the old +// baseline this was 0. It must be 585 — everything that run earned. +eq('a returning player earns the whole run, not zero', + seasonXpGain({ xp: 8581, lastRecordedXp: 0 }, 585), 585) -// The migration case: every doc that exists today has NO seasonId at all. -const legacy = { xp: 2655 } -ok('a pre-existing doc with no seasonId is baselined at its career total', - S.seasonBaseline(legacy, 2655, AUG) === 2655) -ok('so nobody carries a head start into the first season under the new rule', - S.seasonXpFrom(legacy, 2655, AUG) === 0) +// ── first sight anchors, it does not award ────────────────────────────────── +// Every existing doc lands here exactly once at rollout. Awarding the figure +// would hand a veteran their entire career total as a season score. +eq('a doc that has never reported is anchored, not paid', seasonXpGain({ xp: 8581 }, 8581), 0) +eq('and so is a brand-new wallet', seasonXpGain(null, 0), 0) +eq('even one that arrives mid-run', seasonXpGain(undefined, 1200), 0) -// ── the baseline must be STICKY within a season ─────────────────────────── -// Re-deriving it on each write is the obvious bug: every sync would reset the -// player's season score to zero and the board would never move. -const midSeason = { xp: 9000, seasonId: AUG, seasonBaseXp: 8581 } -ok('a baseline already taken this season is kept, not re-derived', - S.seasonBaseline(midSeason, 9000, AUG) === 8581) -ok('so the score keeps climbing across writes', S.seasonXpFrom(midSeason, 9000, AUG) === 419) -ok('and a later write with more XP climbs further', - S.seasonXpFrom(midSeason, 12000, AUG) === 3419) +// ── ordinary progress ─────────────────────────────────────────────────────── +eq('the increase between reports is the score', seasonXpGain({ lastRecordedXp: 100 }, 175), 75) +eq('an unchanged report is worth nothing', seasonXpGain({ lastRecordedXp: 175 }, 175), 0) +// Two writers (updateLeaderboardEntry and recordRunProgress) call this with the +// same figure. The second must not pay twice. +eq('so two writers with the same figure cannot double-count', + seasonXpGain({ lastRecordedXp: 175 }, 175), 0) -// ── the baseline is last season's END, not this write's value ───────────── -// If a player's first sync of the month happens AFTER they have already played, -// baselining on the incoming figure would throw that session away. -const playedBeforeSync = { xp: 8581, seasonId: JUL, seasonBaseXp: 0 } -ok('a first sync that arrives after some play still counts that play', - S.seasonXpFrom(playedBeforeSync, 8581 + 1500, AUG) === 1500, - String(S.seasonXpFrom(playedBeforeSync, 8581 + 1500, AUG))) +// ── New Game, which the baseline could never handle ───────────────────────── +// p.xp only goes DOWN when a run restarts, so the lower figure is a fresh run +// and every point of it is new. +eq('a restart counts in full rather than reading as no progress', + seasonXpGain({ lastRecordedXp: 8581 }, 40), 40) +eq('and keeps accumulating from there', seasonXpGain({ lastRecordedXp: 40 }, 260), 220) -// ── never negative ──────────────────────────────────────────────────────── -// "New Game" restarts a run at 0 XP. Career XP is kept as a high-water mark, -// but a doc written before that rule could hold a baseline above the total. -const stale = { xp: 9000, seasonId: AUG, seasonBaseXp: 9000 } -ok('a career total below the baseline scores 0, never a negative', - S.seasonXpFrom(stale, 100, AUG) === 0, String(S.seasonXpFrom(stale, 100, AUG))) -ok('which matters because a negative would sort a leader to the very bottom', - S.seasonXpFrom(stale, 0, AUG) >= 0) +// ── never negative, never nonsense ────────────────────────────────────────── +ok('a gain is never negative', [ + seasonXpGain({ lastRecordedXp: 500 }, 0), + seasonXpGain({ lastRecordedXp: 500 }, 499), + seasonXpGain({ lastRecordedXp: 0 }, 0), +].every((n) => n >= 0)) +eq('a garbage report is worth nothing', seasonXpGain({ lastRecordedXp: 10 }, NaN), 0) +eq('so is a negative one', seasonXpGain({ lastRecordedXp: 10 }, -5), 0) +eq('and Infinity is not a score', seasonXpGain({ lastRecordedXp: 10 }, Infinity), 0) -// ── a season id is a string, and must be compared as one ────────────────── -ok('a numeric-looking seasonId from an old doc does not falsely match', - S.seasonBaseline({ xp: 500, seasonId: 202608, seasonBaseXp: 0 }, 700, AUG) === 500) +// ── a season is a sum, and it adds up ─────────────────────────────────────── +// Walk a veteran through a month: anchored, two runs, a New Game, two more. +{ + const reports = [8581, 8700, 8950, 120, 400, 900] + let doc = { xp: 8581 } + let season = 0 + for (const xp of reports) { + season += seasonXpGain(doc, xp) + doc = { xp: Math.max(doc.xp, xp), lastRecordedXp: xp } + } + // 0 + 119 + 250 + 120 + 280 + 500 + eq('a full month of play sums to what was actually earned', season, 1269) + ok('and the career high-water mark is untouched by any of it', doc.xp === 8950, String(doc.xp)) +} -// ── two seasons on, the baseline still rolls ────────────────────────────── -const augVeteran = { xp: 20000, seasonId: AUG, seasonBaseXp: 8581 } -ok('September rebaselines off August\'s final career total', - S.seasonBaseline(augVeteran, 20000, '202609') === 20000) -ok('so each month genuinely starts level', S.seasonXpFrom(augVeteran, 20000, '202609') === 0) +// ── the baseline is gone, not merely unused ───────────────────────────────── +const read = (p) => require('fs').readFileSync(path.join(__dirname, '..', p), 'utf8') +ok('seasonBaseline no longer exists to be called by mistake', + !/export function seasonBaseline/.test(read('lib/seasonXp.ts'))) +const svc = read('lib/leaderboardService.ts') +ok('and nothing writes seasonBaseXp any more', !/seasonBaseXp: /.test(svc)) +ok('both writers accumulate instead', (svc.match(/seasonXpGain\(data, xp\)/g) || []).length === 2) +// The rule that already cost this file every kill for a month. +ok('and neither of them reads after it writes', (() => { + const bodies = svc.split('runTransaction(db, async (tx) => {').slice(1) + return bodies.every((b) => { + const body = b.slice(0, b.indexOf('\n })')) + const firstSet = body.indexOf('tx.set'), lastGet = body.lastIndexOf('tx.get') + return firstSet < 0 || lastGet < firstSet + }) +})()) console.log(fails ? ` ${fails} GAGAL` : ' semua lolos') -try { fs.unlinkSync(out) } catch {} process.exit(fails ? 1 : 0)