Video Poker: saturate the payout so a big win cannot overflow the bank - #240
Merged
Conversation
closes #239) score += bet * paytable[] was computed entirely in int, so both the payout and the resulting bank could overflow. At the reported bet that corrupted the score, and the score <= 0 check right below then dropped the player on the "You have run out of money!" screen - a jackpot reported as a bankruptcy. Depending on the bet it could instead wrap positive and silently pay out the wrong amount. The new bank total is now worked out in 64 bits and saturated. This is the only place the score grows, so the bank - and highscore, which just copies it - can no longer overflow. The ceiling is MAX_BANK rather than INT_MAX on purpose: parking the bank on INT_MAX would have made a pre-existing overflow reachable, since half-bank then double leaves the bet at INT_MAX-1 and the +10 step then overflows. Capping at roughly half of INT_MAX keeps the whole bet domain away from the type limit instead of needing a per-site guard, and since clamp_bet already binds bet to score, bounding score covers bet too. Also hoist the doubled recognize() call into hand_rank; it is idempotent, so this is behaviour-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repository owner
locked as resolved and limited conversation to collaborators
Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #239
score += bet * paytable[]was computed entirely inint, so both the payout and the resulting bank total could overflow. At the bet reported in #239 that corrupted the score, and thescore <= 0check immediately below then dropped the player on the "You have run out of money!" screen - a jackpot reported as a bankruptcy. Depending on the bet it could instead wrap positive and silently pay out the wrong amount, with no game-over at all.The new bank total is now worked out in 64 bits and saturated:
This is the only place the score grows (the other three writes are the two
= 1000resets and the pledge, which subtracts), so the bank - andhighscore, which just copies it - can no longer overflow.Why MAX_BANK and not INT_MAX
An earlier draft saturated at
INT_MAX. Review caught that this makes a pre-existing overflow newly reachable: with the bank pinned at exactlyINT_MAX, hold Up (bet = score/2) then tap Up (the doubling guard passes with equality, bet =INT_MAX-1) then press Right, and thebet += 10step overflows. Before the fix the bank could never sit atINT_MAX- it wrapped negative instead - so that window was unreachable.Capping at roughly half of
INT_MAXkeeps the whole bet domain away from the type limit rather than requiring a per-site overflow guard, and it composes with the existing invariant:clamp_bet()already bindsbettoscore, so boundingscoreboundsbetfor free. It also avoids editing the betting logic that just shipped in 1.4 (#163). Every arithmetic site was enumerated against the new ceiling; the tightest (bet += 10) peaks at 1,000,000,009, leaving overINT_MAX/2of slack.The cap is disclosed in the changelog rather than being silent policy, and it only ever bites on the way up - a capped win can never look like a loss.
Also in this change
recognize()call intohand_rank. It is idempotent (it only rebuilds its ownshandscratch buffer fromhand), so this is behaviour-preserving, and it saves a full selection sort plus up to seven predicate checks per paying hand.fap_version1.4 -> 1.5 with a CHANGELOG entry covering both failure symptoms and the new ceiling.Verification
Builds clean under ufbt (API 88.0),
ufbt formatapplied. Reviewed by the code-review, silent-failure and comment agents plus a four-angle simplify pass; the silent-failure pass fuzzed 20,000 modelled sessions across banks from 1 toINT_MAXand saw zero wins ending as game-over, zero negative banks and zero invalid bets.Two pre-existing issues were found during review and deliberately left out of scope: the draw callback still calls
recognize()twice per frame, andplaycard()has an out-of-boundshold[5]stack access.🤖 Generated with Claude Code