🔒 Locked

[C++] weird problem (solved)

Started by XTAL256 Aug 19, 2008 at 3:58 AM 9 replies 1.5k views

Original Post

Aug 19, 2008 at 3:58 AM

I am making a map editor for my game using Dev-C++ (i.e. g++ compiler) and OpenGL. I have a global static object: Map map; Which is the map object currently being edited. Since this is just a simple editor for my main project, i am not as concerned with the design, so i don't care if there is a better way of doing this. The object is obviously created at runtime (since it's not a pointer) and it is initialised with a new blank map. Then when i load a map, i create the data. But for some reason, i get some strange problems. 1) When i load the background image (as an OGL texture), it is drawn as a blank white square. I have debugged it (as far as i can with Dev-C++'s not so great debugger) and i have found that the image is passed to my draw function with a texture id of 2 and the function gets an id of 0. At first i found that it was because i had created a global static object as well as a pointer, i.e. i had created two objects. Once that was fixed it worked but now it doesn't again. 2) When i try to initialise my map with data from an XML file i get a runtime error which seems to come from when i create a tile object. Here is the source code: Note 1: The object map is declared at global scope in Main.cpp and declared as extern in the header file which Map.cpp includes. Note 2: i have marked the areas where the problems seem to occur.


/////////////////////////////////////////////////////////////////////
// File: Map.cpp                                                   //
/////////////////////////////////////////////////////////////////////

#include "Main.h"
#include "Map.h"


Map::Map() {
    isNew = true;
    isSaved = true;
    path = "";
    fileName = "Untitled.bmf";
}

Map::Map(string p, string fName) {
    isNew = false;
    isSaved = true;
    path = p;
    fileName = fName;
}

Map::~Map() {
    this->distroy();
}

void Map::create(XMLNode node) {
    XMLNode child;

    // Load background image
    child = node.getChild("background");
    background.image = new Image(child, "image");
    XMLNode bgChild = child.getChild("parallax");
    if (!bgChild.isEmpty()) {
        background.parallaxX = stringTo<int>(bgChild.getAttribute("x"));
        background.parallaxY = stringTo<int>(bgChild.getAttribute("y"));
    }
    else {
        background.parallaxX = background.parallaxY = 0;
    }
    bgChild = child.getChild("scroll");
    if (!bgChild.isEmpty()) {
        background.scrollX = stringTo<int>(bgChild.getAttribute("x"));
        background.scrollY = stringTo<int>(bgChild.getAttribute("y"));
    }
    else {
        background.scrollX = background.scrollY = 0;
    }
    bgChild = child.getChild("wrapMode");
    string type = bgChild.getAttribute("x");
    if (type == "NONE")         background.wrapX = NONE;
    else if (type == "TILE")    background.wrapX = TILE;
    else if (type == "STRETCH") background.wrapX = STRETCH;
    type = bgChild.getAttribute("y");
    if (type == "NONE")         background.wrapY = NONE;
    else if (type == "TILE")    background.wrapY = TILE;
    else if (type == "STRETCH") background.wrapY = STRETCH;

    // Load tiles from composite images
    const int numImages = node.numChild("cimg");
    for (int i = 0; i < numImages; i++) {
        cimages.push_back(new CompositeImage(node.getChild("cimg",i)));
    }

    const int numTiles = node.numChild("tileList");
    MapTile* temp;
    for (int i = 0; i < numTiles; i++) {
        child = node.getChild("tileList",i);
        //### Runtime error here, it seems to be in MapTile construction
        temp = new MapTile();
        temp->set = cimages[stringTo<int>(child.getAttribute("imgId"))-1];
        temp->imgTile = temp->set->tiles[stringTo<int>(child.getAttribute("tileId"))-1];
        temp->x = stringTo<int>(child.getAttribute("x"));
        temp->y = stringTo<int>(child.getAttribute("y"));
        tiles.push_back(temp);
    }
}

void Map::createNew() {
    distroy();    // Delete old map
    name = "Untitled";  description = "";
    fileName = "Untitled"; path = "";
    width = 800; height = 600;
    viewX = 0;   viewY = 0;
    zoom = 1.0f; maxPlayers = 8;
    background.image = NULL;
    background.wrapX = NONE;
    background.wrapY = NONE;
    background.parallaxX = background.parallaxY = 0;
    background.scrollX   = background.scrollY   = 0;
}

void Map::distroy() {
    cimages.clear();
    tiles.clear();
    delete background.image;
    delete preview;
}

void Map::draw(Graphics &g) {
    if (background.image)
        //### Here is where the image is drawn
        g.drawImage(background.image, 0, 0, 400, 300);

    for (int i = 0; i < tiles.size(); i++) {
        // This should work but is just for testing at the moment
        g.drawImage(tiles->set->image, tiles->x, tiles->y);
    }
}

/* Loads data from an XML file */
void Map::load() {
    XMLNode node = XMLNode::readFile((path+"/"+fileName).c_str());

    viewX = 0;  viewY = 0;
    name = node.getChild("name").getText();
    description = node.getChild("description").getText();
    width = stringTo<int>(node.getChild("size").getAttribute("w"));
    height = stringTo<int>(node.getChild("size").getAttribute("h"));
    preview = new Image(node, "preview");
    this->create(node);
}


/////////////////////////////////////////////////////////////////////
// File: Map.h                                                     //
/////////////////////////////////////////////////////////////////////

#ifndef _MAP_H_
#define _MAP_H_

#include "Graphics.h"
#include "Tile.h"


/* Terrain type and RGB colour */
enum TerrainType {
    TER_NONE  = 0,  TER_COL_NONE  = 0x00000000,   // Black
    TER_SOLID = 1,  TER_COL_SOLID = 0x00FFFFFF,   // White
    TER_DIRT  = 2,  TER_COL_DIRT  = 0x00FF0000,   // Red
    TER_ROCK  = 3,  TER_COL_ROCK  = 0x000000FF,   // Blue
    TER_SAND  = 4,  TER_COL_SAND  = 0x00FFFF00    // Yellow
};


class Map {
public:
    bool isNew, isSaved;
    string path, fileName;           // Directory where map data is
    string name, description;        // Name of the map and a short description
    Image* preview;                  // Preview image to show in GUI
    int width, height;               // Size of map (px)
    int viewX, viewY;                // Position of map in view
    float zoom;                      // Zoom factor
    int maxPlayers;                  // Maximum number of players
    struct {
        Image* image;                // Background image
        int parallaxX, parallaxY;    // Parallax speed
        int scrollX, scrollY;        // Scroll speed
        int wrapX, wrapY;            // Tile or stretch background
    } background;
    vector<CompositeImage*> cimages; // Array of tile sets (Composite Images)
    vector<MapTile*> tiles;          // Array of tiles on the map

    Map();
    Map(string path, string file);
    ~Map();
    inline void setPath(string p) { path = p; }
    inline void setFileName(string n) { fileName = n; }
    inline void setFullName(string full) {
        path =  full.substr(0, full.rfind('\\'));
        fileName =  full.substr(full.rfind('\\')+1);
    }
    inline void setFullName(string p, string n) {
        path = p; fileName = n;
    }
    void load();
    void save();
    void create(XMLNode node);
    void createNew();
    void distroy();
    void draw(Graphics &g);
};

#endif  // _MAP_H_


/////////////////////////////////////////////////////////////////////
// File: Tile.cpp                                                  //
// Date: 17/8/08                                                   //
// Desc: Defines a map tile object and composite image (tile set)  //
//                                                                 //
/////////////////////////////////////////////////////////////////////

#include "Main.h"
#include "Map.h"
#include "Tile.h"

CompositeImage::CompositeImage(XMLNode node) {
    load(node);
}

CompositeImage::~CompositeImage() {
    delete image;  // Delete image data
    delete terrain;
    tiles.clear();
}

void CompositeImage::load(XMLNode node) {
    numTiles = node.numChild("tile");
    int ch;      // Number of channels the original image had

    // Load image and terrain data
    image = new Image(node.getAttribute("img"));
    terrain = new Image(node.getAttribute("terrain"));

    XMLNode child;
    ImageTile* temp;
    for (int i = 0; i < numTiles; i++) {
        child = node.getChild("tile", i);
        temp = new ImageTile();
        temp->x = stringTo<int>(child.getAttribute("x"));
        temp->y = stringTo<int>(child.getAttribute("y"));
        temp->width = stringTo<int>(child.getAttribute("w"));
        temp->height = stringTo<int>(child.getAttribute("h"));
        tiles.push_back(temp);
    }
}


/////////////////////////////////////////////////////////////////////
// File: Tile.h                                                    //
// Date: 17/8/08                                                   //
// Desc: Defines a map tile object and composite image (tile set)  //
//                                                                 //
/////////////////////////////////////////////////////////////////////

#ifndef _TILE_H_
#define _TILE_H_


class MapTile;
class CompositeImage;

class ImageTile {
public:
    int x, y;            // Position of tile on composite image
    int width, height;   // Size of tile

    ImageTile() : x(0), y(0), width(0), height(0) {}
    ImageTile(int x, int y, int w, int h) :
         x(x), y(y), width(w), height(h) {}
    ~ImageTile() {}
};

class MapTile {
public:
    int x, y;            // Position of tile on map
    CompositeImage* set; // Tile set this belongs to
    ImageTile* imgTile;

    MapTile() : set(NULL), imgTile(NULL), x(0), y(0) {}
    MapTile(CompositeImage* cimg, ImageTile* imgTile, int x, int y) :
         set(cimg), imgTile(imgTile), x(x), y(y) {}
    ~MapTile() {}
};

class CompositeImage {
public:
    int width, height;
    Image* image;             // Image data
    Image* terrain;           // Terrain map
    vector<ImageTile*> tiles; // List of tiles on this image
    int numTiles;

    CompositeImage(XMLNode node);
    ~CompositeImage();
    void load(XMLNode node);
};

#endif  // _TILE_H_

The full source code can be found here. I have tried everything to find the problem but unfortunately i have no idea what's going wrong. Any help would be appreciated. thanks [Edited by - XTAL256 on August 20, 2008 4:22:10 AM]

Gage64

Aug 19, 2008 at 4:25 AM

If you declare it like you showed, than it is just a global, not a static global (there's a difference). BTW, where is this object declared?

Now, if this is indeed a static initialization dependency issue, then you can solve it as follows:

Map & getMap() {    static Map map;    return map;}

Now code like this:

map.func();

Becomes this:

getMap().func();

Or, if you need to make more than one function call:

Map ↦ = getMap();map.func1();map.func2();

Note that this function should not be inline; See here and here for more info and potential problems.

I would encourage you to get rid of the global, but others will give a better argument for this than I can.

XTAL256

Aug 19, 2008 at 5:44 AM

Quote:
Original post by Gage64
If you declare it like you showed, than it is just a global, not a static global (there's a difference).

Oh, yeah. there are so many different meanings of static, it's confusing.

Quote:
BTW, where is this object declared?

I forgot to mention that in my first post. I will edit the code in the post, but basically i declared it at global scope in Main.cpp and declared as extern in the header file which Map.cpp includes.

Quote:
Now, if this is indeed a static initialisation dependency issue...

Well i'm not sure. As far as i know, that only happens if one object is initialised at runtime but uses another object that is also initialised at runtime but hasn't been init'd yet. And my code seems to work fine until i attempt to load a map. Of course, that problem may stem from a static initialisation issue but i can't find any such issue, can anyone else?

Gage64

Aug 19, 2008 at 6:15 AM

This is just a shot in the dark, but many of your objects use raw pointers (not a good thing, BTW), which means that their default copy constructors and assignment operators won't work properly. It might be that such a copy constructor or assignment operator is called somewhere, which causes the crash.

To fix this, implement appropriate versions for object that need to be copied or assigned, and disallow copy/assignment for objects that don't.

On a slightly related note, pass your objects to functions by const reference as much as possible. This is both safer and more efficient.

Also, can you post the exact line that causes the crash? (and the function containing that line)

XTAL256

Aug 19, 2008 at 8:14 AM

By "raw" pointers, do you mean declarations like this:
Map map; (as opposed to a pointer Map* map = new or null)
I usually use pointers but most of the code is recycled from an older project so i just changed the classes and names but still declared objects in that way (which is how it is done in the GUI API i am using, RAD-C++). Now i understand pointers a bit better but i figured since it worked in the previous project it should work now. Unfortunately it's not and it may be because of an assignment operator like you said. Although i am using pointers to all members of the Map class. The map is made up of a number of tile images, each of which come from one or more tile sets (a single large image with lots of tiles on it). I store these as pointers in a std::vector, so assignment operators should work there.

As for the const reference, i will try to keep that in mind (i try to pass std::strings as const whenever possible).

Oh, and the exact line that causes the crash. I keep forgetting to mention that [embarrass]. I have edited my first post and marked the line with //***

Gage64

Aug 19, 2008 at 8:56 AM

By "raw pointers" I meant something like:

MyObject *obj;

As opposed to:

boost::shared_ptr obj;

Using smart pointers like this is less error-prone and can prevent memory leaks.

Quote:
i try to pass std::strings as const whenever possible

All the strings in your code are passed by (non-const) value.

About the crash - does the code enter MapTile's constructor? If not, it's possible that new throws a bad_alloc exception (though I kinda doubt it). Try surrounding your code in a try { ... } catch (std::exception &) block and see if you catch anything.

BTW, what is the exact error message that you are getting?

XTAL256

Aug 19, 2008 at 7:20 PM

Quote:
Original post by Gage64
By "raw pointers" I meant something like:
MyObject *obj;
As opposed to:
boost::shared_ptr obj;

Oh, i see. Well what do you call it when you don't use a pointer, i.e.
MyObject obj;? Is there a technical name for that type of declaration/usage?

As for the strings, i am recycling most of the code from an older project as i mentioned before, so i just left most of the stuff the way it is. I did go through my main project and put const in the function parameters once i realised that passing a constant string is better. I learned C++ a few years ago but i am still learning [grin].

Back to my problem. Dev-C++'s debugger is not as good as Visual Studio's (which i am using for my main project but can't for this because the GUI library only works with gcc), so i can't view the disassembly or anything but when i stepped through it seemed to crash at the new allocator. The error i get is:
Microsoft Visual C++ Runtime Library Runtime Error! Program: ...\BEdit.exe The application has requested the Runtime to terminate... etc.

theOcelot

Aug 19, 2008 at 9:13 PM

Code::Blocks can use GCC, will let you view the assembly, and is generally a much better deal than Dev-C++, not to mention being free and open-source. You might consider switching.

I bet that error message means there is an uncaught exception.

Just out of curiosity, what GUI library is that?

XTAL256

Aug 19, 2008 at 10:04 PM

Problem solved! I was trying to access the wrong XML node [embarrass]! I thought it might have been something stupid like that but i only checked that i was accessing the correct attributes, i forgot about the node. I store the tiles like this:

I got the tileList node but forgot to get each tileElement node.

But that still doesn't explain why the background image is blank. The only reason i can think of is that i have declared 2 different objects in separate translation units (i.e declared static in .h then included in two or more .cpp files), but i thought i had fixed that.

@theOcelot: yeah, i downloaded CB once before to compile a project that used it but i got rid of it since i already had Dev-C++. Maybe i should switch.
And the GUI lib i use is RAD-C++. It's a free Windows API wrapper library, it encapsulates Win32 controls in C++ classes and makes GUI creation a whole lot easier. I learned Java as my first programming language so i am used to making GUIs using Java's GUI classes and RAD-C++ is very similar. Unfortunately it's a one man project and he doesn't have much time to work on it so although there is a Visual Studio build of the library, it is very buggy at the moment.

theOcelot

Aug 20, 2008 at 1:49 AM

So it's a windows based library, that doesn't work with MS compilers? [lol] I guess that's because it comes with .a libraries, but surely you could build it from source with an MS compiler? Oh well.

XTAL256

Aug 20, 2008 at 4:21 AM

Problem 2 solved. The reason why the images where blank (i.e. not drawn) is because i didn't switch the graphics context to the map window. See i have 2 OpenGL graphics contexts, one for the selection window which is used to select a tile from a tile set, and the second for drawing the map. I switch contexts when i draw the map but i forgot to do it when i create the images, which is why they didn't get created properly.

@theOcelot: it does work with MSVC but it has a lot of bugs. It was originally built for gcc (i.e. it's a .a library) but the author made version 2.0 for a lot more compilers (MSVC, Borland, others i think). The problem is that the MSVC one kept giving runtime errors and it doesn't work as well with the OpenGL windows. And the author has a lot of other work related stuff going on so he hasn't been able to fix the bugs.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.