Merge pull request #60 from BlazingTwist/33-fix-castle-king-check

Fix: Castling verify KingMove not attacked

src/main/java/jchess/common/moveset/special/Castling.java

@@ -41,7 +41,7 @@ public Castling(
             IChessGame game, PieceIdentifier kingId, PieceType rookTypeId, int kingToRookDir, int numStepsKing
     ) {
         this.game = game;
-        this.kingMove = repeat(filter(neighbor(kingToRookDir), Castling::kingStepFilter), numStepsKing, numStepsKing, true).toV1(kingId);
+        this.kingMove = repeat(filter(neighbor(kingToRookDir), this::kingStepFilter), numStepsKing, numStepsKing, true).toV1(kingId);
         this.kingId = kingId;
         this.rookTypeId = rookTypeId;
         this.kingToRookDir = kingToRookDir;
@@ -57,11 +57,11 @@ public Castling(
         game.getEventManager().getEvent(PieceMoveEvent.class).addListener(this::onPieceMove);
     }
 
-    private static boolean kingStepFilter(Entity moveTo) {
+    private boolean kingStepFilter(Entity moveTo) {
         // Castling requires that:
         // - the tiles the king moves over are empty
         // - the tiles the king moves over are not attacked
-        return moveTo.piece == null && !moveTo.isAttacked();
+        return moveTo.piece == null && !moveTo.isAttacked(kingId.ownerId());
     }
 
     private void lookupRook() {

src/main/java/jchess/ecs/Entity.java

@@ -25,11 +25,13 @@ public Stream<MoveIntention> findValidMoves(IChessGame game, boolean verifyKingS
 
     public boolean isAttacked() {
         if (tile == null || piece == null) return false;
+        return isAttacked(piece.identifier.ownerId());
+    }
 
-        final int ownerId = piece.identifier.ownerId();
-        return tile.attackingPieces.stream().anyMatch(attacker -> {
+    public boolean isAttacked(int attackedPlayer) {
+        return tile != null && tile.attackingPieces.stream().anyMatch(attacker -> {
             assert attacker.piece != null; // attacker must be a piece.
-            return attacker.piece.identifier.ownerId() != ownerId;
+            return attacker.piece.identifier.ownerId() != attackedPlayer;
         });
     }
 }

src/test/java/jchess/RegressionTests.java

@@ -9,6 +9,9 @@
 import jchess.gamemode.hex3p.Hex3PlayerGame;
 import jchess.gamemode.hex3p.Hex3pPieceLayouts;
 import jchess.gamemode.hex3p.Hex3pPieces;
+import jchess.gamemode.square2p.Square2PlayerGame;
+import jchess.gamemode.square2p.Square2pPieceLayouts;
+import jchess.gamemode.square2p.Square2pPieces;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -21,6 +24,10 @@ private static Entity getTileAtPosition(IChessGame game, int x, int y) {
                 .findFirst().orElse(null);
     }
 
+    private static void movePiece(IChessGame game, Entity from, Entity to) {
+        NormalMove.getMove(game, from, to).onClick().run();
+    }
+
     /**
      * Simulates an error-condition caused by incorrect MoveSimulator#revert logic, as described in Issue #27
      */
@@ -53,4 +60,42 @@ public void test_issue27_kingCheckException() {
 
         Assertions.assertDoesNotThrow(() -> game.getEventManager().getEvent(BoardClickedEvent.class).fire(distance2.tile.position));
     }
+
+    @Test
+    public void test_issue33_castleMoveNoCheck() {
+        Square2PlayerGame game = new Square2PlayerGame(new PieceStore(Square2pPieces.values()), Square2pPieceLayouts.Standard);
+        game.start();
+
+        Entity x1 = getTileAtPosition(game, 1, 0);
+        Entity x2 = getTileAtPosition(game, 2, 0);
+        Entity x3 = getTileAtPosition(game, 3, 0);
+        Entity x4 = getTileAtPosition(game, 4, 1);
+
+        Entity blackKingTile = getTileAtPosition(game, 4, 0);
+
+        Entity bishop0 = getTileAtPosition(game, 2, 7);
+        Entity bishop1 = getTileAtPosition(game, 6, 3);
+
+        x1.piece = null;
+        x2.piece = null;
+        x3.piece = null;
+        x4.piece = null;
+
+        movePiece(game, bishop0, bishop1);
+
+        Assertions.assertTrue(blackKingTile.piece != null && blackKingTile.tile != null);
+        Assertions.assertNotNull(x3.tile);
+
+        Assertions.assertSame(blackKingTile.piece.identifier.pieceType(), PieceType.KING);
+        Assertions.assertSame(blackKingTile.piece.identifier.ownerId(), 1);
+
+        Assertions.assertTrue(x3.isAttacked(1), "white bishop should be attacking this tile");
+        Assertions.assertFalse(blackKingTile.findValidMoves(game, true).anyMatch(move -> move.displayTile() == x2),
+                "black king should not be able to castle");
+
+        movePiece(game, bishop1, bishop0);
+        Assertions.assertFalse(x3.isAttacked(1), "white bishop should not be attacking this tile anymore");
+        Assertions.assertTrue(blackKingTile.findValidMoves(game, true).anyMatch(move -> move.displayTile() == x2),
+                "black king should be able to castle");
+    }
 }