Stockfish: No longer parsing Scid/Lichess-style Crazyhouse FENs

[niklasf]

Test case 1:

setoption name uci_variant value crazyhouse
position fen r1b3R1/ppp2p1p/4pN1k/4P2Q/3n4/7p/P1P2PPP/R4RK1/PPbbnqbpn b - - 41 21
d

Got:

r1b3R1/ppp2p1p/4pN1k/4P2Q/3n4/7p/P1P2PPP/R4RK1[] b - - 0 1

(but expected pieces in pocket)

---

Test case 2:

setoption name uci_variant value crazyhouse
position fen rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR/ w KQkq - 0 1 moves e2e4
d

Got:

 | r | n | b | q | k | b | n | r |
 +---+---+---+---+---+---+---+---+
 | p | p | p | p | p | p | p | p |
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   |
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   |
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   |
 +---+---+---+---+---+---+---+---+
 |   |   |   |   |   |   |   |   |
 +---+---+---+---+---+---+---+---+
 | P | P | P | P | P | P | P | P |
 +---+---+---+---+---+---+---+---+
 | R | N | B | Q | K | B | N | R |
 +---+---+---+---+---+---+---+---+

Fen: rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR[] b - - 0 1
Key: D1B7544BDF1344F9
Checkers: 

(but expected 1. e4 to be played)

---

According to git bisect 4e183c19fcbf6696c8ff036717a920c055ad2a2f is the first bad commit.

[ornicar]

Ironically enough https://github.com/ddugovic/Stockfish/commit/4e183c19fcbf6696c8ff036717a920c055ad2a2f

Make crazyhouse FEN parsing more robust

[ddugovic]

@ianfab @niklasf @ornicar Does b7d94fcc94ff45427376727058a4100c6a81db34 look good? It handles these two use cases as well as r1b3R1/ppp2p1p/4pN1k/4P2Q/3n4/7p/P1P2PPP/K/PPbbnqbpn (note the FEN does not specify squares B1-H1 which are assumed to be left empty).

[niklasf]

Nice, looks good to me.

[ddugovic]

Thanks, @niklasf .

I've also added an assert which tests the accuracy of positions set up during bench (although this required formatting strings already in benchmark.cpp) and fixed an uncommon en passant square bug.

Given the scope of what changed I'm being irrationally nervous about closing this...

[niklasf]

Works just fine. I'll do some more testing and then a release for lichess.

[ianfab]

I apparently forgot that we support the SCID/lichess-style FEN when I was refactoring the FEN parsing.

@ddugovic For me it mostly looks good, but the assertion is maybe a bit too much since it also fails for input that is allowed (like the lichess-style crazyhouse FEN, or pieces in the pocket in different order).

We could maybe add positions in lichess-style FEN, etc., to bench in order to ensure that bench changes if we break the FEN parsing of non-standard FENs.

[niklasf]

Ah, I agree. If anything we could assert the benchmark suite is given in canonical notation.

Edit after your edit: Or not, if we decide to test mixed formats in the bench suite.

[ddugovic]

It's true that running in debug mode, assertions of valid input fail (such as pocket pieces in a different order). I guess if we want to add mixed-format FENs to the benchmark suite, at that time it's useful to remove the assertion (at the expense of complicating testing of position setup the next time a variant is introduced; for example this revealed that a two-kings FEN in benchmark.cpp were specified with illegal castling rights).

[ianfab]

I would not have any objections against applying the assertion to bench positions, but using it in general is problematic. Debug mode should not affect functionality for valid input.

[niklasf]

Thanks again, I'll just close this now as the original issue is fixed without regressions.

Further testing improvements are always possible. I'll start by adding a Crazyhouse FEN to the fishnet client's test suite itself.