Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error? Losing the castling for no apparent reason #7

Closed
boulayo opened this issue May 26, 2022 · 5 comments · Fixed by #8
Closed

Error? Losing the castling for no apparent reason #7

boulayo opened this issue May 26, 2022 · 5 comments · Fixed by #8
Assignees
Labels
bug Something isn't working

Comments

@boulayo
Copy link

boulayo commented May 26, 2022

Try this code:

$a = "d4 Nc6 c4 Nf6 d5 Na5 b3 c6 dxc6 bxc6 Nf3 g6 Nc3 Bg7 Bb2 Ne4 Qc2 Nxc3 Bxc3 Bxc3+ Nd2 Bxa1 b4 Nb7 e3 Bg7 Bd3 a5 bxa5 Rxa5 O-O Qc7 Nb3 Rh5 h3 d6 Be2 Rh6 f4 O-O Rd1 e5 fxe5 dxe5 e4 f5 Bf3 fxe4 Bxe4 Bf5 Rf1 Bxe4 Rxf8+ Bxf8 Qxe4 Nd6 Qd3 e4 Qe2 Rh5 c5 Nf5 Qxe4 Qa7 Qe6+ Kh8 Qxc6 Ne7 Qf6+ Bg7 Qf2 Rf5 Qc2 Qa4 Nd4 Qxd4+ Kh2 Be5+ g3 Rf2+ Qxf2 Qxf2+ Kh1 Bd4 c6 Qg1#";
$pgn = new Onspli\Chess\PGN($a);
for ($i=0; $i < $pgn->get_last_halfmove_number(); $i++) {
$fen = new Onspli\Chess\FEN($pgn->get_fen_after_halfmove($i));
$move = $pgn->get_halfmove($i+1);
$resultingfen = new Onspli\Chess\FEN($pgn->get_fen_after_halfmove($i+1));
}

In this game at move 19 the fen is this:
2b1k2r/1nq1ppbp/2pp2p1/7r/2P5/1N2P2P/P1Q1BPP1/5RK1 b k -
From there I move "Rh6" and it loses the possibility to castle for no apparent reason. The new fen is this:
2b1k2r/1nq1ppbp/2pp2pr/8/2P5/1N2P2P/P1Q1BPP1/5RK1 w - -

Is it a bug?
Thank you

@onspli onspli self-assigned this May 26, 2022
@boulayo
Copy link
Author

boulayo commented May 27, 2022

I'm pretty sure that the error is here.

private function after_move_update_castling_availability(string $piece_type, Square $origin_square) : void
{
$origin_file = $origin_square->get_file();
if ($piece_type == 'R' && $origin_file == 'a') {
$this->set_castling_availability($this->get_active_piece('Q'), false);
} else if ($piece_type == 'R' && $origin_file == 'h') {
$this->set_castling_availability($this->get_active_piece('K'), false);
} else if ($piece_type == 'K' && $origin_file == 'e') {
$this->set_castling_availability($this->get_active_piece('Q'), false);
$this->set_castling_availability($this->get_active_piece('K'), false);
}
}

It seems that you assume that if a rock is on a file it has to be the same rock that starts on that file.

Maybe this would fix it but I'm not 100% sure

private function after_move_update_castling_availability(string $piece_type, Square $origin_square) : void
{
$origin_file = $origin_square->get_file();
if ($piece_type == 'R' && (($origin_square=="a8" && $this->get_active_color()=="b") || ($origin_square=="a1" && $this->get_active_color()=="w")) {
$this->set_castling_availability($this->get_active_piece('Q'), false);
} else if ($piece_type == 'R' && (($origin_square=="h8" && $this->get_active_color()=="b") || ($origin_square=="h1" && $this->get_active_color()=="w")) {
$this->set_castling_availability($this->get_active_piece('K'), false);
} else if ($piece_type == 'K' && $origin_file == 'e') {
$this->set_castling_availability($this->get_active_piece('Q'), false);
$this->set_castling_availability($this->get_active_piece('K'), false);
}
}

@onspli
Copy link
Owner

onspli commented May 27, 2022

Thanks for reporting the issue, I'll take a look. What version do you use - v0.1.0 or dev-master?

@boulayo
Copy link
Author

boulayo commented May 27, 2022

v0.1.0

@onspli onspli added the bug Something isn't working label May 27, 2022
@onspli onspli linked a pull request May 27, 2022 that will close this issue
@onspli onspli closed this as completed in #8 May 27, 2022
@onspli
Copy link
Owner

onspli commented May 27, 2022

Fix was released in v0.2.0.

Offtopic: I noticed this piece of code in your sample

$fen = new Onspli\Chess\FEN($pgn->get_fen_after_halfmove($i));

It is not very efficient as it converts FEN -> String -> FEN. You can pass parameter bool $as_object (docs) to get FEN object directly (it creates a copy of FEN)

$fen = $pgn->get_fen_after_halfmove($i, true);

@boulayo
Copy link
Author

boulayo commented May 27, 2022

Thanks for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants