From 56d4a6a4c1c38ef5a609d5d11497ce4124b499eb Mon Sep 17 00:00:00 2001 From: Jeremy Smart Date: Wed, 24 Jun 2026 14:14:45 -0400 Subject: [PATCH] Fix off-by-one errors in board and promotion click mapping squareFromMouse assumed the board's top cell was at terminal row 0, but puzzle and replay screens render a title + blank line first, placing the board at row 2. Clicks below the top row of each square spilled into the square below. Add SetBoardOriginY so screens declare the board's vertical origin, and translate the mouse y by it. handlePromoClick had the same class of bug: it hit-tested the promotion piece cells at promoPopupY+2, but the cells render at promoPopupY+1 (the row right after the "Promote pawn:" label). This rejected the top piece row and accepted the key-label row below the pieces. Fix to +1. Add regression tests for both. Co-Authored-By: Claude Opus 4.8 --- internal/client/screens/localmoveinput.go | 13 +++- .../client/screens/localmoveinput_test.go | 73 +++++++++++++++++++ internal/client/screens/puzzle.go | 2 + internal/client/screens/replay.go | 2 + 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/internal/client/screens/localmoveinput.go b/internal/client/screens/localmoveinput.go index b237e73..5c98abf 100644 --- a/internal/client/screens/localmoveinput.go +++ b/internal/client/screens/localmoveinput.go @@ -19,6 +19,7 @@ type LocalMoveInput struct { pendingPromoFrom chess.Square pendingPromoTo chess.Square promoPopupY int + boardOriginY int flipped bool } @@ -43,6 +44,13 @@ func (li *LocalMoveInput) SetPromoPopupY(y int) { li.promoPopupY = y } +// SetBoardOriginY records the terminal row at which the board's top cell is +// drawn, so mouse clicks can be translated into board coordinates. Screens that +// render content (e.g. a title) above the board must set this to that offset. +func (li *LocalMoveInput) SetBoardOriginY(y int) { + li.boardOriginY = y +} + // PendingPromo reports whether a promotion selection is awaiting user input. func (li *LocalMoveInput) PendingPromo() bool { return li.pendingPromo @@ -153,6 +161,7 @@ func (li *LocalMoveInput) handleSquareClick(sq chess.Square, board *render.Board func (li *LocalMoveInput) squareFromMouse(x, y int, board *render.Board) (chess.Square, bool) { cellCols := board.CellCols() cellRows := board.CellRows() + y -= li.boardOriginY if x < 2 || x > 2+8*cellCols-1 || y < 0 || y > 8*cellRows-1 { return 0, false } @@ -224,7 +233,9 @@ func (li *LocalMoveInput) promoSAN(key string, game *chess.Game) string { func (li *LocalMoveInput) handlePromoClick(x, y int, board *render.Board, game *chess.Game) string { cols := board.CellCols() rows := board.CellRows() - pieceY := li.promoPopupY + 2 + // promoPopupY is the row of the "Promote pawn:" label; the piece cells are + // drawn on the rows immediately below it. + pieceY := li.promoPopupY + 1 if y < pieceY || y >= pieceY+rows { return "" } diff --git a/internal/client/screens/localmoveinput_test.go b/internal/client/screens/localmoveinput_test.go index 4f8cdbd..9f581fb 100644 --- a/internal/client/screens/localmoveinput_test.go +++ b/internal/client/screens/localmoveinput_test.go @@ -123,6 +123,30 @@ func TestLocalMoveInput_HandleMsg_MouseClickSelectsPiece(t *testing.T) { } } +func TestLocalMoveInput_HandleMsg_BoardOriginY_OffsetsClick(t *testing.T) { + // Screens like puzzle/replay draw a title before the board, so the board's + // top cell is at terminal row 2. With cellRows=3, e2 (cellRow 6) is drawn at + // rows 18,19,20 in board-local space, i.e. terminal rows 20,21,22. Clicking + // the bottom of that cell (row 22) must still resolve to E2, not the square + // below it. This is the off-by-one regression. + game := chess.NewGame() + board := render.NewBoard(game.Position(), false) + li := NewLocalMoveInput(false) + li.SetBoardOriginY(2) + // Bottom row of the e2 cell: x=26, board-local y=20 → terminal y=22. + msg := tea.MouseMsg{X: 26, Y: 22, Action: tea.MouseActionPress, Button: tea.MouseButtonLeft} + san, handled, _ := li.HandleMsg(msg, board, game) + if !handled { + t.Fatalf("expected handled=true for click on board") + } + if san != "" { + t.Fatalf("expected empty san on first click, got %q", san) + } + if !li.hasSelected || li.selectedSq != chess.E2 { + t.Fatalf("expected selectedSq=E2 after clicking bottom of e2 cell, got selected=%v sq=%v", li.hasSelected, li.selectedSq) + } +} + func TestLocalMoveInput_HandleMsg_MouseClick_ConvertsToSAN(t *testing.T) { game := chess.NewGame() board := render.NewBoard(game.Position(), false) @@ -201,6 +225,55 @@ func TestLocalMoveInput_HandleMsg_PromoMode_QSelectsQueen(t *testing.T) { } } +func TestLocalMoveInput_HandlePromoClick_TopPieceRowSelectsQueen(t *testing.T) { + // The popup is laid out as: row promoPopupY = "Promote pawn:", then the + // piece cells on rows promoPopupY+1 .. promoPopupY+cellRows. Clicking the + // FIRST piece row (promoPopupY+1) on the queen (leftmost, x=2) must select + // the queen. This is the vertical off-by-one regression. + fen, _ := chess.FEN("3k4/4P3/8/8/8/8/8/4K3 w - - 0 1") + game := chess.NewGame(fen) + board := render.NewBoard(game.Position(), false) + li := NewLocalMoveInput(false) + li.pendingPromo = true + li.pendingPromoFrom = chess.E7 + li.pendingPromoTo = chess.E8 + li.SetPromoPopupY(26) + // Top piece row, queen column: x=2, y=27. + click := tea.MouseMsg{X: 2, Y: 27, Action: tea.MouseActionPress, Button: tea.MouseButtonLeft} + san, handled, _ := li.HandleMsg(click, board, game) + if !handled { + t.Fatalf("expected handled=true for click on promo popup") + } + if san != "e8=Q+" && san != "e8=Q" { + t.Fatalf("expected queen promo SAN from top piece row, got %q", san) + } + if li.pendingPromo { + t.Fatalf("expected pendingPromo cleared after selection") + } +} + +func TestLocalMoveInput_HandlePromoClick_LabelRowIsNotAPiece(t *testing.T) { + // The key-label row below the pieces (promoPopupY+cellRows+1) must NOT + // register as a piece selection. + fen, _ := chess.FEN("3k4/4P3/8/8/8/8/8/4K3 w - - 0 1") + game := chess.NewGame(fen) + board := render.NewBoard(game.Position(), false) + li := NewLocalMoveInput(false) + li.pendingPromo = true + li.pendingPromoFrom = chess.E7 + li.pendingPromoTo = chess.E8 + li.SetPromoPopupY(26) + // cellRows defaults to 3, so the label row is at 26+3+1 = 30. + click := tea.MouseMsg{X: 2, Y: 30, Action: tea.MouseActionPress, Button: tea.MouseButtonLeft} + san, _, _ := li.HandleMsg(click, board, game) + if san != "" { + t.Fatalf("expected no promo selection on label row, got %q", san) + } + if !li.pendingPromo { + t.Fatalf("expected pendingPromo to remain set after clicking label row") + } +} + func TestLocalMoveInput_HandleMsg_PromoMode_EscCancels(t *testing.T) { fen, _ := chess.FEN("3k4/4P3/8/8/8/8/8/4K3 w - - 0 1") game := chess.NewGame(fen) diff --git a/internal/client/screens/puzzle.go b/internal/client/screens/puzzle.go index 85a9b6a..1654436 100644 --- a/internal/client/screens/puzzle.go +++ b/internal/client/screens/puzzle.go @@ -516,6 +516,8 @@ func (m *PuzzleModel) View() string { var sb strings.Builder sb.WriteString(puzzleTitleStyle.Render("Puzzle Mode")) sb.WriteString("\n\n") + // Title on line 0, blank on line 1, so the board's top cell is on line 2. + m.input.SetBoardOriginY(2) boardView := m.board.View() right := m.rightPanel() sb.WriteString(lipgloss.JoinHorizontal(lipgloss.Top, boardView, " ", right)) diff --git a/internal/client/screens/replay.go b/internal/client/screens/replay.go index 35dc4f3..3100486 100644 --- a/internal/client/screens/replay.go +++ b/internal/client/screens/replay.go @@ -407,6 +407,8 @@ func (m *ReplayModel) View() string { var sb strings.Builder sb.WriteString(replayTitleStyle.Render("Replay")) sb.WriteString("\n\n") + // Title on line 0, blank on line 1, so the board's top cell is on line 2. + m.input.SetBoardOriginY(2) boardView := m.board.View() moveView := m.moveList.View() left := boardView