Repository navigation
refactor(game): share hand limit constant - #1457
Merged
Merged
Conversation
th060316-lgtm
marked this pull request as ready for review
October 24, 2026 12:47
Haviles04
added
the
version-patch
Haviles04
approved these changes
Oct 24, 2026
Haviles04
added
dev experience
itsalaidbacklife
added a commit
that referenced
this pull request
Oct 25, 2026
Follows #1457, which replaced the hard-coded 8 in the other move helpers but predates discardFour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
itsalaidbacklife
added a commit
that referenced
this pull request
Oct 28, 2026
…rd (#1443) * feat: rework fours to reveal three cards, caster chooses two to discard The second of the two Four variations trialled this Spades season. A four one-off now makes the opponent reveal three cards of their choice from hand, and the four's player chooses two of those for them to discard. Replaces the 3.0.0 random discard. Rules version bumped to 4.0.0. Rules: - 4+ cards in hand: the victim reveals three (RESOLVING_FOUR_REVEAL), then the caster chooses two of them (RESOLVING_FOUR_DISCARD). - Exactly 3 cards: the reveal is skipped; the whole hand is revealed. - 2 or fewer: discarded outright as the four resolves. A four stays playable against any hand, including an empty one. Implementation: - New GamePhases RESOLVING_FOUR_REVEAL (8) and RESOLVING_FOUR_DISCARD (9), and MoveTypes revealFour / discardFour with validate/execute helpers, policy arms, AI move-body generation, active-player cases, and log lines. - Revealed cards persist between the two moves in a new nullable jsonb `revealedCards` column on GameStateRow, threaded through pack/unpack, validate-gamestate, deal-cards, load-fixture-gamestate, and the socket game payload (sent to both players, outside opponent-hand hiding). - get-log keeps the "at random" wording for rules 3.x games only. - Legacy RESOLVING_FOUR / resolveFour replay paths are retained untouched. - New reveal and choose-discard dialogs, waiting overlays (the victim sees their revealed cards), store getters/actions, and socket handling. - Announcement copy, rules text, and move-effect text updated in all locales, under a new announcement id so it shows again. Deploy: production and staging use migrate: 'safe', so before deploying run ALTER TABLE gamestaterow ADD COLUMN IF NOT EXISTS "revealedCards" jsonb; Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(get-log): narrate a four against an empty hand A four resolved against an empty hand carries no discardedCards, so it was mistaken for a legacy (rules < 3.0.0) row and logged "must discard two cards". Identify legacy rows by their RESOLVING_FOUR phase instead, which every pre-3.0.0 four resolve entered and no later one does, and give the empty hand its own line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(fours): reveal short-hand discards to the caster, not the victim create-socket-events flips playedBy on a resolve to the player who played the one-off, but the RESOLVE case 4 handler assumed it was the resolver. So the victim saw their own discards flash in the caster's hand, and the caster never saw the reveal. Invert the check, correct the comment, and cover both sides in 4_fours.spec.js. Fixes #1440 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(fours): rename revealedCards to fourRevealedCards Scope the column, socket payload field, revealFour move body and store state to the four, leaving revealedCards free for #1388's running record of every card both players have seen. Deploy note: the column to add is now ALTER TABLE gamestaterow ADD COLUMN IF NOT EXISTS "fourRevealedCards" jsonb; Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: address first round of review feedback on #1443 - four.js: comment the reveal and reveal-whole-hand branches - AI move bodies: drop the hand-limit recursion-bound comment now that getCombinations serves several move types - Announcement: "Reveal-Three Fours" activator and "Reveal three, discard two" heading, in all five locales - four.spec.js: group the validation error tests in describe('Errors') - 4_fours.spec.js: assert the turn passes via #turn-indicator, per the basicMoves.spec.js convention Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: address review decisions on #1443 - GameStateRow: document that fourRevealedCards is backfilled by the deploy SQL's DEFAULT '[]' rather than left nullable - inGameEvents: restore main's RESOLVE case 4 comment, plus one line on which hands are discarded on resolve - four.spec.js: validate the resolve tests too, and assert valid moves with expect(...).not.to.throw() - Replace the staged RESOLVING_FOUR_DISCARD e2e test with a reconnect test from the four's player's side, and drop the load-fixture override it needed Deploy note: the column to add is now ALTER TABLE gamestaterow ADD COLUMN IF NOT EXISTS "fourRevealedCards" jsonb NOT NULL DEFAULT '[]'::jsonb; Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(discard-four): use shared HAND_LIMIT constant Follows #1457, which replaced the hard-coded 8 in the other move helpers but predates discardFour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Shares the 8-card hand limit between the client and server instead of defining the same value in multiple places.
The client previously exported
HAND_LIMIT = 8fromsrc/stores/game.js, while backend move helpers repeated the literal value8when checking whether a player must discard. Keeping those values separate makes it possible for the client and server rules to drift apart.This change adds a shared
HAND_LIMITvalue inutils/GameConstants.jsonand uses it from both sides without changing the current 8-card gameplay rule.Changes
HAND_LIMIT: 8toutils/GameConstants.jsonsrc/stores/game.jsto export the same shared value for the clientTesting
Resolves #1449