Reasoning for External Command Execution

[dehlen]

Hi,

first of all congrats for a truly great package. The provided API looks really good and provides a real benefits to the iOS ecosystem which is lacking a good UCI implementation.

i was wondering about the reason you chose to fork Stockfish and lc0 in order to allow external command execution. I found similar packages for flutter/capacitor used by the lichess mobile app and to me it seems like they are sending commands through a stdin pipe. This way the engines source code does not need to be modified and I assume it makes updating the engine version much easier. Also you reduce the risk of introducing bugs/crashes by the given changes.

Would you mind sharing your thoughts on that?

The other thing i was wondering about is that you stated in #6 that you experienced some threading/concurrency issues. I saw that you recently worked on some related things. Do you still see these issues? Any update here would be great in order to decide whether using this library would work for me.

Kind regards, David

[pdil]

@dehlen thank you for your comments!

Regarding the external command execution, at the time I first wrote the package I was having issues getting Stockfish to recognize commands sent through stdin. There was this post I found explaining a way it can be done: https://forums.developer.apple.com/forums/thread/690310 but it makes use of Process() which isn't available on iOS. I can try to revisit now and see if I can find another way because I agree it would be a lot simpler and more future-proof to not have to change the internal Stockfish code. I have tried to make it as simple a change as possible by extracting the command execution in UCI::loop directly with no other changes but I will look at the libraries you mentioned to see if there's another way.

Regarding the performance, yes I've moved the communication to a background thread and it has improved greatly in my experience.

[dehlen]

Hi,

Thanks for getting back to me. I was especially looking at https://github.com/lichess-org/dart-stockfish/blob/master/ios/FlutterStockfish/ffi.cpp as this is used in the rewritten Lichess mobile app. I would assume this wrapper could be used as well in this package as already everything is in place for an Objective-C++ wrapper. You could then probably use the created pipe to create a FileHandle for writing. Although CPP is not exactly my speciality I am a professional iOS developer myself. So if i can help I would happily do so.

Kind regards, David

[pdil]

@dehlen good news! I managed to get the pipe working in the other direction so we can write directly to stdin. I've opened PR #12 for this.

I also updated the Stockfish fork to now use the latest 16.1. The one change I couldn't avoid however is that I had to rename main.cpp to _main.cpp and introduce a header file to import it for use with the engine messenger. This is because Swift Package Manager complains when there is a main.cpp in a package that does not have an executable target. I'll try to find a way around this but for now this is a reasonable compromise as it requires no modification of the Stockfish (and presumably any other engine) code!

Here is the commit with these changes if you're interested: https://github.com/chesskit-app/Stockfish/pull/3/commits/419cbc2477c5a451f7aa9458f4ace18b414b8cf8

[dehlen]

That was fast thank you very much 😄

The problem with main is something i also encountered while having a look but I think it is a reasonable change, if everything else can be untouched. Of course this is your package so whatever way you decide to go I think this implementation looks quite good. If you would like I can have a closer look at the Draft PR and provide code review comments. Anyway thanks for your time and the ongoing effort. I myself wrote a Swift package to parse PGN files which I ported from https://github.com/niklasf/rust-pgn-reader/tree/master. If you are interested I could share my implementation which does not rely on any regex to be quite performant. It can also handle multiple games in a single file just fine.

[pdil]

Yes any comments on the PR would be great.

I would also love to see the PGN parser. I have my own in my other chess package: https://github.com/chesskit-app/chesskit-swift/blob/master/Sources/ChessKit/Parsers/PGNParser.swift but it's nothing special and I haven't really tested the performance of it much. It's also still missing a few things like parsing move assessments and variations.

[dehlen]

Ok i'll have a look and will add my comments once i find time (probably will take me some days). Regarding the pgn parser i just made my WIP repo public for you to have a look at. As I am on paternity leave I find myself regularly away from the keyboard so this isn't really tested yet. However I added unit tests and on my first look at it it seemed to work just fine. A few quick notes on the implementation:

• It operates on bytes (UInt8) for performance reasons
• It uses a RingByteBuffer again for performance reasons. This way even large PGN files can be read since I only load parts of it into memory at once
• It does not check whether moves are valid as this would have a performance penalty and ultimately I think is job of the Chess logic SDK one is using
• It can read multiple games in a single PGN file just fine. One can either call readGames or readGame. There also is an Iterator available.
• Annotations and Variations are supported
• You can use the Visitor pattern of the library to transform the parsed data into your own data structure. I think it should be fairly simple to use it in your ChessKit package

No hard feelings if it doesn't fit your needs. If it does, feel free to use though. I thought i would bring it up in order to give you something in return of your work on this engine package.

[pdil]

Looks great! I will take a closer look when I have some time and see if it's a good fit for ChessKit.