bhlangonijr / chesslib Goto Github PK

When loading a PGN with 250 moves, this takes a second. However, when loading a PGN with 9'000 moves, this goes off. The time it takes more is not in proportion to the additional moves and is much too long. Please see the attached PGN (I adapted the second for import).

Of course a game with 9'000 moves is not realistic, but this long time is unexpected. I hope this observation helps to improve your API.

for (final String fileName : Arrays.asList("nikolic_arsovic_1989.pgn", "longest.pgn")) {
  System.out.printf("Processing %s%n", fileName);
  final PgnHolder pgn = new PgnHolder("C:\\temp\\test\\" + fileName);

  final long msBefore = System.currentTimeMillis();
  pgn.loadPgn();
  final Game game = pgn.getGames().get(0);
  game.loadMoveText();
  final long msDuration = System.currentTimeMillis() - msBefore;
  System.out.printf("Loading duration seconds: %f%n", msDuration / 1000.0);
  System.out.printf("Moves: %s%n", game.getHalfMoves().size() / 2.0);
}

Output is

Processing nikolic_arsovic_1989.pgn
Loading duration seconds: 0.494000
Moves: 269.0
Processing longest.pgn
Loading duration seconds: 116.542000
Moves: 8848.5

longest.pgn.txt
nikolic_arsovic_1989.pgn.txt

There is some method to mark a game as abandoned or offer draw?, I can not find the way to mark a game and get your PGN when a player leaves or pacts draw game.

For example, if I read a PGN from a game that has been tied, the isDraw method returns false.

1. e4 1/2-1/2

Thanks

I propose that GameLoader and the other relevant classes are changed to read PGN tag "Variant" instead of the current "Variation". Wikipedia has this to say about the subject:

Handling chess variants
Many chess variants can be recorded using PGN, provided the names of the pieces can be limited to one character, usually a letter and not a number. They are typically noted with a tag named "Variant" giving the name of the rules. The term "Variation" must be avoided, as that refers to the name of an opening variation.

As a practical example from the wild, Lichess exports the games with the "Variant" tag.

My hope would be that eventually the game loading system could be modified to ignore the variants that the library does not support, because when you export your games from Lichess, you get all the variant games as well, and the library crashes when it needs to parse certain, say, Crazyhouse moves.

If you don't see the complete renaming as a feasible plan, then a hacky way to fix the issue would be to treat the Variant tag as a synonym to the Variation tag. A third way would be to add "Variant" as a new attribute next to "Variation", keeping both. I would be willing to work on this issue and to create a pull request, once we just figure out the best way to approach it. My vote would go renaming Variation to Variant completely, though.

A function to count the number of games in a PGN file before loading would be interesting, as this would also allow a PgnLoadListener to calculate the exact percentage of games currently processed. I will create a pull request for this implementation.

One suggestion :

• Probably "Site" tag should be available in Game also

Reason lichess.org site puts specific game id in site tag in their pgn

Example

[Site "https://lichess.org/<gameid>"]

While u parse the pgn the only way to get site is via PGN's event map
and it only has site for last game of that event and there is now way we can get specific game id

Hi once again)

After I used MoveList.remove(Move) method, MoveList.toSanArray() returns incorrect value, because 'dirty' variable wasn't changed false by default and 'sanArray' is not nul (as I have used toSanArray() before ).

    public String[] toSanArray() throws MoveConversionException {
        if (!dirty && sanArray != null) {
            return sanArray;
        }
       ..//other code
        dirty = false;
        return sanArray;
    }

So, possible solutions are :

1. override other methods from LinkedList where dataset is changing and set 'dirty = true'
2. remove ' if (!dirty && sanArray != null)' and always recreate array from dataset
3. make method 'setDitry(boolean)' - and user can call it before any other methods, like 'remove' etc.

Also I propose to make variables 'protected' instead of 'private' if variable don't have getters/setters. Then users of this library can get access to variables through inheritance.

Thanks,
Eugene

I am using this library to create a chess GUI and I noticed that isMoveLegal validates some moves that are clearly illegal

 Board board = new Board();

// Move pawn at D2 across entire board capturing at H7
Move illegalMove = new Move(Square.D2, Square.H7);

System.out.println(board.isMoveLegal(illegalMove, true));

Expected behavior: print false

Observed behavior: print true

The workaround I am using for now is the following since move generation seems to be correct:

if (board.legalMoves().contains(selected_move)) {
    board.doMove(selected_move);
}

The method Board.isInsufficientMaterial returns true for KNvKN, KBvKN and KNvKB, but should return false (checkmate with cooperation of opponent always possible).

final Board board = new Board();
board.loadFromFen("8/8/4K3/8/1n6/8/5k1N/8 w - - 0 50"); // KNvKN
assertFalse(board.isInsufficientMaterial()); // should be false but is true

board.loadFromFen("1n6/8/8/2k1K3/8/8/8/5B2 w - - 0 50"); // KBvKN, bishop light squares
assertFalse(board.isInsufficientMaterial()); // should be false but is true

board.loadFromFen("1n6/8/8/8/4K3/1k6/1B6/8 w - - 0 50"); // KBvKN, bishop dark squares
assertFalse(board.isInsufficientMaterial()); // should be false but is true

board.loadFromFen("8/k2b4/8/8/2K5/8/8/1N6 w - - 0 50"); // KNvKB, bishop light squares
assertFalse(board.isInsufficientMaterial()); // should be false but is true

board.loadFromFen("5b2/8/3k4/8/8/4K3/8/1N6 w - - 0 50"); // KNvKB, bishop dark squares
assertFalse(board.isInsufficientMaterial()); // should be false but is true

The iterator implementations are unfortunately not correct and can lead to errors:

public boolean hasNext() {
     game = GameLoader.loadNextGame(pgnLines);
     return game != null;
 }

public Game next() {
    return game;
}

According to the Iterator interface, the functions should be implemented as follows:

Therefore, hasNext should be callable any number of times without the iterator loading the next element. Only next shall return the next iterative object.

You are overlooking that castle can cause check or checkmate. The SAN for the move O-O-O# is wrongly generated as O-O-O.

final String san = "1. d4 e6 2. Nf3 f5 3. Nc3 Nf6 4. Bg5 Be7 5. Bxf6 Bxf6 6. e4 fxe4 7. Nxe4 b6 "
    + "8. Ne5 O-O 9. Bd3 Bb7 10. Qh5 Qe7 11. Qxh7+ Kxh7 12. Nxf6+ Kh6 13. Neg4+ Kg5 14. h4+ Kf4 "
    + "15. g3+ Kf3 16. Be2+ Kg2 17. Rh2+ Kg1 18. O-O-O#";

final MoveList moveList = new MoveList();
moveList.loadFromSan(san);
final String sanGeneratedLastMove = moveList.toSanArray()[moveList.toSanArray().length - 1]; 
assertEquals("O-O-O#", sanGeneratedLastMove); // expected O-O-O# but O-O-O is generated

From famous game where last move was 18. Kd2# but could have been 18. O-O-O#.

I suggest to add more tests when reading the FEN. At the very least you should not allow a position where the opponent king of the player having the move is in check! I think as such you could improve the API further. Please see examples below, I hope you get the idea:

final Board board = new Board();

// side field contains invalid char - accepted - not ok
board.loadFromFen("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR x KQkq - 0 1");

// castling field contains invalid char - accepted - not ok
board.loadFromFen("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w xQkq - 0 1");

// position field contains non complete rank - accepted - not ok
board.loadFromFen("rnbqkbnr/pppppppp/7/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1");

// position field contains non possible value (opponent king in check) - accepted - not ok
board.loadFromFen("4k3/8/8/8/8/8/4R3/4K3 w - - 0 1");

// castling field contains non possible value for position - accepted - not ok
board.loadFromFen("4k3/8/8/8/8/8/8/4KR2 w K - 0 1");

// en passant field contains non possible value for position - accepted - not ok
board.loadFromFen("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq e3 0 1");

// halfmove counter contains non possible value (too big in regard to fullmove counter) - accepted - not ok
board.loadFromFen("4k3/8/8/8/8/8/8/4KR2 w - - 500 50");

Hi,

Just tried to parse pgn from 34th European Club Cup (http://chess-results.com/tnr373918.aspx?lan=11) and got the following exception:

Exception in thread "main" com.github.bhlangonijr.chesslib.move.MoveConversionException: Coudn't parse destination square[cxd8Q]: CXD8Q
	at com.github.bhlangonijr.chesslib.move.MoveList.encodeSanToMove(MoveList.java:581)
	at com.github.bhlangonijr.chesslib.move.MoveList.addSanMove(MoveList.java:492)
	at com.github.bhlangonijr.chesslib.move.MoveList.addSanMove(MoveList.java:468)
	at com.github.bhlangonijr.chesslib.move.MoveList.loadFromSan(MoveList.java:529)
	at com.github.bhlangonijr.chesslib.game.Game.loadMoveText(Game.java:872)
	at com.github.bhlangonijr.chesslib.game.Game.loadMoveText(Game.java:694)
	at Parse.main(Parse.java:12)

pgn is attached
cup.pgn.zip

Code fragment is from readme

       PgnHolder pgn = new PgnHolder("D:\\Projects\\chess\\cup.pgn");
        pgn.loadPgn();
        for (Game game: pgn.getGame()) {
            game.loadMoveText();
            MoveList moves = game.getHalfMoves();
            Board board = new Board();
            //Replay all the moves from the game and print the final position in FEN format
            for (Move move: moves) {
                board.doMove(move);
            }
        }

KBvKB positions with bishops on different color squares are not insufficient material positions as well known, as a mate is still possible.

But when I tried, I found this to be treated as insufficient material. Can you please check?

final Board board = new Board();
board.loadFromFen("8/8/8/4k3/5b2/3K4/8/2B5 w - - 0 1");
System.out.println("isInsufficientMaterial: Expected=false, Actual=" + board.isInsufficientMaterial());

The output is as below, with latest version 1.1.22, which is not as expected

isInsufficientMaterial: Expected=false, Actual=true

Hi, is it possible to get individual positions from a pgn file?

For example, when pressing "next" I would like to get the next move and when pressing "previous" I would like to get the previous move.

It would be great if MoveList would also have a toLan() method, which would return the moves in the long algebraic notation.

When loading a PGN which uses a starting position, there is an exception, e.g.

[Event "?"]
[Site "?"]
[Date "????.??.??"]
[Round "?"]
[White "?"]
[Black "?"]
[Result "*"]
[SetUp "1"]
[FEN "3k4/8/8/8/3K4/3R4/8/8 w - - 15 100"]

100. Re3 Kc7 101. Re5 Kc6 *

Hi all,
Some matches give the following MoveConversionException while the matches played are legal themselves.
The following example:
com.github.bhlangonijr.chesslib.move.MoveConversionException: Couldn't parse SAN to MoveList: Illegal move: e1g1 [O-O] on r3k2r/p3p1bp/2p3p1/1p6/3Pp3/1Qn1P2P/3N1PPB/q2BK2R w kq - 2 23
at com.github.bhlangonijr.chesslib.move.MoveList.addSanMove(MoveList.java:496)
at com.github.bhlangonijr.chesslib.move.MoveList.addSanMove(MoveList.java:468)
at com.github.bhlangonijr.chesslib.move.MoveList.loadFromSan(MoveList.java:528)
at com.github.bhlangonijr.chesslib.game.Game.loadMoveText(Game.java:872)
at com.github.bhlangonijr.chesslib.game.Game.loadMoveText(Game.java:694)
at nl.overbeek.chessbuilder.EntryPoint.main(EntryPoint.java:23)
Any glue?
Kind regards
Henk
illegalMove2.txt
illegalMove3.txt
illegalMove.txt

How do I get the comment (and Nag) of a move?

The threefold repetition after 5... Bf8 for the below moves is not detected. Why is that the case? Can you please check? The positions, possibilities to capture en passant and castle rights after 1... e5, 3... Bf8 and 5... Bf8 are the same. This is a threefold repetition after 5... Bf8.

final MoveList moveList = new MoveList(); //use MoveList to play moves on the board using SAN
moveList.loadFromSan("1. e4 e5 2. Be2 Be7 3. Bf1 Bf8 4. Bd3 Bd6 5. Bf1 Bf8");

final Board board = new Board();
final Iterator<Move> moves = moveList.iterator();
while (moves.hasNext()) {
  board.doMove(moves.next());
}
System.out.println(board.isRepetition()); // false but should be true

Found a bug, the Queen was able to jump over a pawn
Board board = new Board();
Move mv = new Move(Square.D1, Square.D3);
boolean res = board.doMove(mv, true); // return true

Found there where some problems when runing PgnLoaderTest. I manage to fix it by changing a little the way function parse the line as follows.

Keep checking other errors. I hope this helps.

Interesting I found you put games by round, I don't see the reason. Better aproach would be using a simple List and put games there, then in order to find similar games using Lambda stream functions to filter the list by round, event, player, etc. Let me know if you agree with that.

public static PgnProperty parsePgnProperty(String line) {
        try {
            String parsedLine = line.trim().replace("[", "");
            parsedLine = parsedLine.replace("]", "");
            parsedLine = parsedLine.replace("\"", "");
            String[] values = parsedLine.split(" ",2);

            return new PgnProperty(values[0], values[1]);
        } catch (Exception e) {
            // do nothing
        }

        return null;
    }

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".

Just a suggestion to improve the insufficient material check. KBvKB with same coloured bishops is insufficient material, that is checked. But King versus King with any number of same coloured bishops on the board is also insufficient material, that could also be checked. I've seen that in the python chess API.

https://syzygy-tables.info/?fen=B3k3/8/8/8/8/8/8/4KB2_w_-_-_0_1

final Board board = new Board();
board.loadFromFen("B3k3/8/8/8/8/8/8/4KB2 w - - 0 1");
System.out.println(board.isInsufficientMaterial()); // false but actually true

https://syzygy-tables.info/?fen=B1b1k3/3b4/4b3/8/8/8/8/4KB2_w_-_-_0_1

final Board board = new Board();
board.loadFromFen("B1b1k3/3b4/4b3/8/8/8/8/4KB2 w - - 0 1");
System.out.println(board.isInsufficientMaterial()); // false but actually true

Add:

• did it eat something?
  • was it en passant
• did it castle?
• did it promote?

I'm unable to build the project directly after cloning it because of some PgnHolder related unit tests.

git clone https://www.github.com/bhlangonijr/chesslib.git
cd chesslib/
mvn clean compile package install

Outputs:

Results :

Tests in error:
  testUtf8(com.github.bhlangonijr.chesslib.PgnHolderTest): Error parsing PGN[1, ]:
  testPGNLoad4(com.github.bhlangonijr.chesslib.PgnHolderTest): Error parsing PGN[1, ]:

Tests run: 44, Failures: 0, Errors: 2, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  20.043 s
[INFO] Finished at: 2020-04-09T17:34:01+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project chesslib: There are test failures.
[ERROR]
[ERROR] Please refer to D:\Projects\chesslib\target\surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

The board claims a threefold at the end of the game, but this is only a twofold repetition. The reason is the position hashing, which treats different positions here as the same.

final PgnHolder pgn = new PgnHolder("test.pgn");
pgn.loadPgn();
final Game game = pgn.getGames().get(0);
game.loadMoveText();

final Board board = new Board();
for (final Move move : game.getHalfMoves()) {
  board.doMove(move);
}
System.out.println(board.isRepetition()); // prints true but is only twofold repetition

test.pgn.txt

I got an Exception from prft test with depth 5 from fen "8/1pp3p1/4pq1p/PP1bpk2/1Q2p3/4P1P1/2B2P2/6K1 b - - 2 33".

Code:

public void testPerft1() throws MoveGeneratorException {
        long nodes = testPerft("8/1pp3p1/4pq1p/PP1bpk2/1Q2p3/4P1P1/2B2P2/6K1 b - - 2 33", 5);
        assertEquals(4865609, nodes);
    }

Output:

"C:\Program Files\Java\jdk-14.0.2\bin\java.exe" -ea -Didea.test.cyclic.buffer.size=1048576 -javaagent:C:\Users\LociStar\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\202.6397.94\lib\idea_rt.jar=59153:C:\Users\LociStar\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\202.6397.94\bin -Dfile.encoding=UTF-8 -classpath C:\Users\LociStar\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\202.6397.94\lib\idea_rt.jar;C:\Users\LociStar\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\202.6397.94\plugins\junit\lib\junit5-rt.jar;C:\Users\LociStar\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\202.6397.94\plugins\junit\lib\junit-rt.jar;D:\Programms\Java\IdeaProjects\chesslib\target\test-classes;D:\Programms\Java\IdeaProjects\chesslib\target\classes;C:\Users\LociStar\.m2\repository\junit\junit\4.8.2\junit-4.8.2.jar com.intellij.rt.junit.JUnitStarter -ideVersion5 -junit4 com.github.bhlangonijr.chesslib.PerftTest,testPerft1
h6h5: 308869
b7b6: 331767
c7c5: 240775
c7c6: 286383
g7g5: 244940
g7g6: 261434
d5a2: 291547
d5b3: 265493
d5c4: 316180
d5c6: 276039
f6h4: 305289
f6g5: 304930
f6g6: 241227
f6e7: 386238
f6f7: 335636
f6d8: 448502
f6f8: 452807
depth 2 - ply 4
com.github.bhlangonijr.chesslib.move.MoveGeneratorException: Couldn't generate Legal moves: 
	at com.github.bhlangonijr.chesslib.move.MoveGenerator.generateLegalMoves(MoveGenerator.java:353)
	at com.github.bhlangonijr.chesslib.PerftTest.perft(PerftTest.java:217)
	at com.github.bhlangonijr.chesslib.PerftTest.perft(PerftTest.java:223)
	at com.github.bhlangonijr.chesslib.PerftTest.perft(PerftTest.java:223)
	at com.github.bhlangonijr.chesslib.PerftTest.perft(PerftTest.java:223)
	at com.github.bhlangonijr.chesslib.PerftTest.perft(PerftTest.java:223)
	at com.github.bhlangonijr.chesslib.PerftTest.testPerft(PerftTest.java:200)
	at com.github.bhlangonijr.chesslib.PerftTest.testPerft1(PerftTest.java:26)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
	at org.junit.runners.BlockJUnit4ClassRunner.runNotIgnored(BlockJUnit4ClassRunner.java:79)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:71)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:49)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 64 out of bounds for length 64
	at com.github.bhlangonijr.chesslib.Bitboard.getPawnAttacks(Bitboard.java:469)
	at com.github.bhlangonijr.chesslib.Board.squareAttackedBy(Board.java:1057)
	at com.github.bhlangonijr.chesslib.Board.isKingAttacked(Board.java:1137)
	at com.github.bhlangonijr.chesslib.move.MoveGenerator.generateCastleMoves(MoveGenerator.java:278)
	at com.github.bhlangonijr.chesslib.move.MoveGenerator.generatePseudoLegalMoves(MoveGenerator.java:314)
	at com.github.bhlangonijr.chesslib.move.MoveGenerator.generateLegalMoves(MoveGenerator.java:346)
	... 29 more
depth 3 - ply 3
  ..........

Test Results — PerftTest.testPerft1.pdf

I find it a big problem that when I create a new move as below, I cannot find the SAN for the move:

final Board board = new Board();
final Move pawnMove = new Move(Square.E2, Square.E4);
board.doMove(pawnMove);
System.out.println(pawnMove.getSan());

For example pawnMove.getSan() above is null. This attribute is only set, if a game was loaded from a PGN. But I find it essential to have the SAN for a move constructed as above. Is it anywhere? MoveList offers some methods for SAN generation, but I could not look through.

Also is there a method which supports specifying a move by the SAN? Move(String move, Side side) does not accept SAN, also not LAN, only some sort of "improvised" notation (e.g. e2e4, e7e5 is ok, but then Qe2 or Qd1e2 is not supported. Am I missing something?

In the test/resources folder there are exactly two PGN stored as UTF-8, "Morphy_UTF8.pgn" and "redqueen.pgn". I found that the corresponding test cases PgnHolderTest.testPGNLoad4 and PgnHolderTest.testUtf8 sometimes work and sometimes not. Encoding related, this problem is probably not caused by the project. I found that when running the JUnit tests on the project, it sometimes worked, sometimes not. Same for pom.xml. I use Eclipse under Windows; the chesslib project text file encoding is UTF-8.s

When it does not work, the project does not build, so it is worth having a look here.

Hi, is it possible to this to generate all valid moves for a given piece ? I've only found general purpose move generations in class MoveGenerator.
Thanks

There are a few game end relevant methods not yet implemented. I suggest adding them to improve the API further.

The methods are mentioned below, taken from the python chess API:

has_insufficient_material(color: chess.Color)

is_seventyfive_moves()
is_fivefold_repetition()

can_claim_draw()
can_claim_fifty_moves()
can_claim_threefold_repetition()

When making any of the castling king or rook moves with any piece (e.g. Qe1g1), isCastleMove on GameContext is true. It does not trigger any problems in the code but is confusing when using the method. I expect it to be only true for castling.

final Board board = new Board();
board.loadFromFen("8/5k2/8/8/8/8/5K2/4Q3 w - - 0 1");

final Move whiteQueenMoveE1G1 = new Move("e1g1", Side.WHITE);
final boolean gameContextCastleMove = board.getContext().isCastleMove(whiteQueenMoveE1G1);
System.out.println("gameContext.isCastleMove: Expected=false, Actual=" + gameContextCastleMove);

The output is as below, with latest version 1.1.23, which I think is a bit confusing:

gameContext.isCastleMove: Expected=false, Actual=true

Given the board configuration "r2k3r/p1q1b1p1/4b2p/nB1pp3/8/P2PB3/1PP2PPP/R3K2R b KQkq - 0 2". The following error is given:
com.github.bhlangonijr.chesslib.move.MoveGeneratorException: Couldn't generate Legal moves: at com.github.bhlangonijr.chesslib.move.MoveGenerator.generateLegalMoves (MoveGenerator.java:352) at com.github.bhlangonijr.chesslib.Board.legalMoves (Board.java:1399) at nl.ndl.automatedchessboard.Main.lambda$main$0 (Main.java:27) at java.lang.Thread.run (Thread.java:833) Caused by: java.lang.NullPointerException: Cannot invoke "com.github.bhlangonijr.chesslib.PieceType.equals(Object)" because "fromType" is null at com.github.bhlangonijr.chesslib.Board.isMoveLegal (Board.java:1122) at com.github.bhlangonijr.chesslib.move.MoveGenerator.lambda$generateLegalMoves$0 (MoveGenerator.java:349) at java.util.Collection.removeIf (Collection.java:576) at com.github.bhlangonijr.chesslib.move.MoveGenerator.generateLegalMoves (MoveGenerator.java:349) at com.github.bhlangonijr.chesslib.Board.legalMoves (Board.java:1399) at nl.ndl.automatedchessboard.Main.lambda$main$0 (Main.java:27) at java.lang.Thread.run (Thread.java:833)
There are still moves remaining obviously therefore there is likely a bug within the code.

Currently you can only load pgn format from a file. It is also a valid scenario to have a single pgn string that represents a single match.

Hi

First thanks for this cool API.

When performing the move e1g1 by a rook or queen the move is interpreted as castle move. That is board.getContext().isCastleMove(move) is true. Which I think should not be the case. If intended it is misleading, as not being a castle move.

But then, as a consequence, in the MoveBackup the rook castle move is set, which is wrong, as this should only be set for castling moves. From the code it looks like board.getContext().isCastleMove(move) is missing a check for the moved piece being a king.

final Board board = new Board();
board.loadFromFen("8/5k2/8/8/8/8/5K2/4R3 w - - 0 1");

final Move whiteRookMoveE1G1 = new Move("e1g1", Side.WHITE);
final boolean gameContextCastleMove = board.getContext().isCastleMove(whiteRookMoveE1G1);
System.out.println("gameContext.isCastleMove: Expected=false, Actual=" + gameContextCastleMove);
board.doMove(whiteRookMoveE1G1);
final MoveBackup moveBackup = board.getBackup().getLast();
System.out.println("moveBackup.isCastleMove: Expected=false, Actual=" + moveBackup.isCastleMove());
System.out.println("moveBackup.rookCastleMove: Expected=null, Actual=" + moveBackup.getRookCastleMove());

The output is now with latest version 1.1.22 as below, so not as expected:

gameContext.isCastleMove: Expected=false, Actual=true
moveBackup.isCastleMove: Expected=false, Actual=true
moveBackup.rookCastleMove: Expected=null, Actual=h1f1

The same is for the other king castling moves e1c1, e8g8 and e8c8. Can you please have a look. Thanks.

When using MoveList one can use SAN like "e4" to specify moves, that is fine:

final MoveList moveList = new MoveList();
moveList.loadFromSan("1. e4 Nf6 2. e5 d5 3. Bc4 Nc6 4. Bf1 Nb8 5. Bc4 Nc6 6. Bf1 Nb8");

final Board board = new Board();
final Iterator<Move> moves = moveList.iterator();
while (moves.hasNext()) {
  board.doMove(moves.next());
}

But when using the Board, I don't see an option to specify moves using SAN. I can only specify moves using a notation like board.doMove(new Move("b1c3", Side.WHITE)) (attention this is not LAN notation, which would be Nb1c3). Using SAN board.doMove(new Move("Nc3", Side.WHITE)); throws an exception. It would be a great enhancement of the API if it would support SAN here, for that is where I usually specify the moves.

For a promotion move the moving piece is set as the promoted piece in the MoveBackup. This is used as such in the code. If that must be so, I suggest using another name instead of "moving piece". The moving piece for a promotion move is a pawn, so setting the moving piece to the promoted piece is semantically wrong for my understanding. I ran into a small bug for expecting this. Example:

final Board board = new Board();
board.loadFromFen("8/P3k3/8/8/8/8/4K3/8 w - - 0 1");

final Move whitePawnPromotion = new Move("a7a8Q", Side.WHITE);
board.doMove(whitePawnPromotion);
final MoveBackup moveBackup = board.getBackup().getLast();
System.out.println("moveBackup.movingPiece: Expected=WHITE_PAWN, Actual=" + moveBackup.getMovingPiece());

The output is as below, with latest version 1.1.22, which is not as expected for me:

moveBackup.movingPiece: Expected=WHITE_PAWN, Actual=WHITE_QUEEN

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.

I suggest to include the board history, that is the initial position and move history in the board comparison. The reason for is that two equal boards (by the equals method) should behave the same for all the same future moves. E.g. the fifty-move rule and the threefold repetition rule look at the board history. E.g. now for two equal boards one can later flag a position as threefold repetition, the other not. Please check the example below.

final Board board1 = new Board();
board1.doMove(new Move("b1c3", Side.WHITE));
board1.doMove(new Move("b8c6", Side.BLACK));
board1.doMove(new Move("c3b1", Side.WHITE));
board1.doMove(new Move("c6b8", Side.BLACK));
board1.doMove(new Move("b1c3", Side.WHITE));
board1.doMove(new Move("b8c6", Side.BLACK));
board1.doMove(new Move("c3b1", Side.WHITE));

final Board board2 = new Board();
board2.loadFromFen(board1.getFen());

System.out.println(board1.equals(board2)); // true - because history is not compared

board2.doMove(new Move("c6b8", Side.BLACK));

System.out.println(board1.isRepetition()); // true - board1 sees the full history
System.out.println(board2.isRepetition()); // false - board2 hasn't got the full history

Hi,

I noticed that the Board class has responsibility for loading and outputting FEN strings, and that you have a PgnHolder for reading and writing PGNs to files. I wondered if I could contribute a more java.io idiomatic way of doing this, e.g.

Iterator<Game> iter = new PgnParser(reader).iterator();
Game game = iter.next();

PgnWriter writer = new PgnWriter(appendable);
writer.write(game);

What do you think?

Code

            if (result) {
                result = getSideToMove().equals(board.getSideToMove());
                result = result || getCastleRight(Side.WHITE).equals(board.getCastleRight(Side.WHITE));
                result = result || getCastleRight(Side.BLACK).equals(board.getCastleRight(Side.BLACK));
                result = result || getEnPassant().equals(board.getEnPassant());
            }

returns wrong result.
Operators || should be replaced with &&

The README.md could do with an example showing how to traverse a Game, both the mainline and variations.

JCenter and Bintray is shutting down so please migrate to Maven Central

Bintray Anouncement
Maven guide

I have implemented the following Custom PgnLoadListener class:

import com.github.bhlangonijr.chesslib.pgn.PgnLoadListener;

public class CustomPgnLoadListener implements PgnLoadListener {
  @Override
  public void notifyProgress(int i) {
    System.out.println("Loaded " + i + " games...");
  }
}

And my main method looks like this:

(...)
public static void main(String[] args) throws Exception {
   CustomPgnLoadListener listener = new CustomPgnLoadListener();

    PgnHolder pgn = new PgnHolder("C:\\lichess\\lichess_db_standard_rated_2013-01.pgn");
    pgn.getListener().add(listener );
    pgn.loadPgn();

    System.out.println(pgn.getGames().size() + " Games loaded");
}
(...)

Unfortunately, however, the CustomPgnLoadListener.notifyProgress method is never invoked. Am I doing something wrong?

Hello,

Is it possible to obtain a pgn string having only one board where movements have been made? The only way I see is creating a game but I can not add my board

First of all, many thanks for this library, it is very helpful.
During using it I found a bug, I suppose. Steps to reproduce are:

1. e2 e4
2. e7 e5 (or any other move)
3. e4 e2

Expected result - "e4 e2" is illegal move for pawn. The Pawn can never move backwards.
Actual result - board.doMove(move, true) returns true.

I suppose that need to change something in method isMoveLegal(Move move, boolean fullValidation) to make similar moves illegal.

When the time control tag is invalid, a PGN exception is thrown

Is this a good reason to break the whole PGN parsing process? The logic for parsing the time control tag allows that the time control tag could be absent and in that case the time control is not parsed along with the PGN, I think the same thing should happen when there is an error with parsing the time control, the library could assume its absent and not parse it.

There are a lot of apps these days with invalid time control tags like chess.com.

I would like to know what you think about this.

e.g. 1. e4 { [%clk 03:00] }

The line List<Move> moves = board.legalModes(); should be: ... legalMoves() I believe.