From c01244c850bca6019bbf46ac047ecc02732dcc6a Mon Sep 17 00:00:00 2001 From: "claude (blind_chess)" Date: Mon, 18 May 2026 21:45:42 -0400 Subject: [PATCH] fix: promotion dialog only fires for genuine pawn promotions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Promote pawn" dialog popped for any pawn "moved" toward the last rank: the commit paths checked piece type + destination rank but never the pawn's SOURCE rank. With the phantom layer now filling ranks 7-8 with tappable phantom pieces, tapping one (which falls through to the real-move handler) while a real pawn was armed triggered the dialog for a move no pawn could make — and for any phantom type, not just pawns. Root cause: incomplete promotion detection, duplicated in Game.svelte `onCommit` and the server's `isPromotionRequired`. Replaced with one shared `isPromotionMove(piece, from, to)` — pawn, from the rank adjacent to promotion, to the promotion rank, at most one file over — used by both. 7 unit tests in packages/shared. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/client/src/lib/Game.svelte | 16 +++++----- packages/server/src/commit.ts | 8 ++--- packages/shared/src/index.ts | 1 + packages/shared/src/promotion.ts | 23 ++++++++++++++ packages/shared/test/promotion.test.ts | 42 ++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 packages/shared/src/promotion.ts create mode 100644 packages/shared/test/promotion.test.ts diff --git a/packages/client/src/lib/Game.svelte b/packages/client/src/lib/Game.svelte index 0a6bd4e..6b58048 100644 --- a/packages/client/src/lib/Game.svelte +++ b/packages/client/src/lib/Game.svelte @@ -9,7 +9,7 @@ import { pieceGlyph } from './pieces.js'; import { phantoms } from './stores/phantoms.svelte.js'; import { phantomDrag } from './stores/phantom-drag.svelte.js'; - import type { PromotionType, Square } from '@blind-chess/shared'; + import { isPromotionMove, type PromotionType, type Square } from '@blind-chess/shared'; interface Props { gameId: string; } let { gameId }: Props = $props(); @@ -40,13 +40,13 @@ function onCommit(from: Square, to: Square) { const piece = game.state.view?.pieces[from]; if (!piece) return; - // Promotion check (white pawn to rank 8, black pawn to rank 1). - if (piece.type === 'p') { - const rank = to[1]; - if ((piece.color === 'w' && rank === '8') || (piece.color === 'b' && rank === '1')) { - pendingPromotion = { from, to }; - return; - } + // A pawn promotes only from the rank adjacent to its promotion rank — + // isPromotionMove checks the source rank, destination rank, and file delta, + // so a pawn elsewhere "moved" toward the last rank no longer pops this + // dialog (which a click on a phantom-occupied back-rank square could do). + if (isPromotionMove(piece, from, to)) { + pendingPromotion = { from, to }; + return; } game.commit(from, to); } diff --git a/packages/server/src/commit.ts b/packages/server/src/commit.ts index 3636163..70f820e 100644 --- a/packages/server/src/commit.ts +++ b/packages/server/src/commit.ts @@ -1,6 +1,7 @@ import type { Move } from 'chess.js'; import { geometricMoves, + isPromotionMove, type Announcement, type Color, type Piece, @@ -124,9 +125,6 @@ function chessJsLegalFrom(game: Game, from: Square): string[] { function isPromotionRequired(game: Game, from: Square, to: Square): boolean { const piece = game.chess.get(from); - if (!piece || piece.type !== 'p') return false; - const toRank = to[1]; - if (piece.color === 'w' && toRank === '8') return true; - if (piece.color === 'b' && toRank === '1') return true; - return false; + if (!piece) return false; + return isPromotionMove({ color: piece.color, type: piece.type }, from, to); } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 85fbe3f..07fdaf7 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -3,3 +3,4 @@ export * from './moderator.js'; export * from './protocol.js'; export * from './geometric.js'; export * from './phantoms.js'; +export * from './promotion.js'; diff --git a/packages/shared/src/promotion.ts b/packages/shared/src/promotion.ts new file mode 100644 index 0000000..2a65538 --- /dev/null +++ b/packages/shared/src/promotion.ts @@ -0,0 +1,23 @@ +import { fileIndex, type Piece, type Square } from './types.js'; + +/** + * True iff moving `piece` from→to is a pawn promotion. + * + * A promotion is a pawn advancing from the rank adjacent to its promotion rank + * onto the promotion rank (7→8 for White, 2→1 for Black), at most one file + * over — a straight push or a diagonal capture. + * + * Checking only the piece type and the destination rank — as the move-commit + * paths previously did — wrongly flags e.g. a pawn on the 2nd rank "moved" to + * the 8th, popping the promotion dialog for a move no pawn could ever make. + */ +export function isPromotionMove( + piece: Piece | undefined, + from: Square, + to: Square, +): boolean { + if (!piece || piece.type !== 'p') return false; + if (Math.abs(fileIndex(from) - fileIndex(to)) > 1) return false; + if (piece.color === 'w') return from[1] === '7' && to[1] === '8'; + return from[1] === '2' && to[1] === '1'; +} diff --git a/packages/shared/test/promotion.test.ts b/packages/shared/test/promotion.test.ts new file mode 100644 index 0000000..ce842b0 --- /dev/null +++ b/packages/shared/test/promotion.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from 'vitest'; +import { isPromotionMove } from '../src/promotion.js'; +import type { Piece } from '../src/types.js'; + +const wp: Piece = { color: 'w', type: 'p' }; +const bp: Piece = { color: 'b', type: 'p' }; +const wn: Piece = { color: 'w', type: 'n' }; + +describe('isPromotionMove', () => { + it('white pawn from the 7th rank to the 8th is a promotion', () => { + expect(isPromotionMove(wp, 'e7', 'e8')).toBe(true); + expect(isPromotionMove(wp, 'd7', 'e8')).toBe(true); // capture-promotion + }); + + it('black pawn from the 2nd rank to the 1st is a promotion', () => { + expect(isPromotionMove(bp, 'e2', 'e1')).toBe(true); + expect(isPromotionMove(bp, 'd2', 'c1')).toBe(true); + }); + + it('a pawn NOT on the rank adjacent to promotion is not a promotion', () => { + expect(isPromotionMove(wp, 'e2', 'e8')).toBe(false); // the reported bug + expect(isPromotionMove(wp, 'e5', 'e8')).toBe(false); + expect(isPromotionMove(bp, 'e7', 'e1')).toBe(false); + }); + + it('an ordinary pawn move is not a promotion', () => { + expect(isPromotionMove(wp, 'e2', 'e4')).toBe(false); + expect(isPromotionMove(bp, 'e7', 'e5')).toBe(false); + }); + + it('a non-pawn reaching the last rank is not a promotion', () => { + expect(isPromotionMove(wn, 'g6', 'e8')).toBe(false); + }); + + it('a pawn move spanning more than one file is not a promotion', () => { + expect(isPromotionMove(wp, 'a7', 'h8')).toBe(false); + }); + + it('an undefined piece is not a promotion', () => { + expect(isPromotionMove(undefined, 'e7', 'e8')).toBe(false); + }); +});