patternpythonMinor
Beginner Python Dungeon Crawler RPG
Viewed 0 times
beginnerpythonrpgcrawlerdungeon
Problem
I made a game in python which plays a bit like old terminal based RPGs, though a lot more basic. I had originally intended to implement a weapons and armour system and thus remnants of this can still be seen in the latest version of the code.
Please review this, from the documentation to the actual code.
```
from random import randrange
critter_names = ["JOHN CENA", "Shrek", "A Troll", "Maymay", "Ur mum", "A Pink guy", "A Roman bust", "A Rampant AI", "A NSA operative", "A Klu Klux Klan Member",
"An iPhone user","A Mac user", "Someone wearing a snapback", "Someone wearing an Unknown Pleasures T-shirt who hasn't even listened to the album",
"Larry Page","Someone who illegitimately won the NCSS", "A Lad", "An Illuminatus"]
character = {"name": "YUNG LEAN", "armour_name": "Boardies", "armour_rating": 1, "weapon_name": "Meaty fists", "weapon_rating" : 10, "hp": 100}
size = input("How big do you want the map? (recomended between 10 and 20): ")
char_xy = [0,0]
critter_list = {}
door = [randrange(0, int(size)), randrange(0, int(size))]
new_level = True
player_in_range = False
level = 0
def attack(name):
critter_list[name][2] = critter_list[name][2] - character["weapon_rating"]
def defend(name):
character["hp"] = character["hp"] - (critter_list[name][3]/character["armour_rating"])
def critter_gen():
for i in critter_names:
if randrange(1, 6) == 3:
critter_list[i] = [randrange(0, int(size)), randrange(0, int(size)), randrange(1, 100), randrange(1, 25)] #[xpos, ypos, hp, attack]
def player_input():
move = input("It's your move! ")
if move == "w" and char_xy[0] > 0:
char_xy[0] -= 1
elif move == "a" and char_xy[1] > 0:
char_xy[1] -= 1
elif move == "s" and char_xy[0] 0:
gen[critter_list[i][0]][critter_list[i][1]] = "M"
else:
gen[critter_list[i][0]][critter_list[i][1]] = "X"
gen[door[0]][door[1]] = "D"
print('\n'.join(' '.join(row) for ro
Please review this, from the documentation to the actual code.
```
from random import randrange
critter_names = ["JOHN CENA", "Shrek", "A Troll", "Maymay", "Ur mum", "A Pink guy", "A Roman bust", "A Rampant AI", "A NSA operative", "A Klu Klux Klan Member",
"An iPhone user","A Mac user", "Someone wearing a snapback", "Someone wearing an Unknown Pleasures T-shirt who hasn't even listened to the album",
"Larry Page","Someone who illegitimately won the NCSS", "A Lad", "An Illuminatus"]
character = {"name": "YUNG LEAN", "armour_name": "Boardies", "armour_rating": 1, "weapon_name": "Meaty fists", "weapon_rating" : 10, "hp": 100}
size = input("How big do you want the map? (recomended between 10 and 20): ")
char_xy = [0,0]
critter_list = {}
door = [randrange(0, int(size)), randrange(0, int(size))]
new_level = True
player_in_range = False
level = 0
def attack(name):
critter_list[name][2] = critter_list[name][2] - character["weapon_rating"]
def defend(name):
character["hp"] = character["hp"] - (critter_list[name][3]/character["armour_rating"])
def critter_gen():
for i in critter_names:
if randrange(1, 6) == 3:
critter_list[i] = [randrange(0, int(size)), randrange(0, int(size)), randrange(1, 100), randrange(1, 25)] #[xpos, ypos, hp, attack]
def player_input():
move = input("It's your move! ")
if move == "w" and char_xy[0] > 0:
char_xy[0] -= 1
elif move == "a" and char_xy[1] > 0:
char_xy[1] -= 1
elif move == "s" and char_xy[0] 0:
gen[critter_list[i][0]][critter_list[i][1]] = "M"
else:
gen[critter_list[i][0]][critter_list[i][1]] = "X"
gen[door[0]][door[1]] = "D"
print('\n'.join(' '.join(row) for ro
Solution
First of all you seem to have made a working game, and there is a certain game logic. But your overall coding style and data structures needs updating. If you haven't done so already, I would strongly suggest reading PEP8.
Actually your style isn't really bad, but the entire game is crippled by no use of vertical spacing. You have mostly good names, albeit some of them are a little short, but you need to add some vertical space in there. And you need to add a little comments here and there.
I suggest the follwing for adding vertical space:
Look into using named tuples and/or classes, i.e. your main character could be well served using either. That would allow for coding like
Using a named tuple for coordinates could also make some of your logic easier, as you could use loops to check for all the different positions, instead of large amounts of
You also use some anti-patterns, as they are called. I.e.
A similar anti-pattern is setting the
Regarding all those if statements, you could also benefit from combining them using
Lastly, I would look into print formatting which could simplify print statements to:
The manual seems OK, but I would most likely manually word wrap at around 72 characters, to make it read easier most places. But that kind of depends upon where and you are displaying the text.
This is not a complete review, but you should have some pointers to get you started on refactoring your code. You are very welcome to post a new question with your revised edition of your game.
Actually your style isn't really bad, but the entire game is crippled by no use of vertical spacing. You have mostly good names, albeit some of them are a little short, but you need to add some vertical space in there. And you need to add a little comments here and there.
I suggest the follwing for adding vertical space:
- Add two newlines before any function, method or class
- Add newline within functions before logical groups of code, i.e. initialising local variables,
if ... elifblocks,fororwhileloops
Look into using named tuples and/or classes, i.e. your main character could be well served using either. That would allow for coding like
while character.hp > 0 or character.move('e'). Which in turn could use a namedtuple with x and y coordinates. Using a named tuple for coordinates could also make some of your logic easier, as you could use loops to check for all the different positions, instead of large amounts of
if ... elif blocks.You also use some anti-patterns, as they are called. I.e.
if new_level == True: followed by elif new_level == False:. First of all this could be written like if new_level: followed by elif not new_level:, but the second part is redundant, as new_level has only two values. So you could and should use if new_level: followed by else:. A similar anti-pattern is setting the
player_in_range = True in each and every if statement. Set it once to True before all the if statements, and let the else: statement reset it to False. Looks nicer, and is easier to follow.Regarding all those if statements, you could also benefit from combining them using
or statements in between. Or even better to make a position class with a class method so that you could do stuff like: if character.position.in_range_of(critter[i]). That would look a lot nicer...Lastly, I would look into print formatting which could simplify print statements to:
print("W E A P O N: {} {}".format(character["weapon_rating"], character["weapon_name"])). Or if using named tuples or classes: ... .format(character.weapon_rating, character.weapon_name)). The manual seems OK, but I would most likely manually word wrap at around 72 characters, to make it read easier most places. But that kind of depends upon where and you are displaying the text.
This is not a complete review, but you should have some pointers to get you started on refactoring your code. You are very welcome to post a new question with your revised edition of your game.
Context
StackExchange Code Review Q#111369, answer score: 5
Revisions (0)
No revisions yet.