Two-player dice game for NEA task computer science (Updated)

Question

This is a game for two users who roll 2 dice 5 times. If the total of dice is even the player gains 10 points; if it is odd, they lose 5.

If there is a draw after five rounds then both users will have to roll one die to determine the winner.

Some updates that I have done to this code include adding functions to it so that it reduces the size of the code, removing repeated code, acting upon the suggestions that were given to me on my old code, and trying to improve my DRY (don't repeat yourself) skills.

I just want suggestions on how I could improve this updated code.

import time
import sys
import random
import operator
total_score2 = 0
total_score1 = 0
rounds = 0
playerOnePoints = 0
playerTwoPoints = 0
counter = 0

print("*****************Welcome To The DICE Game*******************")
print("Please enter 'n' if you are a new user and 'e' if you are a exsiting user and enter 's' to display scores")
ens=input("")
while ens not in ('e', 'n', 's'): # if anything else but these characters are entered it will loop until it is correct
    print("Please enter 'n' if you are a new user and 'e' if you are a exsiting user and enter 's' to display scores")
    ens = input()

if ens == "s":
    s = open("scores.txt","r")
    file_content = s.read().splitlines()
    users_points = {i.split()[0]: int(i.split()[2]) for i in file_content}
    best_player = max(users_points.items(), key=operator.itemgetter(1))[0]
    print("LeaderBoard: ")
    print("\n")
    print('player with maximum points is {}, this player has {} points'.format(best_player, users_points[best_player]))
    best_players = sorted(users_points, key=users_points.get, reverse=True)
    for bp in best_players:
        print('{} has {} points'.format(bp, users_points[bp])) # This prints all players scores
    print("\n")
    print("Please enter 'n' if you are a new user and 'e' if you are a exsiting user and enter 's' to display scores")
    ens=input("")

if ens == "n":
    file = open("accountfile.txt","r+")
    text = file.read().strip().split()
    check = True
    while check:
        username=input("Please enter appropiate username: ") #Takes input of a username from user
        if username == "": #if no value is entered for the username
            continue
        if username in text: #username in present in the text file
            print("Username is taken please try another one")
        else: #username is absent in the text file
            print("Username has been accepted")
            check = False
            check = True
            while check:
                password1=input("Please enter password: ")
                password2=input("Please re-enter password: ")
                if password1 == password2:
                    if password2 in text:
                        print("Password has been taken please try another one")
                    else:
                        print("Username and Password have sucessfully been made Thankyou")
                        file.write("username: " + username + " " + "password: " + password2 + "\n")
                        file.close()
                        check = False
                else:
                    print("passwords do not match please try again")                         
    file.close()

def write1():
    print("Player 1 ",username1," Wins!")
    file = open("scores.txt","a")
    file.write(username1 + " has " + str(total_score1) + " points" + "\n")
    file.close()
    sys.exit()
def write2():
    print("Player 2 ",username2," Wins!")
    file = open("scores.txt","a")
    file.write(username2 + " has " + str(total_score2) + " points" + "\n")
    file.close()
    sys.exit()
def validation():
    global counter
    print("Sorry, this username or password does not exist please try again")
    counter = counter + 1
    if counter == 3:
        print("----------------------------------------------------")
        print("You have been locked out please restart to try again")
        sys.exit()
def game():
    global total_score1
    global total_score2
    global rounds
    global number
    global number2
    global playerOnePoints
    global playerTwoPoints
    total_score2 = total_score2 + playerTwoPoints
    total_score1 = total_score1 + playerOnePoints
    rounds = rounds + 1
    number = random.randint(1,6)
    number2 = random.randint(1,6)
    playerOnePoints = number + number2
    print("-------------------------------------------")
    print("Round",rounds)
    print("-------------------------------------------")
    print("Player 1's turn    Type 'roll' to roll the dice")
    userOneInput = input(">>> ")
    if userOneInput == "roll":
        time.sleep(1)
        print("Player 1's first roll is", number)
    print("Player 1's second roll    Type 'roll' to roll the dice")
    userOneInput = input(">>> ")
    if userOneInput == "roll":
        time.sleep(1)
        print("player 1's second roll is", number2)
    if playerOnePoints % 2 == 0:
        playerOnePoints = playerOnePoints + 10
        print("Player 1's total is even so + 10 points")
        print("-------------------------------------------")
        print("Player 1 has",playerOnePoints, "points")
    else:
        playerOnePoints = playerOnePoints - 5
        print("player 1's total is odd so -5 points")
        print("-------------------------------------------")
        print("Player 1 has",playerOnePoints, "points")
    number = random.randint(1,6)
    number2 = random.randint(1,6)
    playerTwoPoints = number + number2
    print("-------------------------------------------")
    print("Player 2's turn    Type 'roll' to roll the dice")
    userTwoInput = input(">>> ")
    if userTwoInput == "roll":
        time.sleep(1)
        print("Player 2's first roll is", number)
    print("Player 2's second roll    Type 'roll' to roll the dice")
    userTwoInput = input(">>> ")
    if userTwoInput == "roll":
        time.sleep(1)
        print("player 2's second roll is", number2)
    if playerTwoPoints % 2 == 0:
        playerTwoPoints = playerTwoPoints + 10
        print("Player 2's total is even so + 10 points")
        print("-------------------------------------------")
        print("Player 2 has",playerTwoPoints, "points")
    else:
        playerTwoPoints = playerTwoPoints - 5
        print("player 2's total is odd so -5 points")
        print("-------------------------------------------")
        print("Player 2 has",playerTwoPoints, "points")

if ens == "e":
    counter = 0 
    check_failed = True
    while check_failed:
        print("Could player 1 enter their username and password")
        username1=input("Please enter your username ")
        password=input("Please enter your password ")
        with open("accountfile.txt","r") as username_finder:
            for line in username_finder:
                if ("username: " + username1 + " password: " + password) == line.strip(): 
                    print("you are logged in")
                    check_failed = False
                    check_failed = True
                    while check_failed:
                        print("Could player 2 enter their username and password")
                        username2=input("Please enter your username ")
                        password=input("Please enter your password ")
                        with open("accountfile.txt","r") as username_finder:
                            for line in username_finder:
                                if ("username: " + username2 + " password: " + password) == line.strip():
                                    print("you are logged in")
                                    check_failed = False
                                    time.sleep(1)
                                    print("Welcome to the dice game")
                                    time.sleep(1)
                                    while rounds < 5:
                                        game()
                                    print("-------------------------------------------")
                                    print("Total score for player 1 is", total_score1)
                                    print("-------------------------------------------")
                                    print("Total score for player 2 is", total_score2)
                                    print("-------------------------------------------")
                                    if total_score1 > total_score2:
                                        write1()
                                    if total_score2 > total_score1:
                                        write2()
                                    if total_score1 == total_score2:
                                        print("Its a draw!")
                                        game()
                                        if total_score1 > total_score2:
                                            write1()
                                        if total_score1 < total_score2:
                                            write2()
                            else:
                                validation()

            else:
                validation()      

This is the link to my old code

Answer 1

You should make better use of functions to reduce duplication.

Here:

def write1():
    print("Player 1 ",username1," Wins!")
    file = open("scores.txt","a")
    file.write(username1 + " has " + str(total_score1) + " points" + "\n")
    file.close()
    sys.exit()

def write2():
    print("Player 2 ",username2," Wins!")
    file = open("scores.txt","a")
    file.write(username2 + " has " + str(total_score2) + " points" + "\n")
    file.close()
    sys.exit()

Note how 99% of those functions are identical. The only difference is the usernames and scores. Add those as parameters to the function and pass the data in as arguments. This will make even more sense once you get rid of global variables as the other answer suggested:

def write_score(username, score):
    print(username," wins!") # Got rid of the "Player #" part for simplicity
    file = open("scores.txt","a")
    file.write(username + " has " + str(score) + " points" + "\n")
    file.close()

     # This arguably shouldn't be here.
     # Do you really want it to be necessary to close the program after writing?
    sys.exit()

Then change the calling code to something like:

if total_score1 > total_score2:
    write_score(total_score1, username1)

elif total_score2 > total_score1:
    write_score(total_score2, username2)

You don't gain as much with functions if you're just moving two nearly identical chunks of code into two separate, nearly identical functions.

If code looks almost the same, here's a simple way to turn it into a common function: Look at the pieces of code, and determine what is the same, and what is different. Make the different parts parameters of the function and pass that data in, and make the identical parts the body of the function. You can see how I used that with the example above.

This can be made much neater though if you tuck the name and score away into a Player object. That way you aren't needing to deal with those bits of data separately.

---

Everything under if ens == "e": should be moved out into its own function. There's no reason to have that much dense code all lumped together. It makes your code much harder to read, and forces you to have a ridiculous amount of nesting/indentation. You should create a function that reads the account information from file, another function that takes that information and checks the supplied login credentials, a function that takes input from the user, and a main procedure function that encompasses the logic of the game.

Splitting it up like that will not only reduces duplication (since then you can, for example, call the "get user input" function twice instead of copying and pasting nearly the same chunk of code), but it will make the program easier to test. To see if loading information works, you just need to feed data to the "load" function and see what it returns back. With how you have it setup now, you need to run the entire program just to see if a small part of it works.

---

Another example of reducing duplication is creating a function to test if a username/password combo is correct. It would make much more sense to write something like:

def verify_login(username, password, login_data):
    for line in login_data:
       if ("username: " + username + " password: " + password) == line.strip():
           return True

    return False

And then call this function for each player in the main routine.

with open("accountfile.txt","r") as username_finder:
    username1 = input(...)
    password1 = input(...)

    if verify_login(username1, password1, username_finder):
        print("you are logged in")

        username2 = input(...)
        password2 = input(...)

        if verify_login(username2, password2, username_finder):

Now, you can test this functionality without ever needing to load data from a file, or even run the program, and you don't need to deal with the whole check_failed mess.

That part could be extracted out too so the user is asked to enter a username/password pair until it matches:

def ask_for_login(login_data):
    while True:
        username = input(...)
        password = input(...)

        if verify_login(username, password, login_data):
            # Return the verified username that the user entered
            return username

        else:
            validation()

Then use it as:

with open("accountfile.txt","r") as username_finder:
    username1 = ask_for_login(username_finder)
    username2 = ask_for_login(username_finder)

    # Once you get rid of the globals, you'd pass "username1" and
    #  "username2" as arguments to "game"
    game()

Notice how much nesting this gets rid of.

---

Practice looking at similar looking code and thinking about how it could be made into a function. This entire piece of code could be reduced by probably half once all the duplication is removed.

Answer 2

Try to avoid using so many globals. Your code would be better-structured if you made a Game class and captured most or all of that state as class member variables.

You made the same spelling mistake here as you did in your previous question. "exsiting" is spelled "existing".

In this code:

s = open("scores.txt","r")

You open, but fail to close, s. Convert this to a with statement.

This:

users_points = {i.split()[0]: int(i.split()[2]) for i in file_content}

relies on this format:

file.write(username1 + " has " + str(total_score1) + " points" + "\n")

As such, you can convert your users_points initialization to:

users_points = {}
for line in file_content:
    user, points = re.match('r(\w+) has (\d+) points').groups()
    users_points[user] = int(points)

However, that's not ideal. If scores.txt doesn't need to be human-readable, then you should store it in a different format - probably JSON. That way, your loading and store can be made much more simple.

Move your global code to a main method.

As I recommended in the previous incarnation of this question, and will recommend again, stop issuing blank input calls. This:

print("Please enter 'n' if you are a new user and 'e' if you are a exsiting user and enter 's' to display scores")
ens=input("")

needs to be

ens = input("Please enter 'n' if you are a new user, 'e' if you are an existing user, or 's' to display scores: ")

Try to convert some of your concatenated strings into f-strings:

username1 + " has " + str(total_score1) + " points" + "\n")

should become

f'{username1} has {total_score1} points\n'

This:

counter = counter + 1

should be

counter += 1

This:

check_failed = False
check_failed = True

is quite strange; the first assignment will be overwritten so you should probably just delete it.