Dungeon and Dragons Text Based Game

Question

I've created a text based game based on D&D as part of exercise No.45 Learning Python the Hard Way v3.0

I've implemented a few features from the game: Character Selection, Fight Scene, Rolling.

Full Program World of wizzard and warriors EX45

I've created a sentence handle and would appreciate adivce on how it code be improved. Any advice would be appreciated.

from Loading import*

class Sentence_Handler():

    def sort_words(self,sentence,lexicon): # Checks words against Lexicon list, then pairs the key and values

        result = [] #Match words store in a list.
        words = sentence.lower().split() #split sentence and converts to lower

        for word in words: #cycles through list of split words.

            word = self.string_to_int(word)   #cheks if the word can be converted to a number.

            if word in lexicon: #checks "word" against the lexicon

                word_type = lexicon[word]  #gets the value of the "word"
                pair = (word, word_type) # pairs "word" & "word_type" in a tuple/
                result.append(pair) #adds pair to the result list.

            else: #if word is not in the lexicon

                if (len(result) == 0) and  (word in ["i" ,"i'm" ,'im' ,'am']): #checks if the word refers to first person

                    pair = (word,'subj')
                    result.append(pair)

                elif type(word) == type(1): # checks if the word is of type int

                    pair = (word,'number')
                    result.append(pair)

                else: #all other words will be treated as stop words. which will later be ignored

                    pair = (word,"stop")
                    result.append(pair)

        return result

    def string_to_int(self,word): # converts "str" to "int": str("One") and str("1")  to int(1)


        #if word in Dictionary.numbers:    # number in word form to int
        #    word = Dictionary.numbers[word]

        try:  #number in str form to int
            return int(word)
        except ValueError:
            return word

    def parser(self,word_list,type):  #checks the sorted words and converts them into a usable sentence.
        # type of sentence being looked at: different setences will have different requirements.


        #each of the required words will be sotred in one of these variables.
        noun = None
        verb = None
        subj = None
        direction = None
        number = None
        skill = None
        action = None
        weapon = None

        count = True # Checks if this the first instance of the loop. ie the first word in the list.

        for word in word_list:
            if (count == True) and word[1] == 'subj':
                subj = 'player'
            elif (count == True) and word[1] ==  'noun':
                subj = word[0]
            elif word[1] == 'noun':
                noun = word[0]
            elif word[1] == 'verb':
                verb = word[0]
            elif word[1] == 'direction':
                direction = word[0]
            elif word[1] == 'number':
                number = word[0]
            elif word[1] == 'skill':
                skill = word[0]
            elif word[1]== 'action':
                action = word[0]
            elif word[1] == "weapon":
                weapon =word[0]

            #if there are multiple words with the same type the last one checked will be used.

            if subj == None:
                subj = 'player'
            count = False


        # this will ensure that the correct words that are needed are present depending on setence typ

        if type == 'sentence':
            assert (verb != None),("[A verb was expected]")
            assert (noun != None),("[A noun was expected]")

        elif type == 'direction':
            assert (verb != None),("[A verb was expected]")
            assert (direction != None),("[A direction was expected]")

        elif type == 'number':
            assert (number != None),("[A number was expected]")

        elif type == 'simple':
            assert (verb != None),("[A verb was expected]")

        elif type == 'skill':
            assert (skill != None),("[A skill was expected]")

        elif type == 'action':
            assert (action != None),("[A action was expected]")

        elif type == 'weapon':
            assert (weapon != None),("[A weapon was expected]")
        gap()

        return(subj,verb,noun,skill,weapon,action,direction,number) #set of usable words returned.

    def scanner(self,lexicon,type): #combines sort_word and parser

        while  True:  # will continue to loop if user doesn't enter the required words will also inform user of the type of word that is needed
            gap()
            sentence = input("> ")
            word_list = self.sort_words(sentence,lexicon)

            try:
                phrase = self.parser(word_list,type)
                return phrase
                break
            except AssertionError as error:
                print(error)

    def word_filter(self,check,tuple, list, sentence_type, method, parameters):
        # Bad Code Below:
        # This is an absolute disaster.
        # At the momment it seems to serve the purpose but
        # i know it will cause a number of issues

        #tuple = subj=[0],verb=[1],noun=[2],skill=[3],weapon=[4],action=[5],direction=[6],number=[7]
        #Checks = ([0]= checks, [1] = Methods, [2] = Parameters)
        #sentence_type = type of sentence being handled

        #### These will hold multiple values depending on requirements. ######

        #list = stores accepted words   [list[0],list[1],list[2]

        #method =  function which will be executed   [method[0],method[1],method[2]]

        #parameters =  parameters to be supplied    [paramters[0],paramters[1],paramters[2]]

        ##################################################


        # So this method will user the result from scanner. "tuple"
        # depending on sentence_type certain points will be checked in the tuple.
        # this mehtod allows me to check the tuple against differnt lists if a match is found
        # the appopraite method will be called with paramters if assigned.

        # list, method and paramters can contain multiple valuse,
        # the first value in each will be used in the first check. then the program will move on to the second value in the set.
        #checks tell the program how many lists are being checked, the number of methods being used and number of parameters
        # depending on the number of methods used and parameters differnt checks will be performed.



        exit = -1  # control when the program exits.

        access = -1
        #access will contol what values are being used in each check.
        # first check will use all values stored at [0] then second check [1] and so on.


        while exit != (check[0] - 1): # loop will exit once all checks are performed or when a match is found

            access = access + 1  #

            exit = exit + 1

            if   ( (check[1] > 0) and (check[2] == 0) ):  # Takes multiple Methods and no parameters

                if sentence_type == 'sentence':

                    if tuple[0] == 'player':

                        if (tuple[1] in list[access]) and (tuple[2] in list[access]):

                            (method[access])()
                            break


                    else:

                        if (tuple[0] in list[access]) and (tuple[1] in list[access]):

                            (method[access])()
                            break

                elif sentence_type == 'simple':

                        if tuple[0] == 'player':

                            if (tuple[1] in list[access]) and (tuple[2] in list[access]):

                                (method[access])()
                                break

                            elif tuple[1]  in list[access]:

                                (method[access])()
                                break

                        else:

                            if (tuple[0] in list[access]) and (tuple[1] in list[access]):

                                (method)[access]()
                                break

                            elif (tuple[1] in list[access]) and (tuple[2] in list[access]):

                                (method[access])()
                                break

            elif ( (check[1] == 1) and (check[2] > 0) ):   # Takes one Method and Multiple Parameters

                if sentence_type == 'skill':

                    if tuple[0] == 'player':


                        if (tuple[1] in list[access]) and (tuple[3] in list[access]):

                            (method)(parameters[access])
                            break

                        elif tuple[3]  in list[access]:

                            (method)(parameters[access])
                            break

                    else:

                        if (tuple[0] in list[access]) and (tuple[3] in list[access]):

                            (method)(parameters[access])
                            break

                        elif (tuple[1] in list[access]) and (tuple[3] in list[access]):

                            (method)(parameters[access])
                elif sentence_type == 'simple':
                    if tuple[0] == 'player':

                        if (tuple[1] in list[access]) and (tuple[2] in list[access]):

                            (method)(parameters[access])
                            break

                        elif tuple[1]  in list[access]:

                            (method)(parameters[access])
                            break

                    else:

                        if (tuple[0] in list[access]) and (tuple[1] in list[access]):

                            (method)(parameters[access])
                            break

                        elif (tuple[1] in list[access]) and (tuple[2] in list[access]):

                            (method)(parameters[access])
                            break
            elif( (check[1] > 1) and (check[2] > 1) ): #Takes Multiple Methods and Multiple Parameters

                if sentence_type == 'sentence':

                    if tuple[0] == 'player':

                        if tuple[1] and tuple[2] in list[access]:

                            (method)(parameters[access])
                            break

                        else:

                            if (tuple[0] in list[access]) and tuple[1] in list[access]:

                                (method[access])(parameters[access])
                                break



                elif sentence_type == 'simple':

                    if tuple[0] == 'player':

                        if (tuple[1] in list[access]) and (tuple[2] in list[access]):

                            (method[access])(parameters[access])
                            break


                        elif tuple[1]  in list[access]:

                            (method[access])(parameters[access])
                            break

                        else:

                            if (tuple[0] in list[access]) and (tuple[1] in list[access]):

                                (method[access])(parameters[access])
                                break


                            elif (tuple[1] in list[access]) and (tuple[2] in list[access]):

                                (method[access])(parameters[access])
                                break
                elif sentence_type == 'action':


                    if tuple[0] == 'player':

                        if (tuple[5] in list[access] ):


                            (method[access])(parameters[access])
                            break

There's a bit much in here to do a code review of all of it. Which specific function are you most interested in cleaning up? — Samwise, Dec 19 '19 at 19:19
One very quick note: using typing annotations will make it a lot easier to keep track of what the code's doing, and the parts that are hard to type are the parts where you should rethink the interface. :) — Samwise, Dec 19 '19 at 19:20
function: "word_filter" in the above class: The function takes a set of keywords depending on the key words found function would be called. At the moment, I think i'm passing to much information to the function. I know i could of used an if statement to checks words supplied but i thought the process might have come too repetitive as it's something that has to be done whenever user input is taken. I'll take that into consideration going forward — leban ali, Dec 19 '19 at 20:14
Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Consider posting a follow-up question instead, this time providing the full context on the first revision. Questions get messy otherwise, if different answers review different revisions in the same thread. — Mast, Dec 21 '19 at 9:15
My suggestions to give things names and types still stand. :) — Samwise, Dec 21 '19 at 17:09

Samwise · Accepted Answer · 2019-12-19 22:25:47Z

Two big suggestions:

Give things meaningful names.
Give things meaningful types.

Looking at your word_filter function:

def word_filter(self, check, tuple, list, sentence_type, method, parameters):
    # Bad Code Below:
    # This is an absolute disaster.
    # At the momment it seems to serve the purpose but
    # i know it will cause a number of issues

    # tuple = subj=[0],verb=[1],noun=[2],skill=[3],weapon=[4],action=[5],direction=[6],number=[7]
    # Checks = ([0]= checks, [1] = Methods, [2] = Parameters)
    # sentence_type = type of sentence being handled

    #### These will hold multiple values depending on requirements. ######

    # list = stores accepted words   [list[0],list[1],list[2]

    # method =  function which will be executed   [method[0],method[1],method[2]]

    # parameters =  parameters to be supplied    [paramters[0],paramters[1],paramters[2]]

The code itself isn't the disaster here, it's the interface to it. I don't know what the types of any of these arguments are, even after reading the comments. More to the point, I don't know what this function is even supposed to be doing. The name suggests that it "filters" something, but it doesn't return anything, which is normally what a filter would do. Instead it invokes a "method" function, but I don't know what that function itself does, or how the method relates to the data passed in. According to the docstring, the function "serves the purpose" (what purpose?) and the inputs "depend on requirements" (what requirements?).

I would try to suggest a way to add types to make this more comprehensible, but since there's no documentation on the function's purpose, and since nothing within the code has a meaningful name (you named one of your inputs tuple and another one list), it's hard to even reverse-engineer what the code is supposed to do. So maybe I can start there by just destructuring the inputs based on your comments.

I'm going to change the signature to:

def word_filter(self, check, gross_tuple, accepted, sentence_type, method, parameters):

because tuple and list are really bad names for variables (they match the names of builtin functions). I honestly don't know what tuple even is so I just renamed it gross_tuple for now. Your list is "accepted words" according to the comments so I renamed it "accepted".

Now I can do some simple find+replace to make the body a little more legible:

subject, verb, noun, skill, weapon, action, direction, number = gross_tuple
checks, methods, params = check[0], check[1], check[2]

exit_code = -1  # control when the program exits.

access = -1
# access will contol what values are being used in each check.
# first check will use all values stored at [0] then second check [1] and so on.

while exit_code != (checks - 1):  # loop will exit once all checks are performed or when a match is found
    access = access + 1  #
    exit_code = exit_code + 1
    if methods > 0 and params == 0:  # Takes multiple Methods and no parameters
        if sentence_type == 'sentence':
            if subject == 'player':
                if (verb in accepted[access]) and (noun in accepted[access]):
                    (method[access])()
                    break
            else:
                if (subject in accepted[access]) and (verb in accepted[access]):
                    (method[access])()
                    break
        elif sentence_type == 'simple':
            if subject == 'player':
                if (verb in accepted[access]) and (noun in accepted[access]):
                    (method[access])()
                    break
                elif verb in accepted[access]:
                    (method[access])()
                    break
            else:
                if (subject in accepted[access]) and (verb in accepted[access]):
                    (method)[access]()
                    break
                elif (verb in accepted[access]) and (noun in accepted[access]):
                    (method[access])()
                    break
    elif methods == 1 and params > 0:  # Takes one Method and Multiple Parameters
        if sentence_type == 'skill':
            if subject == 'player':
                if (verb in accepted[access]) and (skill in accepted[access]):
                    method(parameters[access])
                    break
                elif skill in accepted[access]:
                    method(parameters[access])
                    break
            else:
                if (subject in accepted[access]) and (skill in accepted[access]):
                    method(parameters[access])
                    break
                elif (verb in accepted[access]) and (skill in accepted[access]):
                    method(parameters[access])
        elif sentence_type == 'simple':
            if subject == 'player':
                if (verb in accepted[access]) and (noun in accepted[access]):
                    method(parameters[access])
                    break
                elif verb in accepted[access]:
                    method(parameters[access])
                    break
            else:
                if (subject in accepted[access]) and (verb in accepted[access]):
                    method(parameters[access])
                    break
                elif (verb in accepted[access]) and (noun in accepted[access]):
                    method(parameters[access])
                    break
    elif methods > 1 and params > 1:  # Takes Multiple Methods and Multiple Parameters
        if sentence_type == 'sentence':
            if subject == 'player':
                if verb and noun in accepted[access]:
                    method(parameters[access])
                    break
                else:
                    if (subject in accepted[access]) and verb in accepted[access]:
                        (method[access])(parameters[access])
                        break
        elif sentence_type == 'simple':
            if subject == 'player':
                if (verb in accepted[access]) and (noun in accepted[access]):
                    (method[access])(parameters[access])
                    break
                elif verb in accepted[access]:
                    (method[access])(parameters[access])
                    break
                else:
                    if (subject in accepted[access]) and (verb in accepted[access]):
                        (method[access])(parameters[access])
                        break
                    elif (verb in accepted[access]) and (noun in accepted[access]):
                        (method[access])(parameters[access])
                        break
        elif sentence_type == 'action':
            if subject == 'player':
                if action in accepted[access]:
                    (method[access])(parameters[access])
                    break

That's about the point at which I can't figure out anything else to do with this code, because I don't know what access is or what method might be. My hunch is that you want to break this up into different functions to handle the different sentence types.

My suggestion for meaningful typing would look something like:

from enum import Enum, auto
from typing import Callable, List, NewType, Set, Tuple

class SentenceType(Enum):
    action = auto()
    sentence = auto()
    simple = auto()
    skill = auto()

Word = NewType("Word", str)
Subject = NewType("Subject", Word)
Verb = NewType("Verb", Word)
Noun = NewType("Noun", Word)
Skill = NewType("Skill", Word)
Weapon = NewType("Weapon", Word)
Action = NewType("Action", Word)
Direction = NewType("Direction", Word)


def word_filter(
        check: List[int], 
        gross_tuple: Tuple[Subject, Verb, Noun, Skill, Weapon, Action, Direction], 
        accepted: List[Set[Word]], 
        sentence_type: SentenceType, 
        method: Callable[[...], None], 
        parameters: List[...]
):

Stack Exchange Network

current community

your communities

more stack exchange communities

Dungeon and Dragons Text Based Game

1 Answer 1

Your Answer

Not the answer you're looking for? Browse other questions tagged python object-oriented role-playing-game adventure-game battle-simulation or ask your own question.

Hot Network Questions

Dungeon and Dragons Text Based Game

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged python object-oriented role-playing-game adventure-game battle-simulation or ask your own question.

Related

Hot Network Questions