From e0a8af4551db0eeb13b4ca2fcb1081e924f3958e Mon Sep 17 00:00:00 2001
From: Jean Bovet <jean_bovet@mac.com>
Date: Mon, 3 May 2021 08:34:13 -0700
Subject: [PATCH] Ensure PGN in one line is correctly formatted

---
 BChessTests/PGNTests.cpp       |  35 ++++++--
 Shared/Engine/Helpers/FPGN.cpp | 147 ++++++++++++++++++++++-----------
 Shared/Engine/Helpers/FPGN.hpp |   5 +-
 3 files changed, 131 insertions(+), 56 deletions(-)

diff --git a/BChessTests/PGNTests.cpp b/BChessTests/PGNTests.cpp
index 04d1690..e2310b8 100644
--- a/BChessTests/PGNTests.cpp
+++ b/BChessTests/PGNTests.cpp
@@ -220,11 +220,7 @@ TEST_F(PGN, InputMultipleGames) {
     ASSERT_EQ(2, games.size());
 }
 
-TEST_F(PGN, PGNWithVariations) {
-//    std::string pgn = "1. e4 e5 2. Nc3 Nf6 3. f4 d5 4. fxe5 Nxe4 5. Qf3 Nxc3 {We want them to take\
-//    this knight so we will recapture with the b2 pawn and go for d2-d4, with a\
-//    large center, and Bd3/Ne2 setup.} (5... Nc6 {A tricky and challenging move. We\
-//    must prevent Nxe5.}) 6. bxc3 Be7";
+TEST_F(PGN, PGNWithSimpleWhiteVariation) {
     std::string pgn = "1. e4 (1. d4)";
     
     ChessGame game;
@@ -233,7 +229,32 @@ TEST_F(PGN, PGNWithVariations) {
     ASSERT_EQ(1, game.getNumberOfMoves());
     ASSERT_EQ(2, game.getRoot().variations.size());
     
-    // TODO: generate PGN with variation included
     auto pgnAgain = FPGN::getGame(game);
-    ASSERT_EQ("1. e4 *", pgnAgain);
+    ASSERT_EQ("1. e4 (1. d4) *", pgnAgain);
+}
+
+TEST_F(PGN, PGNWithSimpleBlackVariation) {
+//    std::string pgn = "1. e4 e5 2. Nc3 Nf6 3. f4 d5 4. fxe5 Nxe4 5. Qf3 Nxc3 {We want them to take\
+//    this knight so we will recapture with the b2 pawn and go for d2-d4, with a\
+//    large center, and Bd3/Ne2 setup.} (5... Nc6 {A tricky and challenging move. We\
+//    must prevent Nxe5.}) 6. bxc3 Be7";
+    std::string pgn = "1. e4 e5 (1... c5)";
+    
+    ChessGame game;
+    ASSERT_TRUE(FPGN::setGame(pgn, game));
+    
+    ASSERT_EQ(2, game.getNumberOfMoves());
+    ASSERT_EQ(1, game.getRoot().variations.size());
+    ASSERT_EQ(2, game.getRoot().variations[0].variations.size());
+
+    auto pgnAgain = FPGN::getGame(game);
+    ASSERT_EQ("1. e4 e5 (1... c5) *", pgnAgain);
+}
+
+TEST_F(PGN, LineFromCursor) {
+    ChessGame game;
+    ASSERT_TRUE(FPGN::setGame(move1to3, game));
+    
+    auto line = FPGN::getGame(game, FPGN::Formatting::line, 2);
+    ASSERT_EQ("Nf3 Nc6 Bb5 a6", line);
 }
diff --git a/Shared/Engine/Helpers/FPGN.cpp b/Shared/Engine/Helpers/FPGN.cpp
index e20cb67..8ed9bdc 100644
--- a/Shared/Engine/Helpers/FPGN.cpp
+++ b/Shared/Engine/Helpers/FPGN.cpp
@@ -694,72 +694,125 @@ bool FPGN::setGames(std::string pgn, std::vector<ChessGame> & games) {
     return true;
 }
 
-std::string FPGN::getGame(ChessGame game, Formatting formatting, int fromIndex) {
-    // Game used to compute the optimum PGN representation for each move
-    ChessBoard outputBoard;
-    FFEN::setFEN(game.initialFEN, outputBoard);
-    
-    std::string pgn;
-    unsigned fullMoveIndex = 0;
-    for (int index=0; index<game.getNumberOfMoves() && fromIndex < game.getNumberOfMoves(); index++) {
-        auto move = game.getMoveAtIndex(index);
-        auto piece = MOVE_PIECE(move);
+// Returns a PGN representation of a particular node in the game, including all its descendant.
+static void getPGN(ChessBoard board, FPGN::Formatting formatting, ChessGame::MoveNode & node, std::string & pgn, int index, int fromIndex, int fullMoveIndex, bool mainLine) {
+    auto move = node.move;
+    auto piece = MOVE_PIECE(move);
 
-        if (index == fromIndex) {
-            pgn = "";
-        }
-        
-        SANType sanType = SANType::full;
-        auto matchingMoves = getMatchingMoves(outputBoard, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileUndefined, RankUndefined);
+    if (formatting == FPGN::Formatting::line && index == fromIndex) {
+        pgn = "";
+    }
+    
+    FPGN::SANType sanType = FPGN::SANType::full;
+    auto matchingMoves = getMatchingMoves(board, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileUndefined, RankUndefined);
+    if (matchingMoves.size() == 1) {
+        // Only one matching move, we can use the shortest form for PGN
+        // For example: Ne3
+        sanType = FPGN::SANType::tight;
+    } else {
+        matchingMoves = getMatchingMoves(board, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileFrom(MOVE_FROM(move)), RankUndefined);
         if (matchingMoves.size() == 1) {
-            // Only one matching move, we can use the shortest form for PGN
-            // For example: Ne3
-            sanType = SANType::tight;
+            // Use the File to specify the move. For example: Nge3
+            sanType = FPGN::SANType::medium;
         } else {
-            matchingMoves = getMatchingMoves(outputBoard, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileFrom(MOVE_FROM(move)), RankUndefined);
+            matchingMoves = getMatchingMoves(board, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileFrom(MOVE_FROM(move)), RankFrom(MOVE_FROM(move)));
             if (matchingMoves.size() == 1) {
-                // Use the File to specify the move. For example: Nge3
-                sanType = SANType::medium;
+                sanType = FPGN::SANType::full;
             } else {
-                matchingMoves = getMatchingMoves(outputBoard, MOVE_TO(move), piece, MOVE_PROMOTION_PIECE(move), FileFrom(MOVE_FROM(move)), RankFrom(MOVE_FROM(move)));
-                if (matchingMoves.size() == 1) {
-                    sanType = SANType::full;
-                } else {
-                    // Should not happen
-                    printf("Unable to find matching moves\n");
-                }
+                // Should not happen
+                printf("Unable to find matching moves\n");
             }
         }
-        
+    }
+    
+    if (mainLine) {
+        // In the main line, only display the move number for white
         if (index % 2 == 0) {
             fullMoveIndex++;
-            if (formatting != Formatting::line) {
-                pgn += std::to_string(fullMoveIndex) + ". ";
+            if (formatting != FPGN::Formatting::line) {
+                if (pgn.size() > 0) {
+                    pgn += " ";
+                }
+                pgn += std::to_string(fullMoveIndex) + ".";
             }
         }
-                
-        pgn += to_string(move, sanType);
+    } else {
+        // We are in a variation, which means we need to display
+        // the full move number and if it is white or black
+        // to play. For example:
+        // (1. d4)   // variation for white
+        // (1... c5) // variation for black
+        if (index % 2 == 0) {
+            fullMoveIndex++;
+        }
+        if (formatting != FPGN::Formatting::line) {
+            pgn += std::to_string(fullMoveIndex);
+        }
+        if (index % 2 == 0) {
+            pgn += ".";
+        } else {
+            pgn += "...";
+        }
+    }
+            
+    if (pgn.size() > 0) {
+        pgn += " ";
+    }
+    pgn += FPGN::to_string(move, sanType);
 
-        outputBoard.move(move);
-        
-        // Determine if the position is check or mate
-        if (outputBoard.isCheck(outputBoard.color)) {
-            ChessMoveGenerator generator;
-            auto moveList = generator.generateMoves(outputBoard);
-            if (moveList.count == 0) {
-                pgn += "#";
-            } else {
-                pgn += "+";
-            }
+    board.move(move);
+    
+    // Determine if the position is check or mate
+    if (board.isCheck(board.color)) {
+        ChessMoveGenerator generator;
+        auto moveList = generator.generateMoves(board);
+        if (moveList.count == 0) {
+            pgn += "#";
+        } else {
+            pgn += "+";
         }
+    }
         
-        pgn += " ";
+    for (int vindex=0; vindex<node.variations.size(); vindex++) {
+        auto & vnode = node.variations[vindex];
+        if (vindex > 0) {
+            pgn += " (";
+        }
+        getPGN(board, formatting, vnode, pgn, index+1, fromIndex, fullMoveIndex, vindex == 0);
+        if (vindex > 0) {
+            pgn += ")";
+        }
+    }
+}
+
+std::string FPGN::getGame(ChessGame game, Formatting formatting, int fromIndex) {
+    // Game used to compute the optimum PGN representation for each move
+    ChessBoard outputBoard;
+    FFEN::setFEN(game.initialFEN, outputBoard);
+    
+    std::string pgn;
+    unsigned fullMoveIndex = 0;
+    
+    auto rootNode = game.getRoot();
+    for (int index=0; index<rootNode.variations.size(); index++) {
+        auto & node = rootNode.variations[index];
+        if (index > 0) {
+            pgn += " (";
+        }
+        getPGN(outputBoard, formatting, node, pgn, 0, fromIndex, fullMoveIndex, index == 0);
+        if (index > 0) {
+            pgn += ")";
+        }
     }
     
     if (formatting == Formatting::line) {
         return pgn;
     }
-        
+    
+    if (pgn.size() > 0) {
+        pgn += " ";
+    }
+
     switch (game.outcome) {
         case ChessGame::Outcome::black_wins:
             pgn += "0-1";
diff --git a/Shared/Engine/Helpers/FPGN.hpp b/Shared/Engine/Helpers/FPGN.hpp
index 41b3b38..9cf86a4 100644
--- a/Shared/Engine/Helpers/FPGN.hpp
+++ b/Shared/Engine/Helpers/FPGN.hpp
@@ -44,7 +44,8 @@ class FPGN {
         history, // for displaying the history of the game, with new lines every two ply.
     };
     
-    // compact: true to return a compact form of PGN for display in the UI. For example: "Na3 Bxc4 e5+"
-    // fromIndex: the index from which to start building the PGN. Used to display a line from a game.
+    // fromIndex: the index from which to start building the PGN. Used to display a line from a game during analysis,
+    //            which means we only want to display the game after the current move (to show the thinking of the computer).
+    //            This parameter takes effect only if the Formatting is set to line
     static std::string getGame(ChessGame game, Formatting formatting = FPGN::Formatting::storage, int fromIndex = 0);
 };