Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PGN crashes viewer #457

Open
jackstenglein opened this issue Jun 1, 2023 · 9 comments
Open

PGN crashes viewer #457

jackstenglein opened this issue Jun 1, 2023 · 9 comments

Comments

@jackstenglein
Copy link

I try to display the following PGN (which was generated by Chess.com) in the viewer:

[Event "Live Chess"]
[Site "Chess.com"]
[Date "2023.05.31"]
[Round "-"]
[White "JoshMcK"]
[Black "Pre-Successful"]
[Result "1-0"]
[CurrentPosition "2r3kq/pp1bQp1p/4p1pB/8/8/1B6/P4PP1/4R1K1 b - -"]
[Timezone "UTC"]
[ECO "B22"]
[ECOUrl "https://www.chess.com/openings/Alapin-Sicilian-Defense...9.Qe2-O-O-10.Qe4-Bd7"]
[UTCDate "2023.05.31"]
[UTCTime "08:23:38"]
[WhiteElo "1960"]
[BlackElo "2053"]
[TimeControl "5400+30"]
[Termination "JoshMcK won by resignation"]
[StartTime "08:23:38"]
[EndDate "2023.05.31"]
[EndTime "09:41:30"]
[Link "https://www.chess.com/game/live/79260217669"]
[WhiteUrl "https://images.chesscomfiles.com/uploads/v1/user/38553382.715c4459.50x50o.44b69e9f7ddf.png"]
[WhiteCountry "166"]
[WhiteTitle ""]
[BlackUrl "https://www.chess.com/bundles/web/images/noavatar_l.84a92436.gif"]
[BlackCountry "164"]
[BlackTitle ""]
[PlyCount "51"]

1. e4 {My second game in the Chess Dojo Open Classical tournament. Paired
against a higher rated opponent again, but I had the white pieces. I couldn't
find any games of my opponent to prepare with so just revised some of my white
repertoire prior to the start of the game.} 1... c5 {A Sicilian $1 I have been
coming up against this a lot recently, so I like to think my preparation is
fairly solid at the moment.} 2. c3 Nf6 3. e5 Nd5 4. Nf3 e6 {I don't see this
variation very often so have limited preparation against it.} 5. d4 cxd4 6. cxd4
d6 7. Bc4 Nc6 8. O-O Be7 9. Qe2 O-O 10. Qe4 Bd7 11. Bb3 Rc8 12. Nc3 Nxc3 13.
bxc3 dxe5 {This move took me out of preparation, I was more prepared for Na5.}
14. dxe5 Qc7 15. Bc2 {Hoping to provoke pawn weaknesses around black's king -
I did not expect to get mate here.} 15... g6 {Opening up the dark squares around
his king, but also the only real defence.} 16. Bh6 Rfd8 17. h4 Bxh4?? {I think
this move lost the game - black had only one piece on the king side, he had dark
squared weaknesses around his king, and he has given up his dark squared bishop.
It caught me off guard but I was happy to take it. My opponent mentioned
afterwards that he had overestimated the strength of h4.} 18. Qxh4 Nxe5 19. Nxe5
(19. Qf6 Nxf3+ 20. gxf3 Qxc3 {I was very tempted to go Qf6 during the game but
the diagonal is covered so no mating opportunities - the engine still gives
white an advantage but not nearly as nice a position to play.}) 19... Qxe5 20.
Rfe1 Qxc3 21. Rac1 {From this point my strategy was to remove black's queen from
the a1-h8 diagonal and consider a bishop sacrifice on g6 - I was very aware that
if I could lure the c8 rook from the back rank it would be devastating for
black.} 21... Qh8 {This is quickly becoming one of the worst queens in chess
history, refusing to abandon the key diagonal but losing almost all of its
power.} 22. Bb3 Re8 23. Rxc8 Rxc8 24. Qe7 {I was trying to visualise various
rook or queen sacrifices to deliver a double bishop checkmate here. My opponent
resigned in this position.} (24... Bc6 25. Qxe6 fxe6 26. Bxe6# {Would have been
a beautiful way to finish.}) 1-0
pgnEdit(id, {
    pgn: game.pgn,
    pieceStyle: 'alpha',
    theme: 'blue',
    showResult: true,
    notationLayout: 'list',
});

This causes the PgnViewer to crash with the error Uncaught TypeError: g is undefined. Here is the same PGN loaded into a lichess study: https://lichess.org/study/PG1EK14j/zVQH4ISq

Screenshot 2023-05-31 at 8 03 24 PM
@mliebelt
Copy link
Owner

mliebelt commented Jun 1, 2023

Well, the notation is just wrong. I cannot say how that was produced. The wrong move is Bc6. This is part of a variant (starting with character "("), which indicates that this move replaces the previous move in the main variation. And that is just wrong. Last move was Qe7 (white), first move of a variation of this should be from white again.

Just remove the parentheses before Bc6 and after "finish}" and it should work.

@jackstenglein
Copy link
Author

I've asked my user a bit more about this, and I suspect that they may have manually edited the PGN to add those parenthesis, because I got access to their Chess.com analysis and it isn't present there. They say they didn't add the parenthesis, but I'm not sure how else it would have appeared.

Regardless, it would be nice if the viewer either rendered an error message or handled it like lichess does (lichess just dropped off the invalid variation but kept the rest of the game).

@mliebelt
Copy link
Owner

mliebelt commented Jun 1, 2023

Ok, this sounds reasonable. Something like:

  • When a pgn notation can be read by the parser without problem
  • and the following checking by the reader leads to a problem,
  • the system should show and error and at least accept the moves until the wrong one.

@jackstenglein
Copy link
Author

@mliebelt Hello, I have another PGN that crashes the viewer.

See a codesandbox demonstrating the crash here: https://codesandbox.io/s/pgn-viewer-error-7g2gyf?file=/src/App.js

Here is a link to the PGN in a lichess study: https://lichess.org/study/lIKRjbVE/QygmnUCl

And finally, here is the raw PGN:

[Event "Live Chess"]
[Site "Chess.com"]
[Date "2022.12.19"]
[Round "-"]
[White "DustinDreams"]
[Black "Johnny_Chest"]
[Result "1-0"]
[WhiteElo "845"]
[BlackElo "829"]
[TimeControl "3600"]
[Termination "DustinDreams won on time"]
[UTCDate "2022.12.19"]
[UTCTime "01:45:45"]
[Variant "Standard"]
[ECO "B00"]
[Opening "Nimzowitsch Defense: Declined Variation"]
[Annotator "https://lichess.org/@/DustinDreams"]

1. e4 { [%clk 0:59:52] } 1... Nc6 { [%clk 0:59:54] } 2. Nf3 { [%clk 0:59:10] } 2... d5 { [%clk 0:59:48] } 3. d3?! { [%c_effect
d3;square;d3;type;Inaccuracy;persistent;true] } { I wanted to defend the pawn. I'm still material centric, and the thought of him brining out his queen early scared me....which is ridiculous, because I know I can deal with aggressive queen players fine at my level. } { [%clk 0:58:04] } (3. d4) (3. exd5 Qxd5 4. d4 (4. Nc3 Qd8)) 3... f5? { [%c_effect
f5;square;f5;type;Mistake;persistent;true] [%clk 0:59:12] } 4. e5?! { [%c_effect
e5;square;e5;type;Inaccuracy;persistent;true] } { I was thinking he might be interested in trading knights here, which I doubt was advantageous, but for some reason I tend to err on the side of simplification, although here it wasn't necessary. } { [%clk 0:57:43] } 4... d4 { [%clk 0:58:44] } 5. Bf4?! { [%c_effect f4;square;f4;type;Inaccuracy;persistent;true] } { I figured I'd defend the e5 pawn again, but now I'm seeing that I'm just chasing ghosts here. There is no way he takes this pawn with his knight, so I don't know why I wasted the move here. To add insult to injury, my bishop is blocked. } { [%clk 0:56:45] } (5. g3 Qd5 6. c4 Qc5 7. Bg2 Nxe5 8. O-O Nxf3+ 9. Qxf3) 5... e6 { [%clk 0:58:32] } 6. c3 { I go for a pawn break here so I can recapture with my knight and develop the piece. I also want to get rid of his pawn so I can move d3-d4, creating space for my light squared bishop. The bishop is terribly cramped right now, and I'd love to castle kingside asap. } { [%clk 0:54:54] } 6... h6 { [%clk 0:57:19] } 7. cxd4 { I double my pawns here. Not sure if this is good, but my expectation is he takes with his knight. I have plans to move Qa4+ so I can capture his knight on d4. } { [%clk 0:52:53] } 7... Nxd4 { [%clk 0:57:14] } 8. Qa4+ { [%clk 0:52:48] } 8... c6?? { [%c_effect
c6;square;c6;type;Blunder;persistent;true] } { My opponents move gives me the chance to capture his knight. Although I have not reviewed with an engine, chesscom's in-game move list labeled his move as a blunder. I was expecting Qd7 or Bd7, but certainly not c6. } { [%clk 0:56:55] } 9. Qxd4 { [%clk 0:52:40] } 9... Bd7 { [%clk 0:56:49] } 10. Nc3 { I get to develop my knight finally, but I certainly have not achieved one of my mini-goals from earlier. My light-squared bishop is still locked away. His light-squared bishop is locked up right now, so my guess is he's going to play c5 to create space for his bishy and threaten my queen. } { [%clk 0:50:50] } 10... c5 { [%clk 0:56:31] } 11. Qc4 { There weren't a lot of squares for me to consider here. Qe3 seemed like a step in the wrong direction. Qc4 also gives me the chance to move d3-d4, creating space for my light-squared bishop. I also like vacating the d4 square for my knight on f3 in a future move. } { [%clk 0:49:41] } 11... g5 { [%clk 0:55:35] } 12. Be3 { I kept it simple here, choosing to maintain a tiny shred of central control. If he moves f4 to threaten my bishy, I have already planned on Bxc5 with the hopes of trading bishops and picking up a free pawn with the queen. } { [%clk 0:48:29] } 12... a6 { I think he's setting up b5 to bolster his threat against my queen, but he's too late because of my plan to trade bishops. } { [%clk 0:55:13] } 13. Bxc5 { [%clk 0:47:53] } 13... Bxc5 { [%clk 0:54:55] } 14. Qxc5 { I like this position, even though my queen is hanging. I'm eyeing Qd6 here. Loving the freedom without the dark-squared bishops on the board. } { [%clk 0:47:47] } 14... b6 { [%clk 0:54:45] } 15. Qd6 { This move probably isn't that great. I needed a safe square for my queen, but I'm not too sure how safe it is. } { [%clk 0:44:24] } 15... g4 { [%clk 0:54:27] } 16. Nd4 { Nd4 was the natural move here. My eye is on the e6 pawn. I think I can take with Nxe6, Bxe6, Qxe6, and pick up a free pawn while placing my opponent in check. } { [%clk 0:44:13] } 16... Rc8 { [%clk 0:54:10] } 17. Nxe6 { [%clk 0:42:18] } 17... Bxe6 { [%clk 0:54:05] } 18. Qxe6+ { This position is very awkward for me. And what about my light-squared bishop? She's still locked up! So my queen feels very exposed, although I'm not too upset about picking up the free pawn. } { [%clk 0:42:11] } 18... Ne7 { I think this was a good move by my opponent. I had my eyes on the f5 pawn, and his knight shuts that plan down. With this move, I know my queen must find a new square soon because its a sitting duck. } { [%clk 0:54:03] } 19. Rd1 { I chose to defend the pawn here because my plan was to get my knight into the game. Marching the d pawn up the board is also appealing to me so I can add some queenside pressure and free the bishop. } { [%clk 0:41:50] } 19... Rc6 { [%clk 0:53:29] } 20. Qb3 { When you gotta get out of dodge, you gotta get out of dodge. Qb3 maintains some king control by preventing castling, and it is offering some extra protection to my knight as well as the d1 rook. } { [%clk 0:40:54] } 20... f4?! { [%c_effect f4;square;f4;type;Inaccuracy;persistent;true] [%clk 0:52:47] } 21. d4 { Figured I'd get Danny marching and defend Edward. This also opens up the 3rd rank just in case I need to smash some pawn on 3rd rank. } { [%clk 0:39:28] } 21... f3 { [%clk 0:52:43] } 22. g3 { Not a huge fan of messing up my pawns here, especially since I plan on castling soon, but I'm find with this. He doesn't have any bishops, and my king feels safe here. } { [%clk 0:39:10] } 22... Nf5 { [%clk 0:52:29] } 23. Bxa6 { I take the hanging pawn because it was in the way. What I REALLY wanted was to pin his rook with Bb5. I think I can still achieve this, but the a6 needed to go. } { [%clk 0:38:59] } 23... Nxd4 { Something tricky pops up here. My pin idea with Bb5 is still possible...If I give him my queen and take the pin, I can snag his queen with my rook and take his rook with my bishop. Damn, I wish I had better language to describe this! I'm not sure if my calculation is correct here. Perhaps after sacrificing the queen I should take with the bishop Bc6+. } { [%clk 0:52:18] } 24. Bb5 { [%clk 0:36:37] } 24... Nxb3 { [%clk 0:52:10] } 25. Rxd8+?! { [%c_effect d8;square;d8;type;Inaccuracy;persistent;true] [%clk 0:36:36] } (25. Bxc6+ Ke7 26. Rxd8 Rxd8 27. axb3) 25... Kxd8 { [%clk 0:52:06] } 26. Bxc6 { [%c_effect
e1;square;e1;type;Winner,d8;square;d8;type;TimeoutBlack] <br /><br />Game may
have continued... } { My opponent stalled for the remaining 50 mins. Terrible sportsmanship. I picked up the W and used those 50 mins to complete a bunch of polgar haha. } { [%clk 0:36:35] } (26... Nd4 27. Ba4 Ke7 28. h3 gxh3 29. Rxh3 Ke6 30. Rh5 { +7.48 }) 1-0



@jackstenglein
Copy link
Author

After looking at it a bit more, I believe it is crashing due to this Lichess format where you can force a variation to a move that wasn't played:

Screenshot 2023-06-17 at 10 53 56 PM

@mliebelt
Copy link
Owner

In my opinion, this is a bug of Lichess. It is just not a valid notation to have the main line (continuation not played, but not a variant) written down as variant. I tried to create a study and import there your PGN, but the final variation is just ignored when being imported. Could you please copy your study, remove whatever you want, make the study public and insert the link here? I would like to see that, because I am not able to create it.

Ok, played around a little bit, found the menu entry "Force variation", which gives you what you have. But really, that is not valid PGN, even if Lichess supports that. When I export that game then, the variation is omitted. Really strange behavior :-(

Here is the link to my study: https://lichess.org/study/5lEABhpR#Hc1eQeGh

I tend to just close that issue, because it is not a useful behavior ...

@jackstenglein
Copy link
Author

Yeah, I just tested exporting and then reimporting the same study into Lichess and it changes it from a forced variation to the mainline. That is really weird behavior, and I can understand not supporting it in this case. I was able to get a workaround where the viewer doesn't crash and it just drops off the incorrect variation anyway. Feel free to close this issue if you think there aren't any changes to make

@mliebelt
Copy link
Owner

mliebelt commented Nov 17, 2023

I tried the above PGN as well on Lichess, and I have inconsistent behavior there (don't know why):

  • Using the analysis board to do an import ==> PGN is just ignored completely
  • Using a study, importing the PGN there ==> PGN is read until the main line finishes, the rest is ignored

Both is not what the user would expect.


I tried it then in the configuration builder and got then the following error (which I think is the correct one):

... Error: No legal move: Bc6

@mliebelt
Copy link
Owner

So we have 2 options here in that case:

  1. Just close the ticket, because the behavior of the reader/viewer is reasonable, and should not be changed.
  2. Allow a variant after the last move, with the special consideration that the variant continues the main line, and is not an alternative for it.

In my opinion, the second is a good idea, but I don't know if it is "easy enough" to implement it. Therefore leave the ticket open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants