simple Rock, paper, scissors game

Question

I just recently started learning how to program and how to use python and after watching a few courses I made a simple Rock paper scissors game where basically the script randomly chooses Rock, paper or scissors then you make your choice and depending on what the program picked and what you picked you either win, lose or tie. This goes on until either you lose 3 times or win 3 times after which you'll be prompted for a rematch.

I was wondering how good is the code, I'd like Feedback as I want to know what I could have done differently to make the code better, cleaner and so on since I'm sure the code can be improved. I'll be using this sort of as a learning experience so I can better my programming knowledge.

import random
import time
import os
os.system("")

global vic_count
vic_count = 0
global loss_count
loss_count = 0
start = True
global rematchh
rematchh = True
CRED = '\033[91m'
CYEL = '\033[33m'
CGRE = '\033[32m'
CBLU = '\033[34m'
CTIL = '\033[3m'
CEND = '\033[0m'


def game_countinue():
    global vic_count
    global loss_count
    global rematchh
    while True:
        rematch = input("REMATCH? Y/N: ").upper()
        if rematch == "Y":
            vic_count = 0
            loss_count = 0
            rematchh = False
            return rematchh
        elif rematch == "N":
            break
        

def vcount():
    global vic_count
    vic_count += 1
    ccount()    
        
def lcount():
    global loss_count
    loss_count += 1
    ccount()
    
def ccount():
    print(CBLU + f"Player: {vic_count} Computer: {loss_count}" + CEND)    
 

def bot():
    bot_choice = ['Scissors', 'Paper', 'Rock']    
    bot_final_choice = random.choice(bot_choice)
    return bot_final_choice

   
def player():
     while True:
        player_choice = input(CTIL + "(R)ock, (P)aper or (S)cissors: " + CEND).upper()
        if player_choice == "P" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "P" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "P" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break
        if player_choice == "R" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "R" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "R" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break
        if player_choice == "S" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "S" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "S" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break

while start == True:
    yes_or_no = input(CRED + "WARNING!!! THIS GAME IS SO HARD THAT IT WILL PHYSICALLY CHANGE YOUR BEING, THIS IS ONLY MEANT FOR THE TRULY HARDENED GAMERS, ARE YOU SURE YOU WANT TO CONTINUE? Y/N: " + CEND).upper()
    if yes_or_no == "N":
        break
    elif yes_or_no == "Y":
        time.sleep(1)
        print(CYEL + "WIN A FRIENDLY GAME OF ROCK PAPER SCISSORS, THE FIRST ONE TO GET 3 POINTS WINS!" + CEND)
        time.sleep(3)
        start = False
        rematchh = False

while start == False and rematchh == False:
    botchoice = bot()
    print("Choosing..")   
    time.sleep(1)
    print("Done choosing..")
    time.sleep(1)
    player()
    time.sleep(1)
    if vic_count >= 3:
        print(CYEL + "CONGRATULATIONS!" + CEND)
        rematchh = True
        game_countinue()
        
    elif loss_count >= 3:
        print(CRED + "YOU SURE GOT RAZZLE DAZZLED!" + CEND)
        rematchh = True
        game_countinue()

I'm sure the code can be improved, especially with detecting what the player chose and the computer chose to determine who wins, as I ended up just using a ton of if and elif statements that just clutter everything.

Answer 1

Let's address the elephant in the room: the if-else ladder. When you see code duplication, extract common logic into a parametrized function and then call it with different parameters to alter the behaviour. In this case it can be done like so:

victory_table = {ROCK: (SCISSORS,), PAPER: (ROCK,), SCISSORS: (PAPER,)}

def determine_match_result(p1_choice, p2_choice):
    if p2_choice in victory_table[p1_choice]:
        return VICTORY
    elif p1_choice == p2_choice:
        return TIE
    else
        return DEFEAT

def resolve_match(player_choice, bot_choice):
    result = determine_match_result(player_choice, bot_choice)
    time.sleep(1)
    print(f"Players choice: {player_choice}")
    time.sleep(1)
    print(f"Computers choice: {bot_choice}")
    time.sleep(1)
    termcolor.cprint(result, "red")
    ccount()

In this example:

• the entirety of game logic is contained in a dedicated function
• game rules are recorded as a dictionary where each choice corresponds to a tuple of choices it can beat in a game (this solution can be extanded to play rock-paper-scissors-lizard-Spock or any other variant by modifying only one line)
• plain strings are replaced with CONSTANTS that you can specify at the top of your script: python (as any other language) doesn't check for correctness of strings (it doesn't really know what to check for), so it's very easy to break the prorgam by misseplling a wrod, but with constants it's not a problem anymore:

VICTORY = "You win!"
TIE = "Tie!"
...

• Instead of adding CYEL + and + CEND to every line, write a function that does it for you, or even better, use an existing library

---

As for the other problems:

import os
os.system("")

This line does nothing.

global vic_count

As well as this one. global keyword is used inside functions to access names out of their scope.

On the topic of global names: try to avoid them. They create a global state that makes reasoning and debugging much harder: function behaviour starts to depend on something out of its scope. You can do the same by passing required data as function parameters.

Naming is very important. It's much easier for people (and yourself) to read code when things are named based on what they actually do. Long descriptive names are better than short ones like ccount(). If it's a function, make it a verb that explains what it changes / returns. There is no way to know what the function bot() does by looking at its name. start may mean anything, is_game_started is more fitting. Try to find and change other names that don't explain their purpose to the reader.

if vic_count >= 3:
What is 3? Don't use magic numbers, replace them with constants:

SCORE_TO_WIN = 3
...
if vic_count >= SCORE_TO_WIN:

It's not only for readability: if you need to change the number, you no longer need to scan the code to find all the occurances, you just change the constant.

---

Overall very good for one of the first programs, keep it up!

Answer 2

These hard-coded values may give you portability problems:

CRED = '\033[91m'
CYEL = '\033[33m'
CGRE = '\033[32m'
CBLU = '\033[34m'
CTIL = '\033[3m'
CEND = '\033[0m'

The assumption here is that the output stream is connected to a terminal (or terminal emulator) that uses the ANSI sequences. Although such terminals are now in the majority, others do exist, and you'll get undesirable output if you ever use one of them (or if you play the program using a non-terminal channel, e.g. by writing an opponent program).

If you really want to provide this kind of formatting, then you should at least test whether the output stream is a terminal (using isatty()), and if so, examine the TERM environment variable to discover which kind. Libraries exist to do that for you (e.g. curses), rather than having you re-invent a wheel.

Answer 3

This code is good, but it has a few problems.

Review

---

The options

Instead of handling rock, scissors and paper as strings, I'd use an enum:

from enum import Enum
class Options(Enum):
    ROCK = 0
    PAPER = 1
    SCISSORS = 2

---

The if-else ladder

Instead of handling your logic inside the player() function, I'd instead make it a separate function:

def get_outcome(player_choice, other_choice):
    victory_table = {
        Options.ROCK: Options.SCISSORS,
        Options.PAPER: Options.ROCK,
        Options.SCISSORS: Options.PAPER
    }
    if player_choice == other_choice:
        return 0
    elif victory_table[player_choice] == other_choice:
        return 1
    else:
        return -1

You could even incorporate this into the Options class.

---

The game loop

Instead of having the game loop inside your player() function, you could instead do something like:

def main():
    while True:
        ...

Then, I'd handle all the mechanics inside that single loop:

def main():
    while True:
        bot_choice = bot()
        player_choice = player()

        outcome = get_outcome(player_choice, bot_choice)

        if outcome == 1:
            print(CGRE + "You won!" + CEND)
        elif outcome == -1:
            print(CRED + "You lost!" + CEND)
        else:
            print(CYEL + "You drew!" + CEND)

---

The colors

Instead of adding the color strings onto every line, I'd use an existing library like termcolor:

import termcolor

if outcome == 1:
    termcolor.cprint("You won!", "green")
elif outcome == -1:
    termcolor.cprint("You lost!", "red")
 else:
    termcolor.cprint("You drew!", "yellow")

---

Some useless lines

These lines are useless and should be deleted.

What is this even meant to do:

import os
os.system("")

These variables don't need to be global (global should be used inside functions, and not at all if possible).

global vic_count
...
global loss_count
...
global rematchh

---

Variable names

Some variables are poorly named or incorrectly spelt.

vic_count, rematchh and botchoice should be win_count, rematch and bot_choice respectively.

---

The code

Taking into account what I've said and general best practices, this is how I'd code rock-paper-scissors:

import random
import time

from enum import Enum

import termcolor


class Option(Enum):
    ROCK = 0
    PAPER = 1
    SCISSORS = 2

    def __str__(self):
        match self:
            case self.ROCK:
                return "rock"
            case self.PAPER:
                return "paper"
            case self.SCISSORS:
                return "scissors"

    def get_outcome(self, other):
        win_table = {
            self.ROCK: self.SCISSORS,
            self.PAPER: self.ROCK,
            self.SCISSORS: self.PAPER
        }

        if self == other:
            return 0

        elif win_table[self] == other:
            return 1

        else:
            return -1


def player():
    possible_choices = {"R": Option.ROCK, "S": Option.SCISSORS, "P": Option.PAPER}
    choice = ""

    while choice.upper() not in possible_choices.keys():
        choice = input("Choose: R(ock), P(aper), or S(cissors): ")

    return possible_choices[choice.upper()]


def bot():
    return random.choice([Option.ROCK, Option.PAPER, Option.SCISSORS])


def main():
    player_score = bot_score = 0
    while player_score < 3 and bot_score < 3:
        print("Choosing...")
        time.sleep(3)
        bot_choice = bot()
        print("Bot has chosen")

        player_choice = player()

        outcome = player_choice.get_outcome(bot_choice)

        if outcome == 1:
            termcolor.cprint("You won!!!", "green")
            player_score += 1

        elif outcome == -1:
            termcolor.cprint("You lost!", "red")
            bot_score += 1

        else:
            termcolor.cprint("You drew!", "yellow")

        print("Bot choice:", bot_choice)

        print(f"Player score: {player_score}, Bot score: {bot_score}")

    if player_score == 3:
        termcolor.cprint("You thrashed them!", "yellow")

    else:
        termcolor.cprint("You were thrashed!", "red")

    rematch = input("Do you want a rematch? (Y/N)? ")

    if rematch.lower() == "y":
        main()

play = input("Do you want to play? (Y/N)? ")
if play.lower() == "y":
    main()

Answer 4

This looks great! Going forward, I would be sure to give variables/functions proper names. I used to use names like ccount and aamount and ended up being very confusing to read when sharing with others