Take the 2-minute tour ×
Game Development Stack Exchange is a question and answer site for professional and independent game developers. It's 100% free, no registration required.

My app is crashing on SDL_BlitSurface() and i can't figure out why. I think it has something to do with my static object. If you read the code you'll why I think so.

This happens when the limits of the map are reached, i.e. (i>width || j>height). This is the code:

Map.cpp (this render)

Tile const * Map::getTyle(int i, int j) const {
    if (i >= 0 && j >= 0 && i < width && j < height) {
        return data[i][j];
    } else {
        return &Tile::ERROR_TYLE;  // This makes SDL_BlitSurface (called later) crash
        //return new Tile(TileType::ERROR); // This works with not problem (but is memory leak, of course)
    }
}


void Map::render(int x, int y, int width, int height) const {
    //DEBUG("(Rendering...) x: "<<x<<", y: "<<y<<", width: "<<width<<", height: "<<height);
    int firstI = x / TileType::PIXEL_PER_TILE;
    int firstJ = y / TileType::PIXEL_PER_TILE;
    int lastI = (x+width) / TileType::PIXEL_PER_TILE;
    int lastJ = (y+height) / TileType::PIXEL_PER_TILE;

    // The previous integer division rounds down when dealing with positive values, but it rounds up
    // negative values. This is a fix for that (We need those values always rounded down)
    if (firstI < 0) {
        firstI--;
    }

    if (firstJ < 0) {
        firstJ--;
    }

    const int firstX = x;
    const int firstY = y;

    SDL_Rect srcRect;
    SDL_Rect dstRect;

    for (int i=firstI; i <= lastI; i++) {
        for (int j=firstJ; j <= lastJ; j++) {

            if (i*TileType::PIXEL_PER_TILE < x) {
                srcRect.x = x % TileType::PIXEL_PER_TILE;
                srcRect.w = TileType::PIXEL_PER_TILE - (x % TileType::PIXEL_PER_TILE);
                dstRect.x = i*TileType::PIXEL_PER_TILE + (x % TileType::PIXEL_PER_TILE) - firstX;
            } else if (i*TileType::PIXEL_PER_TILE >= x + width) {
                srcRect.x = 0;
                srcRect.w = x % TileType::PIXEL_PER_TILE;
                dstRect.x = i*TileType::PIXEL_PER_TILE - firstX;
            } else {
                srcRect.x = 0;
                srcRect.w = TileType::PIXEL_PER_TILE;
                dstRect.x = i*TileType::PIXEL_PER_TILE - firstX;
            }

            if (j*TileType::PIXEL_PER_TILE < y) {
                srcRect.y = 0;
                srcRect.h = TileType::PIXEL_PER_TILE - (y % TileType::PIXEL_PER_TILE);
                dstRect.y = j*TileType::PIXEL_PER_TILE + (y % TileType::PIXEL_PER_TILE) - firstY;
            } else if (j*TileType::PIXEL_PER_TILE >= y + height) {
                srcRect.y = y % TileType::PIXEL_PER_TILE;
                srcRect.h = y % TileType::PIXEL_PER_TILE;
                dstRect.y = j*TileType::PIXEL_PER_TILE - firstY;
            } else {
                srcRect.y = 0;
                srcRect.h = TileType::PIXEL_PER_TILE;
                dstRect.y = j*TileType::PIXEL_PER_TILE - firstY;
            }

            SDL::YtoSDL(dstRect.y, srcRect.h);

            SDL_BlitSurface(getTyle(i,j)->getType()->getSurface(), &srcRect, SDL::getScreen(), &dstRect); // <-- Crash HERE

            /*DEBUG("i = "<<i<<", j = "<<j);
            DEBUG("srcRect.x = "<<srcRect.x<<", srcRect.y = "<<srcRect.y<<", srcRect.w = "<<srcRect.w<<", srcRect.h = "<<srcRect.h);
            DEBUG("dstRect.x = "<<dstRect.x<<", dstRect.y = "<<dstRect.y);*/
        }
    }
}

Tile.h

#ifndef TILE_H
#define TILE_H

#include "TileType.h"

class Tile {
private:
    TileType const * type;
public:

    static const Tile ERROR_TYLE;

    Tile(TileType const * t);
    ~Tile();

    TileType const * getType() const;
};

#endif

Tile.cpp

#include "Tile.h"

const Tile Tile::ERROR_TYLE(TileType::ERROR);

Tile::Tile(TileType const * t) : type(t) {}

Tile::~Tile() {}

TileType const * Tile::getType() const {
    return type;
}

TileType.h

#ifndef TILETYPE_H
#define TILETYPE_H

#include "SDL.h"
#include "DEBUG.h"

class TileType {
protected:
    TileType();
    ~TileType();

public:
    static const int PIXEL_PER_TILE = 30;

    static const TileType * ERROR;
    static const TileType * AIR;
    static const TileType * SOLID;

    virtual SDL_Surface * getSurface() const = 0;

    virtual bool isSolid(int x, int y) const = 0;
};

#endif

ErrorTyle.h

#ifndef ERRORTILE_H
#define ERRORTILE_H

#include "TileType.h"

class ErrorTile : public TileType {

    friend class TileType;
private:
    ErrorTile();

    mutable SDL_Surface * surface;

    static const char * FILE_PATH;
public:
    SDL_Surface * getSurface() const;
    bool isSolid(int x, int y) const ;
};

#endif

ErrorTyle.cpp

(The surface can't be loaded when building the object, because it is a static object and SDL_Init() needs to be called first)

#include "ErrorTile.h"

const char * ErrorTile::FILE_PATH = ("C:\\error.bmp");

ErrorTile::ErrorTile() : TileType(), surface(NULL) {}

SDL_Surface * ErrorTile::getSurface() const {
    if (surface == NULL) {

        if (SDL::isOn()) {
            surface = SDL::loadAndOptimice(ErrorTile::FILE_PATH);
            if (surface->w != TileType::PIXEL_PER_TILE || surface->h != TileType::PIXEL_PER_TILE) {
                WARNING("Bad tile surface size");
            }
        } else {
            ERROR("Trying to load a surface, but SDL is not on");
        }
    }
    if (surface == NULL) { // This if doesn't get called, so surface != NULL
        ERROR("WTF? Can't load surface :\\"); 
    }

    return surface;
}

bool ErrorTile::isSolid(int x, int y) const {
    return true;
}
share|improve this question
add comment

closed as too localized by Trevor Powell, Byte56, Josh Petrie, Ali.S, bummzack Dec 16 '12 at 13:07

This question is unlikely to help any future visitors; it is only relevant to a small geographic area, a specific moment in time, or an extraordinarily narrow situation that is not generally applicable to the worldwide audience of the internet. For help making this question more broadly applicable, visit the help center.If this question can be reworded to fit the rules in the help center, please edit the question.

3 Answers

up vote 2 down vote accepted

I suspect these are never initialized:

static const TileType * ERROR;
static const TileType * AIR;
static const TileType * SOLID;

You haven't shown code with them ever being initialized. Even if they are, you're likely suffering from the "static initialization fiasco" outlaid here: http://www.parashift.com/c++-faq/static-init-order.html

In general, I'd shy away from all this static data. But for now, I'll note there's a workaround: http://www.parashift.com/c++-faq/static-init-order-on-first-use.html

If this is not the problem, I recommend comparing the address given to you in the access violation or segfault with any and all of the pointers involved in the line of code that's crashing, including the implicit "this" pointer used to access member variables. While it likely won't match exactly due to member variable offsets and the like, this will likely point you to the exact pointer that's causing the crash when dereferenced.

share|improve this answer
add comment

The only thing I can imagine in this situation, is that you are trying to access data far beyond the array limit, just check this...

share|improve this answer
add comment

MaulingMonkey's answer has the gist of it.

Very specifically, Tile::ERROR_TILE is being initialized by storing a copy of Tile::ERROR's value, but this is total garbage (because Tile::ERROR hasn't yet been initialized). Namely:

// this object is constructed before main() is invoked
// and TileType::ERROR is just an uninitialized pointer
// which you're copying into Tile::ERROR_TYPE.type
const Tile Tile::ERROR_TYLE(TileType::ERROR);

As some unsolicited advice, the entire structure of the code is just... awkward. If you find yourself wanting to use the "mutable" keyword, you should pretty much rethink everything you're doing. In almost 20 years of programming C++, I've never once had a legitimate use for that keyword; anything it can solve you can solve by a better abstraction (which may well mean less abstraction).

In your case, I'd keep a TileDatabase that just mapped simple integer IDs to TileType values. It can even just be a simple std::vector wrapper that returns a reference to the error tile for out-of-bound indices (e.g. invalid tile IDs).

You've got a ton of different objects/types defined, which is a usual sign of newer coders who've just learned about object-oriented programming but haven't learned the do's and dont's yet.

Remember: just because you have some conceptual "object" doesn't mean that it deserves its own C++ class. C++ classes and interfaces should focus on how things work in the algorithm and do not focus on how a human would categorize conceptual entities. In other words, don't make overly "concrete" classes, but rather focus on algorithmic abstractions. It takes a lot of practice and experience to really start understanding what that really means. A classical treatise of the subject is the Coffee Maker example, with a good explanation available at http://web.archive.org/web/20081227131937/http://www.objectmentor.com/resources/articles/CoffeeMaker.pdf

share|improve this answer
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.