Tile rendering/position updating performance

Question

The code below grabs the contents of a .txt file, and scans each character of it, using that data to create a tile set. My main concern, is that for each loop, I am opening and closing a file each time, and that is obviously performance-intensive. I want to create an array of images, based on the files, but I'm not sure how to implement that. I have tried to comment out the code for you, but the draw_map function is the main problem.

Here is a pastebin link to the contents of Map00.

Here are the images:

This is made for Linux, but should work on Windows. Compile with the -lSDL and -lSDL_image arguments.

main.cpp

#include <iostream>
#include <SDL/SDL.h>
#include <SDL/SDL_image.h>
#include <string>
#include <fstream>
#include <sstream>

const int SCREEN_WIDTH = 600, SCREEN_HEIGHT = 400, BITDEPTH = 32, FRAMES_PER_SECONDS = 90; //Window Constants

//Surfaces to play with
SDL_Surface *tile0 = 0;
SDL_Surface *screen = 0;
SDL_Surface *userTile = 0;


//Used for keyboard input tracking
SDL_Event event;

//Used for changing image from 24-bit to 32-bit Alpha
SDL_Surface *load_image(std::string filename) {
    SDL_Surface *loaded_image = 0;
    SDL_Surface *optimized_image = 0;

    loaded_image = IMG_Load(filename.c_str());

    optimized_image = SDL_DisplayFormatAlpha(loaded_image);
    SDL_FreeSurface(loaded_image);

return optimized_image;

}

//Display image onto surface
void blit_surface(int x, int y, SDL_Surface *source, SDL_Surface *destination) {
    SDL_Rect offset;

    offset.x = x, offset.y = y;
    SDL_BlitSurface(source, NULL, destination, &offset);

}

//Starts all the processes we need
void initialize() {
    SDL_Init(SDL_INIT_EVERYTHING);
    screen = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, BITDEPTH, SDL_SWSURFACE);
    SDL_WM_SetCaption("TileSystem", NULL);

}

//Loads the images into memory
void load_files() {
    tile0 = load_image("Grass.png");
    userTile = load_image("userCharacter.png");
}

//Removes the images from memory
void clean_up() {
    SDL_FreeSurface(tile0); SDL_FreeSurface(userTile);

}


class user {
    public:
        int x;//Position on screen where the surface currently is
        int y;
        int offsetX;//Used for adjusting everything else to give the illusion of scrolling
        int offsetY;
        int movSpeed;//How fast the user is allowed to move
        user() {};

        void update(std::string direction) {

                if (direction == "UP") {y -= movSpeed;}
                else if (direction == "DOWN") {y += movSpeed;} // Checks for movement, if so, increases appropriate variable
                else if (direction == "LEFT") {x -= movSpeed;}
                else if (direction == "RIGHT") {x += movSpeed;}
                else {
                    x += 0;
                    y += 0;
                }

                if (y > (SCREEN_HEIGHT - userTile->h) ) {
                    y = (y-movSpeed);
                    offsetY -= movSpeed;
                }
                else if (x > (SCREEN_WIDTH - userTile->w) ) { //Checks to see if the user has exceeded the boundaries of the screen, if so, move the user back based up his speed, and increment the offset appropriately
                    x = (x-movSpeed);
                    offsetX -= movSpeed;
                } 
                else if (x < 0) {
                    x = (x+movSpeed);
                    offsetX += movSpeed;
                }
                else if (y < 0) {
                    y = (y+movSpeed);
                    offsetY += movSpeed;
                }
                else {
                    x += 0, y += 0, offsetX += 0, offsetY += 0;
                }

            blit_surface(x, y, userTile, screen);   //Displays the user image on surface
        }


    user(int x, int y) {
        this->x = x;
        this->y = y;
        this->offsetX = 0;//constructor for initialization values
        this->offsetY = 0;
        this->movSpeed = 2;
    }
};

//Loads file into memory, scans through it, displaying the image as it goes
void draw_map(std::ifstream &Map, std::string filename, user User) {
    char charType;
    int tileX = User.offsetX;//Sets initial drawing values to the offset(which will be zero for first draw)
    int tileY = User.offsetY;

    Map.open(filename);//Loads file
    blit_surface(tileX, tileY, tile0, screen);//Displays first tile

    while(Map >> std::noskipws >> charType) {//Changes draw cursor based on height/width
        if (charType == '0') {
            tileX += tile0->w;
        }
        else if (Map.peek() == '\n') {
            tileX = User.offsetX;
            tileY += tile0->h;
        }

    blit_surface(tileX, tileY, tile0, screen);//Displays the rest of the tiles
    }
    Map.close();
}


//Contains functions for keyboard input/game updates
class Display {
    public: 
        void processInput(std::string &direction, bool &game, bool &will_cap) {
            Uint8 *key_held = SDL_GetKeyState(NULL);


            if (key_held[SDLK_UP]) {direction = "UP";}//Checks for keyboard presses
            else if (key_held[SDLK_DOWN]) {direction = "DOWN";}
            else if (key_held[SDLK_LEFT]) {direction = "LEFT";}
            else if (key_held[SDLK_RIGHT]) {direction = "RIGHT";}
            else {
                direction = "";
            }   

            while (SDL_PollEvent ( &event )) {
            if (event.type == SDL_QUIT) {
                game = false;
                }

            if (event.type == SDL_KEYDOWN) {
                if (event.key.keysym.sym == SDLK_RETURN) {//Used for turning the FPS cap on & off
                    will_cap  = (!will_cap);
                }
            }


            }
        };

        void updateEverything(std::ifstream &Map, std::string filename, user &User, std::string direction) { //Handles all updates/display flipping

            draw_map(Map, filename, User);

            User.update(direction);

            SDL_Flip(screen);

        }

};

//Used for measuring framerate, but other uses(such as pause, start, stop), are included for further use
class Timer {
    public:
        int startTicks;

        int pausedTicks;

        bool paused;

        bool started;

    public:

    Timer() {
        startTicks = 0;
        pausedTicks = 0;

        paused = false;
        started = false;
    }

    void start() {
        started = true;

        paused = false;

        startTicks = SDL_GetTicks();
    }

    void stop() {
        started = false;

        paused = false;
    }

    int get_ticks() {
        if (started) {
            if (paused) {
                return pausedTicks;
            }
            else {
                return (SDL_GetTicks() - startTicks);
            }
        }
    return 0;
    }

    void pause() {
        if ((started) && (!paused)) {
            paused = true;

            pausedTicks = (SDL_GetTicks() - startTicks);
        }
    }

    void unpause() {
        if (paused) {
            paused = false;

            startTicks = (SDL_GetTicks() - startTicks);

            pausedTicks = 0;
        }
    }

    bool is_started() {
        return started;
    }
    bool is_paused() {
        return paused;
    }


};






int main(int argc, char* argv[]) {
    std::ifstream Map;//File object
    std::string filename = "Map00.txt";
    int charStartX = 300, charStartY = 200; //Character starting position, the rest of the below variables are self-explanatory
    bool game = true;
    std::string direction;
    int frame = 0;//Used for measuring FPS
    bool will_cap = true;
    Timer fps;


    initialize();//Loads SDL and Co. into memory

    user User(charStartX, charStartY); //Creates user object

    Display display; //Creates display object, used for handling keyboard input/display updates

    load_files();//loads the images into memory

    draw_map(Map, filename, User); //Scans file, displaying it onto the screen as it goes


    while (game) { //Main Game Loop

        fps.start(); //Starts fps timer
        display.processInput(direction, game, will_cap);//Checks for keyboard input

        display.updateEverything(Map, filename, User, direction);//updates and displays all images on screen
        frame++;

        if ((will_cap) && (fps.get_ticks() < 1000 / FRAMES_PER_SECONDS)) { //Makes sure the framerate stays at the prescribed number
            SDL_Delay( ( 1000 / FRAMES_PER_SECONDS ) - fps.get_ticks());
        }

    }



clean_up(); //Removes all surfaces from memory

}

Answer 1

This is one way of reading the contents of the file once and using the contents in the loop.

Function to read the contents

void getCharList(std::string const& filename,
                 std::vector<char>& charList)
{
   std::ifstream infile(filename);
   char ch;
   while ( infile >> std::noskipws >> ch )
   {
      charList.push_back(ch);
   }
}

Updated main

int main(int argc, char* argv[]) {
   std::string filename = "Map00.txt";
   int charStartX = 300, charStartY = 200; //Character starting position, the rest of the below variables are self-explanatory
   bool game = true;
   std::string direction;
   int frame = 0;//Used for measuring FPS
   bool will_cap = true;
   Timer fps;

   initialize();//Loads SDL and Co. into memory

   user User(charStartX, charStartY); //Creates user object

   Display display; //Creates display object, used for handling keyboard input/display updates

   load_files();//loads the images into memory

   std::vector<char> charList;
   getCharList(filename, charList);

   draw_map(charList, filename, User); //Scans file, displaying it onto the screen as it goes

   while (game) { //Main Game Loop

      fps.start(); //Starts fps timer
      display.processInput(direction, game, will_cap);//Checks for keyboard input

      display.updateEverything(charList, filename, User, direction);//updates and displays all images on screen
      frame++;

      if ((will_cap) && (fps.get_ticks() < 1000 / FRAMES_PER_SECONDS)) { //Makes sure the framerate stays at the prescribed number
         SDL_Delay( ( 1000 / FRAMES_PER_SECONDS ) - fps.get_ticks());
      }

   }

   clean_up(); //Removes all surfaces from memory
}

Updated draw_map

void draw_map(std::vector<char> const& charList, std::string filename, user User) {
   int tileX = User.offsetX;//Sets initial drawing values to the offset(which will be zero for first draw)
   int tileY = User.offsetY;

   blit_surface(tileX, tileY, tile0, screen);//Displays first tile

   for ( auto ch : charList ) {//Changes draw cursor based on height/width
      if (ch == '0') {
         tileX += tile0->w;
      }
      else if (ch == '\n') {
         tileX = User.offsetX;
         tileY += tile0->h;
      }

      blit_surface(tileX, tileY, tile0, screen);//Displays the rest of the tiles
   }
}

Updated updateEverything

  void updateEverything(std::vector<char> const& charList, std::string filename, user &User, std::string direction) { //Handles all updates/display flipping

     draw_map(charList, filename, User);

     User.update(direction);

     SDL_Flip(screen);

  }

Other suggestions

There is too much codde in one file. In addition to many helper functions, you have the following classes:

1. user
2. Display
3. Timer

For the sake of naming consistency, I would change the class user to User. I would put the declarations of the classes in three header files - User.h, Display.h, and Timer.h. I would move the implementations to User.cpp, Display.cpp, and Timer.cpp, respectively.

I would put the declarations of all the helper functions in a .h file (call it tileFunctions.h, for example), and put the definitions of the helper functions in a corresponding .cpp file (call it tileFunctions.cpp, for example).

Reduce objects in global scope

You have

SDL_Event event;

which is used only in Display::processInput. I would move the global object to an to be local variable in the function scope.

You also have:

SDL_Surface *tile0 = 0;
SDL_Surface *screen = 0;
SDL_Surface *userTile = 0;

in the global scope. I would create a struct that holds these together and pass the struct around, starting from main. That would remove the need for them to be in global scope.

Use an enum instead of strings for direction

Instead of using strings "UP", "DOWN", "LEFT", and "RIGHT" for directions, use an enum.

enum Direction { UP, DOWN, LEFT, RIGHT };

Answer 2

You don't want to read a file every time you want to display something, that could be incredibly slow (assuming the file was of any reasonable size). You want to read the file once into memory, and draw the map from that.

Build a two dimensional array. Fill that in a load_map() function with the whole file, then draw the tiles out of that at a given offsets every time you update the screen.

EDIT: in response to comments—

Well, this isn't an SDL issue, it's just a C++ issue. I'm answering this on my phone, so I can't really write an elaborate answer, but if it were me I'd use a single-dimensional std::vector or alternatively (less efficiently) a two dimensional std::vector< std::vector< int > > where the value in the array(s) is an int that corresponds to the index of tile that you want to draw. You'll want to encapsulated that into a 'Map' class. I also assume ultimately, you'll want more than two tiles.

The good thing is there are a million tutorials on tile based games in C++.

As far as other critiques, I wouldn't address the rest, because the performance issue that exists is so fundamental that any other issues pale in comparison.