Ping Pong Pi - A Ping Pong score and serving manager

I have spent a few hours on this code, but think it could be improved a bit. It requires eSpeak to be installed to run the speech synthesis, but the voice can be toggled on or off using the togglevoice command. This was created for Python 2.7.3.

My major concerns are the fluctuations of the use of global to define global variables, the re-use of the variable n in functions and readability of the d array. It currently works fine on a Raspberry Pi running the latest version of Raspbian.

```
import os
def talk(n):
if voice_mode == 1:
os.system("espeak -ven+m3 '" + n + "' 2>/dev/null")
else:
print n

def print_score(n):
if n == 0:
other_n = 1
else:
other_n = 0
if d[n][1] == 0:
score1 = "love"
else:
score1 = str(d[n][1])
if d[other_n][1] == 0:
score2 = "love"
else:
score2 = str(d[other_n][1])
talk(score1 + " " + score2)

def win(n):
global first_server
talk(d[n][0] + " wins the game")
d[n][2] += 1
d[0][1] = 0
d[1][1] = 0
deuce_mode = 0
serve_turns = old_serve_turns
if first_server == 0:
first_server = 1
current_server = 1
else:
first_server = 0
current_server = 0
if d[n][2] == match_points:
talk(d[n][0] + " wins the match")
d[0][2] = 0
d[1][2] = 0

def add_score(n):
global deuce_mode
global current_server
global serve_turns
d[n][1] += 1
if n == 0:
other_n = 1
else:
other_n = 0
if deuce_mode == 0:
if d[0][1] == d[1][1] and d[0][1] == game_points - 1:
deuce_mode = 1
old_serve_turns = serve_turns
serve_turns = 1
if current_server == 0:
print_score(0)
else:
print_score(1)
if d[n][1] == game_points:
win(n)
else:
if d[n][1] == d[other_n][1] + 2:
if current_server == 0:
print_score(0)

Line by line analysis

import os

You need 2 blank lines before each global function definition.

def talk(n):

n is not descriptive and associated with a number. Don't use one letter variables unless it's really obvious (for example i loop counter, k and v as dict key and a value, or d as Twisted's deferred object. Something like phrase would be better.

if voice_mode == 1:

Where does the voice_mode come from? Global variables are bad, pas it as a parameter to a talk method. Also, canonical way of checking if boolean is True is simply if voice_mode.

os.system("espeak -ven+m3 '" + n + "' 2>/dev/null")
    else:
        print n

def print_score(n):

Again, n does not tell reader a lot.

if n == 0:
        other_n = 1

What is other_n? If there's no better name for it - add a comment.

else:
        other_n = 0
    if d[n][1] == 0:

Where did d come from? Pass it to a function. Also, better name.

score1 = "love"
    else:
        score1 = str(d[n][1])
    if d[other_n][1] == 0:
        score2 = "love"
    else:
        score2 = str(d[other_n][1])
    talk(score1 + " " + score2)

def win(n):

Same thing with the n variable. Also, docstring might be helpful for this method.

global first_server

Global variables are bad, really bad. Make your method take and return this if you need the variable.

talk(d[n][0] + " wins the game")

This d is really quite obscure.

d[n][2] += 1
    d[0][1] = 0
    d[1][1] = 0
    deuce_mode = 0
    serve_turns = old_serve_turns
    if first_server == 0:
        first_server = 1
        current_server = 1
    else:
        first_server = 0
        current_server = 0
    if d[n][2] == match_points:
        talk(d[n][0] + " wins the match")
        d[0][2] = 0
        d[1][2] = 0

def add_score(n):

Same thing: add docstring, replace n with better name and pass all the global variables as arguments to the function.

global deuce_mode
    global current_server
    global serve_turns
    d[n][1] += 1
    if n == 0:
        other_n = 1
    else:
        other_n = 0
    if deuce_mode == 0:
        if d[0][1] == d[1][1] and d[0][1] == game_points - 1:
            deuce_mode = 1
            old_serve_turns = serve_turns

I don't see old_serve_turns used anywhere.

serve_turns = 1
        if current_server == 0:
            print_score(0)
        else:
            print_score(1)
        if d[n][1] == game_points:
            win(n)
    else:
        if d[n][1] == d[other_n][1] + 2:
            if current_server == 0:
                print_score(0)
            else:
                print_score(1)
            win(n)
        else:
            if current_server == 0:
                print_score(0)
            else:
                print_score(1)
    if (d[0][1] + d[1][1]) % serve_turns == 0:
        if current_server == 0:
            current_server = 1
            talk(d[1][0] + " is serving next")
        else:
            current_server = 0
            talk(d[0][0] + " is serving next")

d = []

You need either a better data structure, or at least a good comment explained what the d is. Also, d needs a better name.

for i in xrange(2):
    d.append(["", 0, 0])
d[0][0] = "Player 1"
d[1][0] = "Player 2"
game_points = 11
match_points = 2
deuce_mode = 0
voice_mode = 1
serve_turns = 2
old_serve_turns = 2
current_server = 2

You only play with server 0 and 1, correct? Why do you need to obscure and add server 2 which is only there to start the game? Better have a flag of some sort.

first_server = 0

talk("Ready to play")
while True:
    c = raw_input("Command: ").lower()

Better name: command.

if c == "z":
        if current_server != 2:
            add_score(0)
        else:
            talk(d[0][0] + " is serving first")
            current_server = 0
            first_server = 0
    elif c == "x":
        if current_server != 2:
            add_score(1)
        else:
            talk(d[1][0] + " is serving first")
            current_server = 1
            first_server = 1
    elif c == "1name":
        d[0][0] = raw_input("Player 1 name: ")
        talk("Welcome, " + d[0][0])
    elif c == "2name":
        d[1][0] = raw_input("Player 2 name: ")
        talk("Welcome, " + d[1][0])
    elif c == "gm":
        game_points = int(raw_input("Game points: "))
        match_points = int(raw_input("Match points: "))
    elif c == "s":
        serve_turns = int(raw_input("Serve turns: "))
    elif c == "togglevoice":
        if voice_mode == 0:
            voice_mode = 1
        else:
            voice_mode = 0
    elif c == "quit":
        quit()
    else:
        talk("Command not recognised")

Even though this if/else tree is acceptable in your scope, you might want to look into Strategy pattern. This is further reading though.

Summary

Overall, your variable naming needs to be more readable. Don't use global variables, just pass the ones

StackExchange Code Review Q#46097, answer score: 2