Cancel an in-progress input move if the piece being held changes

[thearst3rd]

Previously, if you click a piece to select it or start dragging it, and that piece gets changed when the position changes (i.e. the piece you're holding gets captured), the chessboard ends up in a weird state where an invalid piece is still being held. If it's being dragged, that will make the new piece turn invisible (since dragged pieces become invisible in their original spot until the drag finishes) and is otherwise kinda glitchy.

Since there's already a well defined callback for a move getting cancelled, this seems like a good place to use it. Now, when the position changes and there's a piece being held, it checks if the piece on that square is different than what it was and if so, it will cancel the move.

[shaack]

Thanks for the PR — this is a real rough edge and cancelling the move via the existing callback is the right instinct. A few things I'd like to see addressed before merging:

1. The reset path can corrupt the position. positionChanged() runs after setFen(fen) in Chessboard.setPosition(), so this.state.position already holds the new piece. When the move is cancelled, setMoveInputState(MOVE_INPUT_STATE.reset) hits this branch in VisualMoveInput.js:

if (this.fromSquare && !this.toSquare && this.movedPiece) {
    this.chessboard.state.position.setPiece(this.fromSquare, this.movedPiece)
}

That stamps the old held piece back onto fromSquare, overwriting the piece that setFen just placed there (e.g. the capturing knight). Since enqueuePositionChange then clones this.state.position as the animation target, the new position is lost. Could you test the "held piece gets captured via setPosition" case and verify the resulting FEN? The cancel likely needs to skip the piece-restore in this path.

2. Only setPosition is covered. movePiece() and setPiece() change the position too — if the held piece is captured through one of those, the input won't be cancelled. Worth hooking positionChanged() there as well (or invoking it from a single shared place) for consistent behaviour.

3. Minor: pieceName != this.movedPiece should use strict !== to match the rest of the codebase.

The MOVE_CANCELED_REASON.movedPieceChanged addition and the constructor init look good. Happy to merge once the position-corruption case is sorted out.

[thearst3rd]

This is great feedback thanks, not sure how I missed movePiece() and setPiece() (probably that I don't use them in my project 😛)

The reset path corruption is absolutely worth looking into - in my testing it seems to work out ok but I'll see if something I'm doing is masking the issue.

[shaack]

One more thing I noticed while looking at the reset path, to round out the review above:

4. The cancel callback fires with fromSquare === null. In positionChanged() the callback is invoked after setMoveInputState(MOVE_INPUT_STATE.reset):

this.setMoveInputState(MOVE_INPUT_STATE.reset)              // sets this.fromSquare = null
this.moveInputCanceledCallback(this.fromSquare, null, MOVE_CANCELED_REASON.movedPieceChanged)

The reset case nulls this.fromSquare (VisualMoveInput.js:183), so the callback receives null as the square instead of the square the cancelled move started from. The movedOutOfBoard path shows the correct pattern — capture the square before resetting:

const moveStartSquare = this.fromSquare
this.setMoveInputState(MOVE_INPUT_STATE.reset)
this.moveInputCanceledCallback(moveStartSquare, null, MOVE_CANCELED_REASON.movedOutOfBoard)

So the fix for this path likely looks like: remember fromSquare, set this.movedPiece = null to skip the piece-restore from point #1, then reset, then fire the callback with the remembered square.

No rush on this — just capturing it here so the review is complete.