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

Confirm parameter setting with info string uci response #1598

Open
mooskagh opened this issue Jul 1, 2021 · 7 comments
Open

Confirm parameter setting with info string uci response #1598

mooskagh opened this issue Jul 1, 2021 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mooskagh
Copy link
Member

mooskagh commented Jul 1, 2021

Currently we don't return anything on successful setparam set, and return error non-standart response in error.

It would be more visible in GUI if we used info string <text> in both cases.

@mooskagh mooskagh added enhancement New feature or request good first issue Good for newcomers labels Jul 1, 2021
@ErdoganSeref
Copy link

Is setparam a method in a source file? What <text> should we return?

@mooskagh
Copy link
Member Author

Error is output in this line:

SendResponse(std::string("error ") + ex.what());

The setparam is handled here:

options_.SetUciOption(name, value, context);

@ErdoganSeref
Copy link

Fixed part of the issue with #1755.

@rooklift
Copy link
Contributor

Is this necessary? I don't think other engines do this.

@mooskagh
Copy link
Member Author

Other engines probably don't do that indeed, but I believe it makes sense to make errors more visible.

Most of GUIs will show info string messages to the user somehow instead of ignoring, and I think it good and I don't even see drawbacks. Do you see why it could be bad?

@rooklift
Copy link
Contributor

rooklift commented Jun 30, 2022

I guess it will be fine, it's just I already have to parse info string lines for verbose move stats purposes, but I can probably ignore these.

Having said that, I think info lines are generally supposed to be sent during a search. I can't immediately predict what my own GUI will do when they come at other times, let alone other people's GUIs. [OK, I guess mine will ignore it.]

@mooskagh
Copy link
Member Author

Actually, particularly for confirming that the command worked, it may be questionable whether it's good or bad.
But for reporting errors, I think has much more pros than cons.

I'm pretty sure that many engines use info string for various purposes outside of search, most frequently to output license/logo/etc at the startup.

Hold on with the changes though, there's another one coming in which may be useful, I hope to add jsoninfo response soon (work in progress here: https://github.com/mooskagh/lc0/tree/json) which should be much easier to parse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants