bots: Create Chess Bot. #205

marco · 2017-12-07T01:10:00Z

Chess Bot is a bot that allows you to play chess against either another user or the computer. Use start with other user or start as <color> with computer to start a game.

In order to play against a computer, chess.conf must be set with the key stockfish_location set to the location of the Stockfish program on this computer.

(Done for the "Create your own Chess bot." Google Code-in task.)

showell · 2017-12-07T01:17:30Z

zulip_bots/zulip_bots/bots/chess/test_chess.py

+
+To make your next move, respond to Chess Bot with
+
+```rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1: do <your move>```


I feel like it would be easier to communicate this using examples. Chess players are more likely to know chess notation than being able to parse out what you're saying here, I think.

OK, I'll add an example. Thanks! The reason the beginning part (rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1) is needed is because the bot can't carry over data from earlier messages, so instead the board information has to be sent along with the move.

Hey @skunkmb! We do support saving the data. I am in a bit of a hurry so cannot link to exact code samples but take a look at the virtual_fs bot and try running it once as well.

We do not need to send the data with each move and can use our bot handler storage. 😃

@showell @aero31aero I added to my commit, and now the match data is saved between responses. I didn't know you could do that 😃 ! Thanks!

I think this makes it a lot more intuitive for the user to understand what they're supposed to do. Now the user doesn't have to have all of the match data at the front of their response.

showell · 2017-12-07T01:18:20Z

@skunkmb I'm excited about this! I made a couple initial comments.

aero31aero · 2017-12-07T05:50:53Z

Looks great at a glance @skunkmb!

Just a minor request. I'm sure this PR would be going through some further changes before being merged, so can you please add new commits to this instead of amending the previous commit? That would make it easier for you and the reviewers to see at a glance what was changed from a past version. We'll just squash the commits finally before merging. :)

marco · 2017-12-07T13:35:09Z

@aero31aero OK, will do in the future. Thanks!

tommyip

Awesome work! Just some syntactical changes needed.

tommyip · 2017-12-07T16:57:01Z

zulip_bots/zulip_bots/bots/chess/chess.py

+        move_regex_match = re.match(MOVE_REGEX, content)
+        resign_regex_match = re.match(RESIGN_REGEX, content)
+
+        is_with_computer = bot_handler.storage.get('is_with_computer') if (


We usually prefer to use ternary operators only for single line assignments.

@tommyip OK, I simplified it to an if statement in d91b679.

tommyip · 2017-12-07T16:57:12Z

zulip_bots/zulip_bots/bots/chess/chess.py

+            bot_handler.storage.contains('is_with_computer')
+        ) else False
+
+        last_fen = bot_handler.storage.get('last_fen') if (


Save as above

@tommyip OK, I simplified it to an if statement in d91b679.

tommyip · 2017-12-07T16:58:14Z

zulip_bots/zulip_bots/bots/chess/chess.py

+                bot_handler,
+                True if start_computer_regex_match.group(
+                    'user_color'
+                ) == 'white' else False


Maybe just start_computer_regex_match.group('user_color') == white?

@tommyip OK, I changed that in d91b679. Thanks!

tommyip · 2017-12-07T17:12:04Z

zulip_bots/zulip_bots/bots/chess/chess.py

+    def handle_message(self, message, bot_handler):
+        START_REGEX = 'start with other user$'
+        START_COMPUTER_REGEX = (
+            'start as (?P<user_color>white|black) with computer'


Not that it is that important, but we should avoid compiling regex in loops. (Might become more important later on when we decide to run bots directly on the chat server)

@tommyip OK. How should I go about compiling the RegEx outside of the loop? Or is it not something I should worry about for now? Thanks!

Just re.compile it in the global namespace.

By the loop I just mean the handle_message function, since that is called on every message it receives.

@tommyip Oh, I understand now! 😃 Thanks! I fixed that in commit 4a31674.

tommyip · 2017-12-07T17:14:01Z

zulip_bots/zulip_bots/bots/chess/chess.py

+        Replies to the bot handler.
+
+        Parameters:
+             - message (object): The Zulip Bots message object.


We uses Python 3 inline type annotation. (Check out http://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html)

@tommyip OK, I added the type annotations in 95437b0 and 0e159f7. Thanks!

aero31aero · 2017-12-08T06:16:43Z

Hey, not a complete review here, just a minor comment.

You need to add the files to tools/run-mypy's force_include list for mypy to validate the type annotations. When you do that, you can run tools/test-static-analysis to run mypy.

marco · 2017-12-08T15:34:32Z

@aero31aero OK, I added it. I also had to make a few changes in the annotations in order for it to pass. Thanks!

aero31aero · 2017-12-08T15:52:28Z

tools/run-mypy

@@ -46,6 +46,7 @@ force_include = [
    "zulip_bots/zulip_bots/bots/googlesearch/test_googlesearch.py",
    "zulip_bots/zulip_bots/bots/help/help.py",
    "zulip_bots/zulip_bots/bots/help/test_help.py",
+    "zulip_bots/zulip_bots/bots/chess/chess.py",


Also, test_chess.py?

OK, added in in e0ffeef. Thanks!

aero31aero · 2017-12-08T15:54:04Z

zulip_bots/zulip_bots/bots/chess/chess.py

@@ -22,7 +23,7 @@ def usage(self):
            'Stockfish program on this computer.'
        )

-    def initialize(self, bot_handler):
+    def initialize(self, bot_handler: ExternalBotHandler) -> None:


I think we should further apply this style to other bots as well. This is certainly loads better than : Any. @neiljp is this what you were recommending?

aero31aero

LGTM after a code read. Albeit 700 LoC is a lot to go through so I might have missed something. @neiljp can you take a look as well?

i believe if we find bugs we can do incremental and small fixes later.

zulipbot · 2017-12-08T22:58:41Z

Heads up @skunkmb, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

showell · 2017-12-09T02:05:40Z

@skunkmb Go ahead and squash this into a single commit, and we'll merge it. We'll want to showcase it on chat.zulip.org by having you run it off of your laptop pointing at chat.zulip.org.

I agree with @aero31aero that we may have some fix-forward work to do, but that's fine.

WE HAVE A CHESS BOT! hahah 😄

marco · 2017-12-09T03:41:54Z

@showell Squashed! 😃 It's running now on chat.zulip.org as @chess-bot, for example in this topic. Thank you!

Chess Bot is a bot that allows you to play chess against either another user or the computer. Use `start with other user` or `start as <color> with computer` to start a game. In order to play against a computer, `chess.conf` must be set with the key `stockfish_location` set to the location of the Stockfish program on this computer. Use `bot_handler.storage` to preserve game state across messages.

marco · 2017-12-09T16:25:45Z

There was a merge conflict, so I rebased the commit and pushed it. Other than that it's the same as before the rebase.

tommyip · 2017-12-09T18:33:52Z

FYI I approved the GCI task. @skunkmb can you follow with this until it is merged.

marco · 2017-12-09T18:42:58Z

@tommyip Thank you! Yes, I'll definitely follow this. 😃

showell · 2017-12-10T15:08:16Z

I am working on getting this merged. I am converting the tests to use StubBotTestCase, and we need to handle an empty message properly.

showell · 2017-12-10T15:16:37Z

Ok, merged as 700ce6a

Thanks @skunkmb!

marco force-pushed the create-chess-bot branch from 312dc7d to ffa54b3 Compare December 7, 2017 01:11

showell reviewed Dec 7, 2017

View reviewed changes

marco force-pushed the create-chess-bot branch 3 times, most recently from c49cb7e to 91d44e2 Compare December 7, 2017 05:25

tommyip reviewed Dec 7, 2017

View reviewed changes

marco force-pushed the create-chess-bot branch from b85aa04 to 02747f3 Compare December 8, 2017 15:24

aero31aero reviewed Dec 8, 2017

View reviewed changes

aero31aero approved these changes Dec 8, 2017

View reviewed changes

marco force-pushed the create-chess-bot branch from 02747f3 to e0ffeef Compare December 9, 2017 00:16

marco force-pushed the create-chess-bot branch 2 times, most recently from 8da4c59 to 307a1ac Compare December 9, 2017 03:26

marco force-pushed the create-chess-bot branch from 307a1ac to 9811c1a Compare December 9, 2017 16:23

showell closed this Dec 10, 2017


		To make your next move, respond to Chess Bot with

		```rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1: do <your move>```

Uh oh!

bots: Create Chess Bot. #205

bots: Create Chess Bot. #205

Uh oh!

Conversation

marco commented Dec 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

showell commented Dec 7, 2017

Uh oh!

aero31aero commented Dec 7, 2017

Uh oh!

marco commented Dec 7, 2017

Uh oh!

tommyip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marco Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marco Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marco Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marco Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tommyip Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marco Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aero31aero commented Dec 8, 2017

Uh oh!

marco commented Dec 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aero31aero left a comment

Choose a reason for hiding this comment

Uh oh!

zulipbot commented Dec 8, 2017

Uh oh!

showell commented Dec 9, 2017

Uh oh!

marco commented Dec 9, 2017

marco Dec 7, 2017 •

edited

Loading

marco Dec 7, 2017 •

edited

Loading

marco Dec 7, 2017 •

edited

Loading

marco Dec 7, 2017 •

edited

Loading

tommyip Dec 7, 2017 •

edited

Loading

marco Dec 7, 2017 •

edited

Loading