Tile rendering/position updating performance

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
#include
#include
#include
#include
#include

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);

}

c

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& 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 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 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 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:

• user

• Display

• 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 };

StackExchange Code Review Q#78922, answer score: 6