Best way to re factor my code to be more efficient?

Question

I've recently been working on a simple 2D RPG, and it was going well.

What I'm trying to do: 1. Iterate through a text file, initializing classes as it goes(mapcontrol.LoadMap()). 2. Call mapcontrol.Render() that will add each sprite class to the main sprite array(sprite_list). 3. When a certain event happens(reaching end of screen), shift all of the tiles by a variable(cameraX/Y) 4. Draw the tiles 5. Delete the tiles

The problem with this, is that I would have to call the mapcontrol.LoadMap() method every frame, and this causes major fps issues(probably running at 1 frame every two seconds on my 3.8 Ghz dual core).

My question: What is the best way to optimize this program? I know that much of the program is inefficient, so what is the best way to make it better?

I'm willing to take your toughest beating, because I know it's coming. Here it is, sorry if it hurts your eyes.

import pygame
import os
import sys
import time
import random
pygame.init()
class GameState(object):
    def __init__(self):
        self.state = 'start_screen'
    def state_eval(self):
        if self.state == 'start_screen':

            Start_Screen()
        if self.state == 'local_map':

            Local_Map()
        # if self.state == 'world_map':
            # World_Map()
        if self.state == 'in_progress':
            pass
gamestate = GameState()



def Local_Map():
    gamestate.state = 'in_progress'
    cameraX, cameraY = (0,0)
    width, height = 600, 400
    screen = pygame.display.set_mode((width, height))
    pygame.display.set_caption("Frigid Development")
    sprite_list = pygame.sprite.Group()
    clock = pygame.time.Clock()
    tileSize = 32


    class Player(pygame.sprite.Sprite):
        def __init__(self, cameraX, cameraY):
            pygame.sprite.Sprite.__init__(self)
            self.images_w = [pygame.image.load("C:\Users\Software Development\Desktop\Test00.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test01.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test02.png")]
            self.images_s = [pygame.image.load("C:\Users\Software Development\Desktop\Test03.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test04.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test05.png")]
            self.images_a = [pygame.image.load("C:\Users\Software Development\Desktop\Test06.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test07.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test08.png")]
            self.images_d = [pygame.image.load("C:\Users\Software Development\Desktop\Test09.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test10.png"), pygame.image.load("C:\Users\Software Development\Desktop\Test11.png")]
            self.image_list = [self.images_w, self.images_s, self.images_a, self.images_d]
            self.list_index = 1
            self.index = 0
            self.ani_speed = 6
            self.image = self.image_list[self.list_index][self.index]
            self.rect = self.image_list[self.list_index][self.index].get_rect()
            self.rect.x, self.rect.y, = 200, 300
            self.mov = False
        def update(self):
            if self.list_index >= len(self.image_list):
                self.list_index = 0
            if self.index < 0:
                self.index = 2
            if self.index >= len(self.image_list[self.list_index]):
                self.index = 0
            self.image = self.image_list[self.list_index][self.index]
            self.rect.x, self.rect.y = (self.rect.x - cameraX), (self.rect.y - cameraY)
            screen.blit(self.image, (self.rect.x, self.rect.y))




    class Grass(pygame.sprite.Sprite):
            def __init__(self, tileX, tileY):
                pygame.sprite.Sprite.__init__(self)
                self.image = pygame.image.load("C:\Users\Software Development\Desktop\Tile00.png")
                self.rect = self.image.get_rect()
                self.tileX = tileX
                self.tileY = tileY
            def update(self):
                screen.blit(self.image, (self.tileX, self.tileY))

    class Rock(pygame.sprite.Sprite):
            def __init__(self, tileX, tileY):
                pygame.sprite.Sprite.__init__(self)
                self.image = pygame.image.load("C:\Users\Software Development\Desktop\Tile01.png")
                self.rect = self.image.get_rect()
                self.tileX = tileX
                self.tileY = tileY
            def update(self):
                screen.blit(self.image, (self.tileX, self.tileY))


    class Cube(pygame.sprite.Sprite):
            def __init__(self, tileX, tileY):
                pygame.sprite.Sprite.__init__(self)
                self.image = pygame.image.load("C:\Users\Software Development\Desktop\Tile02.png")
                self.rect = self.image.get_rect()
                self.tileX = tileX
                self.tileY = tileY
            def update(self):
                screen.blit(self.image, (self.tileX, self.tileY))


    class MapControl(object):
        def __init__(self, cameraX, cameraY):
            self.tile_dict = {0: pygame.image.load("C:\Users\Software Development\Desktop\Tile00.png"), 1: pygame.image.load("C:\Users\Software Development\Desktop\Tile01.png"), 2: pygame.image.load("C:\Users\Software Development\Desktop\Tile02.png")}
            self.tileX = 0
            self.tileY = 0
            self.length = 0
            self.count = 0
            self.file = open('C:\Users\Software Development\Desktop\Map00L0.txt', 'r')
            self.oix = True
            self.map_array = []

        def LoadMap(self, map, cameraX, cameraY):
            self.tileX = cameraX
            self.tileY = cameraY
            for x in map:
                for tile in x:
                    if tile == '0':
                        grass = Grass(self.tileX, self.tileY)
                        self.map_array.append(grass)
                        self.tileX = self.tileX+32
                    if tile == '1':
                        rock = Rock(self.tileX, self.tileY)
                        self.map_array.append(rock)
                        self.tileX = self.tileX+32
                    if tile == '\n':
                        self.tileX = cameraX
                        self.tileY += 32
                    if tile == '2':
                        cube = Cube(self.tileX, self.tileY)
                        self.map_array.append(cube)
                        self.tileX = self.tileX+32
                self.tileX = cameraX
                self.tileY = cameraY
        def CalcMapBorders(self, map):
            for ba in map:
                for line in self.file:
                    self.length += 1
                    self.count = len(list(line.strip('\n')))



        def Render(self):
            for draw in self.map_array:
                sprite_list.add(draw)

        def CleanUp(self):
            for rm in self.map_array:
                sprite_list.remove(rm)




    file = open('C:\Users\Software Development\Desktop\Map00L0.txt', 'r')   
    map00ly0 = list(file.read())    
    map = [map00ly0]

    def Loop(screen, map, cameraX, cameraY):
        cameraX, cameraY = 0,0
        player = Player(cameraX, cameraY)
        mapcontrol = MapControl(cameraX, cameraY)
        mapcontrol.LoadMap(map, cameraX, cameraY)
        while gamestate.state == 'in_progress':
            sprite_list.add(player) 
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    pygame.quit()


            key = pygame.key.get_pressed()

            if key[pygame.K_DOWN] and not(key[pygame.K_LEFT] or key[pygame.K_RIGHT]):
                time.sleep(0.0)
                player.rect.y += 2
                player.index += 1
                player.list_index = 1

            if key[pygame.K_UP] and not(key[pygame.K_LEFT] or key[pygame.K_RIGHT]):
                time.sleep(0.0)
                player.rect.y -= 2
                player.index += 1
                player.list_index = 0

            if key[pygame.K_RIGHT]:
                time.sleep(0.0)
                player.rect.x += 2
                player.index += 1
                player.list_index = 3

            if key[pygame.K_LEFT]:
                time.sleep(0.0)
                player.rect.x -= 2
                player.index += 1
                player.list_index = 2

            if not key[pygame.K_UP] and not key[pygame.K_DOWN] and not key[pygame.K_LEFT] and not key[pygame.K_RIGHT]:
                player.index = 0    

            screen.fill((0,0,0))

            mapcontrol.CalcMapBorders(map)

            mapcontrol.Render()

            sprite_list.update()

            if player.rect.y > height - 25:
                player.rect.y -= 2
                player.index += 1
                cameraY -= 3
                #mapcontrol.LoadMap(map, cameraX, cameraY)
            if player.rect.y < 0:
                player.rect.y += 2
                player.index += 1
                cameraY += 3
            if player.rect.x > width - 15:
                player.rect.x -= 2
                player.index += 1
                cameraX -= 3
                #mapcontrol.LoadMap(map, cameraX, cameraY)
            if player.rect.x < 0:
                player.rect.x += 2
                player.index += 1
                cameraX += 3
            if player.rect.y < cameraY:
                cameraY -= 3
            if player.rect.x < cameraX:
                cameraX -= 3
            if player.rect.y > (cameraY + (mapcontrol.length*32) - 15):
                cameraY += 3
            if player.rect.x > (cameraX + (mapcontrol.count*32) - 15):
                cameraX += 3

            sprite_list.draw(screen)
            pygame.display.update()
            clock.tick(30)
            mapcontrol.CleanUp()
            gamestate.state_eval()
    Loop(screen, map, cameraX, cameraY)


def Start_Screen():
    gamestate.state = 'in_progress'
    class Menu_Load_bt(pygame.sprite.Sprite):
        def __init__(self):
            pygame.sprite.Sprite.__init__(self)
            self.image = pygame.image.load("C:\Users\Software Development\Desktop\Menu_Load_bt.png")
            self.rect = self.image.get_rect()
            self.rect.x, self.rect.y = 280, 250
        def update(self):
            GUI.blit(self.image, (self.rect.x, self.rect.y))
    class Menu_Exit_bt(pygame.sprite.Sprite):
        def __init__(self):
            pygame.sprite.Sprite.__init__(self)
            self.image  = pygame.image.load("C:\Users\Software Development\Desktop\Menu_Exit_bt.png")
            self.rect = self.image.get_rect()
            self.rect.x, self.rect.y = 300, 350
        def update(self):
            GUI.blit(self.image, (self.rect.x, self.rect.y))
    class Menu_Sel(pygame.sprite.Sprite):
            def __init__(self):
                pygame.sprite.Sprite.__init__(self)
                self.image = pygame.image.load("C:\Users\Software Development\Desktop\Menu_Select.png")
                self.rect = self.image.get_rect()
                self.dir = None
                self.rect.x, self.rect.y = 470, 270
            def update(self):
                if self.dir == None:
                    pass
                if self.dir == 'down':
                    self.rect.y = 350
                if self.dir == 'up':
                    self.rect.y = 270
                GUI.blit(self.image, (self.rect.x, self.rect.y))
    menu_load_bt = Menu_Load_bt()
    menu_exit_bt = Menu_Exit_bt()
    menu_sel = Menu_Sel()
    images = pygame.sprite.Group()
    GUI = pygame.display.set_mode((800, 600))
    pygame.display.set_caption("Main Menu")
    background = pygame.image.load("C:\Users\Software Development\Desktop\Menu_Back.png")
    while gamestate.state == 'in_progress':

            images.add(menu_load_bt)
            images.add(menu_exit_bt)
            images.add(menu_sel)
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    pygame.quit()
                if event.type == pygame.KEYDOWN:
                    if event.key == pygame.K_DOWN:
                        menu_sel.dir = 'down'
                    if event.key == pygame.K_UP:
                        menu_sel.dir = 'up'

                    if event.key == pygame.K_SPACE or event.key == pygame.K_RETURN: 
                        if menu_sel.rect.y == 270:
                            gamestate.state = 'local_map'
                        if menu_sel.rect.y == 350:
                            sys.exit()

            gamestate.state_eval()
            images.update()
            GUI.blit(background, (0,0))
            images.draw(GUI)
            pygame.display.update() 

gamestate.state_eval()

Thanks.

I did what you said Paul, and I and found that the bottleneck comes at mapcontrol.LoadMap(), but the problem is that for my program to work, I need to redraw an entire map every single loop. Maybe I should only redraw it if I NEED to scroll it(that is why I draw it again and again, so that can incrementally shift every single tile.)?

I tried doing that, and the FPS is correct(30), but when I try to scroll down(if player.rect.y > height, etc.) the fps plummets. However, this function(mapcontrol.LoadMap()) is imperative for the scrolling process to take place. Suggestions???

Answer 1

Pro tip: look at loops. Programs spend almost all their time executing loops. Look for things that are inside of loops and could be outside.

For instance, the function CalcMapBorders: In the for loop, you open the same file over and over again. You increment a variable self.lines for each line in the file. But it's inside a loop, so by the time the function is done, self.lines is equal to the number of lines times the number of loop iterations. I don't know what you're trying to do here, but that's crazy.

You set the variable self.count to the length of each line, then overwrite it when you process the next line. So when you're finished, that variable is equal to the length of the last line. Along the way you have set and clobbered this variable umpteen times.

The outer while loop does absolutely nothing, since it doesn't ever get executed twice (you set the loop variable i to False every step of the inner loop). Eliminate the while, and also the variable i.

Do yourself a favor and put class definitions at the outermost level. Put functions inside of classes, not the other way around. You have nested functions, nested classes, nested functions instantiating nested classes. Some of that stuff I've never (well, hardly ever) done in 12 years of Python programming. It's perfectly legal, but it results in a program structure that's almost impossible to follow. It's hard to spot redundancies and unnecessary code when the structure is so unclear.