Hot answers tagged

43

PEP 8 PEP 8 recommends that constants be written as "all capital letters with underscores separating words.". So something like: yellow = (255, 255, 0) Should be: YELLOW = (255, 255, 0) or possibly an enum (more on this later) Inconsistent quotations Usually a project will stick with either "" or '' unless you have particular reason not to.1 But for ...


29

You have docstrings for your functions and classes, which makes your code better than 95% of code submitted to Code Review. The behaviour of the pipes is split into several pieces: (i) the PipePair class; (ii) the motion, drawing, and destruction logic in main; (iii) the scoring logic in main; (iv) the factory function random_pipe_pair. It would make the ...


28

1. Introduction This is not bad overall, considering that this is your first program written with PyGame. I've made many comments below, but don't take the length of this answer to heart: there are always many things to say about a piece of code of this length. 2. Game design issues The game could do with some instructions. I had to look at the source ...


22

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 def collision(): ... return 1 return 0 def wall_collision(): ... return True return False Both ...


15

1. Introduction The code seems pretty good to me overall. I had no difficulty reading it or understanding what it did, and it seems to work well enough. So take all the points below (especially those in section 3) with a pinch of salt. I would have written it differently, but that doesn't mean that every change I have made is an improvement. 2. Major ...


14

I spent some time last night reviewing this code. Here are some specific suggestions I can make: Move your game loop into an if __name__= "__main__": statement at the bottom of the code. Currently, if the module is imported, the person who imports it will immediately start playing chess, which isn't desirable (they may just want to know about the piece ...


13

1. First version I can't run it: >>> import game ImportError: No module named Vec2D There's no Vec2D package available from the Python Package Index. So where does this come from? I guess it must be your own vector package, but if so, you need to post it here. Similar remarks apply to the constants module. I don't know which version of Python to ...


12

Here's my best effort (8 lines): from pygame import * init() screen = display.set_mode((1000,720)) for e in iter(event.wait, event.Event(QUIT)): col = {(1, 0, 0): 'white', (0, 0, 1): 'black'}.get(mouse.get_pressed()) if col and e.type in (MOUSEBUTTONDOWN, MOUSEMOTION): display.update(screen.fill(Color(col), Rect(mouse.get_pos(), (20, 20)))) ...


12

This is pretty nice code! I can still nitpitck a bit :) You could use the cool .. < .. < .. operator for this: in_x_range = bx + BIRD_WIDTH > self.x and bx < self.x + PIPE_WIDTH like this: in_x_range = bx - PIPE_WIDTH < self.x < bx + BIRD_WIDTH Maybe random_pipe_pair will be slightly more readable if you added a few line breaks. Moot ...


12

Extract repeated code into functions. Loading the tile images is the same operation with a different file name. def load_scaled_image(file_name): return pygame.transform.scale(pygame.image.load(file_name), (TILE_SIZE, TILE_SIZE)) VILLAGE = load_scaled_image("village.png") GRASS = load_scaled_image("grass.png") WATER = load_scaled_image("water.png") ...


12

crossImage1 = pygame.image.load('cross.png') crossImage2 = pygame.image.load('cross.png') crossImage3 = pygame.image.load('cross.png') crossImage4 = pygame.image.load('cross.png') crossImage5 = pygame.image.load('cross.png') crossImage6 = pygame.image.load('cross.png') crossImage7 = pygame.image.load('cross.png') crossImage8 = pygame.image.load('cross.png') ...


12

Is this good enough to show this as an example of my...skills for very entry level job in programming? No At least this was my reaction after reading the first few lines of code: you don't just make one giant blob of a mother of god function that does everything; it is just wrong. After reading further on, the code feels better than expected. You seems ...


12

You're right - there is a lot of duplication. Also, some organization is needed. Organize! Before you do anything else, get everything into a function of some kind. All those statements at module scope, move them into a setup function, write yourself a main, and do the standard Python thing: if __name__ == '__main__': main() You can call play_match ...


11

As you asked in your question, I'm going to try and address your performance issues here. Some of this will be a style review though, so bear with me. Style First off, no offense, but this code looks, not so great. You have many major style issues, so here's a list of some of the major ones I see. You have no spaces between operators. You should have ...


10

This is a short answer to your question about line 81. Instead of all the False, you can do this: add_village = random.randint(1, n) == 1; where n is how many false you have. random.randint(1, n) generates a number between 1 and n, and == 1 checks if it is one. It has a 1 in n chance of returning true, just like what you have. This increases the ...


10

Styling Python has its own official style guide (written by the author of Python) called PEP 8. It has some things to say about your code. By the way, you can check your code against PEP 8 by installing the PyPi package or by trying it online. Naming It's great to see that you have constants instead of using those numbers manually. I am also happy to ...


10

size: int ... 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 ...


8

You have several if conditions that cannot be true at the same time: if e.type==MOUSEMOTION: # ... if e.type==KEYUP: # ... if e.type==MOUSEBUTTONUP: # ... Such conditions should be chained with elif in between. Otherwise, even after we already know that e.type == MOUSEMOTION, the other conditions will be evaluated too, needlessly. You do ...


8

One thought ... Instead of having all those else blocks that only have a return statement, switch your if statements around, so that the if block only has a return statement, and then there doesn't need to be an else block for it. E.g., instead of ... if 1<=X1<=8: original_contents_of_if_block else: return -1 write ... if not(1<=X1<=8)...


8

I am not comfortable with pause handling. First, the busy-waiting loop. Second, the paused game just doesn't render anything, but it still serves events, e.g. pipes are added. random_pipe_pair really wants to be a PipePair constructor. Similarly, images['pipe_body'] and images['pipe_end'] should be static members of PipePair.


8

First off, according to this online tool, you have ~119 PEP8 violations. So, for my first tip, I'm going to reccomend reading Python's official style guide, PEP8. The style guide essentially tells you how to format your code, so I'm not going to give you any tips on that in this answer. Secondly, your pool class seems weird, partially because it seems like ...


8

You should always close any file you open. You can guarantee this by using a context manager. with open("some_file_name") as f: x = int(f.readline()) This is a safe (and clean) way to guarantee that a file is closed, no matter what happens inside the with block.


8

Alright, this may be something where you need/desire/end up with multiple reviews - since changing this from it's current (procedural) style to OOP is (very) unlikely to be something someone new to OOP gets right first time. But, there are certain aspects of this code that could be made better even in a procedural style - so I'll start with these... ...


8

Python is not a fast language — it trades execution speed for flexibility and introspectability — and the kind of repetitive numerical computation involved in CPU rendering is pretty much the worst case for Python. It would make much more sense to use a 3D rendering toolkit like PyOpenGL. But so long as we understand that this is just an exercise, there are ...


7

First of all, a lot of people who would answer this question will not do so because you can't just copy/paste the code and run it. The code depends on a lot of external files, like images and textfiles, and to run your code, you basically have to trial'n'error your way while creating a bunch of placeholder images... I'll try to read your code and comment it ...


7

This is neat :) I think that unless you're tracking the rects which have changed, display.update() is no better than display.flip(), though I suppose for a student it would be easier to read. I'm curious as to why collisions are being checked after the view updates? I know that in an event loop situation the actual order of processes can be a little lax, ...


7

Your function initialize_piece could be way simpler if you used a dictionary to map from the piece rank to the file name and used str.format to supply the family into the string: file_names = {"p": "pawn_{}.png", ...} def initialize_piece(): for piece in pie: if piece.life: img = pygame.image.load(file_names[piece.rank]....


7

For efficiency reasons, you should always do x1**2 + y1**2 < r**2 rather than sqrt(x1**2 + y1**2) < r, because sqrt is much much slower than pow. Because You don't need a square root to compare distances. This is the special case of x1**2 + y1**2 < x2**2 + y2**2. Sometimes sqrt distances computing when you have a bunch of things on your screen ...


7

Some of this is nit-pickery, some is more fundamental: Import Order PEP-8 suggests an ordering to imports. No reason not to use it: Imports should be grouped in the following order: Standard library imports. Related third party imports. Local application/library specific imports. You should put a blank line between each group of imports. Code ...


6

Pythonic Basically, a code is pythonic when it uses common Python idioms. In Python, there is usually one (and only one) way to do things, so you should try to know what this thing is, and use it. You won't fight against the language, the code will be easier to read and modify by other programmers. A good way to start writing pythonic code is to follow PEP ...


Only top voted, non community-wiki answers of a minimum length are eligible