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

Board editor: automatically check castling boxes when importing a FEN that allows castling, but the position is not valid #12929

Open
victorchan314 opened this issue May 30, 2023 · 3 comments

Comments

@victorchan314
Copy link

Example link: https://lichess.org/editor/4k3/8/8/8/8/8/8/4K2R_w_Kk_-_0_1?color=white

This FEN is technically not valid because there is no black rook on h8, even though kingside castling is specified for both White and Black. When the board editor opens, the White O-O checkbox is checked, but the Black O-O checkbox is not. In addition, the URL is automatically updated from the first one to the second:
https://lichess.org/editor/4k3/8/8/8/8/8/8/4K2R_w_Kk_-_0_1?color=white
https://lichess.org/editor/4k3/8/8/8/8/8/8/4K2R_w_K_-_0_1?color=white

The URL update makes sense because the FEN is not valid, and it looks like the URL always must remain valid. However, it would be nice if the checkbox for Black O-O is also automatically checked when opening the link. This way, opening the two links above would produce different results.

In other words: imagine opening that link and then manually checking the Black O-O box. There is no way to reach this state from clicking a link without the manual action of clicking the checkbox, but I think that it is reasonable to infer the state of the checkbox from the FEN even if it is invalid. The invalid FEN is already corrected in the address bar anyway, while the board editor would be in the state intended by the author of the link.

Sharing incomplete FENs for the board editor may not be a common use case, but I don't see the harm if the desired state is already reachable through manual actions.

@victorchan314
Copy link
Author

victorchan314 commented May 30, 2023

After doing some more digging around, I see that the checkboxes are populated here: https://github.com/lichess-org/lila/blob/master/ui/editor/src/view.ts#L20
Which are defined here: https://github.com/lichess-org/lila/blob/master/ui/editor/src/ctrl.ts#L178

The objects are defined types in the chessops package.
The import function is defined here: https://github.com/niklasf/chessops/blob/master/src/chess.ts#L114
And the FEN is parsed here: https://github.com/niklasf/chessops/blob/master/src/fen.ts#L86

The parsed FEN actually allows castling rights in the setup.castlingRights SquareSet object (defined as the corners of the board if the rooks are not present), but the Castles.fromSetup function has logic that discards these possibilities if the rooks and kings don't match up. However, I'm pretty sure that the castlingRights SquareSet isn't what we actually want here either, since it is dependent on the locations of the kings and rooks (for example, if there is only 1 king or rook of a certain color in the back rank, we cannot distinguish between K and Q).

Getting into the implementation details, the only options I foresee are to either update the chessops library with more utility functions or to port over some of the logic for parsing the FEN, which would be ugly. The first option might be nice—for example, the chessops library could use an extracted function that splits apart the FEN into its constituent strings instead of merging that logic with the parser. But at the same time, because the FEN suggested in the issue is technically invalid (is it? I couldn't find a spec for FEN), the chessops library might not be the right place for this kind of logic.

Just brainstorming an idea here: in ctrl.setFen, instead of calling Castles.fromSetup, we do the following:

if setup.castlingRights is not empty: // this means that the castling part of the FEN is not empty
    const castlingPart = fen.split(/[\s_]+/)[2]; // extract the castling part
    for (const x of "KQkq") {
        this.castlingToggles[x] = castlingPart.includes(x);
    }

I'm willing to figure out how to make these changes, but I haven't worked on lila before so I'd like to get some thoughts from more experienced contributors first.

@niklasf
Copy link
Member

niklasf commented Jun 1, 2023

The URL update makes sense because the FEN is not valid, and it looks like the URL always must remain valid.

We can relax that, if it's inconvenient.

Getting into the implementation details, ...

There's also X-FEN / Shredder-FEN to take care of. There are some relevant unreleased changes in chessops that might help:

  1. setup.castlingRights now preserves castling rights without matching rooks, substituting A/H for Q/K. For example, KAbq without any rooks is h1 a1 b8 a8.
  2. makeCastlingFen preserves invalid castling rights, always using the explicit file if no matching king and rook is found. So for example HAba.

As you mentioned setup.castlingRights is still not a lossless representation of invalid FENs, but maybe the following is then good enough?

const castlingPart = makeCastlingFen(setup.board, setup.castlingRights);
this.castlingToggles['K'] = castlingPart.includes('K') || castlingPart.includes('H');
// ...

This still would not preserve invalid Chess960 castling rights. And more fundamentally KQkq toggles are not able to represent all possible Chess960 castling rights in any case.

@ohadbitt
Copy link

Also found something similar after creating a new board and king is not on starting position, played vs bot and he castles ??!!??

https://lichess.org/AEapDndy/white

This is my first activity in this repo, btw as a programmer what is my best way to start helping to this project?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants