PEP 8 recommends that constants be written as "all capital letters with underscores separating words.". So something like:
yellow = (255, 255, 0)
YELLOW = (255, 255, 0)
or possibly an enum (more on this later)
Usually a project will stick with either "" or '' unless you have particular reason not to.1 But for ...
In addition to another answer I will add these places to improve (I will not repeat all things from that answer, my answer is just an addition):
1. Code inconsistency
You have several places in your code where your code is inconsistent
self.size = 2
You never read size, and Python Lists already know their own length in case you did need it.
You're just duplicating functionality that List already gives you by manually keeping size in sync with the list length.
Separate game logic from screen rendering details
You keep snake coordinates scaled by pixels. This seems to ...
self.color = WHITE
self.height = 40
self.width = 40
self.x = 5
self.y = 5
def Draw(self, screen):
pygame.draw.ellipse(screen, self.color, [self.x, self.y, 40, 40])
According to PEP-8, methods should use snake_case. MixedCase is reserved for class names. So ...
I wish I could make a more comprehensive suggestion regarding the gigantic mass under while not done:. The sheer amount of duplication there is unsustainable. I'm pretty tired though, so I'll have to stick to simpler issues.
In that chunk, you do something along these lines, many times:
if turn == 0:
spaces_list[#].color = RED
turn = 1
elif turn ...
Follow PEP-0008 guidelines. Use an automated checker (pylint, pyflakes, ...) to check your code for violations.
Variable, function, and method names should be snake_case. For example newBG should be new_background, mainLoop should be main_loop, and drawButton should be draw_button.
No space between a function/method name and the ...
The comment here is not useful, if you have a variable level1, well... that's level1, right?
The same goes for all your other comments in the code. You should think of comments as a way to describe why you did things that specific way or the meaning of something. If you need them to describe what is happening, most likely you need to ...
You have provided a lot of code, and it's not super well organized. So I'm going to focus on that, plus some things I noticed in passing. There's probably enough for another review once you incorporate what you learn from this one.
First, you have some good ideas. Moving "basic" things into a library is a good idea. Using classes is a good idea. Breaking ...
For the physics:
Given that you don't have bodies that are constantly interacting, updating position and velocity based only on the previous step is fine. Be wary of doing this with lots of interacting objects if you decide to add more balls. Because you can't reasonably update simultaneously, you get issues with divergence. Again though, for this game it ...
Good going on your first game. I completely agree with most of the points made by others. Here are a few more nitpicks. This answer is a bit long-winded, but take it as a complement. :-)
Unpack the Coordinates
This is what lines 195-197 currently look like:
for item in snake.get_body():
pygame.draw.rect(window, snake.color, [item, ...
(I'm on my phone on a road trip, so I can't do anything fancy here)
Your issues with roster are because you have it as a attribute of the class, not an instance attribute. That means that every instance shares the same roster. You need to define it in the __init__ (or elsewhere, but ideally in the initializer) as self.roster, just like you did with the ...
Welcome to CodeReview!
Get an IDE such as PyCharm or VSCode that's capable of linting. It will give you several "PEP8" suggestions, such as newlines around functions, that will make your code more legible. Some particulars that will be suggested:
paddle = Paddle (classes)
black = BLACK (global constants)
Ball = ball and vice versa - instance ...
So many good things, so many weird things.
First of all, I love your comments. They made this review very easy.
With that said, I have a couple major complaints:
dict is not very type-safe; it requires you to access common properties by strings. I highly suggest storing those in a class instead, maybe something like AnimalProperties? That would be the ...
First of all, from an Object-Oriented perspective, you have objects that represent real objects within the game with behaviors and attributes, so on that note, well done! There are, however, other areas that need a bit of TLC.
You have an enumeration of colors sitting there as individually-defined constants. For a bit more type-safety, I would suggest ...
The code looks good, but the
for i, array in enumerate(self.tiles):
for j, _ in enumerate(array):
tile = self.tiles[j][i]
stepstaken += self.move_single_tile(tile,-1,0)
if key =="RIGTH":
for i,array in ...
You've followed some conventions very well like CAPS for constants. Very nice. Here are some points:
Standard library imports precede 3rd party ones
should be changed to
The middle line is to differentiate between the two types of imports
Two lines after imports
There should be two lines after ...
The positions of the bodies are modified in-place, while the loop is running. It means that in your universe the action is not equal to reaction. Your simulation will exhibit some nonrealistic behaviors, like drift of the center of the mass.
Euler integration method is not the most accurate. Consider Runge-Kutta. At least, monitor the motion invariants (...
Thanks for the game. I applied most changes suggested by @Dair and @vurmux as well as partially the suggestions from @Peter Cordes (some of them are not so simple). After that, there were still some things left:
I'm not sure whether it was intentional, but the snake does not collide with itself. In usual snake games, if the snake bites into her ...
This answer pertains to using while run vs while True.
Lets say you have this code:
run = True
count = 0
if count == 5:
run = False
count += 1
Simple enough code. When viewing it as this, a programmer might see that the code will stop when count reaches 5. Lets look at the output:
Few areas to improve:
self.vel. When I first saw this field/variable I thought: "What is that?".I guess many other people may have the same first feeling.I would rename it to an explicit and clear name: self.velocity
self.isJump and self.jumpCount violate Python naming conventions (instance variable names should be all lower case, words in an instance ...
In addition to what @Carcigenicate said:
Spell out color instead of col - I thought it was short for column.
pygame has a Rect class. Use it instead of separate x,y,width,height attributes. The Rect class already implements is_over-functionality. If desired, some common code can be pulled out into a parent/base class.
Make text bliting into a standalone ...
# Create a screen
win = pygame.display.set_mode((800,700))
win_info = pygame.display.Info()
win_w, win_h = win_info.current_w, win_info.current_h
should be moved into one or more functions. These:
pad_colour = (255,255,255) #White colour
pad_distance = 50
speed = 10
running = True
Here are a few things that stand out to me:
You import time but don't use it in your code. It should be removed
You should adhere to the PEP 8 style guide. This guide may seem a little superfluous, but having a consistent style makes your code easier to work with should you modify it and having your code follow the same style guide as ...
I'll focus mostly on the performance issue and only hint at a few design ones at the end.
For this use-case, I'd say stay away from numpy. numpy has some pretty high overhead, which you tend to amortize by faster computations at a "massive" scale. Which is not the case here.
You use numpy for vector addition and for trigonometric functions. In the latter ...
I think your button class should do more. It should know if a point is within it's boundaries or not and what its image(s) is (are). It should probably also have an update method that can optionally be called with the mouse position. I would also rename it to singular Button, since each instance of the class is a single button and not a collection of buttons....
Below is the code I have come up with. It basically follows the code used for the web demo in the github repo you had linked to. I will add some explanation to this answer on how I had made some basic optimizations to better fit cython's memoryview model as well as potential room for improvement later, but figured I should post the code now as it might be ...
I'll comment on two specific things I noticed while briefly reviewing (I hope I don't come across as harsh where I'm just stating observations):
OOP: One thing that stood out to me immediately is the dictionaries. The beauty of OOP is that it allows object hierarchies, and for those objects to inherit properties from their base class. A good rule of thumb: ...
You don't necessarily need classes in Python. The helper class approach is the same direction as I would go, just with a plain module.
Basically, your resource_path and img_folder_path are constants, so I would simply create a module constants.py with the following:
RESOURCE_PATH = os.path.join(os.path.realpath(__file__), 'Resources')