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

reg_genmove command #289

Merged
merged 1 commit into from
May 10, 2016
Merged

reg_genmove command #289

merged 1 commit into from
May 10, 2016

Conversation

ujh
Copy link
Owner

@ujh ujh commented May 10, 2016

Simulates a genmove but doesn't play the resulting move. Respects the time settings but doesn't update the remaining time.

Closes #285

Simulates a genmove but doesn't play the resulting move. Respects the
time settings but doesn't update the remaining time.

Closes #285
@ujh ujh added this to the 0.3.2 milestone May 10, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+1.7%) to 93.378% when pulling 58b7deb on reg_genmove into 110d8e5 on master.

@iopq
Copy link
Collaborator

iopq commented May 10, 2016

I'm still incredulous that we have a test for the actual output string that we have to modify every time we add or remove a command.

@ujh
Copy link
Owner Author

ujh commented May 10, 2016

That is not ideal, but then this whole GTP parsing code is a bit of a hack as we duplicate the list of commands. Once for list_commands and known_command and once for the actual command (a big match expression). That's why I think that test is still useful. Otherwise it would be possible to implement reg_genmove in the match expression but it won't appear in list_commands and so external tools wouldn't know it's there.

@iopq
Copy link
Collaborator

iopq commented May 10, 2016

I remember when I had solved this issue 😉

@ujh
Copy link
Owner Author

ujh commented May 10, 2016

In a more dynamic language I would have switched to a hash for this long ago. Use the keys for list_commands and known_command and execute the functions stored as values for the actual commands. Then everything would be in one place. I'm sure this is possible in Rust as well, but I haven't had the time (or the inclination) to figure that out yet.

@ujh ujh merged commit 855b2ff into master May 10, 2016
@ujh ujh deleted the reg_genmove branch May 10, 2016 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants