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

Add MultiPV support to uci.SearchResults #100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kmorrison
Copy link

References #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
Copy link
Owner

notnil commented Apr 21, 2022

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

@cmitsakis
Copy link

cmitsakis commented Sep 23, 2022

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
Copy link

I implemented the changes I described and created a new PR

@kmorrison
Copy link
Author

kmorrison commented Sep 29, 2022 via email

@kogan69
Copy link

kogan69 commented Feb 8, 2023

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

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

Successfully merging this pull request may close these issues.

4 participants