A game of automated tag

Question

This game has 4 people moving randomly, one unit in one of the four directions (up, down, left, right), with a fifth person as a designated tagger. If this fifth person tags someone, the person is out. Game ends when there is one survivor left.

import random


class Player:
    def __init__(self, name, status, speed, health, position):
        self.name = name
        self.status = status
        self.speed = speed
        self.health = health
        self.position = position

    def move(self, arena_limits, player_pos):
        change = {"left": -1, "right": 1, "up": 1, "down": -1}
        direction = random.choice(list(change.keys()))
        if not self.is_stuck(arena_limits, player_pos):
            if direction in ("left", "right"):
                in_limits = 0 <= self.position[0] + (change[direction] * self.speed) <= arena_limits[0]
                if in_limits and [self.position[0] + (change[direction] * self.speed), self.position[1]] not in player_pos:
                    self.position[0] += change[direction] * self.speed
                else:
                    self.move(arena_limits, player_pos)
            if direction in ("up", "down"):
                in_limits = 0 <= self.position[1] + (change[direction] * self.speed) <= arena_limits[0]
                if in_limits and [self.position[0], self.position[1] + (change[direction] * self.speed)] not in player_pos:
                    self.position[1] += change[direction] * self.speed
                else:
                    self.move(arena_limits, player_pos)
        else:
            pass

    def is_stuck(self, arena_limits, player_pos):
        can_go_left = ([self.position[0] + (-1 * self.speed), self.position[1]] not in player_pos) and \
                      0 <= self.position[0] + (-1 * self.speed) <= arena_limits[0]
        can_go_right = ([self.position[0] + (1 * self.speed), self.position[1]] not in player_pos) and \
                       0 <= self.position[0] + (1 * self.speed) <= arena_limits[0]
        can_go_up = ([self.position[0], self.position[1] + (1 * self.speed)] not in player_pos) and \
                    0 <= self.position[1] + (1 * self.speed) <= arena_limits[1]
        can_go_down = ([self.position[0], self.position[1] + (-1 * self.speed)] not in player_pos) and \
                       0 <= self.position[1] + (-1 * self.speed) <= arena_limits[1]
        if not (can_go_up or can_go_down or can_go_left or can_go_right):
            return True
        else:
            return False

    def tag(self, player_list):
        if self.status == "IT":
            done = False
            turns = 0
            while not done:
                turns += 1
                player_pos = [user.position for user in player_list] + [self.position]
                for player in player_list:
                    player.move([8, 8], player_pos)
                    above_or_under = player.position[0] in (self.position[0] + 1, self.position[0] - 1)
                    left_or_right = player.position[1] in (self.position[1] + 1, self.position[1] - 1)
                    if above_or_under or left_or_right:
                        del player_list[player_list.index(player)]
                        print(player.name + " has been eliminated after " + str(turns) + " turn(s)!")
                        if len(player_list) == 1:
                            print(player_list[0].name + " is the winner!")
                            print("-" * 20)
                            done = True
                self.move([8, 8], player_pos)


tagger = Player("TAGGER", "IT", 1, 0, [8, 8])


for b in range(10000):
    victims = []
    for i in range(4):
        victims.append(Player("Player" + str(i), "", 1, 0, [random.randint(0, 6), random.randint(0, 6)]))
    tagger.tag(victims)

Some quick comments about the code:

• Comments will be added to my code soon
• self.health is currently not used but is planned on being used
• The field is 8x8 and each person can move once in one of the four directions. If they are stuck is_stuck(), they will simply not move
• A player is out if they are directly above, next to, or below the tagger. Diagonally does not count

Main question here, can I shorten the code and can I make it more Pythonic?

Answer 1

Buggy game mechanics

Your program reports "Playerp has been eliminated after t turn(s)!", but without any visual confirmation, so you're blindly trusting that the logic is correct. But is it?

If I do add a visualization, then I see bizarre things like this happening (where T represents the tagger, and the numbers represent the pursued players):

⋮
Turn 5:
 ---------
|         |
|     3   |
|         |
| 2       |
|         |
|   0     |
|         |
|         |
|       T |
 ---------
Turn 6:
 ---------
|         |
|    3    |
|         |
|2        |
|         |
|         |
|   0     |
|         |
|      T  |
 ---------
Player0 has been eliminated after 7 turn(s)!
Player3 has been eliminated after 7 turn(s)!
Player2 is the winner!

Let's examine the tagging logic:

above_or_under = player.position[0] in (self.position[0] + 1, self.position[0] - 1)
left_or_right = player.position[1] in (self.position[1] + 1, self.position[1] - 1)
if above_or_under or left_or_right:
    del player_list[player_list.index(player)]
    …

Basically, this isn't a regular game of tag ("A player is out if they are directly above, next to, or below the tagger. Diagonally does not count"), but a game of laser tag with eight laser beams. Stepping on any of these rays would put a player in the kill zone:

       ┊ ┊
       │ │
       │ │
┈──────┼─┼──────┈
       │T│
┈──────┼─┼──────┈
       │ │
       │ │
       ┊ ┊

The moral of the story: verify what is happening visually. Furthermore, examine the progress at each time step. (Here's a similarly buggy simulation that could have benefitted from the same advice.)

As a consequence, I recommend that you split up your tag() method, which runs an entire game, to define a one_turn() method that just advances the game by one turn, to make it easier to examine the progress.

Design

The logic for dealing with illegal moves is cumbersome. It's repetitive, with the in_limits tests within move() being similar to the tests within is_stuck(). In move(), you make a choice of which direction to go, then awkwardly recurse to retry if that choice turns out to be illegal. It would be better to weed out the illegal moves first, then make a choice that is guaranteed to succeed.

Furthermore, move() has two branches of code to handle displacements along the two axes; it would be nice to handle displacements in any direction using the same code.

To help solve these issues, I would define a Coords class to help you figure out what the neighboring positions are. I would make it a namedtuple, because position.x and position.y would look less cluttered than position[0] and position[1].

---

The status member of the Player class is superfluous, as is the if self.status == "IT" check in the tag() method.

---

The fact that the move(), is_stuck(), and tag() methods need so many and such similar parameters passed to them is a hint that your OOP modeling is underdeveloped. In particular, the tag() method does not belong in the Player class. There really ought to be a TagGame class that defines the arena size, keeps track of its players, and encodes the rules of the game.

Style

When you say that the arena limits are [8, 8], you mean that it's a 9×9 board, which is unconventional. Your legal coordinates range from [0, 0] to [8, 8], inclusive. I would expect that when you specify [8, 8] as the arena limits, you would get an 8×8 board (either ranging from [0, 0] to [7, 7] inclusive, or from [1, 1] to [8, 8] inclusive).

When you get around to commenting your code, start by writing docstrings first.

Instead of composing strings by concatenation with +, use one of the several string-formatting options. Personally, I prefer str.format(), which is quite powerful and is supported by many versions of Python.

List comprehensions and generator expressions are your friend for writing compact Pythonic code!

Flag variables, such as done, are almost always a sign that the code could be improved by eliminating them.

else: pass is obviously superfluous.

del player_list[player_list.index(player)] is simply player_list.remove(player).

Suggested solution

To activate the visualization, uncomment the print() statements in TagGame.one_turn().

from collections import namedtuple
from random import choice, randint

class Coords(namedtuple('Coords', 'x y')):
    def is_in_bounds(self, bounds):
        return (0 < self.x <= bounds.x) and (0 < self.y <= bounds.y)

    def neighbors(self, bounds, steps=1):
        """
        Return a list of Coords that are the specified number of steps away in
        each cardinal direction and that are in bounds.
        """
        candidate_coords = [
                        Coords(self.x, self.y - steps),
        Coords(self.x - steps, self.y), Coords(self.x + steps, self.y),
                        Coords(self.x, self.y + steps),
        ]
        return [c for c in candidate_coords if c.is_in_bounds(bounds)]

class Player:
    def __init__(self, name, position, speed=1):
        self.name = name
        self.position = position
        self.speed = speed

    def __str__(self):
        return self.name

    def move(self, game):
        """
        Make a move in a random cardinal direction (unless it is impossible to
        do so without running out of bounds or stepping on another player).
        """
        player_positions = [p.position for p in game.players + [game.it]]
        destinations = [
            c for c in self.position.neighbors(game.bounds, self.speed)
            if c not in player_positions
        ]
        if destinations:
            self.position = choice(destinations)

class TagGame:
    def __init__(self, num_players):
        """
        Create a game with the specified number of players (plus the
        "it" player).
        """
        self.bounds = Coords(8, 8)
        self.it = Player("IT", Coords(8, 8))
        self.players = [
            Player("Player{0}".format(i), Coords(randint(1, 6), randint(1, 6)))
            for i in range(num_players)
        ]
        self.turns = 0

    def one_turn(self):
        """
        Run one turn of the game, yielding the players who are eliminated.
        """
        kill_zone = self.it.position.neighbors(self.bounds)
        for player in self.players:
            player.move(self)
            if player.position in kill_zone:
                self.players.remove(player)
                yield player
                if len(self.players) == 1:
                    # print(self)
                    return
        # print(self)
        self.it.move(self)

    def run(self):
        """
        Run the game until one player remains, and return that winning player.
        """
        while True:
            self.turns += 1
            for loser in self.one_turn():
                print("{0} has been eliminated after {1} turns!".format(
                    loser, self.turns
                ))
                if len(self.players) == 1:
                    print("{0} is the winner!".format(self.players[0]))
                    return self.players[0]

    def __str__(self):
        arena = (
            ' ' + '-' * self.bounds.x + ' \n' +
            '\n'.join(
                '|' + ''.join(
                    next((
                        p.name[-1] for p in [self.it] + self.players
                        if p.position == Coords(x, y)
                    ), ' ')
                    for x in range(1, self.bounds.x + 1)
                ) + "|"
                for y in range(1, self.bounds.y + 1)
            ) +
            '\n ' + '-' * self.bounds.x + ' '
        )
        return "Turn {0}:\n{1}".format(self.turns, arena)

# Your buggy laser tag games tend to finish in a handful of turns.
# A proper game of tag could sometimes take thousands of turns, so I
# wouldn't run 10000 games.
for simulation in range(2):
    TagGame(num_players=4).run()
    print("-" * 20)

Answer 2

If you are interested in seeing my (somewhat OOP underdeveloped) though process, you can look below. However, 200_success has presented a fleshed out solution which appears to have optimally tuned parts, so I would recommend their solution as a final product. I have no final code at the end of mine.

---

Here's some suggestions. This answer is an iterative process of explanation.

---

Initialization

Use if __name__ == '__main__':

In addition to having all of the procedural initialization code at the bottom, prefix it with an if __name__ == '__main__'. This allows other people to import parts of your module (the file) without running the code at the bottom.

(See also: What does if __name__ == “__main__”: do?)

Python's string formatting options are awesome.

Instead of "Player" + str(i)", do f"Player {i}". Prefixing a string with f makes the string a formatted string literal, which makes it easier to concatenate string and other variables by just putting the variable in curly brackets. This is new in Python 3.6.

Utilize list comprehensions

Instead of

victims = []
for i in range(4):
    victims.append(Player(f"Player {i}", "", 1, 0, [random.randint(0, 6) for i in range(2)]))

just do:

victims = [Player(f"Player {i}", "", 1, 0, [random.randint(0, 6), random.randint(0, 6)]) for i in range(4)]

DRYer is sometimes only negligibly better

Instead of repeating random.randint(0, 6) twice in [random.randint(0, 6), random.randint(0, 6)], one could just do a list comprehension [random.randint(0, 6) for i in range(2)] In this example, it makes almost no difference, but as you scale up or generalize, it can be useful to consider.

But this is only a taste of DRYer code; more of this idea will be developed later.

---

Player.move() and Player.is_stuck() methods

Define often-used "constants" as class variables, not local method variables...

Instead of this:

class Player:

    # ...

    def move(self, arena_limits, player_pos):
        change = {"left": -1, "right": 1, "up": 1, "down": -1}

do this:

class Player:

    # ...

    change = {"left": -1, "right": 1, "up": 1, "down": -1}

    def move(self, arena_limits, player_pos):

        # ...

There's no reason to redefine this each time the method is called.

...and utilize enums

Enums can be incredibly useful in any programming language, because they define a restricted set of values. Instead of defining a dictionary of strings, you can define some enums.

For more information about the why of enums, I would recommend reading this Stack Overflow answer.

Use DRY code

When I see something like the below, I immediately know the code can be made more compact, because the entire control flow is repeated.

if direction in ("left", "right"):
    in_limits = 0 <= self.position[0] + (change[direction] * self.speed) <= arena_limits[0]
    if in_limits and [self.position[0] + (change[direction] * self.speed), self.position[1]] not in player_pos:
        self.position[0] += change[direction] * self.speed
    else:
        self.move(arena_limits, player_pos)
if direction in ("up", "down"):
    in_limits = 0 <= self.position[1] + (change[direction] * self.speed) <= arena_limits[0]
    if in_limits and [self.position[0], self.position[1] + (change[direction] * self.speed)] not in player_pos:
        self.position[1] += change[direction] * self.speed
    else:
        self.move(arena_limits, player_pos)

Also you use (change[direction] * self.speed) all over the place in this section, when you could just assign it to a variable (I chose to call it displacement.)

Now, it can be tricky, and require other constructs, and sometimes it's more efficient to repeat code. But generally, 99% of the time, it's a big no-no.

Now, the example I gave is nothing compared to the (apologies) horrendous repetition in this block:

    can_go_left = ([self.position[0] + (-1 * self.speed), self.position[1]] not in player_pos) and \
                  0 <= self.position[0] + (-1 * self.speed) <= arena_limits[0]
    can_go_right = ([self.position[0] + (1 * self.speed), self.position[1]] not in player_pos) and \
                   0 <= self.position[0] + (1 * self.speed) <= arena_limits[0]
    can_go_up = ([self.position[0], self.position[1] + (1 * self.speed)] not in player_pos) and \
                0 <= self.position[1] + (1 * self.speed) <= arena_limits[1]
    can_go_down = ([self.position[0], self.position[1] + (-1 * self.speed)] not in player_pos) and \
                   0 <= self.position[1] + (-1 * self.speed) <= arena_limits[1]
    if not (can_go_up or can_go_down or can_go_left or can_go_right):

... but more on that later.

Avoid unnecessary recursion...

...like this (consider it in context):

def move(self, arena_limits, player_pos):

    # ...

    self.move(arena_limits, player_pos)

This is a bit debatable, but generally it's better practice to program procedurally when it's equal effort. Getting a bit technical, recursion adds variables to the stack when it goes one level deeper, taking up more memory than necessary. This is a bit inefficient, and on a large scale can result in stack overflow (or "RecursionError", because Python catches stack overflow.)

move() refactored I

from enum import Enum

# ...

class Movement
    # (direction, magnitude)
    LEFT = (0, -1)
    RIGHT = (0, 1)
    UP = (1, 1)
    DOWN = (1, -1)

def move(self, arena_limits, player_pos):
    if not self.is_stuck(arena_limits, player_pos):
        while True:
            movement = random.choice(list(self.Movement))
            direction, magnitude = move[0], move[1]
            displacement = magnitude * self.speed
            in_limits = 0 <= self.position[direction] + displacement <= arena_limits[0]
            destination = self.position.copy()
            destination[direction] += displacement
            if in_limits and destination not in player_pos:
                self.position[direction] += displacement
                break

But wait, there's more!

Player.is_stuck() does most of the same logic that is repeated continually in Player.move(). By being clever in refactoring and combining code, one can almost like magic eliminate most of the logic, just letting DRY and efficient code take care of the work. Basically, we just developed a general method to find accessible location in move(), so now we can apply it to the starting logic, and get that logic out the way before ever choosing an individual direction:

move() refactored II

def move(self, arena_limits, player_pos):
    accessible = self.accessible_positions(arena_limits, player_pos)
    if accessible:
        movement = random.choice(accessible)
        self.position[movement[0]] += movement[1] * self.speed

def accessible_positions(self, arena_limits, player_pos):
    accessible = []
    for m in self.Movement:
        move = m.value
        direction, magnitude = move
        displacement = magnitude * self.speed   
        destination = self.position.copy()
        destination[direction] += displacement
        if 0 <= self.position[direction] + displacement <= arena_limits[0] and destination not in player_pos:
            accessible.append(move)
    return accessible

---

Use more standard loop flow

Declaring a boolean to serve as a loop check variable is generally unnecessary and can lead to problems (e.g. the loop continuing to do things before it checks arrives at the condition check). while True: ... if whatever: break is generally a more recognizable, more understandable, and more concise idiom.

---

Conclusion

This answer will stay unfinished, but hopefully the thought process can be useful.