Skip to content
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

Turn in MP4 #2

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Turn in MP4 #2

wants to merge 22 commits into from

Conversation

hrrs
Copy link

@hrrs hrrs commented Oct 29, 2017

No description provided.

@@ -0,0 +1,227 @@
import pygame, sys

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide a few lines of comments explaining what this code is about!


# Game over if player runs into their own body
if (p1.pos) in p1.body:
final_screen()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the conditions that yield final_screen() (or gameover) could be groupd into one function. So you just call that one function, for example, `CheckGameOver()'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parens unnecessary, here and below: if p1.pos in p1.body

### Runtime script
running = True
while running:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make more sense to put this main loop component in a separate file named "main.py" because the name Snake.py sounds like that is should only contain what defines the Snake object not the entire game.

self.body = []

### Colors
PURPLE = (128,0,128)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that you are capitalizing the constants. One nice way to organize these values is to create a file class named Config.py and initialize it as an object before running the main loop of the game.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, constants (and other variables that don't depend on the specific instance) can go outside the method definition, in the class or in the module (top level of the file), even if they don't go in a separate constants.py.

Typically, since this is a constant:

DARK_BLUE = (0, 0, 204)

class Player(object):
    …
    def __init__(self, pos, direction):
        …
        self.head_color = DARK_BLUE

If the value is specific to this particular class (not true here), you could also write:

class Player(object):
    DARK_BLUE = (0, 0, 204)
    …
    def __init__(self, pos, direction):
        …
        self.head_color = DARK_BLUE

def final_screen():
""" Function that prints GAME OVER text
"""
global p1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason why these two players needed to be declared as global variables? Could you avoid using global variables?

Copy link

@SeunginLyu SeunginLyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good! I had no problem following this well-documented code. The game runs great and it's fun to play. I've made some comments that might help you if you want to revise this project for MP5. They are mostly on ways to organize the code in a more structured manner. It might be fun to explore the MVC pattern and see how that applies to this game although it might be an overkill :)

'''
Advances the player's snake one move forward.
'''
if len(self.body)>0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Python, also: if self.body

fruit = (x_fruit, y_fruit)
if len(p2.body) in (2,4,7,10):
speed = speed + 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some repetition for p1 and p2. This is fragile – it's easy to modify one occurrence, and not the other. Think about how to eliminate this, for example for p in (p1, p2):.


if p2.pos[1] <= 0 or p2.pos[1] >= screen_size[1]:
final_screen()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a loop or a helper function to combine these four tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants