🐛 Make .ToEPDString() fully PGN/EPD compliant (#841)

- Remove `e.p.` for en-passant moves - Use `O` instead of `o` for castling moves - Replace existing `ToEPDString()` method with one that accepts a `Position`. Keep the former as internal. This allows to: - Disambiguate `.ToEPDString()` result when both piece types and target squares match - Remove `.ToMoveString()` method, because it's just confusing and a mixture between wanting the speed of UCI but in a easier to read but still not human form.
lynx-chess · Jul 4, 2024 · fcc1dd9 · fcc1dd9
1 parent ba4281f
commit fcc1dd9
Show file tree

Hide file tree

Showing 10 changed files with 184 additions and 36 deletions.
diff --git a/src/Lynx.Dev/Program.cs b/src/Lynx.Dev/Program.cs
@@ -516,7 +516,7 @@ static void GeneralMoveTest(Game game)
         {
             game.CurrentPosition.Print();
 
-            Console.WriteLine(move.ToMoveString());
+            Console.WriteLine(move.ToEPDString(game.CurrentPosition));
             var gameState = game.MakeMove(move);
             game.CurrentPosition.Print();
 
@@ -539,7 +539,7 @@ static void CastlingRightsTest(Game game)
             {
                 game.CurrentPosition.Print();
 
-                Console.WriteLine(move.ToMoveString());
+                Console.WriteLine(move.ToEPDString(game.CurrentPosition));
                 var gameState = game.MakeMove(move);
                 game.CurrentPosition.Print();
                 game.CurrentPosition.UnmakeMove(move, gameState);
@@ -1121,15 +1121,16 @@ static void TestMoveGen(string fen)
         var oldZobristKey = position.UniqueIdentifier;
         foreach (var move in allMoves)
         {
-            Console.WriteLine($"Trying {move.ToEPDString()} in\t{position.FEN()}");
+            var epdMoveString = move.ToEPDString(position);
+            Console.WriteLine($"Trying {epdMoveString} in\t{position.FEN()}");
 
             var newPosition = new Position(position, move);
             var savedState = position.MakeMove(move);
 
             Console.WriteLine($"Position\t{newPosition.FEN()}, Zobrist key {newPosition.UniqueIdentifier}");
             Console.WriteLine($"Position\t{position.FEN()}, Zobrist key {position.UniqueIdentifier}");
 
-            Console.WriteLine($"Unmaking {move.ToEPDString()} in\t{position.FEN()}");
+            Console.WriteLine($"Unmaking {epdMoveString} in\t{position.FEN()}");
 
             //position.UnmakeMove(move, savedState);
 

diff --git a/src/Lynx/Constants.cs b/src/Lynx/Constants.cs
@@ -351,6 +351,8 @@ public static class Constants
         0, 1, 2, 3, 4, 5, 6, 7
     ];
 
+    public static readonly char[] FileString = [ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h' ];
+
     public const int AbsoluteMaxDepth = 255;
 
     /// <summary>

diff --git a/src/Lynx/Engine.cs b/src/Lynx/Engine.cs
@@ -282,13 +282,13 @@ private async ValueTask<SearchResult> SearchBestMove(int maxDepth, int softLimit
         if (searchResult is not null)
         {
             _logger.Info("Search evaluation result - eval: {0}, mate: {1}, depth: {2}, pv: {3}",
-                searchResult.Evaluation, searchResult.Mate, searchResult.Depth, string.Join(", ", searchResult.Moves.Select(m => m.ToMoveString())));
+                searchResult.Evaluation, searchResult.Mate, searchResult.Depth, string.Join(", ", searchResult.Moves.Select(m => m.UCIString())));
         }
 
         if (tbResult is not null)
         {
             _logger.Info("Online tb probing result - mate: {0}, moves: {1}",
-                tbResult.Mate, string.Join(", ", tbResult.Moves.Select(m => m.ToMoveString())));
+                tbResult.Mate, string.Join(", ", tbResult.Moves.Select(m => m.UCIString())));
 
             if (searchResult?.Mate > 0 && searchResult.Mate <= tbResult.Mate && searchResult.Mate + currentHalfMovesWithoutCaptureOrPawnMove < 96)
             {

diff --git a/src/Lynx/Model/Move.cs b/src/Lynx/Model/Move.cs
@@ -322,48 +322,56 @@ public static bool TryParseFromUCIString(ReadOnlySpan<char> UCIString, ReadOnlyS
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool IsCastle(this Move move) => (move & 0xE00_0000) >> SpecialMoveFlagOffset >= (int)SpecialMoveType.ShortCastle;
 
-    /// <summary>
-    /// Typical format when humans write moves
-    /// </summary>
-    /// <returns></returns>
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public static string ToMoveString(this Move move)
+    [Obsolete(
+        "Consider using the override that accepts a position for fully compliant EPD/PGN string representation of the move. " +
+        "This method be removed/renamed in future versions")]
+    internal static string ToEPDString(this Move move)
     {
+        var piece = move.Piece();
+
 #pragma warning disable S3358 // Ternary operators should not be nested
         return move.SpecialMoveFlag() switch
         {
-            SpecialMoveType.ShortCastle => "0-0",
-            SpecialMoveType.LongCastle => "0-0-O",
+            SpecialMoveType.ShortCastle => "O-O",
+            SpecialMoveType.LongCastle => "O-O-O",
             _ =>
-                Constants.AsciiPieces[move.Piece()] +
-                Constants.Coordinates[move.SourceSquare()] +
-                (move.IsCapture() ? "x" : "") +
-                Constants.Coordinates[move.TargetSquare()] +
-                (move.PromotedPiece() == default ? "" : $"={Constants.AsciiPieces[move.PromotedPiece()]}") +
-                (move.IsEnPassant() ? "e.p." : "")
+                (piece == (int)Model.Piece.P || piece == (int)Model.Piece.p
+                    ? (move.IsCapture()
+                        ? Constants.Coordinates[move.SourceSquare()][..^1]  // exd5
+                        : "")    // d5
+                    : char.ToUpperInvariant(Constants.AsciiPieces[move.Piece()]))
+                + (move.IsCapture() == default ? "" : "x")
+                + Constants.Coordinates[move.TargetSquare()]
+                + (move.PromotedPiece() == default ? "" : $"={char.ToUpperInvariant(Constants.AsciiPieces[move.PromotedPiece()])}")
         };
 #pragma warning restore S3358 // Ternary operators should not be nested
     }
 
-    public static string ToEPDString(this Move move)
+    /// <summary>
+    /// EPD representation of a valid move in a position
+    /// </summary>
+    /// <param name="move">A valid move for the given position</param>
+    /// <param name="position"></param>
+    /// <returns></returns>
+    public static string ToEPDString(this Move move, Position position)
     {
         var piece = move.Piece();
 
 #pragma warning disable S3358 // Ternary operators should not be nested
         return move.SpecialMoveFlag() switch
         {
-            SpecialMoveType.ShortCastle => "0-0",
-            SpecialMoveType.LongCastle => "0-0-O",
+            SpecialMoveType.ShortCastle => "O-O",
+            SpecialMoveType.LongCastle => "O-O-O",
             _ =>
                 (piece == (int)Model.Piece.P || piece == (int)Model.Piece.p
                     ? (move.IsCapture()
-                        ? Constants.Coordinates[move.SourceSquare()][..^1]  // exd5
+                        ? global::Lynx.Constants.FileString[global::Lynx.Constants.File[move.SourceSquare()]]  // exd5
                         : "")    // d5
-                    : char.ToUpperInvariant(Constants.AsciiPieces[move.Piece()]))
+                    : (char.ToUpperInvariant(global::Lynx.Constants.AsciiPieces[move.Piece()]))
+                        + DisambiguateMove(move, position))
                 + (move.IsCapture() == default ? "" : "x")
                 + Constants.Coordinates[move.TargetSquare()]
                 + (move.PromotedPiece() == default ? "" : $"={char.ToUpperInvariant(Constants.AsciiPieces[move.PromotedPiece()])}")
-                + (move.IsEnPassant() == default ? "" : "e.p.")
         };
 #pragma warning restore S3358 // Ternary operators should not be nested
     }
@@ -391,4 +399,59 @@ public static string UCIString(this Move move)
 
         return span[..^1].ToString();
     }
+
+    /// <summary>
+    /// First file letter, then rank number and finally the whole square.
+    /// At least according to https://chess.stackexchange.com/a/1819
+    /// </summary>
+    /// <param name="move"></param>
+    /// <param name="position"></param>
+    /// <returns></returns>
+    private static string DisambiguateMove(Move move, Position position)
+    {
+        var piece = move.Piece();
+        var targetSquare = move.TargetSquare();
+
+        Span<Move> moves = stackalloc Move[Constants.MaxNumberOfPossibleMovesInAPosition];
+        var pseudoLegalMoves = MoveGenerator.GenerateAllMoves(position, moves).ToArray();
+
+        var movesWithSameSimpleRepresentation = pseudoLegalMoves
+            .Where(m => m != move && m.Piece() == piece && m.TargetSquare() == targetSquare)
+            .Where(m =>
+            {
+                // If any illegal moves exist with the same simple representation there's no need to disambiguate
+                var gameState = position.MakeMove(m);
+                var isLegal = position.WasProduceByAValidMove();
+                position.UnmakeMove(m, gameState);
+
+                return isLegal;
+            })
+            .ToArray();
+
+        if (movesWithSameSimpleRepresentation.Length == 0)
+        {
+            return string.Empty;
+        }
+
+        int sourceSquare = move.SourceSquare();
+        var moveFile = Constants.File[sourceSquare];
+
+        var files = movesWithSameSimpleRepresentation.Select(m => Constants.File[m.SourceSquare()]);
+
+        if (files.Any(f => f == moveFile))
+        {
+            var moveRank = Constants.Rank[sourceSquare];
+
+            var ranks = movesWithSameSimpleRepresentation.Select(m => Constants.Rank[m.SourceSquare()]);
+
+            if (ranks.Any(r => r == moveRank))
+            {
+                return Constants.Coordinates[sourceSquare];
+            }
+
+            return (moveRank + 1).ToString();
+        }
+
+        return Constants.FileString[moveFile].ToString();
+    }
 }
diff --git a/src/Lynx/Model/TranspositionTable.cs b/src/Lynx/Model/TranspositionTable.cs
@@ -275,7 +275,7 @@ internal static void Print(this TranspositionTable transpositionTable)
         {
             if (transpositionTable[i].Key != default)
             {
-                Console.WriteLine($"{i}: Key = {transpositionTable[i].Key}, Depth: {transpositionTable[i].Depth}, Score: {transpositionTable[i].Score}, Move: {(transpositionTable[i].Move != 0 ? ((Move)transpositionTable[i].Move).ToMoveString() : "-")} {transpositionTable[i].Type}");
+                Console.WriteLine($"{i}: Key = {transpositionTable[i].Key}, Depth: {transpositionTable[i].Depth}, Score: {transpositionTable[i].Score}, Move: {(transpositionTable[i].Move != 0 ? ((Move)transpositionTable[i].Move).UCIString() : "-")} {transpositionTable[i].Type}");
             }
         }
         Console.WriteLine("");

diff --git a/src/Lynx/Search/Helpers.cs b/src/Lynx/Search/Helpers.cs
@@ -233,13 +233,15 @@ private static void PrintPreMove(Position position, int plies, Move move, bool i
             //if (plies < Configuration.Parameters.Depth - 1)
             {
                 //Console.WriteLine($"{Environment.NewLine}{depthStr}{move} ({position.Side}, {depth})");
-                _logger.Trace($"{Environment.NewLine}{depthStr}{(isQuiescence ? "[Qui] " : "")}{move.ToEPDString()} ({position.Side}, {plies})");
+#pragma warning disable CS0618 // Type or member is obsolete
+                _logger.Trace($"{Environment.NewLine}{depthStr}{(isQuiescence ? "[Qui] " : "")}{move.ToEPDString(position)} ({position.Side}, {plies})");
+#pragma warning restore CS0618 // Type or member is obsolete
             }
         }
     }
 
     [Conditional("DEBUG")]
-    private static void PrintMove(int plies, Move move, int evaluation, bool isQuiescence = false, bool prune = false)
+    private static void PrintMove(Position position, int plies, Move move, int evaluation, bool isQuiescence = false, bool prune = false)
     {
         if (_logger.IsTraceEnabled)
         {
@@ -261,7 +263,9 @@ private static void PrintMove(int plies, Move move, int evaluation, bool isQuies
             //Console.WriteLine($"{depthStr}{move} ({position.Side}, {depthLeft}) | {evaluation}");
             //Console.WriteLine($"{depthStr}{move} | {evaluation}");
 
-            _logger.Trace($"{depthStr}{(isQuiescence ? "[Qui] " : "")}{move.ToEPDString(),-6} | {evaluation}{(prune ? " | prunning" : "")}");
+#pragma warning disable CS0618 // Type or member is obsolete
+            _logger.Trace($"{depthStr}{(isQuiescence ? "[Qui] " : "")}{move.ToEPDString(position),-6} | {evaluation}{(prune ? " | prnning" : "")}");
+#pragma warning restore CS0618 // Type or member is obsolete
 
             //Console.ResetColor();
         }
@@ -311,6 +315,7 @@ private void PrintPvTable(int target = -1, int source = -1, int movesToCopy = 0,
             Console.WriteLine($"Copying {movesToCopy} moves");
         }
 
+#pragma warning disable CS0618 // Type or member is obsolete
         Console.WriteLine(
 (target != -1 ? $"src: {source}, tgt: {target}" + Environment.NewLine : "") +
 $" {0,-3} {_pVTable[0].ToEPDString(),-6} {_pVTable[1].ToEPDString(),-6} {_pVTable[2].ToEPDString(),-6} {_pVTable[3].ToEPDString(),-6} {_pVTable[4].ToEPDString(),-6} {_pVTable[5].ToEPDString(),-6} {_pVTable[6].ToEPDString(),-6} {_pVTable[7].ToEPDString(),-6} {_pVTable[8].ToEPDString(),-6} {_pVTable[9].ToEPDString(),-6} {_pVTable[10].ToEPDString(),-6}" + Environment.NewLine +
@@ -323,6 +328,7 @@ private void PrintPvTable(int target = -1, int source = -1, int movesToCopy = 0,
 $" {427,-3}                                                  {_pVTable[427].ToEPDString(),-6} {_pVTable[428].ToEPDString(),-6} {_pVTable[429].ToEPDString(),-6} {_pVTable[430].ToEPDString(),-6}" + Environment.NewLine +
 $" {484,-3}                                                         {_pVTable[484].ToEPDString(),-6} {_pVTable[485].ToEPDString(),-6} {_pVTable[486].ToEPDString(),-6}" + Environment.NewLine +
 (target == -1 ? "------------------------------------------------------------------------------------" + Environment.NewLine : ""));
+#pragma warning restore CS0618 // Type or member is obsolete
     }
 
     [Conditional("DEBUG")]

diff --git a/src/Lynx/Search/NegaMax.cs b/src/Lynx/Search/NegaMax.cs
@@ -355,7 +355,7 @@ private int NegaMax(int depth, int ply, int alpha, int beta, bool parentWasNullM
             Game.PositionHashHistory.RemoveAt(Game.PositionHashHistory.Count - 1);
             position.UnmakeMove(move, gameState);
 
-            PrintMove(ply, move, evaluation);
+            PrintMove(position, ply, move, evaluation);
 
             // Fail-hard beta-cutoff - refutation found, no need to keep searching this line
             if (evaluation >= beta)
@@ -575,7 +575,7 @@ public int QuiescenceSearch(int ply, int alpha, int beta)
             int evaluation = -QuiescenceSearch(ply + 1, -beta, -alpha);
             position.UnmakeMove(move, gameState);
 
-            PrintMove(ply, move, evaluation);
+            PrintMove(position, ply, move, evaluation);
 
             // Fail-hard beta-cutoff
             if (evaluation >= beta)

diff --git a/tests/Lynx.Test/BestMove/RegressionTest.cs b/tests/Lynx.Test/BestMove/RegressionTest.cs
@@ -230,7 +230,7 @@ public void FalseDrawnPositionBy50MovesRule(string positionCommand)
         Assert.False(engine.Game.Is50MovesRepetition());
         var bestMove = engine.BestMove(new GoCommand("go depth 1"));
 
-        engine.AdjustPosition(positionCommand + " " + bestMove.BestMove.ToEPDString());
+        engine.AdjustPosition(positionCommand + " " + bestMove.BestMove.UCIString());
         Assert.IsFalse(engine.Game.Is50MovesRepetition());
     }
 

diff --git a/tests/Lynx.Test/BestMove/WACSilver200.cs b/tests/Lynx.Test/BestMove/WACSilver200.cs
@@ -37,18 +37,19 @@ public void WinningAtChess_DefaultSearchDepth(string fen, string bestMove, strin
     private static void VerifyBestMove(string fen, string bestMove, string id, GoCommand goCommand)
     {
         var engine = GetEngine(fen);
+        var currentPositionClone = new Position(engine.Game.CurrentPosition);
 
         var bestResult = engine.BestMove(goCommand);
 
         var bestMoveArray = bestMove.Split(' ', StringSplitOptions.RemoveEmptyEntries);
         if (bestMoveArray.Length == 1)
         {
             var expectedMove = bestMoveArray[0].TrimEnd('+');
-            Assert.AreEqual(expectedMove, bestResult.BestMove.ToEPDString(), $"id {id} depth {bestResult.Depth} seldepth {bestResult.Depth} nodes {bestResult.Nodes}");
+            Assert.AreEqual(expectedMove, bestResult.BestMove.ToEPDString(currentPositionClone), $"id {id} depth {bestResult.Depth} seldepth {bestResult.Depth} nodes {bestResult.Nodes}");
         }
         else if (bestMoveArray.Length == 2)
         {
-            var bestResultGot = bestResult.BestMove.ToEPDString();
+            var bestResultGot = bestResult.BestMove.ToEPDString(currentPositionClone);
             Assert.True(
                 bestMoveArray[0].TrimEnd('+') == bestResultGot
                 || bestMoveArray[1].TrimEnd('+') == bestResultGot