I made a code for a health system for the game I'm creating. I made it myself and I wanted some criticism on how to make it better. Thanks for viewing.
## Health system
import time
SH = 100
PA = True
while PA == (True):
DMG = input("How much damage would you like to take?" )
Output = int(SH) - int(DMG)
if Output == (80):
print("You took 20 damage!")
print(" --------------")
print("|■しかく■しかく|■しかく■しかく|■しかく■しかく|■しかく■しかく| |")
print(" --------------")
elif Output == (60):
print("You took 40 damage!")
print(" --------------")
print("|■しかく■しかく|■しかく■しかく|■しかく■しかく| | |")
print(" --------------")
elif Output == (40):
print("You took 60 damage!")
print(" --------------")
print("|■しかく■しかく|■しかく■しかく| | | |")
print(" --------------")
elif Output == (20):
print("You took 80 damage!")
print(" --------------")
print("|■しかく■しかく| | | | |")
print(" --------------")
elif Output == (0):
print("You took 100 damage!")
print(" --------------")
print("| | | | | |")
print(" --------------")
print("You died!")
else:
print("That isn't a command")
PA = input("Would you like to play again? |Y/N| ")
if PA == ("Y"):
PA = True
elif PA == ("N"):
PA = False
print("The program will now close")
time.sleep(5)
else:
print("That is not a command...")
-
\$\begingroup\$ Why have that time delay before closing? \$\endgroup\$Malady– Malady2018年05月10日 18:40:18 +00:00Commented May 10, 2018 at 18:40
3 Answers 3
I gotta start by saying it's weird to ask how much damage you want to take, my answer would always be zero, you know!
Your variable names could be more descriptive.
I understand that DMG
means damage, SH
would be.. something health?
Well I guess you see my point, if we don't know what the variable name means, it's harder to figure out what it does.
Considering the PEP8 guidelines towards Python, your variable should never contain upper case.
So I would change :
DMG -> damage
SH -> total_health
Output -> remaining_health
PA -> play_again
In your input you ask how much damage should be dealt. There are much more options that are invalid than valid ones. I mean, I can enter 0,20,40,60,80,100
but I can't enter any other number, so there are much more chances I face the "That isn't a command"
error message than anything else.
If the only options are the values from 0 to a 100 that are multiples of twenty, you should specify it in your message :
"How much damage would you like to take? [0,20,40,60,80,100]"
There is no real advantage for the Output
variable at this moment. You want to print how much damage is dealt and an health bar that corresponds to the remaining life.
if damage == 20: print("You took 20 damage!")
Or, even better, use the .format
method.
print("You took {0} damage!".format(str(damage))
Then you could print the health bar. I don't want to go deep in details but you could write a function like print_remaining_health_bar(total_health,remaining_health)
that would print according to the parameters instead of repeating this piece of code every time.
All in all you could remove the whole if/else business (except for your input validation, which is important) and have :
print("You took {0} damage!".format(str(damage))
remaining_health = total_health - damage
print_remaining_health_bar(total_health,remaining_health)
There's also a small problem with your code. When asked if I want to play again, if I enter an invalid command, I'd just restart the game.
-
1\$\begingroup\$ SH actually does appear to be a constant as written so it might be more appropriate to use TOTAL_HEALTH according to PEP8 \$\endgroup\$poompt– poompt2018年05月10日 22:30:19 +00:00Commented May 10, 2018 at 22:30
-
\$\begingroup\$ Just in terms of user experience, most CLI programs asking for input use parentheses for a list of allowed responses, e.g.
Do you wish to continue (y/n)? [y]
;Enter a number (1-99):
;Enter your name: [leave empty]
, where brackets denote the default option. \$\endgroup\$Daniel– Daniel2018年05月11日 14:17:24 +00:00Commented May 11, 2018 at 14:17
I'll echo @TopinFrassi to have better-named variables.
Since you don't mention a limit on the amount of damage you can take. I believe you need to change your prompt to be something like (and then you'll want to check that they did):
damage = input("How much damage would you like to take?(interval of 20)" )
Or change your logic after your current prompt. First, you should try casting the damage right away and surround it with a try...except
in case they enter a string. Then have a single line to print how much damage was done, even if you stick to multiples of 20. I would make a print_health
function that handles the health bar so that you can get even more precise. If your health is less than or equal to 0 you should stop the game.
While asking to play again it should probably be in a loop in case they enter something invalid. And then I'd move the sleep
and print
outside of the loop.
import time
health= 100
play = True
def print_health(hp):
l='■しかく'
s='|'
for i in range(10):
if i<hp/10:
s+=l # show a bar of health if multiple of 10
else:
s+=" " # else fill the missing hp with space
if i%2==1:
s+='|' # print a | every 2 characters
print(s)
while play:
try:
damage= int(input("How much damage would you like to take?" ))
except ValueError:
print("That isn't a command")
continue # starts the loop over
remaining_health = health - damage
print("You took {0} damage!".format(damage))
health=remaining_health
print(" --------------")
print_health(remaining_health)
print(" --------------")
if remaining_health <= 0:
print("You died!")
break # exit the loop
again = 'x'
while again not in 'YN': # you may want to cast againt to upper
again = input("Would you like to play again? |Y/N| ")
if again == "N":
play = False
elif again != 'Y':
print("That is not a command...")
print("The program will now close")
time.sleep(5)
-
\$\begingroup\$
except:
? Seriously? \$\endgroup\$d33tah– d33tah2018年05月10日 21:04:55 +00:00Commented May 10, 2018 at 21:04 -
\$\begingroup\$ @Graipher that is correct, missed that, have since fixed. However, I do not see the issue with except (is it because I'm not specifying the error type?) \$\endgroup\$depperm– depperm2018年05月10日 22:26:03 +00:00Commented May 10, 2018 at 22:26
-
2\$\begingroup\$ You should know what exceptions might be raised. Don't catch all of them. \$\endgroup\$hjpotter92– hjpotter922018年05月10日 22:49:56 +00:00Commented May 10, 2018 at 22:49
-
1\$\begingroup\$ Also you are setting
PA = False
instead ofplay
. \$\endgroup\$hjpotter92– hjpotter922018年05月10日 22:52:29 +00:00Commented May 10, 2018 at 22:52 -
\$\begingroup\$ @depperm Yes, it is. A bare
except
even catches the user trying to press Ctrl+C. Useexcept ValueError
instead. \$\endgroup\$Graipher– Graipher2018年05月11日 06:13:57 +00:00Commented May 11, 2018 at 6:13
The main problem with your code is that there is no way to reuse it or reuse any part of it (apart from PA
and SH
by importing the module you designed, but they obviously can not be of great use).
I suggest you to redesign your code in terms of functions (this is the minimum, otherwise I prefer the object oriented approach even if some may argue against it).
You declared PA = True
. That means it is a constant, but later in your code you write PA = input("Would you like to play again? |Y/N| ")
... that does not make sense.
Explore related questions
See similar questions with these tags.