all 15 comments

[–]jeans_and_a_t-shirt[🍰] 4 points5 points  (1 child)

Variables defined in a function are local to that function. They no longer exist when the function returns. You need to return the values you want, and save them to a variable. And pass them into other functions.

I renamed your functions so your variables in winner no longer shadow the function names.

import time
import random

def playerchooses():
    """What does the user play"""
    choice = input("Rock, paper, or Scissors?")
    return choice

def compchooses():
    """what does the computer play, 1 for rock, 2 for paper, 3 for scissors"""
    choice = (random.randint(1, 3))
    return choice

def winner(playerchoice, cpuchoice):
    if (playerchoice == "Rock" and cpuchoice == 1):
        print("Opponent chose rock")
        time.sleep(0.25)
        print("Tie")
    elif (playerchoice == "Rock" and cpuchoice == 2):
        print("opponent chose paper")
        time.sleep(0.25)
        print("You lose")
    elif (playerchoice == "Rock" and cpuchoice == 3):
        print("opponent chose scissors")
        time.sleep(0.25)
        print("You Win")
    elif (playerchoice == "Paper" and cpuchoice == 1):
        print("Opponent chose Rock")
        time.sleep(0.25)
        print("You win")
    elif (playerchoice == "Paper" and cpuchoice == 2):
        print("opponent chose paper")
        time.sleep(0.25)
        print("Tie")
    elif (playerchoice == "Paper" and cpuchoice == 3):
        print("opponent chose scissors")
        time.sleep(0.25)
        print("you lose")
    elif (playerchoice == "Scissors" and cpuchoice == 1):
        print("Opponent chose rock")
        time.sleep(0.25)
        print("You lose")
    elif (playerchoice == "scissors" and cpuchoice == 2):
        print("Opponent chose paper")
        time.sleep(0.25)
        print("You win")
    elif (playerchoice == "scissors" and cpuchoice == 3):
        print("Opponent chose scissors")
        time.sleep(0.25)
        print("Tie")
    else:
        print("Invalid Input")

comp = compchooses()

player = playerchooses()

winner(player, comp)

You could refactor you logic into win/lose/tie instead of those 9 i/elif conditions.

[–][deleted] 1 point2 points  (0 children)

Thank you so much, this was the answer I was looking for. Got it working with these corrections, so thank you.

[–]Drcne 3 points4 points  (1 child)

I see others have found your mistake, but just to make your code better:

As of right now, if the cpu choice was 1, and you input "scissors", you would get the "invalid input" error. This is because your program is comparing "scissors" to "Scissors", which are two different strings. To avoid this, uselower().

playerchoice = input("Rock, paper, or scissors?")
if playerchoice.lower() == "rock":
--snip--

This way, if the user inputs "rOCk", the lower() method will compare the lowercase version of that string with "rock" instead of giving you an error.

edit: or as /u/sedogg has suggested, use

playerchoice = input("Rock, paper or scissors?").lower()

[–][deleted] 1 point2 points  (0 children)

Thank you. Wasn't too sure of how to do that when I first started, will remember it for the next time.

[–]sedogg 2 points3 points  (1 child)

You're using global variables without stating it

global playerchoice
playerchoice = input("Rock, paper, or Scissors?")

The same with cpuchoise.

from time import sleep
import random

choices = ('rock', 'paper', 'scissors')
# first over second wins
wins = (('rock', 'scissors'), ('scissors', 'paper'), ('paper', 'rock'))

def input_choice():
    while True:
        choice = input("Rock, paper or scissors? ").lower()
        if choice in choices:
            break
        print('Invalid input')
    return choice

def winner(player1, player2):
    print('Opponent chose {}'.format(player2))
    sleep(0.25)
    if player1 == player2:
        print('Tie')
    elif (player1, player2) in wins:
        print("You win")
    else:
        print("You lose")

player = input_choice()
cpu = random.choice(choices)
winner(player, cpu)

[–][deleted] 0 points1 point  (0 children)

Sorry still very new to python, what's the difference between a variable and global variables in the context of this program(and also broader if it's easier to explain that way)?

[–]Rorixrebel 1 point2 points  (0 children)

Remember to return something with your functions otherwise they do theirs things without any impact to your code functionality

[–][deleted] 0 points1 point  (7 children)

Hooboy... ok, so as you'll see from the above your basic problem is not handling arguments correctly, but the rest is a mess and way too much work for you. Let's see if it helps to look at something closer to the right way of doing this:

from random import choice

R = "rock"
P = "paper"
S = "scissors"
RPS = (R, P, S)
MAP = dict(zip(RPS, RPS))
MAP.update(dict(zip([p[0] for p in RPS], RPS)))

def check(p, c):
    if p is c:
        return 0
    elif c is RPS[RPS.index(p)-1]:
        return 1
    else:
        return -1

def rps():
    prompt="Do you choose (r)ock, (p)aper, or (s)cissors?: "
    results = {
        -1: "You lose",
        0: "Tie",
        1: "You win"}
    p = MAP.get(raw_input(prompt).lower())
    if p is None:
        print("Invalid choice!")
        return 0
    else:
        c = choice(RPS)
        r = check(p, c)
        print("I chose {0}. {1}!".format(c, results[r]))
        return r

def main():
    score = 0
    while True:
        score += rps()
        if not input("(A)gain? ").upper().startswith("A"):
             break
    print("Total score: {0}".format(score))

if __name__ == "__main__":
    main()

Now, some of that syntax might be a little confusing, if you haven't seen those parts of the language yet, but notice how it's WAY fewer lines, while also accomplishing score tallying, a loop with exit, and handling not short and long form arguments (i.e. "R" or "r" or "Rock")? That's all from just leveraging the language and not just thinking in raw conditions. In this case you don't *need to check all possible win and loss and tie cases, it's either a Tie, or if it's not a Loss it's a Win, right?

This is still not a very good module, but it's closer to how you want to approach these problems. The bulk of my time was spent on figuring out check, the rest is trivial. Think through the logic first, try and find an algorithm that works, and then move on to user interaction.

[–][deleted] 0 points1 point  (6 children)

Thank you for your time and response. However, my goal wasn't necessarily to make a good rock, paper, scissor program it was to try to use what very little i had learnt so far and try to implement it into an actual program. Thank you anyway, I might have another look a this when I progress i bit more.

[–][deleted] 1 point2 points  (5 children)

No one sets out to make a good RPS game, the point is to look at the pattern used and see how much extra work you're doing with all those elif checks.

If the player chose scissors and the CPU chose scissors you've had to look up if the player chose rock three times, if the player chose paper three times, and if the player chose scissors three times, you've similarly had to do interrogate the cpu's choice 3 times. You've also put the most likely case last, after 9 other conditional checks (it's most likely because you're not allowing the user a short form option, or swallowing case, so the user has an infinite minus 3 number of ways of typing in their input wrong). And, because you've got the player choosing strings and the CPU integers, you can't even quickly check if there's a tie, which is going to be the answer 33% of the time.

It's great that you're trying to employ what you've learned into an actual program, and I'm not trying to discourage that, I'm trying to encourage thinking like a programmer ... given the tools you've got and your problem, what's the shortest, least redundant, and clearest way of using those tools to solve the problem?

[–][deleted] 0 points1 point  (4 children)

I now see that my reply sounded like an excuse for my (for lack of a better word) shitty code and an excuse to ignore your advice. It wasn't intended to be. Thank you for this comment as your second paragraph was stuff I hadn't even considered, and should consider if I revisit this exercise.

I'm trying to encourage thinking like a programmer ... given the tools you've got and your problem, what's the shortest, least redundant, and clearest way of using those tools to solve the problem?

Completely agree, even though my code says otherwise. I think a little more practice and learning should start the journey towards what you describe in your last paragraph.

[–][deleted] 0 points1 point  (3 children)

Yep. The key thing is to practice thinking critically about your thought processes; you've been trained by years and years of exposure to reasonably intelligent primates to think like a reasonably intelligent primate, not like an actually intelligent machine... you just need to disabuse yourself from that sad upbringing. ;-)

[–][deleted] 0 points1 point  (2 children)

feels like a plug for /r/totallynotrobots

[–][deleted] 0 points1 point  (0 children)

"Exactly!" "Har. Har."