bhlangonijr / chesslib

chess library for legal move generation, FEN/PGN parsing and more
Apache License 2.0
223 stars 78 forks source link

Threefold detection not working anymore after undoing move (again position hash) #55

Closed dlbbld closed 3 years ago

dlbbld commented 3 years ago

In a nutshell, the position hash changes after undoing a move, which is the gravest error as from this point onwards the board cannot make proper decisions on threefolds anymore. It can overlook threefolds, possibly, this I would need to check further but has not much use, it can call wrong threefolds. This goes along testing #48.

I tested about five consecutive builds, all having problems with the position hash. Finally, when all should be ok, on top of it a real bummer.

final Board board = new Board();
final Move e2e4 = new Move(Square.E2, Square.E4);
final Move e7e5 = new Move(Square.E7, Square.E5);
board.doMove(e2e4);
board.doMove(e7e5);
System.out.println(board.getIncrementalHashKey()); // 4604945406196848133

board.undoMove();
board.doMove(e7e5);
System.out.println(board.getIncrementalHashKey()); // -4218352153612380667

Same positions must have same position hashes, otherwise the board doesn't work anymore correctly. This example should be enough, based on this easily examples can be constructed where the board goes totally off.

What is needed is that you use a non-hash for position id, as getPositionId(). By the way, the getPositionId() can be done performance-wise much better, which I am sure you are aware of yourself. Then you test the repetition result (which was shown to be repeatedly wrong and can still be wrong) for the hash key against the correct repetition result for the getPositionId(). This approach tests the hash key and will reveal additional potential bugs. At this stage, one has to assume that there are most likely more bugs in the position hash. Once this is stable, that might be removed, but still kept for test mode.

I will not accept any arguments on performance. When the chess engine gets wrong threefold information, it cannot operate in a normal mode. It will make the wrong decisions. It doesn't help if the board is fast for the chess engine, but you pass wrong information.

bhlangonijr commented 3 years ago

Thank you for chasing this one @dlbbld . I will get it fixed ASAP.

bhlangonijr commented 3 years ago

Fixed in 1.2.5. The bug was introduced after increasing the hash key length.

dlbbld commented 3 years ago

Thanks for looking at this. My tests only cover a small area and are chosen arbitrarily; these tests passed now.