Drawing a tilemap using python and pygame

Question

I am creating a tile-map in python using pygame. The code I have (see below) works OK, but I was wandering if there were any improvements.

This code creates a tilemap and scrolls it horizontally:

from enum import Enum

import pygame
pygame.init()

SCREEN_WIDTH = 800
SCREEN_HEIGHT = 500

TILE_SIZE = 50


class TileTypes(Enum):
    AIR = 0
    GRASS = 1
    ROCK = 2
    LAVA = 3


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

screen.fill((255,) * 3)
clock = pygame.time.Clock()

MAP_WIDTH = SCREEN_WIDTH // TILE_SIZE
MAP_HEIGHT = SCREEN_HEIGHT // TILE_SIZE

map_layout = [[TileTypes.AIR for j in range(MAP_WIDTH)]
              for i in range(MAP_HEIGHT)]


def draw_map():
    y = 0
    for row in map_layout:
        x = 0
        for tile in row:
            match tile:
                case TileTypes.AIR:
                    pass

                case TileTypes.GRASS:
                    pygame.draw.rect(screen, (0, 255, 0), (x, y, TILE_SIZE, TILE_SIZE))

                case TileTypes.ROCK:
                    pygame.draw.rect(screen, (150, 150, 150), (x, y, TILE_SIZE, TILE_SIZE))

            x += TILE_SIZE

        y += TILE_SIZE


def scroll_map():
    for row in map_layout:
        row.append(row.pop(0))

map_layout[-2] = map_layout[-1] = [TileTypes.ROCK for i in range(MAP_WIDTH)]
map_layout[-3] = [TileTypes.GRASS for i in range(MAP_WIDTH)]
map_layout[-4][2:4] = map_layout[-4][6:8] = [TileTypes.GRASS for i in range(2)]


done = False


while not done:
    if any(event.type == pygame.QUIT for event in pygame.event.get()):
        done = True

    scroll_map()

    screen.fill((255,) * 3)
    draw_map()

    pygame.display.flip()

    clock.tick(20)

pygame.quit()

I would be grateful for any improvements.

Answer 1

I like this code. It accomplishes a simple goal in a simple way, very direct and clear.

Conforming to pep8 and using Enum is very nice.

It reminds me of POKE -16298, 0 for LORES graphic mode.

Let's examine low level details first.

---

__main__ guard

pygame.init()
...
screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))
screen.fill((255,) * 3)

I'm not crazy about doing a bunch of stuff at top level. Prefer to protect such statements with a main guard. The standard idiom is:

if __name__ == "__main__":
    pygame.init()
    screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))

Why do we do this? Well, I understand that no file has an import scrolling_game statement, yet. But we anticipate that could happen in future. For example if you write a unit test or use a couple of modules to implement a fancier game.

The idea is that another file should be able to import this one with no side effects, e.g. print() statements or game windows opening. So class / function definitions are fair game, plus constants like width / height. But the meat of the application typically goes into def main(): which we run according to what the module __name__ is. Notice that, for someone importing this module, the module name will look like "scrolling_game" rather than "__main__".

That third statement, which blanks out the screen with white, appears to be leftover from initial development / debug, and should just be deleted. Once the main display loop begins it becomes superfluous.

And defining a WHITE manifest constant wouldn't hurt.

---

cells dictate the display

def draw_map():
            ...
            match tile:
                case TileTypes.AIR:
                    pass

while not done:
    ...
    screen.fill((255,) * 3)  # <-- recommend you delete this
    draw_map()

I'm not keen on the pass, there. Prefer to repaint every tile, every time, rather than defaulting all tiles to AIR followed by selective repaint of non-AIR tiles. The current code makes sense, but I will introduce an alternate repainting strategy below.

Also, the match works out fine. But it would be better to put a color attribute on each Enum value, so we could unconditionally do lookup + render.

---

use an array

map_layout = [[TileTypes.AIR for j in range(MAP_WIDTH)]
              for i in range(MAP_HEIGHT)]

In Lisp and in Python a list-of-lists datastructure can be a very nice representation of N items that are "all the same thing", like tiles. But it suffers from chasing N object pointers, which aren't cache friendly. CPU hardware is good at predicting and prefetching sequential reads, but less so for random reads.

Prefer to represent N tiles with a vector, perhaps an array or an ndarray. That leaves us with something like

map_layout = np.ones((MAP_WIDTH, MAP_HEIGHT)) * TileTypes.AIR

---

slice when scrolling

A central concern is how to efficiently move the grid by one tile in any direction.

Let's start with a level that spans several screens:

level = np.ones((5 * MAP_WIDTH, 3 * MAP_HEIGHT)) * TileTypes.AIR

Pure AIR would be boring, so fill in additional level tiles to taste.

Now let's render a portion of that level:

x, y = get_current_position()
map_layout = level[x : x+MAP_WIDTH][y : y+MAP_HEIGHT]

We can advance x or y by +/- 1 pixel and re-render to scroll to a new spot within the level. Rather than interpreted bytecode we are looping in C, so it's essentially MAP_HEIGHT memcpy()'s per screen refresh, with no random reads from chasing object pointers.

If all levels have similar dimensions, then you could move from two dimensions to a three-dimensional array: level[lvl][x][y]

---

This is solid code. I would be happy to delegate or accept maintenance tasks for it.

Recommend the use of array slicing for frame updates.