takluyver / weatbag

Written by Everyone Altogether, The Big Adventure Game
MIT License
21 stars 11 forks source link

Bug with the action.get_action function #12

Closed Uglemat closed 11 years ago

Uglemat commented 11 years ago
def get_action():
    """Prompts for an action, splits it into words, and removes any prepositions.

    movement actions will be represented by the move token object in this module,
    followed by a one-letter direction.
    """
    action = []
    while len(action) == 0:
        action = input('> ').lower().split()
    for prep in words.prepositions.intersection(action):
        action.remove(prep)

    return action

When the input consists of predispositions only, like 'in', or 'down and under on', it will make the list empty and it will fail in later parts of the program if the list is used like this: action[0]. For a reference, the center tile will fail. It's also worth to note that because words.predispositions is a set and how set.intersection works, if I put into the input "'in in in" it will only remove the first "in", action will then be ["in","in"].

I don't know a good way to solve this.

takluyver commented 11 years ago

The first bit is easy to solve - we can just move the bit removing prepositions inside the while len(action) == 0: block.

The second bit can be solved in various ways, but I've not yet come up with a really clean one.

takluyver commented 11 years ago

Actually, how about:

actions = [w for w in actions if w not in prepositions]
Uglemat commented 11 years ago

For some reason I mistake preposition for predisposition..

Anyway yeah, that'll result in this

def get_action():
    """Prompts for an action, splits it into words, and removes any prepositions.

    movement actions will be represented by the move token object in this module,
    followed by a one-letter direction.
    """
    action = []
    while len(action) == 0:
        action = input('> ').lower().split()
        action = [w for w in action if w not in words.prepositions]
    return action

It might be considered a little intrusive because certain tiles might want to use prepositions, but it did that from beforehand and now it's probably simpler for tile creators to predict the outcome of input, because before it was like "sometimes, prepositions works", now it's like "prepositions never work" which is simpler.

takluyver commented 11 years ago

Yep, that sounds sensible. Do you want to make a pull request?

I have an idea for a way to tiles to request their actions 'raw', but I'm leaving it until someone decides that they need it.

Uglemat commented 11 years ago

Ok, I'll make a pull request. :)

Uglemat commented 11 years ago

I don't know how to make the commit attached to this issue, I guess you can just close this issue. I'm not very experienced with github.

takluyver commented 11 years ago

It's OK, this is partly a way to practice collaborating on code.

For reference, if you put closes gh-12 into the commit message, it will automatically close that bug number when it gets merged. If we forget, then we just close it manually, like I'm about to do with this.

Another top tip is to use named branches - before you start committing your changes, run git checkout -b fix-get-action (the last bit is the branch name). That means that you can use the master branch to track the changes in the main project codebase.

Thanks!

Uglemat commented 11 years ago

Cool, thanks.