Fix insufficient material detection in Horde, add more test cases

CHANGELOG.md

@@ -1,6 +1,12 @@
+## 0.12.1
+
+- Fix a bug in `Horde.hasInsufficientMaterial` that would cause a stack overflow if only one pawn was left.
+- Fix insufficient material detection when the Horde has a lone Queen.
+- Horde positions where the king is white instead of black are now correctly considered invalid.
+
 ## 0.12.0
 
-- Fix an en passant bug in crackyhouse and atomichess variants. Now the perft tests
+- Fix an en passant bug in crazyhouse and atomicchess variants. Now the perft tests
   cover these variants as well.
 
 ## 0.11.1

lib/src/board.dart

@@ -267,6 +267,7 @@ class Board {
           .union(pawnAttacks(attacker.opposite, square).intersect(pawns)));
 
   /// Puts a [Piece] on a [Square] overriding the existing one, if any.
+  @useResult
   Board setPieceAt(Square square, Piece piece) {
     return removePieceAt(square).copyWith(
       occupied: occupied.withSquare(square),
@@ -283,6 +284,7 @@ class Board {
   }
 
   /// Removes the [Piece] at this [Square] if it exists.
+  @useResult
   Board removePieceAt(Square square) {
     final piece = pieceAt(square);
     return piece != null
@@ -309,11 +311,13 @@ class Board {
   }
 
   /// Returns a new board with a new [promoted] square set.
+  @useResult
   Board withPromoted(SquareSet promoted) {
     return copyWith(promoted: promoted);
   }
 
   /// Returns a copy of this board with some fields updated.
+  @useResult
   Board copyWith({
     SquareSet? occupied,
     SquareSet? promoted,

lib/src/position.dart

@@ -1913,7 +1913,7 @@ abstract class Horde extends Position {
       throw PositionSetupException.empty;
     }
 
-    if (board.kings.size != 1) {
+    if (board.kings.size != 1 || board.kingOf(Side.black) == null) {
       throw PositionSetupException.kings;
     }
 
@@ -1934,27 +1934,19 @@ abstract class Horde extends Position {
     }
   }
 
-  // get the number of light or dark square bishops
-  int _hordeBishops(Side side, SquareColor sqColor) {
-    if (sqColor == SquareColor.light) {
-      return board
-          .piecesOf(side, Role.bishop)
-          .intersect(SquareSet.lightSquares)
-          .size;
-    }
-    // dark squares
-    return board
-        .piecesOf(side, Role.bishop)
-        .intersect(SquareSet.darkSquares)
-        .size;
-  }
+  /// get the number of light or dark square bishops of the [side]
+  int _numBishops(Side side, SquareColor sqColor) => board
+      .piecesOf(side, Role.bishop)
+      .intersect(sqColor == SquareColor.light
+          ? SquareSet.lightSquares
+          : SquareSet.darkSquares)
+      .size;
 
-  SquareColor _hordeBishopColor(Side side) {
-    if (_hordeBishops(side, SquareColor.light) >= 1) {
-      return SquareColor.light;
-    }
-    return SquareColor.dark;
-  }
+  /// Number of light or dark square bishops of the horde (white)
+  int _hordeBishops(SquareColor color) => _numBishops(Side.white, color);
+
+  /// Number of light or dark square bishops of the pieces (black)
+  int _piecesBishops(SquareColor color) => _numBishops(Side.black, color);
 
   bool _hasBishopPair(Side side) {
     final bishops = board.piecesOf(side, Role.bishop);
@@ -1966,18 +1958,18 @@ abstract class Horde extends Position {
 
   @override
   bool hasInsufficientMaterial(Side side) {
-    // side with king can always win by capturing the horde
-    if (board.piecesOf(side, Role.king).isNotEmpty) {
+    // Black can always win by capturing the horde
+    if (side == Side.black) {
       return false;
     }
 
-    // now color represents horde and color.opposite is pieces
+    // now side represents horde (white) and side.opposite is pieces (black)
     final hordeNum = board.piecesOf(side, Role.pawn).size +
         board.piecesOf(side, Role.rook).size +
         board.piecesOf(side, Role.queen).size +
         board.piecesOf(side, Role.knight).size +
-        math.min(_hordeBishops(side, SquareColor.light), 2) +
-        math.min(_hordeBishops(side, SquareColor.dark), 2);
+        math.min(_hordeBishops(SquareColor.light), 2) +
+        math.min(_hordeBishops(SquareColor.dark), 2);
 
     if (hordeNum == 0) {
       return true;
@@ -1989,7 +1981,9 @@ abstract class Horde extends Position {
     }
 
     final hordeMap = board.materialCount(side);
-    final hordeBishopColor = _hordeBishopColor(side);
+    final hordeBishopColor = _hordeBishops(SquareColor.light) >= 1
+        ? SquareColor.light
+        : SquareColor.dark;
     final piecesMap = board.materialCount(side.opposite);
     final piecesNum = board.bySide(side.opposite).size;
 
@@ -2010,9 +2004,7 @@ abstract class Horde extends Position {
       return hordeNum == 2 &&
           hordeMap[Role.rook]! == 1 &&
           hordeMap[Role.bishop]! == 1 &&
-          (_pieceOfRoleNot(
-                  piecesNum, _hordeBishops(side.opposite, hordeBishopColor)) ==
-              1);
+          (_pieceOfRoleNot(piecesNum, _piecesBishops(hordeBishopColor)) == 1);
     }
 
     if (hordeNum == 1) {
@@ -2030,18 +2022,19 @@ abstract class Horde extends Position {
 
         return !(piecesMap[Role.pawn]! >= 1 ||
             piecesMap[Role.rook]! >= 1 ||
-            _hordeBishops(side.opposite, SquareColor.light) >= 2 ||
-            _hordeBishops(side, SquareColor.dark) >= 2);
+            _piecesBishops(SquareColor.light) >= 2 ||
+            _piecesBishops(SquareColor.dark) >= 2);
       } else if (hordeMap[Role.pawn] == 1) {
         // Promote the pawn to a queen or a knight and check whether white can mate.
         final pawnSquare = board.piecesOf(side, Role.pawn).last;
 
-        final promoteToQueen = copyWith();
-        promoteToQueen.board
-            .setPieceAt(pawnSquare!, Piece(color: side, role: Role.queen));
-        final promoteToKnight = copyWith();
-        promoteToKnight.board
-            .setPieceAt(pawnSquare, Piece(color: side, role: Role.knight));
+        final promoteToQueen = copyWith(
+            board: board.setPieceAt(
+                pawnSquare!, Piece(color: side, role: Role.queen)));
+
+        final promoteToKnight = copyWith(
+            board: board.setPieceAt(
+                pawnSquare, Piece(color: side, role: Role.knight)));
         return promoteToQueen.hasInsufficientMaterial(side) &&
             promoteToKnight.hasInsufficientMaterial(side);
       } else if (hordeMap[Role.rook] == 1) {
@@ -2068,8 +2061,8 @@ abstract class Horde extends Position {
         // a pawn/opposite-color-bishop on A4, a pawn/opposite-color-bishop on
         // B3, a pawn/bishop/rook/queen on A2 and any other piece on B2.
 
-        return !(_hordeBishops(side.opposite, hordeBishopColor.opposite) >= 2 ||
-            (_hordeBishops(side.opposite, hordeBishopColor.opposite) >= 1 &&
+        return !(_piecesBishops(hordeBishopColor.opposite) >= 2 ||
+            (_piecesBishops(hordeBishopColor.opposite) >= 1 &&
                 piecesMap[Role.pawn]! >= 1) ||
             piecesMap[Role.pawn]! >= 2);
       } else if (hordeMap[Role.knight] == 1) {
@@ -2092,13 +2085,12 @@ abstract class Horde extends Position {
                 (piecesMap[Role.bishop]! >= 1 && piecesMap[Role.pawn]! >= 1) ||
                 (_hasBishopPair(side.opposite) &&
                     piecesMap[Role.pawn]! >= 1)) &&
-            (_hordeBishops(side.opposite, SquareColor.light) < 2 ||
-                (_pieceOfRoleNot(piecesNum,
-                        _hordeBishops(side.opposite, SquareColor.light)) >=
+            (_piecesBishops(SquareColor.light) < 2 ||
+                (_pieceOfRoleNot(
+                        piecesNum, _piecesBishops(SquareColor.light)) >=
                     3)) &&
-            (_hordeBishops(side.opposite, SquareColor.dark) < 2 ||
-                (_pieceOfRoleNot(piecesNum,
-                        _hordeBishops(side.opposite, SquareColor.dark)) >=
+            (_piecesBishops(SquareColor.dark) < 2 ||
+                (_pieceOfRoleNot(piecesNum, _piecesBishops(SquareColor.dark)) >=
                     3)));
       }
     } else if (hordeNum == 2) {
@@ -2122,9 +2114,8 @@ abstract class Horde extends Position {
       } else if (hordeMap[Role.bishop]! >= 1 && hordeMap[Role.knight]! >= 1) {
         // horde has a bishop and a knight
         return !(piecesMap[Role.pawn]! >= 1 ||
-            _hordeBishops(side.opposite, hordeBishopColor.opposite) >= 1 ||
-            (_pieceOfRoleNot(piecesNum,
-                    _hordeBishops(side.opposite, hordeBishopColor)) >=
+            _piecesBishops(hordeBishopColor.opposite) >= 1 ||
+            (_pieceOfRoleNot(piecesNum, _piecesBishops(hordeBishopColor)) >=
                 3));
       } else {
         // The horde has two or more bishops on the same color.
@@ -2138,11 +2129,11 @@ abstract class Horde extends Position {
         // have a pawn and an opposite color bishop.
 
         return !((piecesMap[Role.pawn]! >= 1 &&
-                _hordeBishops(side.opposite, hordeBishopColor.opposite) >= 1) ||
+                _piecesBishops(hordeBishopColor.opposite) >= 1) ||
             (piecesMap[Role.pawn]! >= 1 && piecesMap[Role.knight]! >= 1) ||
-            (_hordeBishops(side.opposite, hordeBishopColor.opposite) >= 1 &&
+            (_piecesBishops(hordeBishopColor.opposite) >= 1 &&
                 piecesMap[Role.knight]! >= 1) ||
-            (_hordeBishops(side.opposite, hordeBishopColor.opposite) >= 2) ||
+            (_piecesBishops(hordeBishopColor.opposite) >= 2) ||
             piecesMap[Role.knight]! >= 2 ||
             piecesMap[Role.pawn]! >= 2);
       }

test/position_test.dart

@@ -1,6 +1,7 @@
 import 'package:fast_immutable_collections/fast_immutable_collections.dart';
 import 'package:dartchess/dartchess.dart';
 import 'package:test/test.dart';
+import 'dart:io' as io;
 
 void main() {
   group('Position', () {
@@ -1116,19 +1117,67 @@ void main() {
   });
 
   group('Horde', () {
-    test('insufficient material', () {
-      for (final test in [
-        ['8/5k2/8/8/8/4NN2/8/8 w - - 0 1', true, false],
-        ['8/8/8/2B5/p7/kp6/pq6/8 b - - 0 1', false, false],
-        ['8/8/8/2B5/r7/kn6/nr6/8 b - - 0 1', true, false],
-        ['8/8/1N6/rb6/kr6/qn6/8/8 b - - 0 1', false, false],
-        ['8/8/1N6/qq6/kq6/nq6/8/8 b - - 0 1', true, false],
-        ['8/P1P5/8/8/8/8/brqqn3/k7 b - - 0 1', false, false],
-      ]) {
-        final pos = Horde.fromSetup(Setup.parseFen(test[0] as String));
-        expect(pos.hasInsufficientMaterial(Side.white), test[1]);
-        expect(pos.hasInsufficientMaterial(Side.black), test[2]);
+    group('insufficient material', () {
+      for (final line
+          in io.File('test/resources/horde_insufficient_material.csv')
+              .readAsLinesSync()) {
+        final [fen, expected, tag] = line.split(',');
+        test('[$tag] $fen', () {
+          final pos = Horde.fromSetup(Setup.parseFen(fen));
+          expect(pos.hasInsufficientMaterial(Side.white), expected == 'true');
+          expect(pos.hasInsufficientMaterial(Side.black), false);
+        });
       }
     });
+
+    group('Position validation', () {
+      test('Empty board', () {
+        expect(
+            () => Horde.fromSetup(Setup.parseFen(kEmptyFEN)),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.empty)));
+      });
+      test('Missing kings', () {
+        expect(
+            () => Horde.fromSetup(Setup.parseFen(
+                'rnbq1bnr/pppppppp/8/1PP2PP1/PPPPPPPP/PPPPPPPP/PPPPPPPP/PPPPPPPP w - - 0 1')),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.kings)));
+      });
+      test('King is white', () {
+        expect(
+            () => Horde.fromSetup(Setup.parseFen(
+                'rnbq1bnr/pppppppp/8/1PPK1PP1/PPPPPPPP/PPPPPPPP/PPPPPPPP/PPPPPPPP w - - 0 1')),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.kings)));
+      });
+      test('Both sides have a king', () {
+        expect(
+            () => Horde.fromSetup(Setup.parseFen(
+                'rnbqkbnr/pppppppp/8/1PPK1PP1/PPPPPPPP/PPPPPPPP/PPPPPPPP/PPPPPPPP w kq - 0 1')),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.kings)));
+      });
+      test('Opposite check', () {
+        expect(
+            () => Horde.fromSetup(
+                Setup.parseFen('3k4/8/1B6/8/8/8/8/8 w - - 0 1')),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.oppositeCheck)));
+      });
+      test('Backrank pawns (black)', () {
+        expect(
+            () => Horde.fromSetup(
+                Setup.parseFen('2k3p1/8/8/8/8/3P4/8/8 w - - 0 1')),
+            throwsA(predicate((e) =>
+                e is PositionSetupException &&
+                e.cause == IllegalSetupCause.pawnsOnBackrank)));
+      });
+    });
   });
 }