Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

In my pygame game, I build a level by adding a bunch platform objects to an array called instancelist. Then pygame tests for each instance in instancelist and blits it's sprite over it's rect position.

@kobejohh suggested that I turn it into a dict so that python wouldn't have to test the player's coordinates against every instance in the list, but I have no idea how to do this.

Here is the code:

import pygame,random
from collections import namedtuple
from pygame.locals import *
pygame.init()
pygame.display.set_caption('Legend of Zelda | By Sam Tubb')
screen=pygame.display.set_mode((640,480))
instancelist=[]
players=[]
enemys=[]
clock=pygame.time.Clock()
Move = namedtuple('Move', ['up', 'left', 'right'])

#load sprites
block1=pygame.image.load('block1.png').convert()

#load/init player stuffs
playersprites=[pygame.image.load('link1.png').convert(),pygame.image.load('link2.png').convert(),
pygame.image.load('link3.png').convert(),pygame.image.load('linkatk.png').convert(),pygame.image.load('linkup.png').convert()]
for s in playersprites:
    s.set_colorkey((255,0,0))
frame=0
frameplus=1
frametime=0
psprite=playersprites[frame]
max_gravity = 75
left=0
atk=0
atktime=0
grounded=0

class Enemy(object):
    def __init__(self,x,y):
        self.x=x
        self.y=y
        self.sprite=playersprites[0]
        self.rect=self.sprite.get_rect(left=x,top=y)
class Player(object):
    sprite=psprite
    def __init__(self, x, y):
        self.rect = self.sprite.get_rect(centery=y, centerx=x)
        # indicates that we are standing on the ground
        # and thus are "allowed" to jump
        self.on_ground = True
        self.xvel = 0
        self.yvel = 0
        self.jump_speed = 7
        self.move_speed = 3

    def update(self, move, blocks):

        # check if we can jump 
        if move.up and self.on_ground:
            self.yvel -= self.jump_speed

        # simple left/right movement
        if move.left:
                self.xvel = -self.move_speed
        if move.right:
                self.xvel = self.move_speed

        # if in the air, fall down
        if not self.on_ground:
            self.yvel += 0.3
            # but not too fast
            if self.yvel > max_gravity: self.yvel = max_gravity

        # if no left/right movement, x speed is 0, of course
        if not (move.left or move.right):
            self.xvel = 0

        # move horizontal, and check for horizontal collisions
        self.rect.left += self.xvel
        self.collide(self.xvel, 0, blocks)

        # move vertically, and check for vertical collisions
        self.rect.top += self.yvel
        self.on_ground = False;
        self.collide(0, self.yvel, blocks)

    def collide(self, xvel, yvel, blocks):
        # all blocks that we collide with
        for block in [blocks[i] for i in self.rect.collidelistall(blocks)]:

            # if xvel is > 0, we know our right side bumped 
            # into the left side of a block etc.
            if xvel > 0:
                    self.rect.right = block.rect.left
            if xvel < 0:
                    self.rect.left = block.rect.right

            # if yvel > 0, we are falling, so if a collision happpens 
            # we know we hit the ground (remember, we seperated checking for
            # horizontal and vertical collision, so if yvel != 0, xvel is 0)
            if yvel > 0:
                self.rect.bottom = block.rect.top+0.4
                self.on_ground = True
                self.yvel = 0
            # if yvel < 0 and a collision occurs, we bumped our head
            # on a block above us
            if yvel < 0: self.rect.top = block.rect.bottom

class Block(object):
    def __init__(self,x,y,sprite):
        self.x=x
        self.y=y
        self.sprite=sprite
        self.rect=self.sprite.get_rect(x=self.x,y=self.y)

x = y = 0
level = [
"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB",
"B                                       B",
"B                                       B",
"B               L                       B",
"B                                       B",
"B                                       B",
"B                                       B",
"B                       E               B",
"B                                       B",
"B         BBBBBBBBBBBBBBB               B",
"B        B                              B",
"B     E B                               B",
"B      B                                B",
"B     B                                 B",
"B    B                                  B",
"B   B                                   B",
"B                   E                   B",
"B                                       B",
"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB",
]
# build the level
for row in level:
    for col in row:
        if col == "B":
            p = Block(x,y,block1)
            instancelist.append(p)
        if col=="L":
             players.append(Player(x, y))
        if col=="E":
            enemys.append(Enemy(x,y))
        x += 16
    y += 16
    x = 0

#mainloop
while True:
    grounded = 0
    for p in players:
        for b in instancelist:
            if b.rect.collidepoint(p.rect.centerx,p.rect.bottom):
                grounded=1
                break
    print grounded
    #total_level_width  = len(level[0])*16
    #total_level_height = len(level)*16
    screen.fill((0,0,0))
    mse=pygame.mouse.get_pos()
    key=pygame.key.get_pressed()
    if atk==0:
        if not key[K_w]:
            if key[K_a]:
                left=1
                if frameplus==1:
                    if frame<2:
                        frameplus=0
                        frametime=5
                        frame+=1
                    else:
                        frame=0
                psprite=pygame.transform.flip(playersprites[frame],1,0)
            if key[K_d]:
                left=0
                if frameplus==1:
                    if frame<2:
                        frameplus=0
                        frametime=5
                        frame+=1
                    else:
                        frame=0
                psprite=playersprites[frame]
            if not key[K_a]:
                if not key[K_d]:
                    if left==0:
                        psprite=playersprites[0]
                    else:
                        psprite=pygame.transform.flip(playersprites[0],1,0)
        else:
            if left==0:
                psprite=playersprites[4]
            else:
                psprite=pygame.transform.flip(playersprites[4],1,0)
    if key[K_SPACE]:
        if atk==0:
            atk=1
            atktime=15
            if left==0:
                psprite=playersprites[3]
                p.rect.right+=4
            else:
                psprite=pygame.transform.flip(playersprites[3],1,0)
                p.rect.right-=16
    for e in pygame.event.get():
        if e.type==QUIT:
            exit(1)
    for inst in instancelist:
        screen.blit(inst.sprite,inst.rect)
    for p in players:
        if atk==0:
            p.rect=psprite.get_rect(x=p.rect.left,y=p.rect.top)
            move = Move(key[K_w], key[K_a], key[K_d])
            p.update(move, instancelist)
        #scrolling
        if p.rect.right>=500:
            for inst in instancelist:
                inst.rect.left-=4
            for e in enemys:
                e.rect.left-=4
            p.rect.left-=4
        if p.rect.left<=140:
            for inst in instancelist:
                inst.rect.left+=4
            for e in enemys:
                e.rect.left+=4
            p.rect.left+=4
        if p.rect.top<=80:
            for inst in instancelist:
                inst.rect.top+=4
            for e in enemys:
                e.rect.top+=4
            p.rect.top+=4
        if p.rect.bottom>300:
            for inst in instancelist:
                inst.rect.bottom-=4
            for e in enemys:
                e.rect.bottom-=4
            p.rect.bottom-=4


        for b in instancelist:
            if p.rect.colliderect(b.rect):
                p.ground=1
        screen.blit(psprite,p.rect)

        for e in enemys:
            if atk==1:
                if left==0:
                    if p.rect.left<e.rect.left:
                        if p.rect.colliderect(e.rect):
                            enemys.remove(e)
                else:
                    if p.rect.right>e.rect.right:
                        if p.rect.colliderect(e.rect):
                            enemys.remove(e)
            screen.blit(e.sprite,e.rect)

    #timers
    if frametime>0:
        frametime-=1
    else:
        frameplus=1

    if atktime>0:
        atktime-=1
    else:
        if atk==1:
            if left==1:
                p.rect.right+=16
            else:
                p.rect.right-=4
        atktime=0
        atk=0

    p.rect=psprite.get_rect(x=p.rect.left,y=p.rect.top)
    pygame.draw.rect(screen, ((255,0,0)), p.rect, 1)
    clock.tick(60)
    pygame.display.flip()

Could someone lend a hand?

share|improve this question
add comment

1 Answer

I want to help with this but I have limited time so I will share what I have noted so far. In summary, I think if you keep programming for a while (say, a year) and look at this code again, you will see many things to improve. I guess that is true for everyone.

Here is some code and comments for the grid idea followed by general comments. Remember I have never worked with pygame so this is almost certainly not game programming best practice.

The dict idea

In summary, when looking for interaction between objects in the game, comparing every object to every other object will become prohibitively expensive when your game gets bigger. As an idea to reduce the amount of comparison going on, you might use a local grid system. This only works when your objects are all the same size. Variable sizes would require a rethink.

  • Split the screen into a grid (like you did with a 16x16 pixel size for each character in the source grid). This could be a dict which acts as a sparse matrix. Or you could use a row+column list of lists to store a complete matrix.

  • Whenever you create an object, place it in the list for it's current grid section (say, whichever section contains the top, left point of the object). Most grid sections will not even exist in the dict. Only when you add something will the section get a list.

  • Make sure each object has a method that can tell you all adjacent grid sections.

Here is an example of setting up the Objects with common behavior for working with the grid:

class BaseItem(object):
    SIZE = 8  # for example. whatever is fine.
    def __init__(self, origin_xy, sprite):
        self.origin = origin_xy
        self.sprite = sprite  # or a dict or a list or whatever
        self.rect = self.sprite.get_rect(x=self.origin.x, y=self.origin.y)

    def collisions(self, grid):
        """Generate a sequence of items that have some collision with self."""
        for grid_coordinates in self._adjacent_grid_coordinates():
            try:
                nearby_items = grid[grid_coordinates]
            except KeyError:
                nearby_items = tuple()  # nobody is nearby --> empty sequence
            for item in nearby_items:
                if item is self:
                    pass  # ignore self
                if self.rect.colliderect(item):
                    yield item  # check out generators

    def _adjacent_grid_coordinates(self):
        """Generate a sequence of grid coordinates that are adjacent to this
        item, including the grid coordinates for self.
        """
        # you may want to do min/max to avoid going outside the map
        # although with a grid dict it won't really matter
        left, right = self.origin.x - 1, self.origin.x + 1
        up, down = self.origin.y - 1, self.origin.y + 1
        for y in range(up, down + 1):
            for x in range(left, right + 1):
                yield (x, y)  # check out generators

class Player(BaseItem):
    def __init__(self, origin_xy, sprite, something_new):
        super(Player, self).__init__(origin_xy, sprite)  # don't forget this
        # do something with something_new

Here is an example of setting up the grid and objects:

# build the level:
# (0, 0) is top left
# grid is sections of the map with coordinates at top left of each section
# positions are pixel positions with coordinates at top left of each item
grid_size = 16
for grid_y, row in enumerate(level):
    for grid_x, col in enumerate(row):
        position_x, position_y = grid_size * grid_x, grid_size * grid_y
        if col == "B":
            item = Block(position_x, position_y, block1)
            #instancelist.append(p)
            # This uses grid as a sparce matrix representing sections of content
            # in the game (not exact positions). Sparse just means not every
            # section has content.
            # grid.setdefault gets the existing list for the section with
            # coordinates (row, col) or if nothing is in that section yet,
            # creates an empty list.
            # It then appends the new block to that list.
            grid_coordinates = (grid_x, grid_y)
            blocks.append(item)
            grid.setdefault(grid_coordinates, list()).append(item)
        if col == "L":
            item = Player(position_x, position_y)
            players.append(item)
            grid.setdefault(grid_coordinates, list()).append(item)
        if col == "E":
            item = Enemy(position_x, position_y)
            enemys.append(item)
            grid.setdefault(grid_coordinates, list()).append(item)

With that setup, when you want to check for interaction between, for example, the player and all other objects, you just need to do player.collisions(grid) and handle each result according to whatever rules you have.

major points

  • It more or less works. That's great. I've never made a pygame app so it was fun to bounce around the room in yours.
  • When you have data that is considered a direct part of an object (like that atk status or psprite) keep that within the object. In this case, Player.
  • On the other hand, if a Class becomes just a collection of data with no behavior (like Block), consider using a dict or namedtuple
  • The above points will help, but further avoid cluttering the namespace by capturing some common or complex behavior in functions. Your main code will become more readable and you will have a lower risk of accidentally mixing names (like using x in a loop that was defined somewhere else rather than in the loop which uses col instead).

minor points

  • Readability and style standardization - read PEP 8! Don't stress out about it. Just notice a few things and try to adjust your writing style a little at a time. For example:
    • 1 space around operators and assignments (x = 0) (if x == 0) (x + 1) (but not around assignments inside functions f(x=0))
    • 1 space after commas
    • avoid c-style x + 16, y + 16, x++ type statements. use enumerate or other automatic abilities if possible
share|improve this answer
 
I've got a bunch of schoolwork to do, but I will read it later :) Thanks! –  Sam Tubb Oct 16 '13 at 20:39
 
I got around to reading it! Thanks for the help, I really loved that first major point :3 . I will see how successfully I can implement this on my own. If I need any help is there some how I can contact you? –  Sam Tubb Oct 17 '13 at 3:17
1  
@Sam Tubb if it's a point directly related to the dict idea, you can probably update here and post a note @kobejohn. If it's a new issue/question, post a new question here or in stackoverflow and add a note here @kobejohn to link to it. If you can keep the size of the code smaller and keep the scope of your questions focused, I think you will get lots of help. –  kobejohn Oct 17 '13 at 3:36
 
Ok, got it. Thanks for your help. Glad someone like you exists. –  Sam Tubb Oct 17 '13 at 10:47
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.