0
\$\begingroup\$

So there is this game 2048. You can play it here

I wrote a python script which "plays" the game on the website by clicking the arrow keys.

I would like to know what can be improved. Is it a good idea to check the Try again button with exceptions? Is there a better way?

Currently it takes alot of tries to get a high score for the bot. Can the logic be improved?

play_2048.py

"""
Opens the webpage of the 2048 game and plays it repeadly.
Needs chromedriver in the directory of the script. Download it here:
https://sites.google.com/a/chromium.org/chromedriver/downloads
"""

from selenium import webdriver
from selenium.webdriver.common.keys import Keys

def play_2048():
    """
    Main lopp of script. First opens the browser to the game page.
    Then pushes the arrow keys to play the game.
    If a game over occurs the try again button on the page is klicked
    to repeat the game
    """
    browser = webdriver.Chrome()
    browser.get('https://play2048.co/')

    html = browser.find_element_by_tag_name('html')

    while True:
        html.send_keys(Keys.RIGHT)
        html.send_keys(Keys.UP)
        html.send_keys(Keys.LEFT)
        html.send_keys(Keys.DOWN)

        try:
            retry = browser.find_element_by_link_text('Try again')
        except:
            continue
        else:
            retry.click()

if __name__ == '__main__':
    play_2048()
\$\endgroup\$
4
\$\begingroup\$

There isn't really much here to comment on, since there isn't even AI involved.

The only thing that stands out to me is your use of a silent "catch-all" try block:

try:
    retry = browser.find_element_by_link_text('Try again')

except: # Big problem here
    continue

else:
    retry.click()

You're catching every exception, and simply ignoring them. In this particular case the ramifications aren't huge. Say in the future though you add a second function to do some processing and decide to put it in the try (for whatever reason). Something like:

try:
    retry = browser.find_element_by_link_text('Try again')
    do_something_with_retry(retry) # This throws. Would you ever know that though?

except:
    continue

else:
    retry.click()

And lets say that do_something_with_retry is improperly tested, or some unforeseen circumstance comes up, and suddenly do_something_with_retry starts throwing exceptions. Would you ever even know that there was an exception thrown? How would you debug your code failing in that case? It would be much better to be specific about what you need to catch, and deal with that only. As you can see in the API documentation, find_element_by_link_text is documented to throw a NoSuchElementException if something fails. Catch that instead, so irrelevant errors aren't silently ignored as well:

try:
    retry = browser.find_element_by_link_text('Try again')

except NoSuchElementException as e: # Be specific!
    continue

else:
    retry.click()

Now anything other than a NoSuchElementException will cause your program to crash. This is a good thing though. You don't want fundamentally broken code to run.

Don't make your code vulnerable to silly mistakes that may happen in the future. Silly mistakes happen (like improperly tested functions). It's best that they fail catastrophically so they can be detected and fixed.

Of course though, the better option to handling the addition of do_something_with_retry would be, if possible, to just not group it in the try in the first place. Again though, mistakes happen.


As for

Currently it takes alot of tries to get a high score for the bot. Can the logic be improved?

That involves writing up an AI to play the game, and is no small feat. That would be beyond the scope of a Code Review unfortunately.

\$\endgroup\$
  • \$\begingroup\$ if i define name 'NoSuchElementException' is not defined it is not defined \$\endgroup\$ – Sandro4912 Dec 11 '18 at 16:50
  • \$\begingroup\$ @Sandro4912 You need to import it from selenium. That's not a standard exception. \$\endgroup\$ – Carcigenicate Dec 11 '18 at 16:52
  • \$\begingroup\$ it worked by adding from selenium.common.exceptions import NoSuchElementException \$\endgroup\$ – Sandro4912 Dec 11 '18 at 16:54
1
\$\begingroup\$

A first step towards writing an AI is to give this an interface.

First, let's rewrite the "AI" you are currently using as a class:

from itertools import cycle

class Cycler:
    def __init__(self):
        self.cycle = cycle([Keys.RIGHT, Keys.UP, Keys.LEFT, Keys.DOWN])

    def next(self, *args):
        return next(self.cycle)

Which you can use like this in your current code:

ai = Cycler()
while True:
    html.send_keys(ai.next())
    try:
        ...

In the future you might want to be smarter than that and for that you would need the state of the board to be passed along to the AI.

Class GameStatePrinter(Cycler):
    def next(self, game_state, *args):
        print(game_state)
        return super(self, GameStatePrinter).next(*args)

This is not really any smarter, but you can pass it the current game state and it will print it:

ai = GameStatePrinter()
while True:
    current_state = get_current_state(html)
    html.send_keys(ai.next(current_state))
    ...

An actual smart bot would then of course act differently according to the information in the board, whereas this one just does the same thing as Cycler, which it inherits from.

This logic is left as an exercise for the reader.

\$\endgroup\$
  • \$\begingroup\$ i tryed adding the first part but in the line self.cycle = cycle[Keys.RIGHT, Keys.UP, Keys.LEFT, Keys.DOWN] it throws 'type' object is not subscriptable so maybe the keys don't work with cycler \$\endgroup\$ – Sandro4912 Dec 11 '18 at 17:05
  • \$\begingroup\$ @Sandro4912: You missed the parenthesis around the list, it should be self.cycle = cycle([Keys.RIGHT, Keys.UP, Keys.LEFT, Keys.DOWN]) \$\endgroup\$ – Graipher Dec 11 '18 at 17:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.