lichess.org
Donate

Bug in Atomic Chess replay

My awesomeness exceeded the abilities of the replay engine. The mate with two bishops cannot be revealed. Is it an en passant bug? Only Happy0 can tell.

http://en.lichess.org/JQxmxkI8

Move 15 is buggy; move 17 is magical.
Oh man, I was about to check out your stream... and then realized today isn't Tuesday!

Maybe the server was so shocked that you didn't play 13. Qa4+ that it's still in denial about how the game actually ended? :-)
But more likely there's a bug whereby if an en passant capture is available but you did some other PxP move it gets all confused? I agree, only Happy0 can tell.
It must be related to the possibility of en passant. Good find.
I think I have found the genesis of these playback ep bugs in Atomic.

An analysis of github.com/ornicar/chess.js/blob/master/chess.js

Note the make_move() and undo_move() are being called constantly in the replayer, to check for legality and SAN for instance, so any mishap in them will show up.

Consider a case of dxc6 en passant.

At line 833, c6 is set to a wP and d5 is cleared.
At line 838-839, c5 is cleared.
Now at 854 there are explosions.
The first, set at line 429, is a black (sic) pawn at c6, normally this is the explosion of the capturer, and the stored information (color/type) is the captured piece. In the ep case, move.to and captures_on differ. The second explosion, set at line 445, is a black pawn at c5, which I think is redundant, but can't hurt.

Now over to the undo, where at line 952 the explosions are undone, first there is *incorrectly* set a black pawn at c6, and then (correctly) a black pawn at c5. This explains why these ghost pawns appear when the ep capture is not made, in happens always in Atomic replaying. At line 955, d5 is set to a wP, and then at line 973 c5 is (redunantly, but correctly) set to a bP.

So to fix this, I think one only need to remove the extra pawn that appeared from this process in the ep case, say as part of the line 966 conditional.

board[move.to]=null;

A simple test case:

[White "me"]
[Black "you"]
[Variant "atomic"]
[Result "*"]

1. d4 Nf6 2. d5 c5 3. h4 *

Currently playing h4 on the replay board makes a bP at c6 appear, and hopefully my one-line fix will remedy this.
Thanks guys.

Cheers for going the extra mile and diagnosing the bug, YetAnotherAccount. I've referenced the post on an existing bug on the issues tracker: github.com/ornicar/lila/issues/273

I'll have a closer look at what you said when I get around to fixing it. Alternatively - if you like, I can help you set up the project and you can submit a pull request with the fix yourself - but there's no expectation or anything. Just let me know :P.
YetAnotherAccount's analysis seems accurate. I'm curious if the following solution (which might seem more intuitive) could also work.

It seems "color" and "type" are only used for undoing moves and wouldn't affect the making of moves? So an "explosion undo" that sets the en passant square to be empty couldn't hurt:
// explode capturer
move.explosion = [{
square: move.to,
color: board[move.to].color, // instead of captures_on
type: board[move.to].type // instead of captures_on
}];
I am no expert in Javascript, but would not be sure "board[move.to].color" is valid when "board[move.to]" is null (as I think it is when the square is empty). See line 505 for example, where first "piece" is checked if it is null before referencing "piece.color" and comparing it.

Your solution might work if one only wanted "board[move.to].type" to be null and not "board[move.to]" itself, but I don't think that is the case. Though maybe Javascript automatically makes "null.attribute" be "null", in which case it could suffice.

But enpassant is so unintuitive, I am not sure any one solution is better than another.
Oops, you're correct. I suppose my idea would involve changing "explosion" to combine "type" and "color" -- maybe call it "piece" -- and then change line 953:
board[kaboom.square] = kaboom.piece;

but now my lack of JavaScript expertise is showing: unless doing a null assignment, board.js only once does that sort of direct assignment (by reference?) so there is probably a reason for that:
board[move.to] = board[move.from];

(On the other hand, maybe in the scope of "explosion" and "kaboom" it's OK to assign by reference since we know that board[index] at each of those explosion squares are about to be set to null, and the references aren't accessed again until we try to undo the move. But clearly my sort of change would require careful testing.)

This topic has been archived and can no longer be replied to.