11
\$\begingroup\$

I've written a program to simulate a game of Mafia among several bots. Here's the directory structure the program lives in (to make understanding parts of the program easier):

/
 start
 README.md      # Readme file
 wtfpl.txt      # License
 controller/
    main.py
    messages.py # Contains a list of strings. I only put it into
                # a separate file because it's long.
 players/
    log
    player_folder/ # Every player has one of these
        to_server
        from_server
        players
        run

main.py is the actual program that I'm looking for feedback with, and it is run by running ./start in the root folder (which just executes controller/main.py, nothing fancy).

I have a few concerns, most of which come down to "good style". I tried to follow PEP8 where appropriate (with the exception of tabs vs. spaces, because TABS FOREVER) but I'm sure there are places I screwed that up. Largely, though, I'm just concerned because I'm not an incredibly experienced programmer and a not-insignificant portion of that already limited experience is code golf, so I have almost no idea of how to make an actual real program not suck. I'm just hoping people can take a look at this and suggest ways to make it "better", for whatever definition of better they choose.

I think I've commented heavily enough/chosen good enough variable and function names to explain what the program actually does, but if you have any questions I'd be happy to answer. Additionally, I know I'm not supposed to edit this question with updates as I make them, but if you want to know what my code looks like right now I'll be updating the git repo here with any updates as I make them.

Finally, one last thing: I use some try: ... except: blocks that blindly catch any exception and ignore it. I'm aware of how terrible this is, however, these bots are going to be written by other people, and I want to be catching any error at all and defaulting to "ignore the input", which is why I've done that.

#!/bin/python2

import os
import sys
import random
import textwrap

from messages import messages

class Player(object):
    """Player object for each bot."""
    vote = None
    role = 0

    # Messages to the player are stored in the player object themself until they
    # are sent, allowing more than one message to be sent with relative ease.
    messages = ''

    def __init__(self, name):
        self.name = name

    def add_message(self, s):
        self.messages += textwrap.dedent(s + '\n')

    def get_role(self):
        if self.role == 1:
            return 'a mafioso'
        if self.role == 2:
            return 'the cop'
        if self.role == 3:
            return 'the doctor'
        return 'a villager'

def m_read(p):
    """Return contents of player p's 'to_server' file, stripped of special
    characters, and clear the file.
    """
    with open(p.name + '/to_server', 'r+') as f:
        s = filter(lambda c: c.isalnum() or c == ' ', f.read())
        f.truncate(0)
    return s

def m_write(players):
    """Write messages to each player to that player's 'from_server' file"""
    # Convert Player object to a 1-element list to allow calling for a single
    # player
    if isinstance(players, Player):
        players = [players]
    for p in players:
        with open(p.name + '/from_server', 'w') as f:
            f.write(p.messages)
        p.messages = ''

def execute(p):
    """Executes the bot associated with player p"""
    os.chdir(p.name)
    os.system('./run')
    os.chdir('..')

def log(message):
    """Append message + newline to the log file. This happens a lot, so this
    function exists as shorthand.
    """
    with open('log', 'a') as f:
        f.write(textwrap.dedent(message + '\n'))

def get_players(players):
    """Return a list of player objects for each player name in the input. Also
    return the doc, cop and a list of the mafia seperately.
    """
    def assign_roles(players):
        # Shuffling the whole list ensures you aren't assigning multiple roles to one
        # player, which we would have to account for with random.choice()
        random.shuffle(players)

        # Make 1/3rd of the players mafia, one player a doctor and one player a cop
        for _ in xrange(len(players) / 3):
            players[0].role = 1
            players.append(players.pop(0))

        players[0].role = 2
        players.append(players.pop(0))

        players[0].role = 3

        # Return list of players, list of mafia, cop and doctor
        return (players,
                filter(lambda p: p.role == 1, players),
                players[-1],
                players[0])

    # At least six players are required. Ensure that we have six. Do this by 
    # testing for at least seven, because...
    if len(players) < 7:
        sys.exit('Not enough players.')

    # ...The log file is kept in the same directory as the players. Get rid of it
    players.remove('log')

    # Convert to Player object, assign roles, and return
    return assign_roles(map(lambda p: Player(p), players))

def kill(p, players, mafia, cop, doctor):
    """Return every role the player p might be filling, with them removed."""
    players.remove(p)
    if p in mafia:
        mafia.remove(p)
    if cop is p:
        cop = None
    if doctor is p:
        doctor = None

    return players, mafia, cop, doctor  

def main():
    # Get player objects for all players, the doc, the cop, and a list of mafia
    os.chdir('players')
    players, mafia, cop, doctor = get_players(os.listdir('.'))

    # Give everyone a list of players
    for p in players:
        with open(p.name + '/players', 'w') as f:
            # Sort it so that it isn't ordered by role
            f.write('\n'.join(sorted([l.name for l in players])))

    # Clear the log file, so it's fresh for the new game
    with open('log', 'w') as f:
        f.truncate(0)

    # Create a dictionary allowing you to look up player objects by their name
    name_to_player = dict(map(lambda p: (p.name, p), players))

    day = 0

    # Day 0 doesn't have a suspect or victim, every subsequent day does
    suspect, victim = None, None

    # Game loop, exits when mafia is dead or mafia outnumbers village
    while mafia and (len(players) - len(mafia)) > len(mafia):
        log('Day {} begins.'.format(day))

        # Randomize turn order every day. Bots /shouldn't/ be able to figure
        # this out, but who knows what you crazy kids will come up with. :P
        random.shuffle(players)

        # Print a message at the beginning of each day. On the first day, power
        # roles need to be old their role and mafia members need to know their
        # allies. On every other day, the cop needs to know the result of their
        # investigation and all players need to know who died.
        if day == 0:
            log("""\
          Cop: {}
          Doctor: {}
          Mafia: {}""".format(
          cop.name, doctor.name, ', '.join(m.name for m in mafia)))

            for p in players:
                p.add_message("""\
                              Rise and shine! Today is day 0.
                              No voting will occur today.
                              Be warned: Tonight the mafia will strike.""")

            for m in mafia:
                m.add_message("""\
                      You are a member of the mafia.
                      Your allies are:""")
                m.add_message('\n'.join(p.name for p in mafia if p is not m))

            cop.add_message('You are the cop.')
            doctor.add_message('You are the doctor.')
        else:
            for p in players:
                p.add_message('Dawn of day {}.'.format(day))
                if victim is not None:
                    p.add_message('Last night, {} was killed.'.format(victim.name))

            if victim is not None:
                players, mafia, cop, doctor = kill(victim, players, mafia, cop, doctor)
                log('{}, {}, was killed.'.format(victim.name, victim.get_role))

            if suspect is not None:
                cop.add_message('Investigations showed that {} is {}-aligned.'.format(
                suspect.name, 'mafia' if suspect.role == 1 else 'village'))

            log('These players are still alive: {}'.format(
            ', '.join(p.name for p in players)))

        m_write(players)

        # During a day, players may perform up to 50 actions (Action= vote or talk)
        for r in xrange(50):
            for p in players:
                try:
                    execute(p)
                    command = m_read(p).split()
                    if command[0] == 'vote':
                        # Set the player's vote
                        if day != 0:
                            if command[1] == 'no':
                                if command[2] == 'one':
                                    p.vote = None
                                    log('{} has voted to lynch no one.'.format(p.name))
                                    for l in players:
                                        l.add_message('{} has voted to lynch no one.'.format(p.name))
                            else:
                                p.vote = name_to_player[command[1]]
                                log('{} has voted to lynch {}.'.format(p.name, command[1]))
                                for l in players:
                                    l.add_message(
                                    '{} has voted to lynch {}.'.format(p.name, command[1]))
                    elif command[0] == 'say':
                        # Send a message to all players
                        message = '{} says "'.format(p.name)
                        # Messages with an id higher than 4 have the name of a bot attached
                        # This screws with parsing a bit so we handle them seperately
                        if int(command[1]) > 4:
                            if len(command) == 4:
                                # Convert from a name to a player object and back to ensure
                                # that it's a correct name
                                message += '{}, '.format(name_to_player[command[3]].name)
                            message += messages[int(command[1])]
                            message += '{}"'.format(name_to_player[command[2]].name)
                        else:
                            if len(command) == 3:
                                message += '{}, '.format(name_to_player[command[2]].name)
                            message += '{}"'.format(messages[int(command[1])])
                        log(message)
                        for l in players:
                            l.add_message(message)
                except:
                    # Do nothing on invalid input
                    pass

            m_write(players)

        # Tally up the votes for each player
        votes = [p.vote for p in players]

        # Shuffle to eliminate max() bias
        random.shuffle(votes)

        # The most voted player is lynched, with ties broken randomly
        lynched = max(votes, key=votes.count)
        if lynched is not None:
            log('The town has killed {}!'.format(lynched.name))
            log('They were {}.'.format(lynched.get_role()))
            for p in players:
                p.add_message("""\
                The town has killed {}!
                They were {}.""".format(lynched.name, lynched.get_role))
            players, mafia, cop, doctor = kill(lynched, players, mafia, cop, doctor)
        else:
            log('The town opted to lynch no one today.')
            for p in players:
                p.add_message('The town opted to lynch no one today.')

        m_write(players)
        for p in players:
            execute(p)
            p.vote = None

        # Don't go to night if a win condition's been met.
        if not mafia or (len(players) - len(mafia)) <= len(mafia):
            break

        # Day ends, night begins

        # MAFIA NIGHT ACTION
        # Each mafioso votes for a victim. The most voted player is then killed,
        # unless saved that night by the doctor.
        for m in mafia:
            m.add_message('It is night. Vote for a victim.')
        m_write(mafia)

        victim_votes = []       
        for m in mafia:
            try:
                execute(m)
                victim_votes.append(name_to_player[m_read(m)])
                log('{} votes to kill {}.'.format(m.name, victim_votes[-1].name))
            except:
                # Vote to kill no one on invalid input
                victim_votes.append(None)
                log(m.name + ' votes to kill no one.')

        # Shuffle to eliminate max() bias
        random.shuffle(victim_votes)

        # The victim is the player most voted for by the mafia, with ties broken
        # randomly.
        victim = max(victim_votes, key=victim_votes.count)
        log('The mafia collectively decides to kill {}.'.format(
            victim.name if victim is not None else 'no one'))

        # COP NIGHT ACTION
        # The cop chooses a player to investigate. At the dawn of the next day,
        # they are told whether that player is village- or mafia-aligned.
        if cop is not None:
            cop.add_message('It is night. Who would you like to investigate?')
            m_write(cop)
            try:
                execute(cop)
                suspect = name_to_player[m_read(cop)]
                log('{} spends the night investigating {}.'.format(
                cop.name, suspect.name))
            except:
                # Investigate no one on invalid input
                suspect = None
                log('{} chooses not to investigate anyone.'.format(cop.name))

        # DOCTOR NIGHT ACTION
        # The doctor chooses a player they expect the mafia to try to kill. If they
        # are right, the mafia gets no kills that night.
        if doctor is not None:
            doctor.add_message('It is night. Who would you like to save?')
            m_write(doctor)
            try:
                execute(doctor)
                patient = name_to_player[m_read(doctor)]
                if patient == victim:
                    victim = None
                    log('{} was able to save {} from near-certain death.'.format(
                    doctor.name, patient.name))
                else:
                    log('{} tried to save {}, but they were not the target.'.format(
                    doctor.name, patient.name))
            except:
                # Save no one on invalid input
                log('{} took tonight off.'.format(doctor.name))

        log('')
        day += 1

    if mafia:
        print 'MAFIA VICTORY'
        log('MAFIA VICTORY')
    else:
        print 'VILLAGE VICTORY'
        log('VILLAGE VICTORY')

if __name__ == '__main__':
    main()
\$\endgroup\$
1
  • \$\begingroup\$ Love the code looks awesome. You going to compile to exe so we can all play it? \$\endgroup\$ – Oliver Perring Apr 30 '14 at 15:53
7
\$\begingroup\$

Overall, it looks pretty good to me. I agree that you have good commenting and variable and function naming. I see just a few things to comment on.

  • You have assign_roles() nested inside get_players(). Is there a reason for doing this? Unless there's a good one, I'd recommend putting assign_roles() at the top level along with everything else.

  • Could you put the log file someplace like '/tmp/log' so it doesn't mess up your list of players? Besides, what if you have a player named 'log'? Also, you might consider looking into python's logging module. It can be a bit complex, but provides a lot of nice features for managing log files.

  • I see an opportunity for further modularizing your main() routine -- maybe routines like day_action(), maybe_lynch_somebody(), mafia_night_action(), cop_night_action(), and doctor_night_action(). It might involve passing your lists around some, but you're already doing that up above, so it shouldn't be an issue.

Looks like an interesting game.

\$\endgroup\$
7
  • \$\begingroup\$ 1. I was under the impression that are only ever called from inside another function are generally kept inside that other function for clarity reasons (with the exception of main(), of course). If I'm mistaken, I can fix that easily. 2. That's true, I could. Having log under players/ is an artifact of before I made the log() function. I was writing to it with the whole with open(... syntax each time and didn't to navigate to a different folder each time. Now that I almost always do it from the function, that wouldn't be an issue at all. I didn't know python had a logging module... \$\endgroup\$ – undergroundmonorail Apr 23 '14 at 8:14
  • \$\begingroup\$ ...but I'll check it out! 3. That's true, I should do that. If I'm being honest with you, I was starting to near the end of the project when I got there and having it all dumped into main() is probably an artifact of me growing lazy. :P Thank you for your help! \$\endgroup\$ – undergroundmonorail Apr 23 '14 at 8:16
  • \$\begingroup\$ You're welcome. 1. With regard to nested functions, it may just be a matter of preference on my part. I think it's easier to find things when they're all at the top level. PEP20, The Zen of Python (legacy.python.org/dev/peps/pep-0020) does say, "Flat is better than nested." However, if nesting works well for you to indicate that the subordinate function is only called by its parent, I wouldn't stop doing that just because some guy on the internet says to. Now, if twenty guys on the internet say so, that's something else again. :) Let's see if anybody else has an opinion. \$\endgroup\$ – Tom Barron Apr 23 '14 at 10:37
  • \$\begingroup\$ 2. For putting the log file in /tmp, I didn't mean to suggest that you should os.chdir("/tmp") before opening the file. Just use the full path in the open statement: open('/tmp/log', 'a'). That will work from wherever you are. \$\endgroup\$ – Tom Barron Apr 23 '14 at 10:40
  • \$\begingroup\$ re: nested functions: I'm going to be honest and say that the coding style opinions of a guy who actively posts answers on codereview.SE probably trumps those of a guy who did a CodeAcademy course and uses reddit. :P \$\endgroup\$ – undergroundmonorail Apr 23 '14 at 12:03

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.