Skip to content

Commit

Permalink
chess_move: Remove "takes" validation for SAN
Browse files Browse the repository at this point in the history
This change modifies the from_san parsing to fix an issue where
un-annotated en passant (missing the `e.p.` suffix) is not found as a
valid move.

This bug stems from the fact that the SAN parser:

* records if a proposed move is a capture by looking for the `x`
  character
* matching the proposed move's capture status with whether each legal
  move is a capture, in a slightly buggy way - by detecting if the
  destination square holds a piece.

En passant is the only capture type during which the destination square
doesn't hold the captured piece. When the `e.p.` suffix is present in
the input SAN move, this check is skipped; however, the check is not
skipped when the suffix is not present, and such an en passant SAN input
will fail to match its corresponding legal move.

This change drops the check for takes. This will allow en passant SAN
inputs to match their corresponding legal move, but it will also alias
capture SAN inputs to a non-capture legal move (Nxf3 will be interpreted
as Nf3 if Nf3 happens to be legal, rather than being rejected) and vice
versa.

Since this crate will only be used when we assume the SAN is valid, we
don't care about this aliasing.

Fixes: jordanbray#54
  • Loading branch information
minor-fixes committed Apr 23, 2021
1 parent 4232f3c commit 03184db
Showing 1 changed file with 2 additions and 20 deletions.
22 changes: 2 additions & 20 deletions src/chess_move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl ChessMove {
_ => None,
};

let takes = if let Some("x") = move_text.get(cur_index..(cur_index + 1)) {
let _takes = if let Some("x") = move_text.get(cur_index..(cur_index + 1)) {
cur_index += 1;
true
} else {
Expand Down Expand Up @@ -288,6 +288,7 @@ impl ChessMove {
None
};

#[allow(unused_assignments)]
if let Some(s) = move_text.get(cur_index..(cur_index + 1)) {
let _maybe_check_or_mate = match s {
"+" => {
Expand All @@ -302,16 +303,6 @@ impl ChessMove {
};
}

let ep = if let Some(s) = move_text.get(cur_index..) {
s == " e.p."
} else {
false
};

//if ep {
// cur_index += 5;
//}

// Ok, now we have all the data from the SAN move, in the following structures
// moveing_piece, source_rank, source_file, taks, dest, promotion, maybe_check_or_mate, and
// ep
Expand Down Expand Up @@ -347,15 +338,6 @@ impl ChessMove {
return Err(error);
}

// takes is complicated, because of e.p.
if !takes && board.piece_on(m.get_dest()).is_some() {
continue;
}

if !ep && takes && board.piece_on(m.get_dest()).is_none() {
continue;
}

found_move = Some(m);
}

Expand Down

0 comments on commit 03184db

Please sign in to comment.