Fixes Chess960 X-FEN parser and generator

README.md

@@ -38,7 +38,6 @@ Some code may seem not very elegant as it uses "old fashion" *for* structures in
     - Implement Quiesce moves
 - UCI stuff needs to be tested (especially if stop works with JChessEngine and perfT test that probably use the last move comparator, which is perfectly useless - Engine should have a perfT mode or, at least a way to set in perftT test if moves are sorted or not).  
 For PerfT stuff, making ...generic.MovesBuilder#getMoves writing something to System.out when moveComparator is null, could be helpful.
-- Finish FENParser for Chess960
 - Implement Capablanca chess?
 - General things ... if it does not alter performance too much:
     - Move generation improvements:

src/main/java/com/fathzer/jchess/Board.java

@@ -51,8 +51,10 @@ default boolean isWhiteToMove() {
 
 	/** Gets the initial rook position of a castling.
 	 * @param castling The castling
-	 * @return The initial position of the rook involved in the castling.
+	 * @return The initial position of the rook involved in the castling.<br>
+	 * Please note that the returned value is unpredictable if {@link #hasCastling(Castling)} returns false.
 	 * @see Board#getCoordinatesSystem()
+	 * @see #hasCastling(Castling)
 	 */
 	int getInitialRookPosition(Castling castling);
 
@@ -69,20 +71,13 @@ default boolean isWhiteToMove() {
 	 * @param from The king's starting position.
 	 * @param to The king's end position.
 	 * <br>Please note this position is the representation of the king's destination in the encoded move,
-	 * not necessarily the 'effective' king's position after the move. For example, in chess360, the castling move
+	 * not necessarily the 'effective' king's position after the move. For example, in chess960, the castling move
 	 * is encoded as 'king moves to the rook it castles with', but the 'effective' end position is the same as in standard chess.
-	 * @return The castling if the move is a castling, null if it is a king standard move.
-	 * <br>The default implementation, returns true if the king moves more than 1 cell or on a cell occupied by a rook of the same color).
+	 * @return The castling if the move is a castling, null if it is a king standard move.<br>
+	 * Please note it is perfectly legal to return here a castling prohibited by some attacked or non empty cells. In other words,
+	 * this method check if the move is a castling, not if it is valid.
 	 */
-	default Castling getCastling(int from, int to) {
-		final int offset = Math.abs(to-from);
-		boolean castling = offset>=2 && (getCoordinatesSystem().getRow(from)==getCoordinatesSystem().getRow(to));
-		if (!castling) {
-			final Piece rook = isWhiteToMove() ? Piece.WHITE_ROOK : Piece.BLACK_ROOK;
-			castling = rook.equals(getPiece(to));
-		}
-		return castling ? Castling.get(getActiveColor(), to>from) : null;
-	}
+	Castling getCastling(int from, int to);
 
 	Piece getPiece(int position);
 

src/main/java/com/fathzer/jchess/chess960/Chess960Board.java

@@ -68,6 +68,17 @@ private void fillRookPosition(PieceWithPosition p) {
 			initialRookPositions[castling.ordinal()] = position;
 		}
 	}
+
+	@Override
+	public Castling getCastling(int from, int to) {
+		if (getCoordinatesSystem().getRow(from)!=getCoordinatesSystem().getRow(to)) {
+			// Doesn't move horizontally or by only one cell
+			return null;
+		}
+		final Castling castling = Castling.get(getActiveColor(), to>from);
+		return to==getInitialRookPosition(castling) ? castling : null;
+	}
+
 
 	@Override
 	protected void copy(Board<Move> other) {

src/main/java/com/fathzer/jchess/fen/FENParser.java

@@ -193,27 +193,53 @@ private int[] getInitialRookColumns(Dimension dimension, List<PieceWithPosition>
 			if (castlings.isEmpty()) {
 				return null;
 			}
-			final int[] positions = new int[Castling.ALL.size()];
-			Arrays.fill(positions, -1);
+			final int[] columns = new int[Castling.ALL.size()];
+			Arrays.fill(columns, -1);
 			final int blackKingColumn = getColumn(pieces, p->p.getPiece()==BLACK_KING);
 			final int whiteKingColumn = getColumn(pieces, p->p.getPiece()==WHITE_KING);
 			final int defaultKingColumn = dimension.getWidth()/2;
 			boolean isDefault = true; // isDefault will remain true if kings and rooks involved in castling are at their default position
 			for (Castling castling : castlings) {
 				final int kingRow = castling.getColor()==WHITE ? dimension.getHeight()-1 : 0;
 				final int kingColumn = castling.getColor()==WHITE ? whiteKingColumn : blackKingColumn;
-				//TODO Support inner rook position as start position
-				// Initial rook position is the furthest rook from the king
-				int rookPosition = getFurthest(pieces, castling.getColor()==BLACK ? BLACK_ROOK : WHITE_ROOK, kingRow, kingColumn, castling.getSide());
-				isDefault = isDefault && rookPosition==getStandardRookColumn(dimension, castling) && kingColumn==defaultKingColumn;
-				positions[castling.ordinal()] = rookPosition;
+				// Check for chess 960 like rook start position
+				int rookColumn = getRookStartColumn(castlingsString, kingColumn, dimension.getWidth(), castling);
+				if (rookColumn<0) {
+					// Initial rook position is the furthest rook from the king
+					rookColumn = getFurthest(pieces, castling.getColor()==BLACK ? BLACK_ROOK : WHITE_ROOK, kingRow, kingColumn, castling.getSide());
+				}
+				isDefault = isDefault && rookColumn==getStandardRookColumn(dimension, castling) && kingColumn==defaultKingColumn;
+				columns[castling.ordinal()] = rookColumn;
 			}
-			return isDefault ? null : positions;
+			return isDefault ? null : columns;
 		} catch (NoSuchElementException e) {
 			throw new IllegalArgumentException(e);
 		}
 	}
 
+	private static int getRookStartColumn(String value, int kingColumn, int width, Castling castling) {
+		char firstColumn = castling.getColor()==WHITE ? 'A':'a';
+		char c = (char) (firstColumn+kingColumn);
+		if (castling.getSide()==KING) {
+			char max = (char) (firstColumn+width);
+			while (c<max) {
+				c++;
+				if (value.indexOf(c)>=0) {
+					return c-firstColumn;
+				}
+			}
+		} else {
+			char min = firstColumn;
+			while (c>min) {
+				c--;
+				if (value.indexOf(c)>=0) {
+					return c-firstColumn;
+				}
+			}
+		}
+		return -1;
+	}
+
 	private int getColumn(List<PieceWithPosition> pieces, Predicate<PieceWithPosition> p) {
 		return pieces.stream().filter(p).findAny().orElseThrow().getColumn();
 	}

src/main/java/com/fathzer/jchess/fen/FENUtils.java

@@ -1,7 +1,11 @@
 package com.fathzer.jchess.fen;
 
+import java.util.stream.IntStream;
+
+import com.fathzer.games.Color;
 import com.fathzer.jchess.Board;
 import com.fathzer.jchess.Castling;
+import com.fathzer.jchess.Castling.Side;
 import com.fathzer.jchess.Move;
 import com.fathzer.jchess.Piece;
 
@@ -12,7 +16,6 @@ public class FENUtils {
 	public static final String NEW_STANDARD_GAME = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1";
 
 	public static String to(Board<Move> board) {
-		//TODO output X-FEN castling when chess960 needs it
 		final StringBuilder b = new StringBuilder();
 		for (int i = 0; i <board.getDimension().getHeight() ; i++) {
 			b.append(getRow(board, i));
@@ -33,13 +36,33 @@ public static String to(Board<Move> board) {
 		return b.toString();
 	}
 
-	protected static void addCastlings(final StringBuilder b, Board<Move> board) {
+	private static void addCastlings(final StringBuilder b, Board<Move> board) {
 		final int initialSize = b.length();
-		Castling.ALL.stream().filter(board::hasCastling).forEach(c -> b.append(c.getCode()));
+		Castling.ALL.stream().filter(board::hasCastling).forEach(c -> b.append(getCode(board, c)));
 		if (b.length()==initialSize) {
 			b.append('-');
 		}
 	}
+
+	private static String getCode(Board<Move> board, Castling castling) {
+		final int rookPos = board.getInitialRookPosition(castling);
+		int column = board.getCoordinatesSystem().getColumn(rookPos);
+		if (column==0 || column==board.getDimension().getWidth()-1) {
+			// If standard rook position, return the FEN code
+			return castling.getCode();
+		}
+		// Check if we should use X-FEN code => Check if there's another rook farther from the king than the initial rook's position
+		final boolean isWhite = castling.getColor()==Color.WHITE;
+		final Piece searched = isWhite ? Piece.WHITE_ROOK : Piece.BLACK_ROOK;
+		final IntStream indexes = castling.getSide()==Side.KING ? IntStream.range(rookPos+1, rookPos-column+board.getDimension().getWidth()) :
+			IntStream.range(rookPos-column, rookPos);
+		final boolean isFursthest = indexes.noneMatch(i -> searched.equals(board.getPiece(i)));
+		if (isFursthest) {
+			return castling.getCode();
+		} else {
+			return String.valueOf((char)((isWhite ? 'A' : 'a')+column)); 
+		}
+	}
 
 	private static CharSequence getRow(Board<Move> board, int row) {
 		final StringBuilder b = new StringBuilder();
@@ -65,5 +88,4 @@ private static CharSequence getRow(Board<Move> board, int row) {
 	public static Board<Move> from(String fen) {
 		return new FENParser(fen).get();
 	}
-
 }

src/main/java/com/fathzer/jchess/generic/ChessBoard.java

@@ -390,7 +390,8 @@ private void onRookEvent(int cell) {
 	 * <br>This generic implementation returns the corner that contains the rook in standard chess
 	 */
 	@Override
-	public int getInitialRookPosition(Castling castling) { //TODO Possible to optimize using same structure as in chess960
+	public int getInitialRookPosition(Castling castling) {
+		//TODO Possible to optimize using same structure as in chess960
 		final CoordinatesSystem cs = board.getCoordinatesSystem();
 		if (Castling.BLACK_QUEEN_SIDE.equals(castling)) {
 			return cs.getIndex(0, 0);

src/main/java/com/fathzer/jchess/standard/StandardBoard.java

@@ -40,4 +40,13 @@ private void checkCastling(Castling castling) {
 			throw new IllegalArgumentException("Invalid castling: Rook is not at its initial position");
 		}
 	}
+
+	@Override
+	public Castling getCastling(int from, int to) {
+		if (Math.abs(to-from)<2 || getCoordinatesSystem().getRow(from)!=getCoordinatesSystem().getRow(to)) {
+			// Doesn't move horizontally or by only one cell
+			return null;
+		}
+		return Castling.get(getActiveColor(), to>from);
+	}
 }

src/test/java/com/fathzer/jchess/fen/FENParserTest.java

@@ -21,6 +21,9 @@ void test() {
 		assertEquals(Piece.WHITE_KING, board.getPiece(cs.getIndex("e1")));
 		assertEquals(Piece.WHITE_QUEEN, board.getPiece(cs.getIndex("e4")));
 		assertEquals(Piece.BLACK_BISHOP, board.getPiece(cs.getIndex("h6")));
+		assertEquals(cs.getIndex("h1"), board.getInitialRookPosition(Castling.WHITE_KING_SIDE));
+		assertEquals(cs.getIndex("a1"), board.getInitialRookPosition(Castling.WHITE_QUEEN_SIDE));
+		assertEquals(cs.getIndex("a8"), board.getInitialRookPosition(Castling.BLACK_QUEEN_SIDE));
 
 		assertEquals(fen, FENUtils.to(board));
 
@@ -45,15 +48,26 @@ void bug20230601() {
 
 	@Test
 	void test960() {
-		final String fen = "nbbqrknr/pppppppp/8/8/8/8/PPPPPPPP/NBBQRKNR w KQkq - 0 1";
-		Board<Move> board = FENUtils.from(fen);
+		final var fen = "nbbqrknr/pppppppp/8/8/8/8/PPPPPPPP/NBBQRKNR w KQkq - 0 1";
+		var board = FENUtils.from(fen);
 		assertEquals(fen, FENUtils.to(board));
 
 		final String fenWithInnerRook = "rn2k1r1/ppp1pp1p/3p2p1/5bn1/P7/2N2B2/1PPPPP2/2BNK1RR w Gkq - 4 11";
 		board = FENUtils.from(fenWithInnerRook);
 		assertTrue(board.hasCastling(Castling.WHITE_KING_SIDE));     
 		assertFalse(board.hasCastling(Castling.WHITE_QUEEN_SIDE));     
 		assertTrue(board.hasCastling(Castling.BLACK_KING_SIDE));     
-		assertTrue(board.hasCastling(Castling.BLACK_QUEEN_SIDE));     
+		assertTrue(board.hasCastling(Castling.BLACK_QUEEN_SIDE));
+
+		final CoordinatesSystem cs = board.getCoordinatesSystem();
+		assertEquals(cs.getIndex("g1"), board.getInitialRookPosition(Castling.WHITE_KING_SIDE));
+		assertEquals(cs.getIndex("g8"), board.getInitialRookPosition(Castling.BLACK_KING_SIDE));
+		assertEquals(cs.getIndex("a8"), board.getInitialRookPosition(Castling.BLACK_QUEEN_SIDE));
+
+		assertEquals(fenWithInnerRook, FENUtils.to(board));
+
+		final String otherFenWithInnerRook = "1r2k1r1/ppp1pp2/3p2pp/5bn1/P7/2N2B2/1PPPPP2/RR2K3 w Bkq - 4 11";
+		board = FENUtils.from(otherFenWithInnerRook);
+		assertEquals(otherFenWithInnerRook, FENUtils.to(board));
 	}
 }

src/test/java/com/fathzer/jchess/fischerrandom/ChessBoardTest.java → src/test/java/com/fathzer/jchess/fischerrandom/Chess960BoardTest.java

@@ -7,7 +7,10 @@
 
 import org.junit.jupiter.api.Test;
 
+import com.fathzer.games.Color;
+import com.fathzer.games.MoveGenerator.MoveConfidence;
 import com.fathzer.jchess.Board;
+import com.fathzer.jchess.Castling;
 import com.fathzer.jchess.CoordinatesSystem;
 import com.fathzer.jchess.Move;
 import com.fathzer.jchess.MoveBuilder;
@@ -16,8 +19,9 @@
 import com.fathzer.jchess.chess960.Chess960Board;
 import com.fathzer.jchess.fen.FENParser;
 import com.fathzer.jchess.fen.FENUtils;
+import com.fathzer.jchess.generic.BasicMove;
 
-class ChessBoardTest implements MoveBuilder {
+class Chess960BoardTest implements MoveBuilder {
 	@Test
 	void test() {
 		final List<PieceWithPosition> pieces = new FENParser("rnbqkbnr/pppppppp/8/8/8/2PP4/PP2PPPP/2RK3R w - - 0 1").getPieces();
@@ -64,4 +68,39 @@ void testTrickyLegalCastling() {
 		board.unmakeMove();
 		assertTrue(board.getLegalMoves().contains(move));
 	}
+
+	@Test
+	void notRightRookCastling() {
+		final String fenWithRook = "rn2k2r/ppp1pp1p/3p2p1/5bn1/P7/2N2B2/1PPPPP2/2BNK1RR w Kkq - 4 11";
+		var board = FENUtils.from(fenWithRook);
+		final CoordinatesSystem cs = board.getCoordinatesSystem();
+//		// Verify e1-g1 which is a castling with the wrong rook is not in legal moves
+//		assertFalse(board.getLegalMoves().stream().map(m->m.toString(cs)).toList().contains("e1-g1"));
+//		// Can't castling with the wrong rook
+//		final BasicMove wrongMove = new BasicMove(board.getKingPosition(Color.WHITE), cs.getIndex("g1"));
+//		assertFalse(board.makeMove(wrongMove, MoveConfidence.UNSAFE));
+//		if (board.getMoves().stream().map(m->m.toString(cs)).toList().contains(wrongMove.toString(cs))) {
+//			assertFalse(board.makeMove(wrongMove, MoveConfidence.PSEUDO_LEGAL));
+//		}
+
+		final String fenWithInnerRook = "rn2k1r1/ppp1pp1p/3p2p1/5bn1/P7/2N2B2/1PPPPP2/2BNK1RR w Gkq - 4 11";
+		board = FENUtils.from(fenWithInnerRook);
+		assertTrue(board.hasCastling(Castling.WHITE_KING_SIDE));     
+		assertFalse(board.hasCastling(Castling.WHITE_QUEEN_SIDE));     
+		assertTrue(board.hasCastling(Castling.BLACK_KING_SIDE));     
+		assertTrue(board.hasCastling(Castling.BLACK_QUEEN_SIDE));
+
+		assertEquals(cs.getIndex("g1"), board.getInitialRookPosition(Castling.WHITE_KING_SIDE));
+		// Verify e1-h1 which is a castling with the wrong rook is not in legal moves
+		assertFalse(board.getLegalMoves().stream().map(m->m.toString(cs)).toList().contains("e1-h1"));
+		// Verify making e1-h1 move fails
+		assertFalse(board.makeMove(new BasicMove(board.getKingPosition(Color.WHITE), cs.getIndex("h1")), MoveConfidence.UNSAFE));
+		// Verify castling with the right rook succeeds
+		assertTrue(board.makeMove(new BasicMove(board.getKingPosition(Color.WHITE), cs.getIndex("g1")), MoveConfidence.UNSAFE));
+		board.unmakeMove();
+		// Verify it still succeeds if wrong rook moves
+		assertTrue(board.makeMove(new BasicMove(cs.getIndex("h1"), cs.getIndex("h2")), MoveConfidence.UNSAFE));
+		assertTrue(board.makeMove(new BasicMove(cs.getIndex("h7"), cs.getIndex("h6")), MoveConfidence.UNSAFE));
+		assertTrue(board.makeMove(new BasicMove(board.getKingPosition(Color.WHITE), cs.getIndex("g1")), MoveConfidence.UNSAFE));
+	}
 }