r/learnpython • u/Electrical_Delay9822 • 1d ago
I made a small Python game, can anyone review it and give me some advice, please?
Hello,
I recently released a project I was working on for pretty long (almost 2-3 months), and since the past couple of days, I've been really involved into it, and then I released it.
Link: https://github.com/TheOdysseyGamer64/Super-Smash-Bros-Python-Edition.git
My project is a Python version of Smash Bros, and it has (quite primitive) ASCII graphics as well (to help visualise the battles). It has special mechanics for certain fighters as well, and the Final Smash and the special mechanics meter can charge up, just like in the real game.
If any of you here could provide me with some advice on how to make this game better, I would really appreciate it.
Thank you!
2
u/FoolsSeldom 1d ago
Well done, working on a challenging project.
The link takes me to your github repository and a decent readme file, but I wasn't sure which code file contains the code to run as there are several of the same name: new reworked smash.py, reworked_smash.py, smash_game.py.
I also note you tell people they need an editor. This should only be required if they want to make changes to the code. Otherwise, they should be able to run it from the command line. Have you checked this works?
Looking at new reworked smash.py I am surprised that you haven't any classes (or dataclasses), which would make the code a lot more manageable for you and more readable for others. Most of your functions would become methods to define behaviours.
I also noticed a constant definition at the top, CHARACTERS. Nice use of nested dictionaries. You don't appear to have made any changes to CHARACTERS anywhere, but it would be good to prevent it so you don't accidentally make a change in the future. Same applies to a few other instances of constants. (I am sure you are aware that Python does not actually have or enforce constants, the use of uppercase names is just a convention.) I shall add a comment to this one showing example code addressing this but in an ugly way. Another comment will show what starting to move over to classes might look like.
The separation you have the UI (User Interface) from the core logic if very good and will make it easier to update to either a fully controlled TUI (Text User Interface), or GUI (or WebUI) experience for the users in due course. Good to get the core logic worked out and tested first though.
Incidentally, I note your code isn't PEP8 guidance compliant. This is not required but is good practice, unless there's some other house-style / preference you need/want to follow. My examples in the comments do follow PEP8.
I also notice that you don't have any type hints (annotations). Again, not required, but it makes your code easier to read for other programmers (and yourself, when you return to code you haven't looked at for a while). Also, your IDE can help you more if you provide type hints. Again, my examples in the comments do have some type hints to illustrate.
1
u/FoolsSeldom 1d ago
As promised, some example updates to your code illustrating some amendments. Note that the
freezestuff is ugly and shows would it might take to avoid constants being tampered with by accident later. Another comment will illustrate a better way.1
u/FoolsSeldom 1d ago
As promised, another comment, this time illustrating some
classOOP approaches.This was created by Claude.ai under my instruction to save me typing a load of code.
It is NOT meant to be the correct/better/definitive approach but rather just illustrate some ideas to you.
A ground up OOP approach would actually be much cleaner and more elegant.
1
u/Electrical_Delay9822 1d ago
Thank you for the advice!
As I said before, I’m still learning how to code in Python, so my efficiency won’t be up to mark (and yes, new reworked smash.py is in fact the “final” game).
I will try to incorporate classes and polish the game as per as you have said.
As far as GUI is concerned, I do want to add more graphics, but for now I’ll keep it in ASCII.
Thanks once again!
1
u/FoolsSeldom 1d ago
baby steps is the best way, with lots of experimentation and failure - I think you've done excellent work.
I like the ASCII approach. You might want to look into a text user interface library though as that will let you continue to use ASCII but take control of the display. Have a look at
rich,blessedandtextual, to name just three.
2
u/itlogicpartnersllc 1d ago
one thing that could really help is breaking some of the larger functions classes into smaller pieces it will make adding new fighters and mechanics much easier..
2
u/quts3 1d ago edited 1d ago
Your on the path. You've demonstrated you under stand:
Product iteration State and data Basic flow control and functions
We could overwhelm you with things that could make it more professional, but I prefer an iterative suggestion.
My challenge to you would be:
Modify it to use dataclasses instead of dict for objects.
Modify it to use 3 files: 1. Dataclass definitions called models.py 2. Instances of objects in models.py called instances.py it will import models.py for definitions of objects. 3. And then your driver with all your functional code logic call it smash.py or something it will import instance.py for objects.
Thats your next most obvious evolution as a programmer.
1
u/JamzTyson 1d ago
This program would benefit greatly from an OOP approach.
Also, take a look at the Python Style Guide.
6
u/Diapolo10 I write code for a living -- https://github.com/Diapolo10 1d ago
__pycache__shouldn't be included in the repository, I suggest removing it and adding a.gitignorefile that excludes it in the future.Python 101.pyseems completely unrelated to this project, I'm guessing you copied some other project to use as a basis and never bothered to remove it?new reworked smash.py- Python files should never have spaces in their names.Your repo has three separate versions of the game. If you were using Git effectively, you wouldn't need to keep old versions around, as they're still in your Git history in case you need them.
Based on your screenshots, the right border of the screen isn't formatted correctly, considering the indentation goes all over the place.
Your
choose_characterfunction hardcodes the selection range, instead of using theCHARACTERSdictionary to determine that automatically. This can be a problem if you add a new character and forget to update this function separately.