5
\$\begingroup\$

I've made a python game called 2d minecraft (guess where I got the idea) that has an infinite chunk system. I believe the idea I have is alright but my coding is bad and I have tried to improve it as much as I can before posting here.

Here is the code:

import pygame

# set up variables
title, size = "2d_MC", (1365, 705)
pygame.init()
screen, window_rect, clock = pygame.display.set_mode(size), pygame.Rect((0, 0), size), pygame.time.Clock()
pygame.display.set_caption(title)
pygame.event.set_allowed([pygame.QUIT])
block_size = 64
block_texture_names = ["textures\\void.png", 
                       "textures\\air.png", 
                       "textures\\grass_block.png", 
                       "textures\\dirt.png", 
                       "textures\\stone.png", 
                       "textures\\sandstone.png", 
                       "textures\\sand.png", 
                       "textures\\bedrock.png", 
                       "textures\\oak_log.png", 
                       "textures\\oak_leaves.png", 
                       "textures\\cobblestone.png"]
block_textures = []

def block_texture(texture):
    # does the image loading
    return pygame.transform.scale(pygame.image.load(texture), (block_size, block_size)).convert_alpha()

def convert(pos, convert_value_x, convert_value_y):
    # converts coordinate systems
    nx, rx = divmod(pos.x, convert_value_x)
    ny, ry = divmod(pos.y, convert_value_y)
    return ((nx, ny), (rx, ry))

class chunk():
    width, height = 8, 8

    def __init__(self, pos):
        self.pos = pygame.Vector2(pos)
        self.blocks = {}
        # sets up the range and blocks
        self.range = (range(0, self.width), range(0, self.height))
        for x in self.range[0]:
            for y in self.range[1]:
                self.blocks[(x, y)] = 0

    def load(self):
        # will be of more use in the future
        self.generate()

    def generate(self):
        # simple generation for now
        for x in self.range[0]:
            for y in self.range[1]:
                x_pos, y_pos = int(self.pos.x)+x, int(self.pos.y)+y
                surface = 0
                bedrock = 16
                soil_amount = 3
                if y_pos == surface:
                    block = 2
                elif y_pos > surface and y_pos <= surface + soil_amount:
                    block = 3
                elif y_pos > surface + soil_amount and y_pos < bedrock:
                    block = 4
                elif y_pos == bedrock:
                    block = 7
                elif y_pos > bedrock:
                    block = 0
                else:
                    block = 1
                self.blocks[(x, y)] = block

    def blocks_str(self):
        # string representation of the blocks in the chunk, for testing purposes
        return [self.blocks[(x, y)] for x in self.range[0] for y in self.range[1]]

    def save(self):
        # will be used in furture
        pass

    def unload(self):
        # will be of more use in the future
        for x in self.range[0]:
            for y in self.range[1]:
                self.blocks[(x, y)] = 0


class world():
    def __init__(self, loader_pos, loader_distance):
        self.loaded_chunks = {}
        self.loader_pos = loader_pos
        self.loader_chunk_pos = pygame.Vector2(convert(loader_pos, chunk.width*block_size, chunk.height*block_size)[0])
        self.loader_distance = loader_distance
        self.rendered = pygame.Surface((1, 1)).convert_alpha()

    def handle_chunk_loader(self):
        # get chunks needed and unloads and loads acordingly
        chunks_needed = [chunk_pos for chunk_pos in self.chunks_to_load(self.loader_chunk_pos)]
        chunks_currently_loaded = [chunk_pos for chunk_pos in self.loaded_chunks]
        self.load_chunks([chunk_pos for chunk_pos in chunks_needed if chunk_pos not in chunks_currently_loaded])
        self.unload_chunks([chunk_pos for chunk_pos in chunks_currently_loaded if chunk_pos not in chunks_needed])

    def load_chunks(self, chunks_pos):
        for chunk_pos in chunks_pos:
            self.loaded_chunks[(chunk_pos.x, chunk_pos.y)] = chunk(chunk_pos)
            self.loaded_chunks[(chunk_pos.x, chunk_pos.y)].load()

    def unload_chunks(self, chunks_pos):
        for chunk_pos in chunks_pos:
            chunk = self.loaded_chunks.pop(chunk_pos)
            # chunk.save()
            chunk.unload()
    
    def get_block(self, pos):
        # will be of more use in the future
        chunk_pos, block_pos = convert(pos, chunk.width, chunk.height)
        try:
            return self.loaded_chunks[chunk_pos].blocks[block_pos]
        except Exception as e:
            return 0

    def chunks_to_load(self, loader_pos):
        return [pygame.Vector2(chunk_pos_x*chunk.width, chunk_pos_y*chunk.height) 
               for chunk_pos_x in range(int(loader_pos.x)-self.loader_distance, int(loader_pos.x)+self.loader_distance+1) 
               for chunk_pos_y in range(int(loader_pos.y)-self.loader_distance, int(loader_pos.y)+self.loader_distance+1)]

    def change_pos(self, pos):
        self.loader_pos += pos
        self.loader_chunk_pos = pygame.Vector2(convert(self.loader_pos, chunk.width*block_size, chunk.height*block_size)[0])

    def set_pos(self, pos):
        self.loader_pos = pos
        self.loader_chunk_pos = pygame.Vector2(convert(pos, chunk.width*block_size, chunk.height*block_size)[0])
    
    def render(self, screen, use_pos, other_offset=0):
        for chunk_pos, ch in self.loaded_chunks.items():
            chunk_screen_pos = pygame.Vector2(chunk_pos[0], chunk_pos[1])*block_size
            for block_pos, block in ch.blocks.items():
                screen.blit(block_textures[block], ((chunk_screen_pos + pygame.Vector2(block_pos)*block_size) - self.loader_pos*int(use_pos))+other_offset)


def handle_events():
    for e in pygame.event.get():
        if e.type == pygame.QUIT:
            return 1

    keys = pygame.key.get_pressed()

    if keys[pygame.K_UP]:
        overworld.change_pos(pygame.Vector2(0, -2))
    if keys[pygame.K_DOWN]:
        overworld.change_pos(pygame.Vector2(0, 2))
    if keys[pygame.K_LEFT]:
        overworld.change_pos(pygame.Vector2(-2, 0))
    if keys[pygame.K_RIGHT]:
        overworld.change_pos(pygame.Vector2(2, 0))

    return 0

def draw():
    screen.fill((255, 255, 255))
    overworld.render(screen, True, other_offset=window_rect.center)
    #screen.blit(overworld.rendered, (0, 0))

def game_logic():
    overworld.handle_chunk_loader()

def run_game():
    # set up textures
    for texture in block_texture_names:
        block_textures.append(block_texture(texture))

    while 1:
        game_logic()

        if handle_events():
            break

        draw()

        pygame.display.update(window_rect)

        clock.tick()
        print(clock.get_fps())

    pygame.quit()

overworld = world(pygame.Vector2(0, 0), 2)
run_game()

It runs at a minimum of 30 fps and maximum of 50 and I would like to get it to 60 at least. I would also not mind reducing the code.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Yay, minecraft!


# set up variables
title, size = "2d_MC", (1365, 705)
pygame.init()

Delete the vacuous # comment, as it doesn't tell us anything beyond what the code is clearly explaining.

Break the tuple assign into two lines, as they are unrelated variables. Similarly for the ..., ..., clock = assignment. In contrast, the width, height = 8, 8 assignment further down is perfect as-is, since it helpfully points out that there is a single concept (with two values) being assigned.

That .init() call is troublesome. Protect it within an if __name__ == "__main__": guard. Why? At some point you, or another maintainer, will want to import this module, for example when running unit tests. And there should be no interesting side effects at import time. We should be able to run headless, without a display. Some related calls should be moved along with the init call.

block_texture_names = ["textures\\void.png", ...

Consider spelling it "textures/void.png", so the code will portably run on more than one OS. If that is infeasible, at least spell it r"textures\void.png".


def convert(...):
    # converts coordinate systems

That is a valuable comment. I thank you for it.

But python has a special way of explaining what a function does:

def convert(...):
    """Converts coordinate systems."""

Recommend you prefer a """docstring""" over # comments.


class chunk():

Pep-8 asks that you spell it class Chunk. And also World.

No need for those extra ( ) parens, as you're not inheriting from anything.


    def __init__(self, pos):
        self.pos = pygame.Vector2(pos)

I don't understand what's going on there. An optional type hint of ...(self, pos: tuple[int, int]): would go a long way toward helping me out.

As it stands, I believe we're getting a Vector2 position passed in, and then we turn it into an identical Vector2. Couldn't we have simply assigned it, without creating a copy?

        self.range = (range(0, self.width), range(0, self.height))

This is nice enough. Consider breaking it into a pair of assignments so you instead have x_range and y_range. The [0] / [1] subscripts aren't terrible, but they're not super convenient either -- names would improve clarity.

                self.blocks[(x, y)] = 0

Usually zero (and one) get a pass when it comes to magic numbers. But here it seems like you should define an enum or manifest constant of TEXTURE_VOID to denote the zeroth index into that array. Oh, and we see it a little farther down in generate(), where 1, 2, 3, 4, & 7 are similarly mysterious / magical.

The generate() function is not too long. But consider breaking out the inner if's as a trivial helper method.


    def blocks_str(self):
        # string representation of the blocks in the chunk, for testing purposes
        return [self.blocks[(x, y)] for x in self.range[0] for y in self.range[1]]

I don't understand that at all. It clearly isn't a string. That is, both the identifier and the comment are lying to us.

Do consider def __str__(self):, so that str(my_chunk) and print(my_chunk) will offer nice debugging behaviors.


Your self.blocks is a dict, which is nice enough. Consider turning it into a numpy ndarray.

You would save on the overhead of storing 962_325 object pointers. More importantly you could linearly scan those ~ 1 M elements without random reads, so the code runs at closer to memory bandwidth speed.

And then unload() would become a simple self.blocks = np.zeros(self.range)


    def get_block(self, pos):
        # will be of more use in the future
        chunk_pos, block_pos = convert(pos, chunk.width, chunk.height)
        try:
            return self.loaded_chunks[chunk_pos].blocks[block_pos]
        except Exception as e:
            return 0

This "more code coming in future!" seems to be a theme. Resist it. YAGNI. Wait until some future time when new code is needed, and refactor at that point.

Some of the implemented methods don't seem to be called, and could be safely removed until needed.

Catching Exception is worrisome. Apparently the Author's Intent was to catch IndexError, so specify just that. Habitually catching "too broadly" is an easy way to accidentally mask bugs, which then become hard to diagnose.


You say you want this to run ~ twice as fast. There's no profiler output in this submission that would help to pinpoint where the bulk of the time is being spent. I imagine it is probably in the render() loop.

I wonder if tracking "screen update deltas" would speed it up? That is, if you're scrolling horizontally and a row is usually TEXTURE_BEDROCK both before / after the scroll, would it help to not re-render it? (I don't know -- time it!)

An obvious thing to do would be choose lower resolution, that is, bigger texture blocks. Then you're blit'ing fewer blocks.

A less obvious thing to do is inspired by the progressive rendering of e.g. PNG and JPEG images, and by the fact that sometimes the user is rapidly scrolling across blocks and sometimes not. During rapid scrolling with e.g. pygame.K_LEFT depressed, query the current FPS and if it is "slow" (e.g. 30 FPS) then reduce resolution so you're blit'ing fewer blocks. When the arrow key is released we're no longer scrolling so there are zero deltas and FPS should be "fast". At that point we can afford to render at maximum resolution, so the scrolling effect is low-res "blur" and when we halt then all the detailed blocks will progressively render to show a hi-res image.


EDIT

Suppose you can only do K block blits per frame if you want to stay at 60 FPS.

Init a list of all (x, y) block positions. At start of each frame, shuffle it, and blit just the initial K locations. (Use the sorted() initial locations to take better advantage of the memory hierarchy. Or equivalently, instead of shuffle use random draws compared against threshold so that, in expectation, you blit about K blocks.) During rapid scrolling there will be visual artifacts, but upon halting the screen will rapidly update and stabilize. Actually, no need to shuffle each time, doing that at init suffices. Just keep walking your index circularly through the scheduled block coordinates, and on next frame pick up where you left off. Randomizing at the horizontal raster level, rather than the block level, would be enough.

Better, let's assume you can do about K blits and "little" or "big" blits have about the same cost. Then we want to cache giant pre-rendered multi-block regions.

Remove the K_LEFT / K_RIGHT actions for the moment, so now we have an app which can only scroll up / down. We render a raster of blocks near the middle of the screen. Notice that the same raster will be horizontally displayed at one of the {above, below, same} rows on next frame, according to whether the keyboard demands scrolling or is idle. So upon laboriously looping over the whole raster and rendering each block, we finish up by creating a cached raster by blit'ing the screen result into the cache. On the next render(), we probe the cache and either do a single giant blit or else we render block-by-block.

Now suppose we only honor K_LEFT / K_RIGHT events. Same thing, pretty much, just cache vertical columns of pre-rendered blocks.

Now combine those behaviors, where rendering is aware of the recent scroll direction.

We don't need to cache a full raster -- using quarter rasters might still be helpful, and would more robustly handle diagonal scrolling. Caching larger regions, say 16 x 16, might also work well.


This code achieves most of its objectives. It would benefit from some minor refactoring. The structure is solid, as the two classes nicely reflect Separation of Concerns.

I would be willing to delegate or accept maintenance tasks on this codebase.

\$\endgroup\$
4
  • \$\begingroup\$ Thank for that last comment and sorry I am doing this as a hobby only and other reasons. Point 6, I knew it, someone was going to suggest numpy... Other than that I consider your advice very valuable and will try to do everything you suggested. :) \$\endgroup\$
    – coder
    Commented May 14, 2023 at 21:12
  • \$\begingroup\$ update: right now it runs at 38 to 55 fps, not using numpy or rendering improvements, will try profile it. \$\endgroup\$
    – coder
    Commented May 14, 2023 at 21:35
  • \$\begingroup\$ ok i just made it so each chunk prerenders then render() just blits each chunk, my fps just went nuts, 100 to 150, thanks for the help, you deserve the accept \$\endgroup\$
    – coder
    Commented May 14, 2023 at 22:15
  • 1
    \$\begingroup\$ Yay, laziness FTW! People say they want to "optimize", but what that really means is to tell the CPU to sit back in its recliner, put its feet up, and do just ten percent of what it used to be doing. \$\endgroup\$
    – J_H
    Commented May 14, 2023 at 22:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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