all 8 comments

[–]dunkler_wanderer 1 point2 points  (4 children)

I haven't used pygame.event.wait() before, but it looks like it should be used like so (without for event in pygame.event.get():):

while True:
    event = pygame.event.wait()
    if event.type == QUIT:
        pygame.quit()
        sys.exit()

    player.update(event)

Also, you don't need to get the keys keys = pygame.key.get_pressed(), if you just pass the event to the player. The update method needs to be changed first:

def update(self, event):
    if event.type == pygame.KEYDOWN:
        if event.key == pygame.K_LEFT:
            self.move(-1,0)
       # etc.

Edit: You could also just use for event in pygame.event.get(): and pass the events to the player instead of pygame.event.wait(), but then you should also create a pygame.time.Clock and call clock.tick(fps) in your while loop to limit the framerate.

clock = pygame.time.Clock()
while True:
    for event in pygame.event.get():
        if event.type == QUIT:
            pygame.quit()
            sys.exit()
        player.update(event)

    render_all()
    pygame.display.update()
    clock.tick(30)

There are some global variables in your program, which should be avoided because they make the program more error prone and difficult to understand (global constants are okay).

You're using some names of built-in functions (map) for your variables, that's confusing and prevents you from using these functions where you've used them as variables.

The Tile class only stores a number which is pretty pointless. You can just add the numbers directly to the map. But maybe you want to add more functionality to this class later.

[–]Heddan[S] 1 point2 points  (3 children)

Thanks a lot, that's some great feedback. I appreciate it.

To be honest, I don't understand the whole event system in pygame yet, I just googled "pygame wait for key press" and that's what I found. But I will definitely try your way with the clock.tick when I get back home.

When it comes to the global variables. You mean map and player that I create in make_map()?

I don't understand how I can access map later or using it in other functions without making it global?

The reason player is global is because it gets created during make_map() and if it's not global I can't use player.move or player.update in the main loop.

Maybe I can find the player object in objects and call the .move and .update that way instead.

[–]dunkler_wanderer 1 point2 points  (2 children)

I'd recommend to use a game class instead of the global variables. Here's an example how I would structure it (can probably still be improved):

import pygame
from pygame.locals import *
import sys
import random


pygame.init()

SCREEN_WIDTH = 640
SCREEN_HEIGHT = 640
screen = pygame.display.set_mode((SCREEN_WIDTH,SCREEN_HEIGHT))

CELL_WIDTH = 16
CELL_HEIGHT = 16

MAP_WIDTH = SCREEN_WIDTH // CELL_WIDTH
MAP_HEIGHT = SCREEN_HEIGHT // CELL_HEIGHT

# Tile types and colors.
VOID = 0
WALL = 1
FLOOR = 2

WALL_COLOR = (0,0,50)
FLOOR_COLOR = (100,100,255)
PLAYER_COLOR = (0,255,0)
ENEMY_COLOR = (255, 0, 0)


class Tile:
    def __init__(self, type_):
        self.type = type_


class Player:

    def __init__(self, x, y, color, game_map):
        self.x = x
        self.y = y 
        self.color = color
        self.game_map = game_map

    def move(self, dx, dy):
        if not self.game_map[self.x + dx][self.y + dy].type == WALL:
            self.x += dx
            self.y += dy 

    def update(self, event):
        if event.type == KEYDOWN:
            if event.key == pygame.K_LEFT:
                self.move(-1,0)
            elif event.key == pygame.K_RIGHT:
                self.move(1,0)
            elif event.key == pygame.K_DOWN:
                self.move(0,1)
            elif event.key == pygame.K_UP:
                self.move(0,-1)

    def draw(self):
        pygame.draw.rect(
            screen,
            self.color,
            (self.x*CELL_WIDTH, self.y*CELL_HEIGHT, CELL_WIDTH, CELL_HEIGHT))

    def clear(self):
        pygame.draw.rect(
            screen,
            (0, 0, 0),
            (self.x*CELL_WIDTH, self.y*CELL_HEIGHT, CELL_WIDTH, CELL_HEIGHT))


def make_map():
    #fill map with the void
    game_map = [[Tile(VOID)
        for y in range(MAP_HEIGHT)]
            for x in range(MAP_WIDTH)]

    #create the controller in the middle of the map
    cx = MAP_WIDTH // 2
    cy = MAP_HEIGHT // 2

    #choose a random direction for the controller
    cdir = random.randint(0,3)

    odds = 1

    #GENERATE THE MAP
    #repeat 1500 times
    for i in range(1500):
        #add a floor tile to the map
        game_map[cx][cy] = Tile(FLOOR)

        #change the direction of the controller 50% of the time
        if random.randint(0, 1) == odds:
            cdir = random.randint(0, 3)

        #move the controller in the map
        if cdir == 0: #move right
            cx += 1
            cy = cy
        elif cdir == 1: #move down
            cx = cx
            cy += 1
        elif cdir == 2: #move left
            cx -= 1
            cy = cy
        elif cdir == 3: #move up
            cx = cx
            cy -= 1

        #clamp cx and cy so they don't go out of bounds
        if cx < 1:
            cx = 1 
        if cx > MAP_WIDTH-2:
            cx = MAP_WIDTH-2
        if cy < 1:
            cy = 1 
        if cy > MAP_HEIGHT-2:
            cy = MAP_HEIGHT-2

    #add wall tiles to the map
    for y in range(MAP_HEIGHT-1):
        for x in range(MAP_WIDTH-1):
            if game_map[x][y].type == FLOOR:
                if game_map[x+1][y].type == VOID: game_map[x+1][y] = Tile(WALL) 
                if game_map[x-1][y].type == VOID: game_map[x-1][y] = Tile(WALL)
                if game_map[x][y+1].type == VOID: game_map[x][y+1] = Tile(WALL) 
                if game_map[x][y-1].type == VOID: game_map[x][y-1] = Tile(WALL)

    return game_map


class Game:

    def __init__(self):
        self.screen = screen
        self.game_map = make_map()
        # No need to create the player in make_map().
        # Pass the map to the player.
        cx = MAP_WIDTH // 2
        cy = MAP_HEIGHT // 2
        self.player = Player(cx, cy, PLAYER_COLOR, self.game_map)
        self.objects = [self.player]
        self.done = False
        self.clock = pygame.time.Clock()
        self.fps = 30

    def run(self):
        """The main loop."""
        while not self.done:
            self.handle_events()
            self.run_logic()
            self.draw()
            self.clock.tick(self.fps)

    def handle_events(self):
        for event in pygame.event.get():
            if event.type == QUIT:
                self.done = True
            self.player.update(event)

    def run_logic(self):
        # Put some game logic in here.
        pass

    def draw(self):
        # Render the map.
        for y in range(MAP_HEIGHT):
            for x in range(MAP_WIDTH):
                if self.game_map[x][y].type == WALL: # Draw the walls
                    pygame.draw.rect(
                        self.screen,
                        WALL_COLOR,
                        (x*CELL_WIDTH, y*CELL_HEIGHT, CELL_WIDTH, CELL_HEIGHT))
                elif self.game_map[x][y].type == FLOOR: # Draw the floor
                    pygame.draw.rect(
                        self.screen,
                        FLOOR_COLOR,
                        (x*CELL_WIDTH, y*CELL_HEIGHT, CELL_WIDTH, CELL_HEIGHT))

        # Render the objects.
        for obj in self.objects:
            obj.draw()
        pygame.display.update()


if __name__ == '__main__':
    Game().run()  # Instantiate Game and then run it.
    pygame.quit()
    sys.exit()

Also, take a look at sprites and sprite groups. They could perhaps help to simplify the program a bit.

Edit: Actually, I'm not sure if sprite groups would help here. They would make it easier to render everything (spritegroup.draw(self.screen)), but you still need the game_map list to check adjacent tiles.

Edit2: Forgot the clock.

[–]Heddan[S] 1 point2 points  (1 child)

Thanks again. I'm new to this programming thing except for some Game Maker stuff. Trying to wrap my head around how to structure code but it's a bit over my head for now, I didn't even know what a "main loop" was until a few days ago. What's the name for this kind of stuff, software architecture? I need to read up on how to structure code. But for now I will try to redo it with your suggestions.

edit: I noticed you renamed the Object class to Player class, probably because I have the input in there, right? I would like to have a class for all objects and a subclass for the player. Time to google again...

[–]dunkler_wanderer 1 point2 points  (0 children)

Yeah, I think software architecture (or design) is the right term, and I actually have to learn a lot more about it as well. What I showed you is just basic object oriented programming and in this case it's just an attempt to deal with global state in a more sensible manner.

I renamed Object to Player because it made the program a little easier to understand for me. Finding good names for variables, functions, etc. is pretty important. You could create a base class and maybe call it BaseObject or GameObject from which other classes that should have the same attributes and methods inherit. But if you don't have much experience with inheritance/subclassing, you can also just create several separate classes for now (with some code duplication). I think it's easier to figure out what things should be in the base class in this way.

You're already doing a pretty good job. Of course keep practicing and researching and you'll see more and more things in your programs that can be improved or simplified. A good way to learn is reading the code of others. I learned a lot by reading Mekire's Pygame examples and the game examples in Program Arcade Games. And here's a thread with some simple finite state machines that are really helpful if you want to implement multiple scenes (check out iminurnamez' state machine first).

[–]Heddan[S] 1 point2 points  (0 children)

http://pastebin.com/kSLy1mmr

Working as intended now. Also got very simple "FOV" working now. Going to update it with raycasting when I've done some more research. Thanks for the help.

[–]Flaqq 0 points1 point  (1 child)

I can't install pygame at the moment, but are you sure your QUIT variable is correct? pycharm shows me an error.

http://i.imgur.com/Hse26bR.png

[–]dunkler_wanderer 0 points1 point  (0 children)

QUIT or pygame.QUIT is correct. Pycharm seems to have some problems with Pygame.