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

Complete interactive programming #3

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

Conversation

vivienyuwenchen
Copy link

No description provided.

@@ -0,0 +1,165 @@
import os, sys

Choose a reason for hiding this comment

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

Please add a few lines of comments explaining what this cool game is about!

Game4.py Outdated
"""Main screen for game."""
# initialize player
count = 0
play_len = 25

Choose a reason for hiding this comment

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

It might be easier to follow the code if all the constants (or config variables) are all in one file. You can make a config.py class and put all the constants there, and load all the config variables at once when initializing the game. Also, it's a convention to use capital letters for constants e.g) PLAY_LEN = 25

Copy link
Contributor

Choose a reason for hiding this comment

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

This convention (and others) come from https://www.python.org/dev/peps/pep-0008/#constants

Game4.py Outdated
obstacles = []
new_obstical = False
# uncomment and place line below in [] of obstacles = [] to generate random obstacles:
# Obstacle(obs_x, obs_y, obs_len, self.screen,'BLUE')

Choose a reason for hiding this comment

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

Could you let user change these settings without interacting with the code? (uncommenting and stuff)

Game4.py Outdated
obs_dt = 500

# main event loop
while 1:
Copy link

@SeunginLyu SeunginLyu Dec 3, 2017

Choose a reason for hiding this comment

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

while 1 does not provide any direct information about when the loop is supposed to end. Using a boolean variable named running = true provides better documentation in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, prefer True or False for a boolean.

Going beyond: you can also use itertools to produce a continuously increasing set of integers, like range(1, 100) except it keeps going forever instead of stopping at 100:

for count in itertools.count(1):

misc.py Outdated
@@ -0,0 +1,128 @@
import os, sys

Choose a reason for hiding this comment

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

I think this file can be named models.py. The current file name misc.py does not really provide helpful documentation.

for event in pygame.event.get():
if event.type == pygame.QUIT:
done = True
#if event.type == pygame.KEYDOWN:

Choose a reason for hiding this comment

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

Please remove comments when submitting the final code

Copy link
Contributor

Choose a reason for hiding this comment

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

Check. “comments” == “commented-out code”, in this context.


jumpover = False

#if False:#if detect_key_press() == 'space':

Choose a reason for hiding this comment

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

please remove comments

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.

Nicely done overall! The documentation is detailed and complete, and the game itself is fun to play. If you think about revising this project for MP5, I strongly suggest you explore MVC pattern and really clean up the code stylistically.

Game4.py Outdated
if event.type == pygame.KEYDOWN:
# z to create obstacle at ground level
if event.key == pygame.K_z:
new_obstical = True
Copy link
Contributor

Choose a reason for hiding this comment

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

sp. new_obstacle

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using constants to document the levels:

GROUND_LEVEL_X_OFFSET = 0
FIRST_LEVEL_X_OFFSET = 25
SECOND_LEVEL_X_OFFSET = 50obstacle_height = obs_y - FIRST_LEVEL_X_OFFSET

The general principle is that you can name things instead of commenting them. The names are more likely to stay in sync with the code that the comments are, and are also evident everywhere the named thing is used.

You could also use e.g.

LEVEL_OFFSETS = {
   'ground': 0,
  'first': 25,
  'second': 50,
}
…
    obstacle_height = obs_y - LEVEL_OFFSETS['first']

or:

LEVEL_OFFSETS = {
  0: 0,
  1: 25,
  2: 50,
}
…
    obstacle_height = obs_y - LEVEL_OFFSETS[1]
LEVEL_OFFSETS = [0, 25, 50]
…
    obstacle_height = obs_y - LEVEL_OFFSETS[1]

This makes it easier to add levels. It also allows for downstream simplifications, for example a map from event.key to level name or index.

Game4.py Outdated
pressed = pygame.key.get_pressed()
# up to jump
if pressed[pygame.K_UP] and P1_stamina_bar.bars >= player.jumpcost:
if player.play_y == (360 - player.play_len):
Copy link
Contributor

Choose a reason for hiding this comment

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

360 is a “magic number”. Replace it by a constant, or an attribute that is initialized next to wherever this value is coming from.

Game4.py Outdated
def gameOver(self,score):
"""Game over screen."""
# main event loop
while 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, while True is clearer.

Game4.py Outdated
while 1:
for event in pygame.event.get():
if event.type == pygame.QUIT:
sys.exit()
Copy link
Contributor

Choose a reason for hiding this comment

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

Going beyond: you can also use either of the following. Experiment with these styles, and see if they're to your liking.

if any(event.type == pygame.QUIT for event in pygame.event.get()):
    sys.exit()
if pygame.QUIT  in {event.type event in pygame.event.get()}:
    sys.exit()

({…} makes a set; same as set(…).)

Game4.py Outdated
# display game over screen
self.screen.fill(colors['BLACK'])
font = pygame.font.SysFont("comicsansms", 28)
text = font.render("Player 1 recived a score of " + str(score), True, (255,255,255))
Copy link
Contributor

Choose a reason for hiding this comment

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

(255,255,255) is a good candidate for a constant, especially as it's used more than once.

Think fragility and maintainability: it's easy to type (255,25,255) and not notice; and WHITE leaps out to the reader in a way that (255,255,255) does not.

misc.py Outdated
"""A square obstacle, defined by its top left hand coordinate, length, and color.
Also takes in screen as an argument to draw the obstacle."""
def __init__(self, obs_x, obs_y, obs_len, screen, color):
"""Initialize the instance."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment isn't necessary.

misc.py Outdated
return 'Obstacle({}, {}, {}, {})'.format(self.obs_x, self.obs_y, self.obs_len, self.screen)

def moveForward(self):
"""Update horizontal location and draw obstacle."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Python convention is move_forward, is_gone.

move_forward_and_draw is a clearer description of this methods behavior. Separate move_forward and draw might be more naturally units, though.

misc.py Outdated
play_y0 = self.play_y
play_y1 = self.play_y + self.play_len
# check if player coordinates within obstacle coordinates
if (play_x0 in range(obs_x0, obs_x1) or play_x1 in range(obs_x0, obs_x1)) and (int(play_y0) in range(obs_y0, obs_y1) or int(play_y1) in range(obs_y0, obs_y1)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shortcut: if (obs_x0 <= play_x0 < obs_x1 or play_x1 <= play_x0 < obs_x1) …

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

4 participants