class that handles the rpg game map

Question

I created a class to handle maps in my RPG game, before it became a class this was a collection of functions I created, but I thought it could be used as a class. although this code is working but i am not sure what i made. changing the function set to OOP confuses me.

import os

import pygame

import helper_func as hf


class Map:
    def __init__(self):
        self.map = []
        self.images = {}
        self.tile_size = (50, 50)

    def load_map(self, path):
        ''' load map from txt to 2d list '''
        f = open(path, "r")
        file = f.read()
        f.close()

        sep = file.split("\n")
        for tile in sep:
            numbers = tile.split(",")
            
            self.map.append(numbers)

    @staticmethod
    def generate_key(image):
        result = []
        for char in image:
            try:
                if isinstance(int(char), int):
                     result.append(char)
            except Exception:
                pass

        return "".join(result)

    def map_images(self, path):
        ''' load tileset image to dictionary '''
        images = os.listdir(path)
        for image in images:
            key = Map.generate_key(image)
            
            self.images[key] = hf.load_image(f"{path}/{image}", self.tile_size[0], self.tile_size[1])

    def render(self, screen):
        y = 0
        for row in self.map:
            x = 0
            for tile in row:
                screen.blit(self.images[tile], (x, y))

                x += self.tile_size[0]

            y += self.tile_size[1]

Is there anything that needs to be fixed?

Answer 1

Map.tile_size seems awkward. You never use the tuple as whole, only the individual [0] and [1] components. Perhaps use two members (Map.tile_width and Map.tile_height) would be clearer, and possibly slightly faster due to removing one level of lookup.

In Map.load_map, you split lines of text on commas, and assign it to numbers, but actually you just have a list of strings which presumably happen to look like numbers. What are these numbers? 0 to 9? 00 to 99? If they are small integers (0-255), you might store them very compactly as a bytearray(), or bytes() object. If the range of values is larger that a byte, you could use array.array().

Map.generate_key: you’re using try: catch: to test if a character is in the range '0' to '9'? That is very expensive test, more so because you then test isinstance(…, int) which must be true if int(char) didn’t return an exception! The isdecimal() function can easily determine if the character is a digit, and the entire function body could be replaced with a single statement:

    return "".join(char for char in image if char.isdecimal())

map_images is a very uninformative name. load_map_images would be better. os.listdir(path) is giving you ALL the files in the directory, which means you can’t have any backup files, readme files, or source-code control files in that directory. You should probably use a glob() to find only *.png files (or .gif, or whatever image format you are using).

Your images are stored by by “key”, which is only the digits in the filename. So, water0.png and grass0.png would both be stored as under the key "0", with the last one overwriting any previous ones. You might want to check for uniqueness or collisions.

Which begs the question, why are you storing the map as numbers? Maybe “grass”, “forest”, “nw-road” would be clearer than “12”, “17”, “42”.

render seems to be only able to render the entire map. Perhaps that is all that is necessary for the game in question, but in many games, you’d usually render a small region around a given x,y coordinate.