Terrain generator in a PyGame game

In my game, there is a terrain generator subsequently resulting in many instances.

I have implemented this code:

for b in blocklist:
    if b.rect.left>=0:
       if b.rect.right<=640:
          screen.blit(b.sprite, b.rect)

It only renders things within the screen (400-500 blocks), but it still runs as if it were rendering all 2000 or so.

Why is it so slow? Does it have anything to do with pygame.display.update() or pygame.display.flip()? Is there even a difference?

Here is the entire code:

```
#Init stuff
import pygame,random
from pygame.locals import *
from collections import namedtuple
import time, string
pygame.mixer.init(frequency=22050, size=-16, channels=2, buffer=500)
f=open('texdir.txt','r')
texdir=f.read()
f.close()
f=open(texdir+"\\splash.txt",'r')
splash=f.read()
splash=splash.replace('(','')
splash=splash.replace(')','')
splash=splash.split(',')
f.close()
splashlen=len(splash)
chc=random.randint(0,int(splashlen))
splash=splash[chc-1]
f=open(texdir+"//backcolor.txt")
pygame.init()
clock=pygame.time.Clock()
screen=pygame.display.set_mode((640,480))
pygame.display.set_caption("PiBlocks | By Sam Tubb")
max_gravity = 100
blocksel=texdir+"\\dirt.png"
btype='block'
backimg = pygame.image.load(texdir+"\\menu.png").convert()
backimg = pygame.transform.scale(backimg, (640,480))
clsimg = pygame.image.load("clear.bmp").convert()
clsimg = pygame.transform.scale(clsimg, (640,480))
ingame=0
sbtn=pygame.image.load("startbtn.png").convert()
qbtn=pygame.image.load("quitbtn.png").convert()
tbtn=pygame.image.load("texbtn.png").convert()
sbtnrect=sbtn.get_rect()
sbtnrect.x=220
sbtnrect.y=190
qbtnrect=qbtn.get_rect()
qbtnrect.x=220
qbtnrect.y=225
tbtnrect=tbtn.get_rect()
tbtnrect.x=220
tbtnrect.y=260
go=0
gotime=35
select=1
colliding = False
Move = namedtuple('Move', ['up', 'left', 'right'])
player=[]
blocklist=[]
font=pygame.font.Font(None,18)

#set cursor
curs = pygame.image.load(texdir+"\\cursor.png").convert()
curs.set_colorkey((0,255,0))

#set backcol

First of all, a lot of people who would answer this question will not do so because you can't just copy/paste the code and run it. The code depends on a lot of external files, like images and textfiles, and to run your code, you basically have to trial'n'error your way while creating a bunch of placeholder images...

I'll try to read your code and comment it step by step.

f=open('texdir.txt','r')
texdir=f.read()
f.close()
f=open(texdir+"\\splash.txt",'r')
splash=f.read()
splash=splash.replace('(','')
splash=splash.replace(')','')
splash=splash.split(',')
f.close()
...
f=open(texdir+"//backcolor.txt")

Here you read some settings/configurations. Better use the ConfigParser module. It will make your code more readable and more easily to follow.

splashlen=len(splash)
chc=random.randint(0,int(splashlen))
splash=splash[chc-1]

To select a random element from as list, you can just use random.choice.

f=open(texdir+"//backcolor.txt")
...
blocksel=texdir+"\\dirt.png"

Sometimes you use // in a path, sometimes \\. Better use os.path.join to create a path so it will work on different operating systems,

COLOR=f.read()
f.close()
COLOR=COLOR.replace('(','')
COLOR=COLOR.replace(')','')
COLOR=COLOR.split(',')
c1=COLOR[0]
c2=COLOR[1]
c3=COLOR[2]

...

if ingame==1:
    screen.fill((int(c1),int(c2),int(c3)))

Well, that's a whole lot of code to read a simple tuple. Let be point to ast.literal_eval, which allows you to savely read a string and convert it into a python structure:

line = f.read()
# assuming 'line' is a string representing a tuple, like (120, 120, 200)
back_color = literal_eval(line) # now, back_color is a tuple

...

if ingame==1:
    screen.fill(back_color)

if key[K_1]:
    blocksel=texdir+"\\dirt.png"
    btype='block'
    select=1
if key[K_2]:
    blocksel=texdir+"\\stonetile.png"
    btype='block'
    select=2

Here, you have three (maybe four if you count the key) values that are connectet to each other, so you should create a type that represents this:

class BlockType(object):
    def __init__(self, name, type, image_path=None, y_offset=0):
        self.name = name
        self.type = type
        if not image_path:
            image_path = name + '.png'
        self.image = pygame.image.load(image_path).convert()
        self.y_offset = y_offset

    def get_rect(self, x, y):
        return self.image.get_rect(top=y+self.y_offset, left=x)

class Block(object):
    def __init__(self, x, y, block_type):
        self.block_type = block_type
        self.image = block_type.image
        self.rect = block_type.get_rect(x, y)

    # don't know if we need the information 'block or slab'         
types = [BlockType('dirt', 'block'), 
         BlockType('stonetile', 'block'), 
         BlockType('stone', 'block'),
         BlockType('grass', 'block'),
         BlockType('sand', 'block'),
         BlockType('woodplank', 'block'),
         BlockType('woodslab', 'slab', y_offset=16)]

block_lookup = {t.name: t for t in types}
key_map = dict(zip([K_1, K_2, K_3, K_4, K_5, K_6], types))

Also, this will allow to remove a lot of code duplication.

if event.button==4:
        if select 1:
                select=select-1
        else:
                select=9

can be simplified to

num_of_types = len(types)

if event.button==4:
    select += 1
elif event.button==5:
    select -= 1
select = max(min(select, num_of_types), 0)

if blklvl==top:
        blocklist.append(Block(across,blklvl,texdir+"\\grass.png",'block'))
if blklvl>top:
        if blklvlcen-1:
        blocklist.append(Block(across,blklvl,texdir+"\\stone.png",'block'))

Notice how you repeat the filenames to each different block type all over again? Let's fix that by creating a factory method that uses our new block_lookup dictionary:

# this lookup could also be directly in the Block class
def block_factory(name, x, y):
    return Block(x, y, block_lookup[name])

...

if blklvl == top:
    blocklist.append(block_factory(across, blklvl , 'grass'))
if blklvl > top and blklvl cen-1:
    blocklist.append(block_factory(across, blklvl , 'stone'))

vers=font.render('PiBlocks Alpha 0.6',True,(255,255,255))
tex=font.render('Selected Texture Pack: '+texdir,True,(255,255,255))

You render the text surface once each frame. Text rendering in pygame is quite slow, so you should cache the text surfaces and reuse them.

for block in blocklist:
    if any(block.rect.colliderect(b.rect) for b in blocklist if b is not block):
        if b.btype=='slab':
            blocklist.remove(block)
        else:
            blocklist.remove(b)

No idea what you try to do here. b is declared in the list comprehension above and should not be used outside of it.

There's a lot more what could be improved, but we've already eliminated two performance bottlenecks (reloading images from disk every time a new block is created, text rendering), and I'm running out of space

StackExchange Code Review Q#32434, answer score: 7