Jump to content
  • Advertisement
Sign in to follow this  
qwasqaws

Peculiar array/looping problem

This topic is 4413 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Yello, I'm teaching myself to program simple games by making a tile based thing with SDL and I've come across an odd problem with loading a tilemap. I load the file locations of the tiles into the bg_tileset[] string array with this:

  ifstream tileset("data/tilesets/default.ts");
  if(tileset.is_open())
  {
    getline (tileset,current_line);                //Get first line which contains number of tiles to load
    num_tiles = atoi(current_line.c_str());        //Convert string to int
    for (int i=1;i<=num_tiles;i++)
      {
        tileset.ignore(256,'#');                   //Ignore anything before '#' sign
        getline (tileset,current_line);
        bg_tileset = current_line;
        
      }
      tileset.close();
  } 
  else fprintf(stderr,"Failed to open tileset.\n");


Later on I want to use bg_tileset[] to load the images and draw them to the screen, but when I refer to it the program crashes out. After I figured out that it was that array causing it I removed the reference and tested it by writing a line that output the contents of the first block of the array to a text file and moved the line to different areas of the code. I found the problem to be in this section which comes directly after the above code:

  ifstream mapfile("data/maps/1.map");
  if(mapfile.is_open())
    {cout << bg_tileset[1];
      //Run through, get tiles from map, form tile array.
      for (int h=1;h<=(HEIGHT/TILE_SIZE);h++)
      {***************
        for (int w=1;w<=(WIDTH/TILE_SIZE);w++)
        {***************
          
          mapfile >> map[h][w];
          
          //cout << h << " " << w << " " << tile << "\t" << map[h][w] << endl;
        }
      }
      mapfile.close();
    }
    
  else fprintf(stderr,"Failed to open mapfile.\n");


Those asterisks mark points of interest if I put the test line there. Anywhere before the first one and it works fine. If I put it on that first line then it outputs lines of those square symbols in notepad, but if I open it in editplus or something it shows the correct lines. Anywhere on or after the second lot of asterisks and it crashes out with no error. Rather peculiar I think you'll agree. I can't see what in those sections of code could cause that so here's the entire file. It is a complete mess though. I started it with a template I got from a tutorial and worked from there.

#include <cstdio>
#include <cstdlib>
#include <stdio.h>
#include <stdlib.h>
#include <iostream>
#include <string>
#include <fstream>
#include <sstream>
using namespace std;

#include <SDL.h>
#include <SDL_image.h>

SDL_Surface *back;
SDL_Surface *temp;
SDL_Surface *front;
SDL_Surface *screen;

int xpos=0,ypos=0;

const int HEIGHT = 1024, WIDTH = 768, TILE_SIZE = 64;

int map[16][12];
string bg_tileset[10];
int num_tiles;

#include "draw.h"

//Load images for level
int LoadLevel()
{
  //Load tileset
  string current_line;
  //Open file
  ifstream tileset("data/tilesets/default.ts");
  if(tileset.is_open())
  {
    getline (tileset,current_line);                //Get first line which contains number of tiles to load
    num_tiles = atoi(current_line.c_str());        //Convert string to int
    for (int i=1;i<=num_tiles;i++)
      {
        tileset.ignore(256,'#');                   //Ignore anything before '#' sign
        getline (tileset,current_line);
        bg_tileset = current_line;
        
      }
      
      tileset.close();
      
  } 
  else fprintf(stderr,"Failed to open tileset.\n");
  
  //Load mapfile
  ifstream mapfile("data/maps/1.map");
  if(mapfile.is_open())
    {cout << bg_tileset[1];
      //Run through, get tiles from map, form tile array.
      for (int h=1;h<=(HEIGHT/TILE_SIZE);h++)
      {
        for (int w=1;w<=(WIDTH/TILE_SIZE);w++)
        {
          
          mapfile >> map[h][w];
          
          //cout << h << " " << w << " " << tile << "\t" << map[h][w] << endl;
        }
      }
      mapfile.close();
    }
    
  else fprintf(stderr,"Failed to open mapfile.\n");
//  back = IMG_Load("data/tiles/grass.png");
//  back = IMG_Load(current_line.c_str());
//  image = IMG_Load("image.bmp");
  return 0;
}



void DrawBG()
{
  Slock(screen);
  
  //Draw tilemap to screen
  for (int h=1;h<=(HEIGHT/TILE_SIZE);h++)
  {
    for (int w=1;w<=(WIDTH/TILE_SIZE);w++)
    {
      for (int i=1;i<=num_tiles;i++)
      {string a = "boo";
        if (map[h][w] == i){temp = IMG_Load(a.c_str());}
      }

      DrawIMG(temp, TILE_SIZE*(h-1), TILE_SIZE*(w-1));
    }
  }
        SDL_Rect dest;
        dest.x = 0;
        dest.y = 0;
        //SDL_BlitSurface(screen, &dest2, back, &dest);
        SDL_BlitSurface(back, NULL, screen, &dest);
    
  Sulock(screen);
}

void DrawScene()
{
  Slock(screen);
  DrawIMG(back, xpos-2, ypos-2, 64, 64, xpos-2, ypos-2);
  DrawIMG(front, xpos, ypos);

  SDL_Flip(screen);
  Sulock(screen);
}

int main(int argc, char *argv[])
{
  Uint8* keys;

  if ( SDL_Init(SDL_INIT_AUDIO|SDL_INIT_VIDEO) < 0 )
  {
    printf("Unable to init SDL: %s\n", SDL_GetError());
    exit(1);
  }
  atexit(SDL_Quit);

  screen=SDL_SetVideoMode(HEIGHT,WIDTH,32,SDL_HWSURFACE|SDL_DOUBLEBUF);
  if ( screen == NULL )
  {
    printf("Unable to set 640x480 video: %s\n", SDL_GetError());
    exit(1);
  }

  LoadLevel();
  DrawBG();
  front = IMG_Load("data/sprites/dudes.png");
  
  int gameover=0;

  while(gameover == 0)
  {
    SDL_Event event;

    while ( SDL_PollEvent(&event) )
    {
      if ( event.type == SDL_QUIT )  {  gameover = 1;  }

      if ( event.type == SDL_KEYDOWN )
      {
        if ( event.key.keysym.sym == SDLK_ESCAPE ) { gameover = 1; }
      }
    }
    keys = SDL_GetKeyState(NULL);
    if ( keys[SDLK_UP] )         { ypos -= 5; }
    if ( keys[SDLK_DOWN] )       { ypos += 5; }
    if ( keys[SDLK_LEFT] )       { xpos -= 5; }
    if ( keys[SDLK_RIGHT] )      { xpos += 5; }
    if ( keys[SDLK_f] )          { screen=SDL_SetVideoMode(HEIGHT,WIDTH,32,SDL_HWSURFACE|SDL_DOUBLEBUF|SDL_FULLSCREEN); DrawBG();}
    if ( keys[SDLK_SPACE] )      { DrawBG(); }

    DrawScene();
    SDL_Delay(10);
  }

  return 0;
}

Share this post


Link to post
Share on other sites
Advertisement
Seems like you got a memory read error.

How big is your array? In your for-loops start at i=0 and end at i<array_size.
this is because array-indices go from 0 to size - 1. So reading location size would cause a crash.

Example: int numbers[5]
- numbers[0] = first number
- numbers[1] = second number
- numbers[2] = third number
- numbers[3] = fourth number
- numbers[4] = fifth number
- numbers[5] = crash

Share this post


Link to post
Share on other sites
0) Why are you using fprintf() to report errors? Just output to cerr, as you would output "normal" text to cout.

1) You've included sstream, but you're not using it to read the number. Please use it instead of things like atoi(), which have icky error handling (you can't tell if the value was garbage or if it was a "real" 0).

2) Your array bounds are totally wrong all over the place. The available indices in an array of size N are 0 up to (N-1) inclusive. N is not available. You should not write to index N. 1 is not the first index. 0 is. You should write to index 0 if you are trying to write to all indices.

The normal idiom for looping over an array, which can be found in millions of places the world over, is:


for (int i = 0; i < N; ++i) {


This way is easy to read, will get it right, doesn't require weird + or -1's, and is relatively difficult to screw up.

3) BUT, don't just use arrays for things when you don't know ahead of time what the size will be. Use std::vector. Then you don't have to worry about what happens when you read in a num_tiles that's bigger than the bg_tilesets array you declared.

4) ifstream::ignore kind of sucks. The value 256 might not be enough. If you want something more robust that doesn't have the feeling of a magic number, try including <limits>, and using std::numeric_limits<streamsize>::max(). It shouldn't be possible for a file on your platform to be bigger (in bytes) than that number :) Alternatively, you could getline() into a string and then just discard it, but that uses temporary memory (may be a problem, if the garbage is likely to be significantly longer than the useful data).

5) Don't explicitly .close() streams unless you have to. Their destructors will do it automatically.

6) Don't return values from functions if the return value is useless. Also, if only success or failure can happen, and there's no reason to differentiate between modes of failure, and you don't want to use exceptions, return a bool, rather than an int. Of course, it's often a better idea, instead of printing an error and returning an error code, to throw an exception, and let the caller *decide* if an error message should be printed (and/or other corrective action taken).

7) Expose mathematical relationships between constants whereever possible. That is, instead of "int map[16][12];", what you really want is "int map[HEIGHT/TILE_SIZE][WIDTH/TILE_SIZE];".

Share this post


Link to post
Share on other sites
Don't feel bad. I can assure you I've had more to say about less code in the past, both inside and outside of FB.

Share this post


Link to post
Share on other sites
I just want to emphasize, in his post above, element 2) is the only one where you have a major fundamental flaw in your code. To reiterate, C is a zero based language for index purposes.

All the rest are just ideas to help you improve your coding methods and style. Almost everything he said are really good ideas, but don't feel overwhelmed that he's found 5+ ways in which your design is not as C++ oriented as it could / should be (most of your code reads like C, not C++).

I second the recommendation that you use std::vector instead of arrays, the std containers (vector, deque, list, set, map) rock rock rock!

I one time prototyped an INI file loading system with this data model:

#include <vector>
#include <map>
#include <string>

using std::deque;
using std::map;
using std::string;

namespace ini
{
typedef vector<string> ValueList;
typedef pair<string, ValueList> Setting; // one line
typedef map<string, Setting> SettingGroup; // one block
typedef map<string, SettingGroup> SettingGroupList; // whole file
};


I later refactored each typedef into classes, but only as my needs waranted it.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!