Add MultiPV support to uci.SearchResults

notnil / chess

chess package for go

MIT License

508 stars 126 forks source link

Add MultiPV support to uci.SearchResults #100

Open kmorrison opened 2 years ago

kmorrison commented 2 years ago

References https://github.com/notnil/chess/issues/99

Hi notnil, big fan of your chess module. I ran into a use-case, as described in the above issue, and thought I'd submit a possible fix. I'm new to go so any input on style or approach is appreciated.

Add new field to SearchResults for MultiPV searches
Add response parser, mostly so I could test the new parsing method with some repeatability. I have no idea how liable the UCI output is to change as versions of stockfish get bumped, so I tried to divorce the parsing of the UCI output from the actual engine definition.
In order to remove the engine knowledge from the parser, I had to create a debug logger method that I could pass in to the parser, in order to keep the e.readLine() behavior. Not sure if this is the best way, but I thought I'd let you decide.

notnil commented 2 years ago

Very cool! I will take a look this weekend. Thanks for the PR!

cmitsakis commented 1 year ago

Your PR has helped me. Thank you.

One issue I noticed is that if you do a MultiPV search, eng.SearchResults().Info shows the score of the last MultiPV result which is the worst move, so the score is wrong.

So I got the score from the 1st MultiPV result: eng.SearchResults().MultiPV[0].Score I believe that's the correct score of the current position.

Also MultiPV is of type []*Info but it could be []Info. It's also not formatted properly you need to run go fmt.

cmitsakis commented 1 year ago

I implemented the changes I described and created a new PR

kmorrison commented 1 year ago

Thank you. Sorry, I've been out of town this weekend. Thank you for fixing

On Thu., Sep. 29, 2022, 7:48 a.m. Charalampos Mitsakis, < @.***> wrote:

I implemented the changes I described and created a new PR

— Reply to this email directly, view it on GitHub https://github.com/notnil/chess/pull/100#issuecomment-1262164252, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE7GS4Y6MX2L7TH2QWQ52DWAV6XJANCNFSM5TEQFNQA . You are receiving this because you authored the thread.Message ID: @.***>

kogan69 commented 1 year ago

Guys, any chance that this will be merged anytime soon? I'm really eager to get this :) And thanks again for the great job!!!