pgn bug fixed (#147)

fixtures/pgns/0015.pgn

@@ -0,0 +1 @@
+1. g1f3 e7e5  *

fixtures/valid_notation_tests.json

@@ -168,17 +168,17 @@
 		"description" : "https://lichess.org/analysis/fromPosition/r7/1R1nk3/2R1p3/p2n1p2/P5p1/4P3/1P2KPP1/8_b_-_-_1_35"
 	},
 	{
-		"pos1": "r7/1R1nk3/2R1p3/p2n1p2/P5p1/4P3/1P2KPP1/8 b - - 1 35",
-		"pos2": "r7/1R1nk3/2R1pn2/p4p2/P5p1/4P3/1P2KPP1/8 w - - 2 36",
-		"algText": "N5f6",
-		"longAlgText": "Nd5f6",
-		"uciText": "d5f6",
-		"description" : "https://lichess.org/analysis/fromPosition/r7/1R1nk3/2R1p3/p2n1p2/P5p1/4P3/1P2KPP1/8_b_-_-_1_35"
+		"pos1": "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1",
+		"pos2": "rnbqkbnr/pppppppp/8/8/8/5N2/PPPPPPPP/RNBQKB1R b KQkq - 1 1",
+		"algText": "Nf3",
+		"longAlgText": "Ng1f3",
+		"uciText": "g1f3",
+		"description" : "https://lichess.org/analysis/rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR_w_KQkq_-_0_1?color=white#1"
 	},
 	{
 		"pos1": "r7/1R1nk3/2R1p3/p2n1p2/P5p1/4P3/1P2KPP1/8 b - - 1 35",
 		"pos2": "r7/1R1nk3/2R1pn2/p4p2/P5p1/4P3/1P2KPP1/8 w - - 2 36",
-		"algText": "Ndf6",
+		"algText": "Nf6",
 		"longAlgText": "Nd5f6",
 		"uciText": "d5f6",
 		"description" : "https://lichess.org/analysis/fromPosition/r7/1R1nk3/2R1p3/p2n1p2/P5p1/4P3/1P2KPP1/8_b_-_-_1_35"

notation.go

@@ -133,44 +133,12 @@ func algebraicNotationParts(s string) (string, string, string, string, string, s
 	return submatches[1], submatches[2], submatches[3], submatches[4], submatches[5], submatches[6], submatches[7], submatches[8], nil
 }
 
-// Decode implements the Decoder interface.
 func (AlgebraicNotation) Decode(pos *Position, s string) (*Move, error) {
-	piece, originFile, originRank, capture, file, rank, promotes, castles, err := algebraicNotationParts(s)
-	if err != nil {
-		return nil, fmt.Errorf("chess: %+v for position %s", err, pos.String())
-	}
-
+	s = sanitizeNotationString(s)
 	for _, m := range pos.ValidMoves() {
-		moveStr := AlgebraicNotation{}.Encode(pos, m)
-		moveSubmatches := pgnRegex.FindStringSubmatch(moveStr)
-		moveCleaned := strings.Join(moveSubmatches[1:9], "")
-
-		cleaned := piece + originFile + originRank + capture + file + rank + promotes + castles
-		if cleaned == moveCleaned {
-			return m, nil
-		}
-
-		// Try and remove the disambiguators and see if it parses. Sometimes they
-		// get extraneously added.
-		options := []string{}
-
-		if piece != "" {
-			options = append(options, piece+capture+file+rank+promotes+castles)            // no origin
-			options = append(options, piece+originRank+capture+file+rank+promotes+castles) // no origin file
-			options = append(options, piece+originFile+capture+file+rank+promotes+castles) // no origin rank
-		} else {
-			if capture != "" {
-				// Possibly a pawn capture. In order to parse things like d4xe5, we need
-				// to try parsing without the rank.
-				options = append(options, piece+originFile+capture+file+rank+promotes+castles) // no origin rank
-			}
-			if originFile != "" && originRank != "" {
-				options = append(options, piece+capture+file+rank+promotes+castles) // no origin
-			}
-		}
-
-		for _, opt := range options {
-			if opt == moveCleaned {
+		for _, v := range notationVariants(s) {
+			moveStr := AlgebraicNotation{}.Encode(pos, m)
+			if moveStr == v {
 				return m, nil
 			}
 		}
@@ -214,7 +182,16 @@ func (LongAlgebraicNotation) Encode(pos *Position, m *Move) string {
 
 // Decode implements the Decoder interface.
 func (LongAlgebraicNotation) Decode(pos *Position, s string) (*Move, error) {
-	return AlgebraicNotation{}.Decode(pos, s)
+	s = sanitizeNotationString(s)
+	for _, m := range pos.ValidMoves() {
+		for _, v := range notationVariants(s) {
+			moveStr := LongAlgebraicNotation{}.Encode(pos, m)
+			if moveStr == v {
+				return m, nil
+			}
+		}
+	}
+	return nil, fmt.Errorf("chess: could not decode long algebraic notation %s for position %s", s, pos.String())
 }
 
 func getCheckChar(pos *Position, move *Move) string {
@@ -301,3 +278,17 @@ func pieceTypeFromChar(c string) PieceType {
 	}
 	return NoPieceType
 }
+
+func sanitizeNotationString(s string) string {
+	s = strings.Replace(s, "!", "", -1)
+	s = strings.Replace(s, "?", "", -1)
+	return s
+}
+
+func notationVariants(s string) []string {
+	return []string{
+		s,
+		s + "+",
+		s + "#",
+	}
+}

notation_test.go

@@ -51,7 +51,7 @@ func TestValidDecoding(t *testing.T) {
 			}
 			postPos := test.Pos1.Update(m)
 			if test.Pos2.String() != postPos.String() {
-				t.Fatalf("starting from board \n%s%s\n after move %s\n expected board to be %s\n%s\n but was %s\n%s\n",
+				t.Errorf("starting from board \n%s%s\n after move %s\n expected board to be %s\n%s\n but was %s\n%s\n",
 					test.Pos1.String(),
 					test.Pos1.board.Draw(), m.String(), test.Pos2.String(),
 					test.Pos2.board.Draw(), postPos.String(), postPos.board.Draw())
@@ -107,13 +107,19 @@ var (
 			Pos:  unsafeFEN("rnbqkbnr/ppp1pppp/8/3p4/3P4/8/PPP1PPPP/RNBQKBNR w KQkq - 0 2"),
 			Text: "bf4",
 		},
+		{
+			// invalid notation
+			N:    AlgebraicNotation{},
+			Pos:  unsafeFEN("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1"),
+			Text: "g1f3",
+		},
 	}
 )
 
 func TestInvalidDecoding(t *testing.T) {
 	for _, test := range invalidDecodeTests {
 		if _, err := test.N.Decode(test.Pos, test.Text); err == nil {
-			t.Fatalf("starting from board\n%s\n expected move notation %s to be invalid", test.Pos.board.Draw(), test.Text)
+			t.Errorf("starting from board\n%s\n expected move notation %s to be invalid", test.Pos.board.Draw(), test.Text)
 		}
 	}
 }

pgn_test.go

@@ -50,6 +50,10 @@ var (
 			PostPos: StartingPosition(),
 			PGN:     mustParsePGN("fixtures/pgns/0012.pgn"),
 		},
+		{
+			PostPos: unsafeFEN("rnbqkbnr/pppp1ppp/8/4p3/8/5N2/PPPPPPPP/RNBQKB1R w KQkq e6 0 2"),
+			PGN:     mustParsePGN("fixtures/pgns/0015.pgn"),
+		},
 	}
 )