SimCity clone with PyGame

I am doing my best to keep this code clean and fast, but as I add more to the game, it seems to get harder to maintain a frame-rate higher than 200! I believe that the culprit is a somewhat large for loop inside of the main loop. It performs all of the actions for the tiles inside of the world, and as more tiles are added, the game gets slower.

```
import pygame
from pygame.locals import *
from random import choice
from math import sqrt

#inits
pygame.init()
font=pygame.font.Font(None, 18)
screen=pygame.display.set_mode((640,480))
pygame.display.set_caption('City Game | Pre-Alpha')
clock=pygame.time.Clock()

#sprites
curspr=pygame.image.load('curs.png').convert()
curspr.set_alpha(100)
grassspr=pygame.image.load('grass.png').convert()
roadspr=pygame.image.load('road.png').convert()
forestspr=pygame.image.load('forest.png').convert()
water1=pygame.image.load('water1.png').convert()
water2=pygame.image.load('water2.png').convert()
power1=pygame.image.load('power1.png').convert()
power2=pygame.image.load('power2.png').convert()
res=pygame.image.load('res.png').convert()
house1_0=pygame.image.load('house1_0.png').convert()
house1_1=pygame.image.load('house1_1.png').convert()
res.set_alpha(215)

#vars and lists
taxrateR=8
tilelist=[grassspr,roadspr,forestspr,water1,res,power1]
namelist=['Grass','Road','Forest','Water','Residental','Power Plant']
costlist=[5,10,20,50,100,250]
tiles=[]
sel=0
money=10000
mse=(0,0)
tileframe=2000
pop=0
month=1
year=1
monthtime=0
R=1
C=1
I=1

def Dist(set1,set2):
vec=(set2[0]-set1[0],set2[1]-set1[1])
dist=(vec[0]2+vec[1]2)
return sqrt(dist)

class Tile(object):
def __init__(self,pos,spr,typ):
self.typ=typ
self.spr=spr
self.pos=pos
self.rect=pygame.rect.Rect(pos[0],pos[1],32,32)
self.adv=0
self.haspower=0

while True:
pygame.display.set_caption(str(clock.get_fps()))
screen.fill((2,110,200))
key=pygame.key.get_pressed()
othertiles=[x for x in tiles if x.spr==p

Humans are bad at figuring out where the slow spots are. You will need to ask the computer by profiling your code. As rolfl said, you're likely better off saving this for when you have more of your code in place and it does enough work that it's actually slowing down.

It's quite possible most of your slowdown related to extra tiles comes from othertiles=[x for x in tiles if x.spr==power1 or x.spr==power2]; perhaps you should update othertiles incrementally as the board changes instead of recalculating it every pygame.event. Other than that, I'm going to concentrate on the inner for t in tiles loop. There are a couple things in there that stand out to me. They may or may not make a real difference to your code's performance.

• The overall body reads like an if tree, yet all scenarios are mutually exclusive. You can probably squeak a little better performance out by changing the second and later ifs to elifs. Make sure they're ordered in terms of descending frequency. If the number of possibilities will grow larger, consider other approaches that don't require a series of if tests. (At some point the overhead of testing each alternative will likely be more expensive than that of using alternate lookups such as draw method on various tile classes.)

-
The distance check loop in a res tile seems flawed. It executes a for loop, but only saves the result from the last element of othertiles. And for each element, it checks if othertiles is empty (aside: you should spell that if othertiles: rather than creating and comparing against an empty list every time). If you want the first result, you can fix both of these by using break and a for/else loop:

for x in othertiles:
    # distance calculation
    break
else:
    t.haspower = 0

(Alternately you could just set t.haspower = 0—or False—before the for loop, and let the for loop set it to 1—or True—if applicable.)

• The distance calculation is computationally heavy. Like my comment on your previous post, this is typically more relevant to C level code, but it's still good to remember that sqrt takes a while to calculate, so you can look for opportunities to avoid calling it. Consider that you compare Dist(...) to a constant 2500. If instead of calling sqrt you just returned the square of the distance, you could compare to 6250000 and have the same effect.

• Calling the distance function, you tear t.pos and x.pos apart and piece it back together for a function that just uses their first two elements. You might save some effort, readability, and possibly gain a hint of performance by calling your distance function with just Dist(t.pos, x.pos).

Two more notes:

-
You probably don't want just the first or last distance to a power tile. Instead you likely want the closest. This might be easier to handle with an approach like distance = min(Dist(t.pos, x.pos) for x in othertiles)) if othertiles else 2501 or even better:

t.haspower = False
if othertiles:
    t.haspower = min(Dist(t.pos, x.pos) for x in othertiles)) < 2500
    # or distance_squared and compare to 6250000 per third bullet

-
It's unusual in python to name your functions with initial capital letters. When I read the line distance=Dist((t.pos[0],t.pos[1]),(x.pos[0],x.pos[1])) I assumed it was instantiating a class (which would probably be even heavier than calculating a square root). I would suggest renaming this use all lowercase letters.

StackExchange Code Review Q#36015, answer score: 5