⚡ Reimplement repetition detection

src/Lynx/Constants.cs

@@ -296,6 +296,8 @@ public static class Constants
     /// </summary>
     public const int MaxNumberOfPossibleMovesInAPosition = 250;
 
+    public const int MaxNumberMovesInAGame = 1024;
+
     public static readonly int SideLimit = Enum.GetValues(typeof(Piece)).Length / 2;
 
     public static readonly int[] Rank =

src/Lynx/Model/Game.cs

@@ -11,7 +11,7 @@ public sealed class Game
     public List<Move> MoveHistory { get; }
 #endif
 
-    public HashSet<long> PositionHashHistory { get; }
+    public List<long> PositionHashHistory { get; }
 
     public int HalfMovesWithoutCaptureOrPawnMove { get; set; }
 
@@ -33,11 +33,11 @@ public Game(ReadOnlySpan<char> fen)
             _logger.Warn($"Invalid position detected: {fen.ToString()}");
         }
 
-        PositionHashHistory = new(1024) { CurrentPosition.UniqueIdentifier };
+        PositionHashHistory = new(Constants.MaxNumberMovesInAGame) { CurrentPosition.UniqueIdentifier };
         HalfMovesWithoutCaptureOrPawnMove = parsedFen.HalfMoveClock;
 
 #if DEBUG
-        MoveHistory = new(1024);
+        MoveHistory = new(Constants.MaxNumberMovesInAGame);
 #endif
     }
 
@@ -78,8 +78,9 @@ public Game(ReadOnlySpan<char> fen, ReadOnlySpan<char> rawMoves, Span<Range> ran
     ///     At depth 3, there's a capture, but the eval should still be 0
     ///     At depth 4 there's no capture, but the eval should still be 0
     /// </remarks>
+    /// <returns>true if threefol/50 moves repetition is possible (since both captures and pawn moves are irreversible)</returns>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public void Update50movesRule(Move moveToPlay, bool isCapture)
+    public bool Update50movesRule(Move moveToPlay, bool isCapture)
     {
         if (isCapture)
         {
@@ -91,19 +92,26 @@ public void Update50movesRule(Move moveToPlay, bool isCapture)
             {
                 ++HalfMovesWithoutCaptureOrPawnMove;
             }
+
+            return false;
         }
         else
         {
             var pieceToMove = moveToPlay.Piece();
 
-            if ((pieceToMove == (int)Piece.P || pieceToMove == (int)Piece.p) && HalfMovesWithoutCaptureOrPawnMove < 100)
+            if (pieceToMove == (int)Piece.P || pieceToMove == (int)Piece.p)
             {
-                HalfMovesWithoutCaptureOrPawnMove = 0;
-            }
-            else
-            {
-                ++HalfMovesWithoutCaptureOrPawnMove;
+                if (HalfMovesWithoutCaptureOrPawnMove < 100)
+                {
+                    HalfMovesWithoutCaptureOrPawnMove = 0;
+                }
+
+                return false;
             }
+
+            ++HalfMovesWithoutCaptureOrPawnMove;
+
+            return true;
         }
     }
 
@@ -112,7 +120,22 @@ public void Update50movesRule(Move moveToPlay, bool isCapture)
     /// </summary>
     /// <returns></returns>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public bool IsThreefoldRepetition(Position position) => PositionHashHistory.Contains(position.UniqueIdentifier);
+    public bool IsThreefoldRepetition()
+    {
+        var currentHash = CurrentPosition.UniqueIdentifier;
+
+        // [Count - 1] would be the last one, we want to start searching 2 ealier and finish HalfMovesWithoutCaptureOrPawnMove earlier
+        var limit = Math.Max(0, PositionHashHistory.Count - 1 - HalfMovesWithoutCaptureOrPawnMove);
+        for (int i = PositionHashHistory.Count - 3; i >= limit; i -= 2)
+        {
+            if (currentHash == PositionHashHistory[i])
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public bool Is50MovesRepetition()
@@ -126,13 +149,28 @@ public bool Is50MovesRepetition()
     }
 
     /// <summary>
-    /// To be used in online tb proving only, with a copy of <see cref="PositionHashHistory"/>
+    /// To be used in online tb proving only, with a copy of <see cref="PositionHashHistory"/> that hasn't been updated with <paramref name="position"/>
     /// </summary>
     /// <param name="positionHashHistory"></param>
     /// <param name="position"></param>
     /// <returns></returns>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public static bool IsThreefoldRepetition(HashSet<long> positionHashHistory, Position position) => positionHashHistory.Contains(position.UniqueIdentifier);
+    public static bool IsThreefoldRepetition(List<long> positionHashHistory, Position position, int halfMovesWithoutCaptureOrPawnMove = Constants.MaxNumberMovesInAGame)
+    {
+        var currentHash = position.UniqueIdentifier;
+
+        // Since positionHashHistory hasn't been updated with position, [Count] would be the last one, so we want to start searching 2 ealier
+        var limit = Math.Max(0, positionHashHistory.Count - halfMovesWithoutCaptureOrPawnMove);
+        for (int i = positionHashHistory.Count - 2; i >= limit; i -= 2)
+        {
+            if (currentHash == positionHashHistory[i])
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
 
     /// <summary>
     /// To be used in online tb proving only, with a copy of <see cref="HalfMovesWithoutCaptureOrPawnMove"/>

src/Lynx/OnlineTablebaseProber.cs

@@ -43,7 +43,7 @@ public static class OnlineTablebaseProber
         TypeInfoResolver = SourceGenerationContext.Default
     };
 
-    public static async Task<(int MateScore, Move BestMove)> RootSearch(Position position, HashSet<long> positionHashHistory, int halfMovesWithoutCaptureOrPawnMove, CancellationToken cancellationToken)
+    public static async Task<(int MateScore, Move BestMove)> RootSearch(Position position, List<long> positionHashHistory, int halfMovesWithoutCaptureOrPawnMove, CancellationToken cancellationToken)
     {
         var fen = position.FEN(halfMovesWithoutCaptureOrPawnMove);
         _logger.Info("[{0}] Querying online tb for position {1}", nameof(RootSearch), fen);

src/Lynx/Search/NegaMax.cs

@@ -219,36 +219,22 @@ private int NegaMax(int depth, int ply, int alpha, int beta, bool parentWasNullM
 
             // Before making a move
             var oldHalfMovesWithoutCaptureOrPawnMove = Game.HalfMovesWithoutCaptureOrPawnMove;
-            Game.Update50movesRule(move, isCapture);
-            var isThreeFoldRepetition = !Game.PositionHashHistory.Add(position.UniqueIdentifier);
+            var canBeRepetition = Game.Update50movesRule(move, isCapture);
+            Game.PositionHashHistory.Add(position.UniqueIdentifier);
 
             int evaluation;
-            if (isThreeFoldRepetition)
+            if (canBeRepetition && (Game.IsThreefoldRepetition() || Game.Is50MovesRepetition()))
             {
                 evaluation = 0;
 
                 // We don't need to evaluate further down to know it's a draw.
                 // Since we won't be evaluating further down, we need to clear the PV table because those moves there
                 // don't belong to this line and if this move were to beat alpha, they'd incorrectly copied to pv line.
                 Array.Clear(_pVTable, nextPvIndex, _pVTable.Length - nextPvIndex);
-
-                // This is the only case were we don't clear position.UniqueIdentifier from Game.PositionHashHistory, because it was already there before making the move
-            }
-            else if (Game.Is50MovesRepetition())
-            {
-                evaluation = 0;
-
-                // We don't need to evaluate further down to know it's a draw.
-                // Since we won't be evaluating further down, we need to clear the PV table because those moves there
-                // don't belong to this line and if this move were to beat alpha, they'd incorrectly copied to pv line.
-                Array.Clear(_pVTable, nextPvIndex, _pVTable.Length - nextPvIndex);
-
-                Game.PositionHashHistory.Remove(position.UniqueIdentifier);
             }
             else if (pvNode && movesSearched == 0)
             {
                 evaluation = -NegaMax(depth - 1, ply + 1, -beta, -alpha);
-                Game.PositionHashHistory.Remove(position.UniqueIdentifier);
             }
             else
             {
@@ -262,7 +248,7 @@ private int NegaMax(int depth, int ply, int alpha, int beta, bool parentWasNullM
                 {
                     // After making a move
                     Game.HalfMovesWithoutCaptureOrPawnMove = oldHalfMovesWithoutCaptureOrPawnMove;
-                    Game.PositionHashHistory.Remove(position.UniqueIdentifier); // We know that there's no triple repetition here
+                    Game.PositionHashHistory.RemoveAt(Game.PositionHashHistory.Count - 1);
                     position.UnmakeMove(move, gameState);
 
                     break;
@@ -324,13 +310,12 @@ private int NegaMax(int depth, int ply, int alpha, int beta, bool parentWasNullM
                     // PVS Hipothesis invalidated -> search with full depth and full score bandwidth
                     evaluation = -NegaMax(depth - 1, ply + 1, -beta, -alpha);
                 }
-
-                Game.PositionHashHistory.Remove(position.UniqueIdentifier);
             }
 
             // After making a move
             // Game.PositionHashHistory is update above
             Game.HalfMovesWithoutCaptureOrPawnMove = oldHalfMovesWithoutCaptureOrPawnMove;
+            Game.PositionHashHistory.RemoveAt(Game.PositionHashHistory.Count - 1);
             position.UnmakeMove(move, gameState);
 
             PrintMove(ply, move, evaluation);

src/Lynx/Search/OnlineTablebase.cs

@@ -5,7 +5,7 @@
 namespace Lynx;
 public sealed partial class Engine
 {
-    public async Task<SearchResult?> ProbeOnlineTablebase(Position position, HashSet<long> positionHashHistory, int halfMovesWithoutCaptureOrPawnMove)
+    public async Task<SearchResult?> ProbeOnlineTablebase(Position position, List<long> positionHashHistory, int halfMovesWithoutCaptureOrPawnMove)
     {
         var stopWatch = Stopwatch.StartNew();
 

tests/Lynx.Test/GameTest.cs

@@ -26,25 +26,20 @@ public void IsThreefoldRepetition_1()
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[1]));
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[2]));
 
-        var newPosition = new Position(game.CurrentPosition, repeatedMoves[3]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[3]));
+        game.MakeMove(repeatedMoves[3]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[4]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[4]));
+        game.MakeMove(repeatedMoves[4]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[5]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[5]));
+        game.MakeMove(repeatedMoves[5]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[6]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[6]));
+        game.MakeMove(repeatedMoves[6]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[7]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[7]));
+        game.MakeMove(repeatedMoves[7]);
+        Assert.True(game.IsThreefoldRepetition());
     }
 
     [Test]
@@ -70,25 +65,20 @@ public void IsThreefoldRepetition_2()
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[1]));
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[2]));
 
-        var newPosition = new Position(game.CurrentPosition, repeatedMoves[3]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[3]));
+        game.MakeMove(repeatedMoves[3]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[4]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[4]));
+        game.MakeMove(repeatedMoves[4]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[5]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[5]));
+        game.MakeMove(repeatedMoves[5]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[6]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[6]));
+        game.MakeMove(repeatedMoves[6]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[7]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[7]));
+        game.MakeMove(repeatedMoves[7]);
+        Assert.True(game.IsThreefoldRepetition());
     }
 
     [Test]
@@ -116,20 +106,19 @@ public void IsThreefoldRepetition_CastleRightsRemoval()
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[1]));
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[2]));
 
-        var newPosition = new Position(game.CurrentPosition, repeatedMoves[3]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
+        game.MakeMove(repeatedMoves[3]);
+        Assert.True(game.IsThreefoldRepetition());
 
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[3]));
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[4]));
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[5]));
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[6]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
+        game.MakeMove(repeatedMoves[6]);
+        Assert.True(game.IsThreefoldRepetition());
 
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[6]));
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[7]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
+        game.MakeMove(repeatedMoves[7]);
+        Assert.True(game.IsThreefoldRepetition());
 
         // Position with castling rights, lost in move Ke1d1
         winningPosition = new Position("1n2k2r/8/8/8/8/8/4PPPP/1N2K2R w Kk - 0 1");
@@ -153,21 +142,21 @@ public void IsThreefoldRepetition_CastleRightsRemoval()
             Assert.DoesNotThrow(() => game.MakeMove(move));
         }
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[^1]);
-        Assert.False(game.IsThreefoldRepetition(newPosition));                      // Same position, but white not can't castle
-        Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[^1]));
+        game.MakeMove(repeatedMoves[^1]);
+        Assert.False(game.IsThreefoldRepetition());                      // Same position, but white not can't castle
 
 #if DEBUG
         Assert.AreEqual(repeatedMoves.Count, game.MoveHistory.Count);
 #endif
+        Assert.AreEqual(repeatedMoves.Count + 1, game.PositionHashHistory.Count);
 
         var eval = winningPosition.StaticEvaluation().Score;
         Assert.AreNotEqual(0, eval);
 
         Assert.DoesNotThrow(() => game.MakeMove(repeatedMoves[5]));
 
-        newPosition = new Position(game.CurrentPosition, repeatedMoves[6]);
-        Assert.True(game.IsThreefoldRepetition(newPosition));
+        game.MakeMove(repeatedMoves[6]);
+        Assert.True(game.IsThreefoldRepetition());
     }
 
     [Test]
@@ -196,6 +185,7 @@ public void FiftyMovesRule_1()
 #if DEBUG
         Assert.AreEqual(101, game.MoveHistory.Count);
 #endif
+        Assert.AreEqual(101 + 1, game.PositionHashHistory.Count);
 
         // If the checkmate is in the move when it's claimed, checkmate remains
         Assert.False(game.Is50MovesRepetition());
@@ -224,6 +214,7 @@ public void FiftyMovesRule_2()
 #if DEBUG
         Assert.AreEqual(100, game.MoveHistory.Count);
 #endif
+        Assert.AreEqual(100 + 1, game.PositionHashHistory.Count);
 
         Assert.True(game.Is50MovesRepetition());
     }
@@ -255,6 +246,7 @@ public void FiftyMovesRule_Promotion()
 #if DEBUG
         Assert.AreEqual(51, game.MoveHistory.Count);
 #endif
+        Assert.AreEqual(51 + 1, game.PositionHashHistory.Count);
 
         Assert.False(game.Is50MovesRepetition());
     }

tests/Lynx.Test/OnlineTablebaseProberTest.cs

@@ -386,8 +386,8 @@ public async Task RootSearch_ForceThreefoldRepetitionWhenLosing()
         Assert.AreEqual(0, result.MateScore);
         Assert.AreEqual("h8g7", result.BestMove.UCIString());
 
-        var lastPosition = new Position(position, result.BestMove);
-        Assert.True(game.IsThreefoldRepetition(lastPosition));
+        game.MakeMove(result.BestMove);
+        Assert.True(game.IsThreefoldRepetition());
 
         // Using local method due to async Span limitation
         static Game ParseGame()
@@ -412,8 +412,8 @@ public async Task RootSearch_ForceThreefoldRepetitionWhenBlessedLosing()
         Assert.AreEqual(0, result.MateScore);
         Assert.AreEqual("h8g7", result.BestMove.UCIString());
 
-        var lastPosition = new Position(position, result.BestMove);
-        Assert.True(game.IsThreefoldRepetition(lastPosition));
+        game.MakeMove(result.BestMove);
+        Assert.True(game.IsThreefoldRepetition());
 
         // Using local method due to async Span limitation
         static Game ParseGame()