mrscbw
mrscbw

Reputation: 19

New to Python, can't find bug

I am new to Python (3rd day) and I was just trying to creat a basic Rock, Paper, Scissors. I am seeing a bug that I can't locate in the code and was hoping somebody could help. Here is the output below with the code following:

Welcome to Rock, Paper, Scissors!
Player 1 name?b
Player 2 name?s
3
2
1
GO!
Rock, Paper or Scissors?rock
b  threw  rock
s  threw  rock
Draw! Get Ready!.
3
2
1
GO!
Rock, Paper or Scissors?rock
b  threw  rock
s  threw  scissors
b  win.
Rematch?no
Goodbye.
b  win.
Rematch?

After entering "no" for rematch, it is printing "b win." also asking "Rematch?" again. Here is the code below:

import time
import random
picks=["rock","paper","scissors"]
answers=["yes","no"]
rock="rock"
paper="paper"
scissors="scissors"
yes="yes"
no="no"
invalid=""
#############################################Defining Functions#########################################################
def rename1():
    global name1
    while True:
        if name1 is invalid:
            name1 = input("Player 1 name?")
        if name1 is not invalid:
            break

def rename2():
    global name2
    while True:
        if name2 is invalid:
            name2 = input("Player 2 name?")
        if name2 is not invalid:
            break

def rematchinvalid():
    global rematch
    while True:
        if rematch not in answers:
            print("Invalid, try again..")
            rematch = input("Rematch?")
        if rematch in answers:
            break

def Rethrow1():
    global P1
    while True:
        if P1 not in picks:
             print("Invalid, try again..")
             P1 = input("Rock, Paper, or Scissors?")
        if P1 in picks:
            break

def start():
    print("3")
    time.sleep(1)
    print("2")
    time.sleep(1)
    print("1")
    time.sleep(1)
    print("GO!")

def RPS():
    global P1
    global P2
    global rematch
    P1 = input("Rock, Paper or Scissors?")
    P2 = random.choice(picks)
    if P1 not in picks:
        Rethrow1()
    battle()
    winner()

def battle():
    print(name1," threw ",P1)
    print(name2," threw ",P2)

def winner():
    global rematch
    if P1 == P2:
        print("Draw! Get Ready!.")
        start()
        RPS()
    if P1 == rock and P2 == scissors:
            print(name1," win.")
    if P1 == rock and P2 == paper:
            print(name2," win.")
    if P1 == scissors and P2 == rock:
            print(name2," win.")
    if P1 == scissors and P2 == paper:
            print(name1," win.")
    if P1 == paper and P2 == rock:
            print(name1," win.")
    if P1 == paper and P2 == scissors:
            print(name2," win.")
    rematch = input("Rematch?")
    if rematch not in answers:
        rematchinvalid()
    replay()

def replay():
    if rematch == yes:
        start()
        RPS()
    if rematch == no:
        print("Goodbye.")
################################################Game Start##############################################################
print("Welcome to Rock, Paper, Scissors!")
name1 = input("Player 1 name?")
if name1 is invalid:
    rename1()
name2 = input("Player 2 name?")
if name2 is invalid:
    rename2()
start()
RPS()

Also, if you have any recommendations on how to clean up the code, that would be appreciated!

Thanks

Upvotes: 1

Views: 142

Answers (3)

gboffi
gboffi

Reputation: 25093

if you have any recommendations on how to clean up the code, that would be appreciated

codes = {'rock':0, 'paper':1, 'scissors':2}
...
def winner():
    outcome = (codes[P1]-codes[P2])%3
    if outcome == 0:
        # it's a draw
    elif outcome == 1:
        # player 1 wins
    else:
        # player2 wins

Upvotes: 0

Hai Vu
Hai Vu

Reputation: 40773

Take a look at your function winner:

def winner():
    global rematch
    if P1 == P2:
        print("Draw! Get Ready!.")
        start()
        RPS()
    if P1 == rock and P2 == scissors:
            print(name1," win.")
    if P1 == rock and P2 == paper:
            print(name2," win.")
    if P1 == scissors and P2 == rock:
            print(name2," win.")
    if P1 == scissors and P2 == paper:
            print(name1," win.")
    if P1 == paper and P2 == rock:
            print(name1," win.")
    if P1 == paper and P2 == scissors:
            print(name2," win.")
    rematch = input("Rematch?")
    if rematch not in answers:
        rematchinvalid()
    replay()

When there is a tie, you print Draw! Get Ready!., call start(), call RPS(), then what? Then, instead of exiting the function, you let the control flowed right into the code below, which displayed the winner name one more time, then asked for rematch one more time before exiting the function. I leave it up to you to fix it.

As for recommendation: please do not use global variables.

Update

Here is some suggestion to eliminate global variables: pass information into functions and return information from function. For example, here is a way to eliminate the global variable rematch. This variable is first used in winner() and then passed to replay(). Also, rematchinvalid() get the user's input into this variable and pass it back to winner, so the information flow for rematch is:

rematchinvalid <==> winner ==> replay

With that in mind, we can fix rematchinvalid() as such:

def rematchinvalid(rematch):
    # Remove the global statement here
    while True:
        if rematch not in answers:
            print("Invalid, try again..")
            rematch = input("Rematch?")
        if rematch in answers:
            break
    return rematch  # Return rematch to the caller

As for winner(), we will receive information from rematchinvalid() and pass it on to replay():

def winner():
    # Remove global statement
    if P1 == P2:
        print("Draw! Get Ready!.")
        start()
        RPS()
        return  # Fix for your problem
    if P1 == rock and P2 == scissors:
            print(name1," win.")
    if P1 == rock and P2 == paper:
            print(name2," win.")
    if P1 == scissors and P2 == rock:
            print(name2," win.")
    if P1 == scissors and P2 == paper:
            print(name1," win.")
    if P1 == paper and P2 == rock:
            print(name1," win.")
    if P1 == paper and P2 == scissors:
            print(name2," win.")
    rematch = input("Rematch?")
    if rematch not in answers:
        rematch = rematchinvalid(rematch)  # Get the valid rematch
    replay(rematch)  # Pass rematch to replay

Finally, for replay, we can accept rematch as a parameter:

def replay(rematch):
    if rematch == yes:
        start()
        RPS()
    if rematch == no:
        print("Goodbye.")

That should take care of rematch. You can apply this method to eliminate other variables as well.

Upvotes: 2

Ic3fr0g
Ic3fr0g

Reputation: 1219

Your bug is primarily a small logic error. While loops with a true condition are generally a bad practice. The crux is that you can rewrite rematchinvalid() like this and still be logically equivalent to what you wanted to achieve.

def rematchinvalid():
    global rematch
    while rematch not in answers:
            print("Invalid, try again..")
            rematch = input("Rematch?")

Furthermore, try implementing this for the other functions as well. Another thing I noticed -

Rock, Paper, or Scissors?paper
a  threw  paper
b  threw  paper
Draw! Get Ready!.
3
2
1
GO!
Rock, Paper or Scissors?paper
a  threw  paper
b  threw  rock
a  win.
Rematch?no
Goodbye.
a  win.
Rematch?no
Goodbye.
Goodbye.
a  win.
Rematch?no
Goodbye.
Goodbye.
a  win.
Rematch?no
Goodbye. 

If you get a draw you should set a flag to exit the function. Otherwise you end up with something like this in the output!

Upvotes: 0

Related Questions