5
\$\begingroup\$

I've been glancing at Game of Life for quite a while, but been reluctant towards graphical packages like pygame. But today I finally got over myself and did it.

Did you know that there the "problem" computationally only depends on one condition? I didn't, and followed the conditions of the game at first.

At first I used list to contain the coordinates if alive bricks or plebes as I call them. But I quickly realized that order did not matter and I abandoned the lists. Way are Python lists so slow?

Peer reading makes me wiser, so feel free to give any pointers.

class GameOfLife:
    def __init__(self, size=800):
        """
        The implementation is based on sets and dict, because order never
         matters in this game. Sets and dicts are faster. There is no need
         for complex data-structures like list of lists, order don't matter.


        :param self.screen: the screen used by pygame.
        :param self.white: color for pygame. alive plebs
        :param self.black: color for pygame. dead plebs or "empty" area

        :param self.width: screen width
        :param self.size: size of a pleb.
        :param self.alive: alive plebs.
        :param self.last_config: if self.alive, has not changed, it will not
         change. So this is an end condition.
        """
        self.screen = pygame.display.set_mode((size, size))
        self.white = (255, 255, 255)
        self.black = (0, 0, 0)

        self.width = size
        self.size = size//100
        self.brick = [self.size, self.size]
        self.alive = set()
        self.last_config = set()

    def show(self):
        pygame.Surface.fill(self.screen, self.black)
        for pos in self.alive:
            x, y = pos
            pos = x*self.size, y*self.size
            pygame.draw.rect(self.screen, self.white, list(pos)+self.brick, 0)
        pygame.display.flip()

    def setup(self):
        """
        Initialize the game, i.e. sets initial alive plebs.
        """

        alive_at_start = Configurations.glider_gun_mirroed.union(
            Configurations.glider_gun)

        self.alive = {(x, y) for x, y in alive_at_start}

    def get_connected_plebs(self, part: tuple) -> list:
        """
        Relative to a grid brick, there is only eight possible other connecting
         bricks. patterns defines them and if the brick is in the grid, it is
         returned
        """
        x, y = part
        patterns = [
            [-1, 0],
            [0, -1],
            [0, 1],
            [1, 0],
            [-1, 1],
            [1, -1],
            [1, 1],
            [-1, -1]
        ]
        return {(x+i, y+j) for i, j in patterns if
                all(k <= self.width//self.size for k in (x+i, y+j))}

    def generation(self):
        """
        For each generation there is only one condition we have to check, i.e,
        if a alive brick will survive. Everything computational else depends on
        this condition.
        """

        next_generation = set()
        cache = Counter()

        for pleb in self.alive:
            neighbors = self.get_connected_plebs(pleb)
            cache.update([n for n in neighbors])

            alive_neighbors = [x for x in neighbors if x in self.alive]
            if 1 < len(alive_neighbors) < 4:
                next_generation.add(pleb)

        for key, value in cache.items():
            if value == 3:
                next_generation.add(key)

        self.alive = next_generation

    def generate(self):
        """ The main loop of this game. """

        self.setup()
        while self.last_config != self.alive:
            self.last_config = self.alive
            self.show()
            self.generation()
            sleep(0.1)
        sleep(4)


def main():
    m = GameOfLife()
    m.generate()


if __name__ == '__main__':
    main()

Many rows and ridiculously much time goes to waste while hard-coding the configurations of this game.

class Configurations:
    """ Starting configurations of the game. Don't examine this. """
    pentadecathlon = {
        (20, 19), (19, 19), (18, 19), (20, 20), (19, 20), (18, 20), (19, 21),
        (19, 22), (19, 23), (18, 24), (20, 24), (18, 27), (20, 27), (19, 28),
        (19, 29), (19, 30), (18, 31), (19, 31), (20, 31), (18, 32), (19, 32),
        (20, 32)
    }
    glider_gun = {
        (10, 32), (11, 30), (11, 32), (12, 20), (12, 21), (12, 28), (12, 29),
        (12, 42), (12, 43), (13, 19), (13, 23), (13, 28), (13, 29), (13, 42),
        (13, 43), (14, 8), (14, 9), (14, 18), (14, 24), (14, 28), (14, 29),
        (15, 8), (15, 9), (15, 18), (15, 22), (15, 24), (15, 25), (15, 30),
        (15, 32), (16, 18), (16, 24), (16, 32), (17, 19), (17, 23), (18, 20),
        (18, 21)
      }
    glider_gun_mirroed = {
        (72, 94), (72, 93), (74, 93), (84, 92), (83, 92), (76, 92), (75, 92),
        (62, 92), (61, 92), (85, 91), (81, 91), (76, 91), (75, 91), (62, 91),
        (61, 91), (96, 90), (95, 90), (86, 90), (80, 90), (76, 90), (75, 90),
        (96, 89), (95, 89), (86, 89), (82, 89), (80, 89), (79, 89), (74, 89),
        (72, 89), (86, 88), (80, 88), (72, 88), (85, 87), (81, 87), (84, 86),
        (83, 86)
    }
\$\endgroup\$
3
  • \$\begingroup\$ I'll post this as a comment not an answer because it's not a review of your code, nor about Python: you might like to read chapters 17 and 18 of Abrash's description of Game of Life implementations, which starts here. Note the compact formats in which the winning solutions store cell-neighbour-states. \$\endgroup\$ – ChrisW Oct 15 '16 at 11:37
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – 301_Moved_Permanently Oct 17 '16 at 8:02
  • \$\begingroup\$ I'll keep that in mind. \$\endgroup\$ – Simon Oct 17 '16 at 12:56
7
\$\begingroup\$

The hard coded configurations look extremely painful. A good technique in this kind of situations is to come up with a convenient way to write the configuration by humans, and some helper functions to translate the human readable configuration to the data structures needed by the program. Something like this:

glider = (
    "###",
    "  #",
    " # ",
)

And then, parse this with a helper function that takes the lines tuple and the x and y offsets where it should start drawing on the canvas.

def parse_config(x, y, lines):
    # ...

This function should create the kind of configurations that you did through no small pain.

Another variation on the same idea:

import textwrap

def normalized_ascii(text):
    return textwrap.dedent(text).strip()

glider_raw = """
    ###
      #
     #
    """

glider_ascii = normalized_ascii(glider_raw)

def parse_config(x, y, ascii):
    pass

The difference is that instead of a list, the raw configuration is a multi-line string, which might be slightly easier to type. The excess indentation indentation is easy to remove with textwrap.dedent, and the excess blank lines with strip. The parse_config helper will need to be rewritten to interpret this format. Actually, it can split the ascii input by line characters, and then it's the same format as the first example.

Make sure to unit test the helper function (parse_config) well. And then you can comfortably start using a much more user-friendly configuration syntax.

Note that it's not worthwhile to worry about the extra computation added by the configuration parser. The overhead is typically insignificant next to the main operations of the program. Making the configuration easy to write and read, at the expense of an extra parsing step is always worth it, and saves you time. This is thanks to eliminating the frustration of misconfiguration, which is a very high risk when the format is not easy to write and read.


The implementation should be written in terms of the domain. self.white and self.black are not terms of the domain, but implementation details. The implementation would read more naturally if you renamed these to self.empty_color and self.alive_color, respectively.


The patterns list is recreated in every call to get_connected_plebs. This is unnecessary, as this list is a constant. It would be better to make it a static attribute of the class.

Also, patterns is not a great name. These are coordinate offsets. So I'd call them offsets. And when iterating over it, instead of i and j as the loop variables, which are commonly used in counting loops, I'd call them dx and dy.


Instead of cache.update([n for n in neighbors]), you can make a copy of a list easier with cache.update(neighbors[:]). But do you actually need a copy here? Wouldn't simply cache.update(neighbors) be enough?

\$\endgroup\$
0
2
\$\begingroup\$

Your use of setup is not very friendly. It would be best to allow the user to specify the configuration itself and, as such, remove its call from generate. I would define it like:

def setup(self, alive_at_start):
    """ Initialize the game, i.e. sets initial alive plebs. """
    self.alive = set(alive_at_start)

You would also need to call it once with the desired configuration before calling generate.


You also happen to have a wrong documentation for get_connected_plebs as you say it returns a list but you return a set.

\$\endgroup\$

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.