TransWikia.com

Easier ways to make an RPG dice roller code?

Code Review Asked by Lucas Melo on January 4, 2022

I’m new to programming and this is the first thing I’m doing on my own. I would appreciate it if you could point me ways to optimize this RPG dice roller code in Python!

import random

def dice_reader():

##this should read messages like '2 d 6 + 3, 5 d 20 + 2 or just 3 d 8'##
    message = input('>')
    reader = message.split(' ')
    times = reader[0]
    sides = reader[2]
    output_with_modifier = []
    result = []

    if len(reader) == 5:
        modifier = reader[4]
    else:
        modifier = 0

    for output in range(int(times)):
        output = random.randint(1, int(sides))
        result.append(output)
        output_with_modifier = [(int(x) + int(modifier)) for x in result]
    print(f' Dice rolls: {tuple(result)}')
    if modifier != 0:
        print(f' With modifier: {tuple(output_with_modifier)}')


    end = False
    while end == False:
        dice_reader()
        end_message = input('Again? ')
        if end_message.lower() == 'no':
            end = True
    else:
        pass

One Answer

Mystery inputs

It's clear that there's an implied structure to this input:

message = input('>')
reader = message.split(' ')

requiring between four and five tokens. I have no idea what those are, and neither does your user. Replace '>' with an actual description of what's expected here.

Unpacking

times = reader[0]
sides = reader[2]

can be

times, _, sides = reader[:3]

though the discarded second item is suspicious. You do need to show what this is, and it probably shouldn't be there.

Looping and overwrites

This:

for output in range(int(times)):
    output_with_modifier = [(int(x) + int(modifier)) for x in result]

does not make sense. If you ask for 100 times, output_with_modifier will be calculated 100 times and thrown away 99 of them. Only the last value will be kept. You probably want to de-indent that last assignment so that it happens outside of the loop.

More looping

end = False
while end == False:
    dice_reader()
    end_message = input('Again? ')
    if end_message.lower() == 'no':
        end = True
else:
    pass

First, delete that else; pass - it isn't doing anything. Also, end == False should be not end; but you shouldn't be using a termination variable at all. If you find a no, simply break.

Suggested code

Some of this may challenge a beginner, but I like to think that CodeReview is for "aspiring advanced programmers". I've tried to comment it extensively, but feel free to ask questions in the comments.

import re
from random import randint
from re import Pattern
from typing import ClassVar, Iterable


class Dice:
    """
    One specification for dice rolls in Dungeons & Dragons-like format.
    """

    def __init__(self, times: int, sides: int, modifier: int = 0):
        if times < 1:
            raise ValueError(f'times={times} is not a positive integer')
        if sides < 1:
            raise ValueError(f'sides={sides} is not a positive integer')

        self.times, self.sides, self.modifier = times, sides, modifier

    # This is a class variable (basically a "static") that only has one copy
    # for the entire class type, rather than a copy for every class instance
    # It is a regular expression pattern that will allow us to parse user
    # input.
    INPUT_PAT: ClassVar[Pattern] = re.compile(
        # From the start, maybe some whitespace, then a group named "times"
        # that contains one or more digits
        r'^s*(?P<times>d+)'    
        
        # Maybe some whitespace, then the letter "d"
        r's*d'
        
        # Maybe some whitespace, then a group named "sides" that contains one
        # or more digits
        r's*(?P<sides>d+)'
        
        # The beginning of a group that we do not store.
        r'(?:'
        
            # Maybe some whitespace, then a "+" character
            r's*+'
        
            # Maybe some whitespace, then a group named "modifier" that
            # contains one or more digits
            r's*(?P<modifier>d+)'
        
        # End of the group that we do not store; mark it optional
        r')?'
        
        # Maybe some whitespace, then the end.
        r's*$',

        # We might use "d" or "D"
        re.IGNORECASE
    )

    # This can only be called on the class type, not a class instance. It
    # returns a new class instance, so it acts as a secondary constructor.
    @classmethod
    def parse(cls, message: str) -> 'Rolls':
        match = cls.INPUT_PAT.match(message)
        if match is None:
            raise ValueError(f'Invalid dice specification string "{message}"')

        # Make a new instance of this class based on the matched regular
        # expression.
        return cls(
            int(match['times']),
            int(match['sides']),
            # If there was no modifier specified, pass 0.
            0 if match['modifier'] is None else int(match['modifier']),
        )

    @classmethod
    def from_stdin(cls) -> 'Rolls':
        """
        Parse and return a new Rolls instance from stdin.
        """

        while True:
            try:
                message = input(
                    'Enter your dice specification, of the formn'
                    '<times>d<sides> [+ modifier], e.g. 3d6 or 4d12 + 1:n'
                )
                return cls.parse(message)
            except ValueError as v:
                print(v)
                print('Please try again.')

    def roll(self, with_modifier: bool = False) -> Iterable[int]:
        """
        Return a generator of rolls. This is "lazy" and will only execute the
        rolls that are consumed by the caller, because it returns a generator
        (not a list or a tuple).
        """
        mod = self.modifier if with_modifier else 0
        return (
            randint(1, self.sides) + mod
            for _ in range(self.times)
        )

    def print_roll(self):
        print(
            'Dice rolls:',
            ', '.join(str(x) for x in self.roll()),
        )

        if self.modifier != 0:
            print(
                'With modifier:',
                ', '.join(str(x) for x in self.roll(with_modifier=True)),
            )


def test():
    """
    This is an automated test method that does some sanity checks on the Dice
    implementation.
    """
    
    d = Dice.parse('3 d 6')
    assert d.times == 3
    assert d.sides == 6
    assert d.modifier == 0

    d = Dice.parse('3D6 + 2')
    assert d.times == 3
    assert d.sides == 6
    assert d.modifier == 2

    try:
        Dice.parse('nonsense')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == 'Invalid dice specification string "nonsense"'

    try:
        Dice.parse('-2d5')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == 'Invalid dice specification string "-2d5"'

    try:
        Dice.parse('0d6')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == "times=0 is not a positive integer"

    d = Dice.parse('100 d 12+3')
    n = 0
    for x in d.roll(True):
        assert 4 <= x <= 15
        n += 1
    assert n == 100


def main():
    test()

    dice = Dice.from_stdin()
    dice.print_roll()


if __name__ == '__main__':
    main()

Answered by Reinderien on January 4, 2022

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP