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

Now move method returns a future. #6

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

Conversation

DrDub
Copy link

@DrDub DrDub commented Dec 7, 2014

Most changes are in Kernel. This solution uses a semaphore to minimize
modifications to the code but a more async solution should be possible.

Closes #3.

Most changes are in Kernel. This solution uses a semaphore to minimize
modifications to the code but a more async solution should be possible.
@ornicar
Copy link
Owner

ornicar commented Dec 7, 2014

I assumed a synchronous API was acceptable here, because the application runs a single bot, and it's ok to block a single thread.
However, I'm OK for moving to a Future signature, but not if it means introducing Java semaphores.

@DrDub
Copy link
Author

DrDub commented Dec 8, 2014

Yes, I understand. I was also disappointed to have to introduce a semaphore.

I'll look for a solution without semaphores (most likely with the type of ExecutionEnvironment used for testing) and update this PR.

(When you say "a synchronous solution here" do you mean to use an Await construct...?)

@ornicar
Copy link
Owner

ornicar commented Dec 8, 2014

I mean that the function doesn't return a Future.
If your implementation uses Futures, then it means using Await.result. Since the starter runs a single bot, there is no harm in it.

@DrDub
Copy link
Author

DrDub commented Dec 10, 2014

OK, I updated it, let me know what you think.

@@ -65,7 +69,7 @@ object Main {
def step(server: Server, input: Input) {
if (!input.game.finished) {
print(".")
step(server, server.move(input.playUrl, bot move input))
step(server, server.move(input.playUrl, Await.result(bot move input, 60.minutes)))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shouldn't wait for 60 minutes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I changed it to 1sec

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.

Bot move function should return a => Future[Dir] ?
3 participants