jordanbray / chess

A rust library to manage chess move generation
https://jordanbray.github.io/chess/
MIT License
231 stars 54 forks source link

FEN - Half-move clock and full-move counter Display Trait implementation #46

Open WalterSmuts opened 3 years ago

WalterSmuts commented 3 years ago

Introduction

I've noticed that in all my games the FEN string always prints 0 and 1 for the half-move clock and full move counter respectively. Here's an example of a game where each rounds' FEN string is printed. Notice that each of the FEN strings has 0 1 at the end. Here's a snip of the first couple of board states FEN representation:

rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1
rnbqkbnr/pppppppp/8/8/2P5/8/PP1PPPPP/RNBQKBNR b KQkq - 0 1
rnbqkbnr/pp1ppppp/8/2p5/2P5/8/PP1PPPPP/RNBQKBNR w KQkq - 0 1
rnbqkbnr/pp1ppppp/8/2p5/2P5/7P/PP1PPPP1/RNBQKBNR b KQkq - 0 1
rnb1kbnr/pp1ppppp/8/q1p5/2P5/7P/PP1PPPP1/RNBQKBNR w KQkq - 0 1
rnb1kbnr/pp1ppppp/8/q1p5/P1P5/7P/1P1PPPP1/RNBQKBNR b KQkq - 0 1

The issue

Looking through the code, I can see the Board's Display trait implementation is just a thin wrapper around the BoardBuilder's Display trait implementation. The half-move clock and the full-move counter is hard-coded to display as 0 1. Neither the Board or BoardBuilder structs have counters to keep track of the half-move clock or full-move counter.

The consequences

My immediate reaction was to wonder how you managed to implement the can_declare_draw function on the Game struct. You seem to keep a list of Actions and essentially calculate the half-move clock from history.

Neat! So no need to keep an actual counter of the half-move clock (and full-move counter, although I don't think there's any chess logic depending on it) since you can derive it from the history you keep for other reasons. Although, this does pose a problem for writing a correct FEN string from the context available in the Board struct.

Possible solutions

I'd suggest adding counters to the Board and BoardBuilder structs to keep track of the half-move clock and full-move counter, then also refactor the can_declare_draw function to make use of these counters to improve performance.

I'm quite keen to make these changes and submit a PR myself. I'm however not sure when I'll get to it so in the mean time I'm just writing this issue to hear your thoughts.

jordanbray commented 3 years ago

So, the issue with what you're requesting is pretty simple. Having the half-move clock would allow you to declare draw by 50 move rule. However, having the half-move clock would not allow you to declare draw by 3-fold repetition.

This is a pretty major concern: the FEN (even when formatted correctly) does not contain enough information to allow a chess game to be resumed.

This is why it is written the way it is. It's to provide a hint that maybe you're using information for something, but you don't have enough information to do the thing you want.

If you want to resume a game, pretty much the only way is via a PGN file, or via the UCI protocol.

WalterSmuts commented 3 years ago

This is a pretty major concern: the FEN (even when formatted correctly) does not contain enough information to allow a chess game to be resumed.

Agreed. The FEN format is not perfect.

This is why it is written the way it is. It's to provide a hint that maybe you're using information for something, but you don't have enough information to do the thing you want.

I'd still advocate for having a correct FEN implementation, conforming to the standard, even though the standard is flawed.

austinabell commented 3 years ago

So, the issue with what you're requesting is pretty simple. Having the half-move clock would allow you to declare draw by 50 move rule. However, having the half-move clock would not allow you to declare draw by 3-fold repetition.

This is a pretty major concern: the FEN (even when formatted correctly) does not contain enough information to allow a chess game to be resumed.

This is why it is written the way it is. It's to provide a hint that maybe you're using information for something, but you don't have enough information to do the thing you want.

If you want to resume a game, pretty much the only way is via a PGN file, or via the UCI protocol.

What benefit do you get over leaving it at - 0 1? Also, the en-passant target square always seems to be kept at -, what benefit is there for this (if also not a bug)?

If you want a consistent way to format this data, you could just define an arbitrary serialization format for the board state with something like serde to be able to handle this case and just document the limitations of FEN?

jordanbray commented 3 years ago

The benefit is quite simple: FEN is a description of the current board state, which does not contain a move counter. PGN is a description of the game state, with a move counter and handling all chess rules.

FEN can be converted into a Board. Board can be converted into an FEN. The Board has no reason to know the half-move clock or the full-move clock.

If there is a clean way to implement this where the data can be stored and tracked, I'd be up for merging something. It's likely true that you would need: a) a custom FromStr for Game that converts a FEN to a game, and b) a custom IntoStr for Game that converts the game into an FEN.

I have no motivation to work on this, as the format only solves a specific incarnation of a more general problem. However, if you were to make it work on Game I'd be happy to review it.

austinabell commented 3 years ago

The benefit is quite simple: FEN is a description of the current board state, which does not contain a move counter. PGN is a description of the game state, with a move counter and handling all chess rules.

FEN can be converted into a Board. Board can be converted into an FEN. The Board has no reason to know the half-move clock or the full-move clock.

If there is a clean way to implement this where the data can be stored and tracked, I'd be up for merging something. It's likely true that you would need: a) a custom FromStr for Game that converts a FEN to a game, and b) a custom IntoStr for Game that converts the game into an FEN.

I have no motivation to work on this, as the format only solves a specific incarnation of a more general problem. However, if you were to make it work on Game I'd be happy to review it.

Okay I see, it is definitely not intuitive or documented that using just the Board results in invalid FEN strings and that it is necessary to use the Game struct for everything.

It definitely doesn't need to be a string, has there been no need to persist game state, or is it just assumed that you recreate moves given PGN?

WalterSmuts commented 3 years ago

if you were to make it work on Game I'd be happy to review it.

I originally disagreed with this... but now am considering. Stay tuned!