-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A bunch of not-really solicited advice #1
Open
Mewp
wants to merge
4
commits into
HubertKwiatkowski:master
Choose a base branch
from
Mewp:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Every frame, the code first moved the snake, then registered input, then updated the screen. And for some reason, updating the screen waits until it's time to draw the next frame. An unfortunate consequence of this is that the game feels laggy – you have to wait until next frame to get your input processed, which takes 100ms (because the timer is set to 10fps) – more than enough for the player to notice. There's a simple fix, though – process input at the start of every frame, so that the snake responds immediately. Also, a small suggestion: run the clock in the main loop, and not in some function – it'll be easier to see that the whole game runs at 10fps, not just the screen update. Or extract the waiting to a separate function – regardless, it's not really related to updating the screen.
Up to now, this game was just a bunch of files. This is fine for very small apps, but quickly becomes problematic, if not for other reasons, because it makes the app uninstallable. A python module is basically a directory with an __init__.py file. It allows you to use relative imports, i.e. from .settings import Settings Thanks to this, if there's ever a module named "settings" outside of your application, your file won't shadow it. If you want to see what I mean by yourself, create a `pygame.py` before applying these changes and see what happens. Proper structure is important, and using modules is a part of it. Now, in order for python to recognize it as a module, you have to execute it as such. There are multiple ways of doing that, but the simplest one is: python3 -m snake_clone This executes the __main__.py file, so I've had to move the startup code to it. Oh, and it has to be in a subdirectory because of the way python's setuptools work. But more on that later.
When making a git repository, it's always a good idea to add a .gitignore file to ignore all the files you didn't write, and were created by something automatically. I generate these files using gitignore.io, I don't even really know what's inside, because I didn't look.
Now, for another method of running a module – installing it. pip install --user -e . Running the above will install the game in a way that it will be always up to date with your code. The `setup.py` file is something that one usually copies from the internet or another project, an changes the intereting bits. I have included a really minimal example here, there's a *lot* of fields you'd want to fill if you were publishing it. There's a convention for specyfing libraries your project depends on in python: the `requirements.txt` file. It can be used to just install the dependencies using: pip install -r requirements.txt But the `setup.py` I've included also reads it. It makes others' lives easier if you specify your dependencies like that. And yours, if you don't want to remember and manually install whatever you depend on. Finally, on the matter of entry points – it's a somewhat complex, and rarely useful concept in general, but there's one often used aspect: console_scripts. This is what allows you to create an executable to run for your package. The way it works is rather simple, you tell it the name of the script, and the function to call. In this case the name is `snake-clone`, and the function is `run` in the `snake_clone.__main__` module (I forgot to mention – files are also python modules). Once installed, you should be able to just run `snake-clone` from your commandline. If not, runnign this, then restarting the console will probably help: echo 'export PATH=$HOME/.local/bin:$PATH` >> .bashrc This is not related to python at all, it just configures your system to recognize packages installed for your user (instead of globally).
Thanks for the feedback, I'll try to head in that direction. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some changes to make your code a more proper python package. Read the full commit messages, they explain the changes extensively.
Important thing: I didn't read most of the code. Don't interpret these as "there's nothing more wrong with it". I just wanted to fix the project's structure for you, so I didn't conduct a thorough review.