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()

tilemap

I would be grateful for any improvements.

• sbottingota: @SᴀᴍOnᴇᴌᴀ thank you for pointing this out to me. I have now fixed the code so that the tilemap scrolls horizontally and made the question about general, not specific improvements, is it on topic now?
• Sᴀᴍ Onᴇᴌᴀ: Thank you. Yes, it appears to be on-topic and I have re-opened it.

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.

• sbottingota: np.ones((MAP_WIDTH, MAP_HEIGHT)) * TileTypes.AIR results in an error: TypeError: unsupported operand type(s) for +: 'float' and 'TileTypes. Wouldn't something like np.full() work better?
• J_H: Agreed. Though we'll still want AIR (or AIR.value) to give us a small integer, even if used as fill_value for .full(). Plus AIR.color should give us an RGB white value, GRASS.color should give a green triple, and so on, which is easily arranged, so no need for a separate enum_to_color dict.