Dice game for two players (Nea)

Question

The rules:

• The points rolled on each player’s dice are added to their score.
• If the total is an even number, an additional 10 points are added to their score.
• If the total is an odd number, 5 points are subtracted from their score.
• If they roll a double, they get to roll one extra die and get the number of points rolled added to their score.
• The score of a player cannot go below 0 at any point.
• The person with the highest score at the end of the 5 rounds wins.
• If both players have the same score at the end of the 5 rounds, they each roll 1 die and whoever gets the highest score wins (this repeats until someone wins)

import pickle
import random
import time

print("")
print("welcome to the dice game")
print("")

with open('users.pickle', 'rb') as f:
    users = pickle.load(f)

print(users)


def login():

    logged_in = False

    username = input('please enter username: ')

    while not logged_in:

        if username in users:
            password = input("enter password: ")
            if password == users[username]:
                print("access granted")
                logged_in = True

        else:
            print("access denied")
            exit()

    return username


print("  ")

rounds = 0


def roll():
    die1 = random.randint(1, 7)
    die2 = random.randint(1, 7)
    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():
    player1_points = 0
    player2_points = 0
    for i in range(1, 6):
        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')

    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)

    player2_win = (player2_points + player2_tiebreaker)
    player1_win = (player1_points + player1_tiebreaker) \

    return (player1_points, player1_win), (player2_points, player2_win)


def add_winner(winner):

    with open('Winner.txt', 'a') as i:
        i.write(','.join(map(str, winner)))
        i.write('\n')


def get_leaderboard():
    with open('Leaderboard.txt', 'r+') as g:
        return [line.replace('\n', '') for line in g.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', 'r+') as h:
        for item in leaderboard:
            h.write(f'{item}\n')


def main():
    user1 = login()
    user2 = login()

    (player1, player1_win), (player2, player2_win) = game()

    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()

Answer 1

Starting off, its not secure the way you are prompting for passwords and storing them. Instead, use the getpass module, and store the hash of the password. I'm not even certain that this way is 100% correct (I'm not a security guru), but its what the Python documentation shows for secure password hashing, and I imagine its better than plaintext.

from getpass import getpass
from hashlib import pbkdf2_hmac as pwhash

def login(users):
    username = input('please enter username: ')
    if not username in users:
        print("access denied")
        exit()

    password_hash = None
    while password_hash != users[username]:
        password_hash = pwhash("sha256", getpass().encode("ascii"), b"t4h20p8g24j", 100000)

    print("access granted")
    return username

Your roll function had a typo in it- the third (optional) roll could only generate numbers [1, 5] inclusive. I also reordered the statements to remove an intermediate variable that didn't contribute anything.

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

I would replace your get_leaderboard body with a shorter and cleaner read call.

def get_leaderboard():
    with open('Leaderboard.txt', 'r+') as g:
        return g.read().split("\n")

And similarly for save_leaderboard

def save_leaderboard(leaderboard):
    with open('Leaderboard.txt', 'r+') as h:
        h.write("\n".join(leaderboard))

You should also try to avoid excessive use of tuples when you need to access the elements individually. Your code will be littered with thingy[0] and thingy[1] and nobody but you (if that) will know what the significance of 0 and 1 are. I revised update_leaderboard with these changes, but I'm still not quite satisfied with it.

def update_leaderboard(leaderboard, winner):
    winner_score, winner_name = winner
    for idx, item in enumerate(leaderboard):
        item_score, item_name = item.split(', ')
        if item.name == winner_name and int(item_score) < int(winner_score):
            leaderboard[idx] = '{}, {}'.format(winner_score, winner_name)
    leaderboard.sort(reverse=True)
    #leaderboard.sort(reverse=True), key=lambda x:int(x.split(', ')[0]))

The data really shouldn't be cast back and forth from strings to lists/tuples so much. It would be best to just make a dictionary of name:score pairs, and write a tiny bit of formatting logic into your get_leaderboard and save_leaderboard functions. I wasn't going to change that much though.

I also want to point out common mistake that could normally create a bug in sorting (in your case it does not because of the hard-coded ,, but still)- you can't rely on str.sort to sort a list of strings by their numerical value. I left a comment showing how you would do this correctly if your delimiter evaluated as greater than "0".

Furthermore- you shouldn't really rely on changing leaderboard outside the function scope. I feel like this is partially why you chose to go with the (unnecessary) enumeration() logic. Instead, return the modified leaderboard and allow the caller to take it from there.

Anyways, the last major change I would make would be to move all your globals (and friends) inside your main() function.

def main():
    print("")
    print("welcome to the dice game")
    print("")

    with open('users.pickle', 'rb') as f:
        users = pickle.load(f)

    print(users)
    print("  ")

    user1 = login(users)
    user2 = login(users)
    ...