search

iigorr / pgn.net

Portable Game Notation (PGN) implementation in .NET
Other
33 stars 24 forks source link

Moveformatter uses capital B whenpawn on b-file captures #11

Closed Fredrik-Lindner closed 10 years ago

Fredrik-Lindner commented 10 years ago

When a pawn on the b-file captures (e.g. bxc6) the formatter wil use captial B making the move Bxc6 denoting a capture by the bishop. I don't know if this occurs all the times a b-pawn captures but it does sometimes.

Example

Original pgn: (note blacks 9:th move)

[Event "DHLC Slow 1-2 Pairing #03"] [Site "Live Chess on Chess.com"] [Date "2013.11.16"] [Round "2"] [White "amichay"] [Black "Alabasterknight"] [Result "1-0"] [ECO "C41"] [WhiteElo "1220"] [BlackElo "754"] [Annotator "Lenning,Bob"] [PlyCount "31"] [EventDate "2013.??.??"] [TimeControl "90"]

  1. e4 e5 2. Nf3 d6 3. d4 Nf6 4. Nc3 Bg4 5. Bg5 Be7 6. h3 Bxf3 7. Qxf3 Nc6 8. Bb5 a6 9. Bxc6+ bxc6 10. dxe5 Nd7 11. Bxe7 Qxe7 12. exd6 Qxd6 13. Rd1 Qc5 14. Qg4 f5 15. Qxg7 Qf8 16. Qxd7# 1-0

Output of game.ToString() (again note the (erronius) 9:th move of black:

"[Event \"DHLC Slow 1-2 Pairing #03\"]\r\n[Site \"Live Chess on Chess.com\"]\r\n[Date \"2013.11.16\"]\r\n[Round \"2\"]\r\n[White \"amichay\"]\r\n[Black \"Alabasterknight\"]\r\n[Result \"1-0\"]\r\n[ECO \"C41\"]\r\n[WhiteElo \"1220\"]\r\n[BlackElo \"754\"]\r\n[Annotator \"Lenning,Bob\"]\r\n[PlyCount \"31\"]\r\n[EventDate \"2013.??.??\"]\r\n[TimeControl \"90\"]\r\n\r\n1. e4 e5 2. Nf3 d6 3. d4 Nf6 4. Nc3 Bg4 5. Bg5 Be7 6. h3 Bxf3 7. Qxf3 Nc6 8. Bb5 a6 9. Bxc6+ Bxc6 10. dxe5 Nd7 11. Bxe7 Qxe7 12. exd6 Qxd6 13. Rd1 Qc5 14. Qg4 f5 15. Qxg7 Qf8 16. Qxd7# 1-0"

iigorr commented 10 years ago

Hi, thanks for reporting.

This is even worse as the bug is in the parser. It does not consider letter case. So B or b are considered capturing moves by a bishop. I will fix it in a minute, but I wonder about this:

Is bxc6 always a pawn move or could it be used for an different piece type if there is no ambiguity, e.g. end-game with only 3 pieces left?

Fredrik-Lindner commented 10 years ago

Hi iigor, thanks for the fix!

According to the PGN spec,http://www6.chessclub.com/help/PGN-spec, moves must be entered using "Standard Algebraic Notation (SAN)", http://en.wikipedia.org/wiki/Algebraic_chess_notation, which states that a Piece (excluding pawns) captures must be denoted with the piece letter (NBRQK), and pawn captures must be denoted using the file letter (a-h) so in your example bxc6 can only be used for a pawn capture.

However, the specification says that software parsers may allow the capital letter 'P' to denote pawns, and the SAN spec (as described in wikipedia) mentions the abbreviated notation for pawn captures (exd or ed rather than e.g. exd5). I don't know if your parser handles that (my F# skills are way to basic).

iigorr commented 10 years ago

Thanks for the response. All of those combinations are handled. exd, ed, exd5 all should work. However I wasn't sure if the capturing piece can always be assumed to be a pawn. For now in the latter form (exd5) I do not automatically assume it to be.

In brief:

However, Wikipedia states this: "When a pawn makes a capture, the file from which the pawn departed is used to identify the pawn. For example, exd5 (pawn on the e-file captures the piece on d5)"

So basically all three variations seem to mean "pawn on file E captures..." Thanks for hinting me to Wikipedia.

iigorr commented 10 years ago

BTW: Are you using the NuGet version of pgn.net? Do you want me to make a new release containing the fixes?

Fredrik-Lindner commented 10 years ago

I was but then I needed the fixes so I included the code in my project and struggled a bit ;-) Seems as FParsec is linked to FSharp.Core 4.0 which is a problem if you target you app to .Net 4.5 which requires FSharp 4.3. However, if you add an assembybinding in the config file FParsec will accept version 4.3

 <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="FSharp.Core" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="4.0.0.0" newVersion="4.3.0.0" />
      </dependentAssembly>
    </assemblyBinding>

Note that the assemblybinding must be in the executable and not in any libraries... took me a while to figure that one out :-)

Fredrik-Lindner commented 10 years ago

Errm... I had some xml snippets in that comment but it doesn't show... This is what I mean: http://stackoverflow.com/questions/11718385/system-wide-bindingredirect-for-f-4-0-0-0-to-4-3-0-0

iigorr commented 10 years ago

You can use ``` (back-ticks) for code blocks. See http://github.github.com/github-flavored-markdown/ I have fixed your comment.

As for the FParsec dependency, there are several problems especially with Windows Phone, I'll see if I can fix them or at least document in the README.

Thanks for pointing that out.

iigorr commented 10 years ago

Hey Fredrik,

I have created a bug fix release (v1.1.1), which includes both fixes. Also I have merged FSharp into the package, so now only the pgn.NET.dll should be required. No need to reference FSharp anymore. There are now two example projects for net4.0 and net4.5, but maybe you could verify that in your project.

Thanks for contributing! :+1:

Fredrik-Lindner commented 10 years ago

No worries, thanks for the lib.

If you ever fancy a game I play as "Bishop-Brask" @chess.com :-)

/F

Dodgero commented 9 years ago

Hi iigorr,

It seems this is still an issue. I have downloaded via nuget v1.1.1.0, and the bug is there when getting moves by .MoveText property. Also, another user signalized that the problem is still there yesterday. Could you please let me know if you could resolve this soon ? I'd appreciate that :)

Kind regards and thank you for this fine library!

iigorr commented 9 years ago

Hi Dodgero

Thanks for reporting! It seems that something went wrong with the package 1.1.1.0. The old broken parsers DLL was packed and I haven't noticed. Sorry for that. I have just published the fixed version as 1.1.1.1. Please let me know if there are any issues, I'll try to fix them asap.

Cheers,

Igor

Dodgero commented 9 years ago

Hi Igor,

This is great, thanks a lot! I will try it out.

Regarding issues, I don't know if what I am going to point out can be called that way, but I have noticed the library does not support some of the pgn header fields. I think it would be great if it would be possible to access them via properties and to get on the output the same what was on the input, so that no fields would be lost in the process. In other words, after operation pgn->reader->writer->pgn, the pgn should be the same on both sides. To make this happen, all headers would have to be processed, now they are cut off. Namely, header fields "WhiteElo", "BlackElo" I find as quite important.

Would you kindly consider adding support for other pgn header fields ? ;)

Cheers, Dodgero

2015-06-10 23:34 GMT+02:00 Igor Lankin notifications@github.com:

Hi Dodgero

Thanks for reporting! It seems that something went wrong with the package 1.1.1.0. The old broken parsers DLL was packed and I haven't noticed. Sorry for that. I have just published the fixed version as 1.1.1.1. Please let me know if there are any issues, I'll try to fix them asap.

Cheers,

Igor

— Reply to this email directly or view it on GitHub https://github.com/iigorr/pgn.net/issues/11#issuecomment-110921408.

iigorr commented 9 years ago

Hi Dodgero,

as the issue with the pawn interpreted as a bishop is hopefully closed, I have created a new issue https://github.com/iigorr/pgn.net/issues/13 for what you are suggesting. Let's discuss it there.

Cheers,

Igor