SAN generation incorrect when pinned pieces involved

[dlbbld]

In the below example, the SAN generated for the last move is incorrect. It should be Ne2 and not Nge2 because the knight on c3 is pinned and cannot move to e2. By the SAN specification, the file is specified when needed for disambiguation, otherwise not. Please see the specification below.

Generating Nge2 instead of Ne2 seems like a small problem because also Nge2 allows identifying the move. The problem is, however, not in identifying the move. For example, this creates a problem when trying to compare two API. When one implements the SAN correctly, it will refuse SAN's like "Nge2", this leads to problems.

final MoveList moveList = new MoveList("4k3/8/8/8/1b6/2N5/8/4K1N1 w - - 0 1");
moveList.add(new Move(Square.G1, Square.E2)); // Ne2
System.out.println(moveList.toSan()); // Nge2 but expected Ne2

On the other side specifying the move as "Nge2" should throw an exception, being an invalid SAN. But this is not the case.

final MoveList moveList = new MoveList("4k3/8/8/8/1b6/2N5/8/4K1N1 w - - 0 1");
moveList.addSanMove("Nge2", false, true); // is accepted but should throw an exception

From PGN specification 8.2.3: Movetext SAN (Standard Algebraic Notation) 8.2.3.4: Disambiguation

[...] Note that the above disambiguation is needed only to distinguish among moves of the same piece type to the same square; it is not used to distinguish among attacks of the same piece type to the same square. An example of this would be a position with two white knights, one on square c3 and one on square g1 and a vacant square e2 with White to move. Both knights attack square e2, and if both could legally move there, then a file disambiguation is needed; the (nonchecking) knight moves would be "Nce2" and "Nge2". However, if the white king were at square e1 and a black bishop were at square b4 with a vacant square d2 (thus an absolute pin of the white knight at square c3), then only one white knight (the one at square g1) could move to square e2: "Ne2".

[dlbbld]

Is there any feedback here? This looks unimportant first, but implementing the specification correctly here is of utmost importance.

[bhlangonijr]

Hi @dlbbld, I am fixing this one too. I'll update this ticket when released. By the way, thank you for reporting the bugs and adding suggestions. This is very important for improving the library.

[dlbbld]

Thanks for the feedback. It goes a bit hand in hand I think. As long the feedbacks are considered it is worth making them. Besides, I think the API is very stable and fine.

[bhlangonijr]

The expected behaviour now is the follow: Chesslib is generating the SAN moves with correct notation, although not too strict when taking the SAN move with addSanMove: It'll will accept Nge2 because this is a legal move, but when generating its string representation back the correct notation without the ambiguity resolution is shown: Ne2.

[dlbbld]

I spot checked the SAN generation and looks fine, thanks. addSanMove is specified as taking a SAN move and performing it. Nge2 is not a SAN move and allowing it is wrong, full stop. I suggest to add a method like addSanMoveNonStrict or whatever if you want to support non SAN moves as well.

[dlbbld]

Can you please recheck. It generates "Ne2 " not "Ne2":

final MoveList moveList = new MoveList("4k3/8/8/8/1b6/2N5/8/4K1N1 w - - 0 1");
moveList.add(new Move(Square.G1, Square.E2)); // Ne2
System.out.println(moveList.toSan()); // "Ne2 "