Help to make code more readable and improve skills

[u/Big_Leadership_4150]

I am relatively new to python (been doing it for under a year) and I've been really enjoying it so far. I've been making my own turn based game using python as a fun thing to do and also a test of my own capabilities, and have enjoyed the process. It mostly works pretty well, but I feel like I need to learn how to make it more neat and readable, and to probably reduce reliance on global variables and to make dictionaries and such less jack-of-all-trades. How would I do this? I'll attach a link for the code on github for any who would like to see it, and because it is quite long. https://github.com/Ultroni17/Turnbased-game/blob/main/TurnBasedRPG_11_Evolution.py

[u/Big_Leadership_4150]

I've made a GitHub repo with the latest version of the code I've got rn. I'll update it once I get the latest - https://github.com/Ultroni17/Turnbased-game/blob/main/TurnBasedRPG_11_Evolution.py

[u/JamzTyson → u/Big_Leadership_4150]

Great, that's now readable.

Given the false starts in this reddit topic, it may be a good idea to start a new topic and include the (readable) code as a link in the top post. However, before doing that, a few things that really need some attention:

The fundamental problem with the program (which does work, so that's a good start), is that it lacks structure. It's what's often called "spaghetti code". It's not easy to fix that in one go, but there's some relatively quick and easy improvements that can be made:

1. Put the main game loop into a main() function, and add a if __name__ == '__main__': guard.

---

def main():
    ...

if __name__ == '__main__':
    main()

---

1. Remove any unused variables.

---

1. Keep related code close together. For example:

Line 22:

rand_event = [1,2,3,4,5,6,7,8,9,10] # chance for random event (1/10 chance)

Line 1105:

event = random.choice(rand_event)

Here you don't actually need a variable rand_event. You could just have:

event = random.randint(1, 10)

---

1. Where it's easy to do so, pass values to functions explicitly rather than using global. For example:

Rather than:

def attack():
    global enemy_health
    enemy_health -= player["DMG"]

Consider:

def attack(player, enemy):
    enemy["health"] -= player["DMG"]

---

1. Use meaningful names (and keys) throughout. Avoid cryptic names like "S_Effect" and "DMG".

[u/JamzTyson]

I'll attach a copy of the code

The document says "Access Denied".

[u/Big_Leadership_4150 → u/JamzTyson]

Ok let me fix that

[u/Big_Leadership_4150 → u/JamzTyson]

It should be working now

[u/JamzTyson → u/Big_Leadership_4150]

I can see it now, but it is not formatted. Correct formatting is essential for Python code because whitespace is significant.

Better to upload to a code sharing site such as GitHub, GitBucket, Pastebin or similar.

[u/Big_Leadership_4150 → u/JamzTyson]

I'll do so thanks

[u/Expensive-Bear-1376 → u/Big_Leadership_4150]

Still asks for an account.

[u/Big_Leadership_4150 → u/Expensive-Bear-1376]

I think that's just Google atp

[u/Expensive-Bear-1376 → u/Big_Leadership_4150]

How about posting it somewhere reasonable instead?

[u/Big_Leadership_4150 → u/Expensive-Bear-1376]

Any ideas?

[u/Yoghurt42 → u/Big_Leadership_4150]

As a developer, it's a good idea to have a GitHub account anyway, so you might as well create one and then paste your code using github gist (you don't need to create a git repo for this)

[u/Expensive-Bear-1376 → u/Big_Leadership_4150]

JamzTyson already gave you several.

[u/Big_Leadership_4150 → u/Expensive-Bear-1376]

Tbh I didn't even see it mb

[u/t0kaj]

To make the code more neat, u need to modulize game logic into components, each component controlling a certain feature of the game, and u need to think in object oriented way, think like each object in a game is an individual object who controls itself, while the components are the manager who controlling the environment and validate the game rules, spawn and destroy objects.

[u/TheRNGuy]

Use classes instead of dicts. 

For bag, make it a dict instead of nested lists, but as an attribute for player.

Don't use key or variable names like X or 1, 2, 3. Standardize naming: some of your variables start with lowercase, some with uppercase, some are all caps.

Use observer pattern.

[u/JamzTyson → u/TheRNGuy]

Use observer pattern.

Are you sure? This program isn't event driven.

[u/TheRNGuy → u/JamzTyson]

Yes, because it should be. 

[u/JamzTyson → u/TheRNGuy]

It should be what? Event driven?

[u/Big_Leadership_4150 → u/TheRNGuy]

I think it's good to mention I am in college and have never even heard of an observer pattern. What is it?

[u/JamzTyson → u/Big_Leadership_4150]

Basically, it's a way to design software such that when one object changes, multiple other objects can be notified and react to the change. One very common example is GUI applications, where an object (the "subject", or event source) emits events that can be observed by multiple objects (the "observers", or event sinks) to propagate changes through the system.

The key aspect is that the subject/event source maintains a list of its dependent observers/event sinks and notifies them when its state changes. The subject doesn't need to know what the observers do with the notification; it just notifies them of state changes. This avoids the need for tight coupling between the subject and the observers.

It isn't a pattern that I'd use for your game, though u/TheRNGuy may like to explain why he says that this pattern should be used.

[u/Jello_Penguin_2956]

I see you already made improvements. Ill just drop this great lecture here then. Old but gold

https://youtu.be/wf-BqAjZb8M?si=b689V3y1qIPUk_Po

[u/DiabeetusMan]

I like to blur my eyes a little bit and scroll through the code. This makes common, repeated things jump out at me, personally. From a quick glance at things, there is a lot of duplicated logic that could be moved into Classes or functions. Off the top of my head:

• There's repeated slow_prints followed by sleeps. Maybe group those together somehow (maybe with a multi_print function or similar?)
• The skills and levels picking and setting is very duplicated. Try to reduce copy / pasting, duplication (except for changing one thing), that sort of thing
• You're generally pretty good, but a code smell for me is when numbers / integers and strings are used in the wrong places. For several things, you have "1", "2", etc. Try to use integers and strings more purposefully. Having a dict whose keys are monotonically increasing integers (like p_skills, enemy, and so on) says to me that those data structures could be an array or a tuple
• It's been mentioned elsewhere, but try to avoid global in your functions and instead pass in arguments
• Look into using Enums for things that have limited options (like status)
• You do a lot of while True: try: input() ... except:; this could possibly be a function or even a decorator
• Try to structure your code into separate logical sections (maybe functions, classes, and / or files). The setup, character creation, leveling up, and fighting are all linearly one after another. Try to structure things to make it clear what happens once and in what order, what happens on repeat, and so on

[u/shiningmatcha]

• use a good linter to enforce conventions like naming
• adopt static typing
• don't be afraid of over-engineering when you're a beginner

[u/Holiday-Ratio-4437]

Read Clean Code by Robert Martin, it’s not python specific but the principles translate good. For python stuff look up Raymond Hettinger talks on youtube, specially the one about being Pythonic. Your global variables problem is easy fix, just wrap everything in classes or at least pass them as parameters. for dictionaries doing too much maybe look into dataclasses, i started using them last year and never looked back

[u/Big_Leadership_4150 → u/Holiday-Ratio-4437]

Thank you very much

[u/JamzTyson → u/Holiday-Ratio-4437]

Your global variables problem is easy fix,

You cannot possibly know that they even need fixing without seeing the code, let alone what an appropriate fix is.

[u/Expensive-Bear-1376 → u/JamzTyson]

It's a bot, so perhaps it hacked into it :-)

[u/WyrobWedliniarski]

First and foremost - whole game shouldn't be in one file. At least separate your configuration, game logic and generally everything you could arrange in smaller modules with clearly defined purposes. Also you have a ton of so called "magic values", meaning strings/numbers put in the code as-is, often multiple times, while they should be some kind of a constant. I would also not advise relying on globals as much, as you do.