4
\$\begingroup\$

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 tie after the five rounds, then the users have to roll one die to determine the winner.

###### IMPORTING RANDOM AND TIME & DEFINING VARIABLES ######

import random
import time

i = 0
Player1Points = 0
Player2Points = 0
Player1Tiebreaker = 0
Player2Tiebreaker = 0
Winner_Points = 0

###### LOGIN CODE ######

### This Sets logged in to false, and then makes sure the username and password is correct before allowing them to continue ###


logged_in1 = False
logged_in2 = False
while logged_in1 == False:
    username = input('What is your username? ')
    password = input('What is your password? ')
    if username == 'User1' or username == 'User2' or username == 'User3' or username == 'User4' or username == 'User5':
        if password == 'password':
            print('Welcome, ',username,' you have been successfully logged in.')
            logged_in1 = True
            user1 = username
        else:
            print('Incorrect password, try again')  
    else:
        print('Incorrect username, try again')

while logged_in2 == False:
    username = input('What is your username? ')
    password = input('What is your password? ')
    if username == 'User1' or username == 'User2' or username == 'User3' or username == 'User4' or username == 'User5':
        if password == 'password':
            print('Welcome, ',username,' you have been successfully logged in.')
            logged_in2 = True
            user2 = username
        else:
            print('Incorrect password, try again')  
    else:
        print('Incorrect username, try again')


###### DEFINING ROLL ######

### Makes the dice roll for the player and works out the total for that roll ###


def roll():

    points = 0

    die1 = random.randint(1,6)

    die2 = random.randint(1,6)

    dietotal = die1 + die2

    points = points + dietotal

    if dietotal % 2 == 0:
        points = points + 10

    else:
        points = points - 5

    if die1 == die2:
        die3 = random.randint(1,6)
        points = points +die3

    return(points)

###### DICE ROLL ######

### This rolls the dice 5 times for the players, and then adds up the total. If the scores are equal, it starts a tie breaker and determines the winner off that ###

for i in range(1,5):
    Player1Points += roll()
    print('After this round ',user1, 'you now have: ',Player1Points,' Points')
    time.sleep(1)
    Player2Points += roll()
    print('After this round ',user2, 'you now have: ',Player2Points,' Points')
    time.sleep(1)

if Player1Points == Player2Points:
    while Player1Tiebreaker == Player2Tiebreaker:


        Player1Tiebreaker = random.randint(1,6)
        Player2Tiebreaker = random.randint(1,6)

    if Player1Tiebreaker > Player2Tiebreaker:
        Player2Points = 0
    elif Player2Tiebreaker > Player1Tiebreaker:
        Player1Points = 0

###### WORKING OUT THE WINNER ######

### This checks which score is bigger, then creates a tuple for my leaderboard code ( Gotton of stack overflow ) ###

if Player1Points>Player2Points:
    Winner_Points = Player1Points
    winner_User = user1
    winner = (Winner_Points, user1)
elif Player2Points>Player1Points:
    Winner_Points = Player2Points
    winner = (Winner_Points, user2)
    winner_User = user2

print('Well done, ', winner_User,' you won with ',Winner_Points,' Points')

###### CODE TO UPLOAD ALL SCORES TO A FILE ######

### This will store the winners username and score in a text file ###

winner = (Winner_Points,',',winner_User)
f = open('Winner.txt', 'a')
f.write(''.join(winner))
f.write('\n')
f.close()


###### CODE TO LOAD, UPDATE AND SORT LEADERBOARD ######

### This loads the leaderboard into an array, then compares the scores just gotton and replaces it ###

f = open('Leaderboard.txt', 'r')
leaderboard = [line.replace('\n','') for line in f.readlines()]
f.close()


for idx, item in enumerate(leaderboard):
    if item.split(', ')[1] == winner[1] and int(item.split(', ')[0]) < int(winner[0]):
            leaderboard[idx] = '{}, {}'.format(winner[0], winner[1])
    else:
        pass 

### This sorts the leaderboard in reverse, and then rewrites it ###

leaderboard.sort(reverse=True)

with open('Leaderboard.txt', 'w') as f:
    for item in leaderboard:
        f.write("%s\n" % item)

This was for my NEA task in computer science, which I have now finished; if anyone has any suggestions on how I could have made it better they will be greatly appreciated. So, please suggest how I can improve it!

\$\endgroup\$

2 Answers 2

8
\$\begingroup\$

Login

  1. Make a function login and move all the code to do with logged_in1 into it.
  2. Use while not logged_in1 rather than while logged_in1 == False.
  3. Rather than using val == 'a' or val == 'b' you can use val in ('a', 'b') .
  4. Rather than nesting if's you can use a guard clause:

    # bad
    if username in ('User1', 'User2', 'User3', 'User4', 'User5'):
        # code
    else:
        print('Incorrect username, try again')
    
    # good
    if username not in ('User1', 'User2', 'User3', 'User4', 'User5'):
        print('Incorrect username, try again')
        continue
    
    # code
    
  5. You can use str.format or f-strings (3.6+) to make your print easier to read.

    print('Welcome, {} you have been successfully logged in.'.format(username))
    print(f'Welcome, {username} you have been successfully logged in.')
    
  6. You can use while True and break from the loop if you login successfully. Alternately as you have a function you can return out of the function.

This changes your login code to:

def login():
    while True:
        username = input('What is your username? ')
        password = input('What is your password? ')
        if username not in ('User1', 'User2', 'User3', 'User4', 'User5'):
            print('Incorrect username, try again')
            continue

        if password != 'password':
            print('Incorrect password, try again')
            continue

        print(f'Welcome, {username} you have been successfully logged in.')
        return username

user1 = login()
user2 = login()

Roll

  1. Reduce the amount of empty lines in roll.
  2. You can use a turnery statement to reduce the amount of lines.

    # from
    if (die1 + die2) % 2 == 0:
        change = 10
    else:
        change = -5
    
    # to
    change = 10 if (die1 + die2) % 2 == 0 else -5
    
  3. You can calculate points in one go, so it's easier to know what it is.

  4. It's easier to read points += 1, which is the same as points = points + 1.
  5. It's common practice to put a space after commas in function calls.
  6. return is a keyword, rather than a function so you should remove the brackets.

This changes roll to:

def roll():
    die1 = random.randint(1, 6)
    die2 = random.randint(1, 6)
    change = 10 if (die1 + die2) % 2 == 0 else -5
    points = die1 + die2 + change
    if die1 == die2:
        points += random.randint(1, 6)
    return points

Game

  1. It's common in Python to use snake_case, so I suggest changing Player1Points to player_1_points or player1_points.
  2. I'd change the code so that it returns a tuple of both the points scored and if they won for both players.

Winner

  1. It'd be easier to make this code in the main function, and so I'd move the calls to login and game here.
  2. It'd be simpler if you focus on just making winner, rather than winner_user and winner_points.

Getting:

def main():
    user1 = login()
    user2 = login()
    (player1, player1_win), (player2, player2_win) = game(user1, user2)
    if player1_win:
        winner = (player1, user1)
    else:
        winner = (player2, user2)
    print('Well done, {winner[1]} you won with {winner[0]} Points')

Winner and Scoreboard

  1. It'd be easier if you used an f-string to format the line to write to the file.
  2. You should use with when using files.
  3. You should add some more functions to get, mutate and write to the leaderboard.
  4. You should add a if __name__ == '__main__': guard to ensure your code doesn't run when you don't want it to.

This gets the following code:

import random
import time


def login():
    while True:
        username = input('What is your username? ')
        password = input('What is your password? ')
        if username not in ('User1', 'User2', 'User3', 'User4', 'User5'):
            print('Incorrect username, try again')
            continue

        if password != 'password':
            print('Incorrect password, try again')
            continue

        print(f'Welcome, {username} you have been successfully logged in.')
        return username


def roll():
    die1 = random.randint(1, 6)
    die2 = random.randint(1, 6)
    change = 10 if (die1 + die2) % 2 == 0 else -5
    points = die1 + die2 + change
    if die1 == die2:
        points += random.randint(1, 6)
    return points


def game(user1, user2):
    player1_points = 0
    player2_points = 0
    for i in range(1,5):
        player1_points += roll()
        print(f'After this round {user1} you now have: {player1_points} Points')
        time.sleep(1)
        player2_points += roll()
        print(f'After this round {user2} you now have: {player2_points} Points')
        time.sleep(1)

    player1_tiebreaker = 0
    player2_tiebreaker = 0
    if player1_points == player2_tiebreaker:
        while player1_tiebreaker == player2_tiebreaker:
            player1_tiebreaker = random.randint(1,6)
            player2_tiebreaker = random.randint(1,6)

    player1_win = (player1_points + player1_tiebreaker) > (player2_points, player2_tiebreaker)
    return (player1_points, player1_win), (player2_points, not player2_win)


def add_winner(winner):
    with open('Winner.txt', 'a') as f:
        f.write('{winner[0]},{winner[1]}\n')


def get_leaderboard():
    with open('Leaderboard.txt', 'r') as f:
        return [line.replace('\n','') for line in f.readlines()]


def update_leaderboard(leaderboard, winner):
    for idx, item in enumerate(leaderboard):
        if item.split(', ')[1] == winner[1] and int(item.split(', ')[0]) < int(winner[0]):
                leaderboard[idx] = '{}, {}'.format(winner[0], winner[1])
        else:
            pass
    leaderboard.sort(reverse=True)


def save_leaderboard(leaderboard):
    with open('Leaderboard.txt', 'w') as f:
        for item in leaderboard:
            f.write(f'{item}\n')


def main():
    user1 = login()
    user2 = login()
    (player1, player1_win), (player2, player2_win) = game(user1, user2)
    if player1_win:
        winner = (player1, user1)
    else:
        winner = (player2, user2)
    print('Well done, {winner[1]} you won with {winner[0]} Points')
    add_winner(winner)
    leaderboard = get_leaderboard()
    update_leaderboard(leaderboard, winner)
    save_leaderboard(leaderboard)


if __name__ == '__main__':
    main()
\$\endgroup\$
8
  • \$\begingroup\$ Tried this code and wasnt working for me. Also, what does the if name == 'main' do \$\endgroup\$
    – Panda32
    Commented Oct 12, 2018 at 17:10
  • \$\begingroup\$ @Panda32 I will look into fixing the errors later today. As for the name thing, it means if you import the file, then then it won't run main. This is good if you put a lot of files in one folder, you add one with the same name as one of the standard libraries, but when it's imported it wipes all your local data. This'd mean if you run one of your other files which accedently runs the wipe code it'd wipe all your data! So it's a good thing to do just to be safe \$\endgroup\$
    – Peilonrayz
    Commented Oct 12, 2018 at 17:12
  • \$\begingroup\$ Ah ok, what do u mean by importing the file ;-; \$\endgroup\$
    – Panda32
    Commented Oct 12, 2018 at 17:15
  • \$\begingroup\$ @Panda32 import <file name>. \$\endgroup\$
    – Peilonrayz
    Commented Oct 12, 2018 at 17:16
  • \$\begingroup\$ oh i never knew you could do that xD. What can you use that for? \$\endgroup\$
    – Panda32
    Commented Oct 12, 2018 at 17:17
7
\$\begingroup\$

@Peilonrayz has a very good review, I just want to add a couple of things to the Login functionality

The login works, but it is not very secure (not that it matters much in this example) but to give you a general idea of how to make this more secure for more (real world) applications

  • It leaks information about your users

         else:
            print('Incorrect password, try again')  
    else:
        print('Incorrect username, try again')
    

    This makes it realy easy for attackers to enumerate users as an attacker will know when a username exists or not.

  • It stores passwords in plaintext

    Best practise is to store hashed passwords only, so if the Database or in this case the source code leaks an attacker will know all the passwords stored there

  • input() for passwords is not secure

    There is a good python module for getting passwords from user input namely getpass

  • (Optional) You could do something if there are multiple wrong passwords

Example Code

import getpass
import hashlib

HASHED_PASSWORD = "b109f3bbbc244eb82441917ed06d618b9008dd09b3befd1b5e07394c706a8bb980b1d7785e5976ec049b46df5f1326af5a2ea6d103fd07c95385ffab0cacbc86"
USERS = ('User1', 'User2', 'User3', 'User4', 'User5')

def login(max_attempts=5):
    attempts = 0
    while attempts < max_attempts:
        username = input('What is your username? ')
        password = getpass.getpass('What is your password? ')

        # [OPTIONAL] add a salt
        if username not in USERS or HASHED_PASSWORD != hashlib.sha512(password.encode()).hexdigest():
            print('Something went wrong, try again')
            attempts += 1
            continue

        print(f'Welcome, {username} you have been successfully logged in.')
        return username
    print("Too many tries, exiting")
    exit()

if __name__ == '__main__':
    user = login()
    # ... rest of the code here
\$\endgroup\$
1
  • \$\begingroup\$ ah okay, I wouldn't have personally included the login code, but was a requirement of the code. Same with the leader-board: I've been told there are much easier ways than using files to do the leader board. \$\endgroup\$
    – Panda32
    Commented Oct 12, 2018 at 13:54

Not the answer you're looking for? Browse other questions tagged or ask your own question.