chess icon indicating copy to clipboard operation
chess copied to clipboard

Add MultiPV support to uci.SearchResults

Open kmorrison opened this issue 2 years ago • 5 comments

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.

kmorrison avatar Apr 11 '22 21:04 kmorrison

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

notnil avatar Apr 21 '22 15:04 notnil

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 avatar Sep 23 '22 10:09 cmitsakis

I implemented the changes I described and created a new PR

cmitsakis avatar Sep 29 '22 11:09 cmitsakis

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: @.***>

kmorrison avatar Sep 29 '22 18:09 kmorrison

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

kogan69 avatar Feb 08 '23 19:02 kogan69