Illegal moves

[TerjeKir]

Very rarely the engine plays an illegal move, searching and moving for the wrong side, despite at least seemingly having read in the correct position (and side) to search.

The bug was discovered during #16 , but was introduced before that as it was confirmed to also happen after #15 . It is assumed to have been introduced after #10 and testing so far seems to confirm this.

[TerjeKir]

200 games at #10 without an illegal move, concluding that the bug was likely introduced later. Starting testing at #12 .

[TerjeKir]

500 games at #12 without an illegal move.

Got an illegal move at #13 after 37 games so I will assume it was introduced here.

[TerjeKir]

Fixed in #19

Turned out to be 2 separate bugs:

• Introduced in Cleanup #13 attack.c commit and extremely rare: InitPawnAttacks incorrectly set the attack bitboards of pawns on rank 8 (relative to their side) as something outside the SetMask array (not sure what exactly), resulting in Kings in particular being unable to move to some of the rank 8 squares due to Weiss thinking the square was attacked by pawns potentially even far away. When an opponent made such a move it was parsed, but taken back, leaving Weiss to play the same position the opponent just played, resulting in moving for the opponent.

This happened because with the old way of using a bitmask and shifting it around, the bitmask was shifted completely out for pawns on their 8. rank, so their attack bitboards would be empty. I failed to remember this particularity when I changed to using SETBIT, and didn't add the required rank 8 check.

• Introduced in Separate threads for search and uci loop #16 New thread for searching commit and somewhat more common: ParsePosition would read past the null terminator, potentially into old input. When a move in the trailing old input was legal in the position it would be made, changing the turn to play to the opponent again, making Weiss move for the opponent like in the other bug.

I didn't realize that ParsePosition would do this and removed the memset that zeroed out input before reading again. #19 fixes ParsePosition so it won't go past a null terminator, and also reintroduces the zeroing out before reading just in case (both separately fix the problem according to testing). This part isn't speed critical as it's done during preparations before the go command, so we may as well.

Finding the errors took a long time as I didn't expect there to be two separate causes of what seemed like one error; thinking it was the opponents turn again. Testing possible solutions, one of the two would always be present, showing illegal moves and making me think the proposed fix had had no effect even in the cases where I had actually fixed one or the other. This confused me as I was unable to pinpoint the cause.

The first one going undetected was unfortunate, but understandable; after print debugging, locating the input bug and fixing it, it took 190 games to get an illegal move. When not looking for them, it's easy to miss that one or two out of a couple hundred games ended in an illegal move. Going forward I will be looking for this more attentively.