[SDL/C++] Broke my draw code somehow.

Started by
3 comments, last by theOcelot 14 years, 4 months ago
This had been working previously; however, I completely rewrote how I stored and instanced gamedata, and also rewrote the related loader functions. Game data is structured as such: * There is a SpriteSheet class, which stores image data and has relevant draw functions. The image data is comprised of an SDL_Surface pointer, information on how large an individual "frame" of a sprite sheet is in pixels, and how large a sprite sheet is in number of frames. Additionally, this includes a draw function, which displays a given frame from the sprite sheet at a certain position on the screen. * There is an Instance class, which represents a game actor as it exists on the map. It contains information related to where the actor is located on the map, its current state, and a pointer to an object of type GameData (see below). The instance class includes a draw method, which calls the draw function of its includes GameData pointer, passing a pointer to itself as an argument. * There is a GameData class, which stores purely-data objects. In this case, the only things they're really storing are a pointer to a SpriteSheet object, a frame number for that sprite sheet, and a string containing a lua script filename. GameData also includes a draw method. Draw takes a pointer to an object of type Instances. From this, it gets the instance's x and y coordinates, and then passes those, as well as the frame number from the GameData object, to the SpriteSheet's draw function. The intended functionality is that there's a list of instances and a while loop iterating through these instances, calling each instances draw() function as it goes. When the instance's draw function is called, it passes a pointer of itself to its GameData's draw function, which gets relevant positioning data from that pointer, and calls its SpriteSheet's draw function to actually blit the proper image to the screen. However, running the program leads to none of the instances actually being drawn. I know SDL itself is work, since I have another variant on the SpriteSheet class that just outputs text from a bitmap font, and that displays fine. I've done a few tests and I'm not seeing any of the SpriteSheets, Instances, or GameDatas going out of scope, so all the pointer passing and loading functionality and whatever else seems to be in line (For instance, I checked the *name* of the SpriteSheets at every point in the draw function chain where it would be used, and the data remained consistent and what it was supposed to be. It's possible that the actual SDL_Surfaces in the SpriteSheets themselves are getting mangled, though). I THINK I'm having issues locating the rendering surface for drawing to but I'm not really sure. Also, actually calling the draw function from main would just be: instanceIterator->draw(); inside of a while(instanceIterator != instances.end()) loop. Code: SpriteSheet header:

#ifndef SPRITE
#define SPRITE

#include "SDL.h"

#include <string>
#include <vector>
#include <cstdarg>
#include <cstdio>

using namespace std;

// Abstract sprite class.
class Sprite {
public:
	virtual void draw() {}
};

// ******************************************************
// * A simple sprite is a single-image, non-animating   *
// * sprite whose only functionality is to be drawn at  *
// * a given location.                                  *
// ******************************************************
class SimpleSprite : public Sprite {
private:
	SDL_Surface* image;
public:

	// Constructor: Simply sets the image surface to the parameter *_image.
	SimpleSprite(SDL_Surface *_image) {
		image = _image; }

    // Destructor: frees the image surface.
    ~SimpleSprite() { SDL_FreeSurface(image); }

    // Draw image at posX by posY on the drawing surface.
    void draw(int posX, int posY);

};

class SpriteSheet : public Sprite {
protected:

	SDL_Surface* image;                         // Pointer to an SDL_Surface containing the sprite's image.

	int sheetHeight;                            // These two variables refer to the width and height
	int sheetWidth;                             // of the sprite sheet *in number of frames.*

	int frameHeight;                            // These two variables give the height and width for an
	int frameWidth;                             // individual sprite frame in the sprite sheet *in pixels.*

	string sheetName;                           // Used for identifying the tileset in question.

public:

	SpriteSheet(string _sheetName,              // Constructor.
        SDL_Surface *_image, int _sheetHeight, int _sheetWidth, int _frameHeight, int _frameWidth) {
		    sheetName = _sheetName;
		    image = _image;
		    sheetHeight = _sheetHeight;
		    sheetWidth = _sheetWidth;
		    frameHeight = _frameHeight;
		    frameWidth = _frameWidth;
		    SDL_SetColorKey(image, SDL_SRCCOLORKEY,
                SDL_MapRGB(image->format,
                0xFF, 0x00, 0xFF)
            );
        }

    SpriteSheet() {}                            // Default constructor.

    ~SpriteSheet() { SDL_FreeSurface(image); }  // Destructor. frees the image surface.

	virtual void draw(int frameNum, int posX, int posY); // Draw a sprite frame from a given
                                                               // row at position x, y.

	string name() const { return sheetName; }   // Return the asset's identifier name.

    bool operator==(const SpriteSheet &rhs) const   // Operator==. Used in find algorithm.
    { if (this->sheetName == rhs.sheetName) {
        return true; }
      else { return false; }
    }

    bool operator==(const string &rhs) const   // Operator==. Used in find algorithm.
    { if (this->sheetName == rhs) {
        return true; }
      else { return false; }
    }
};

// ******************************************************
// * FontSprites are used for handling bitmap fonts.    *
// * They contain a spritesheet representing bitmap     *
// * font and their draw function takes a string of text*
// * which is then displaye at some place on the screen.*
// ******************************************************
class TextRenderer : public SpriteSheet {
    // Stores the current/last position of the 'cursor.'
    int cursorX;
    int cursorY;

    // Divides the text up and re-creates it according to the max linelength.
    void splitText(string text, vector<string> &lines, int lineLength);

	// Draw a string of text at position X, Y of lenght lineLength.
	void subDraw(int posX, int posY, int lineLength, const string &text);
public:
	TextRenderer(string _sheetName, SDL_Surface *_image, int _sheetHeight, int _sheetWidth,
		int _frameHeight, int _frameWidth) {
		    sheetName = _sheetName;
		    image = _image;
		    sheetHeight = _sheetHeight;
		    sheetWidth = _sheetWidth;
		    frameHeight = _frameHeight;
		    frameWidth = _frameWidth;
		    SDL_SetColorKey(image, SDL_SRCCOLORKEY, SDL_MapRGB(image->format, 0xFF, 0x00, 0xFF));
		}

    ~TextRenderer() { SDL_FreeSurface(image); }

    // Format a text string, then call something to draw it to the screen.
	void draw(int posX, int posY, int lineLength, const string &text, ...);


	// Other functions to maybe add: one with a max lines function that prompts
	// the user to press enter to see more lines.

    // Should I add newline support..?
};

#endif



SpriteSheet.cpp

// SpriteSheet function definitions.

void SpriteSheet::draw(int frameNum, int posX, int posY) {
    SDL_Rect clipRect;
    SDL_Rect destPos;
    int column, row;

	destPos.x = posX;
	destPos.y = posY;

	// Determine which frame from the sheet to draw.
	row = frameNum/sheetWidth;
    // Special condition for access the last image in a
    // given row of the sprite sheet.
    if((frameNum % sheetWidth) == 0) {
        column = sheetWidth - 1;
        //row--;
    }
    else {
        column = (frameNum % sheetWidth) - 1;
    }

    // Assign these values to the clipping rectangle.
	clipRect.h = frameHeight;
	clipRect.w = frameWidth;
	clipRect.x = column * frameWidth;
	clipRect.y = row * frameHeight;

    // Draw the sprite.
    SDL_BlitSurface(image, &clipRect, SDL_GetVideoSurface(), &destPos);
}



GameData header:

class GameData {
protected:
    string dName;                               // Name of the GameData. Used for identification/instancing.
    string behaviorScript;                      // Scriptfile defining the GameData's behavior.
    SpriteSheet* spriteSheet;                   // Spritesheet containing the GameData's image.
    int frame;                                  // Specific frame from the spritesheet containing that image.
public:
    GameData(string _dName, string _behaviorScript, SpriteSheet* _spriteSheet, int _frame) {
        dName = _dName;
        behaviorScript = _behaviorScript;
        spriteSheet = _spriteSheet;
        frame = _frame;
    }

    string name() const { return dName; }       // Returns dName.

    virtual int update(Instance* myInst);       // Runs the update logic.
    void draw(Instance* myInst);                // Draws the object on the screen.

    bool operator==(const GameData &rhs) const  // Operator==. Used in find algorithm.
    { if (this->dName == rhs.dName) {
        return true; }
      else { return false; }
    }

    bool operator==(const string &rhs) const  // Operator==. Used in find algorithm.
    { if (this->dName == rhs) {
        return true; }
      else { return false; }
    }

    //
    // DEBUG FUNCTIONS - DO NOT CALL DURING ACTUAL GAME. DELETE SOMETIME.
    //

    string mySheetName() const
    {
        return spriteSheet->name();
    }
};



GameData draw code:

void GameData::draw(Instance* myInst) {
    int xpos, ypos;
    myInst->coordinates(xpos, ypos);
    spriteSheet->draw(frame, xpos, ypos);
}



Instance header:

#ifndef INSTANCE
#define INSTANCE

/*
An Instance represents the existance of a unique copy of some Data object within the
current level. All Instances have code for positioning them, setting their activity,
etc. Additionally, each Instance takes a pointer to a new copy of a GameData object.
When update() and draw() are invoked, the Instance passes a pointer to itself (this)
to the update() and draw() functions of its controlling GameData object.

This solution is based on the Strategy pattern. In this case a GameData object is
the concrete strategy, while the Instance is the context.

This way, the Instance class remains a fairly clean and immutable interface to the
underling GameData object. The Instance does not need to know anything about its
connected GameData object, and whatever the GameData object needs to know about its
instances is handled through the Instance's public interface.
*/

#include <string>
#include <list>
#include <vector>

using namespace std;

#include "GlobalDefs.h"

class GameData;

#define MAX_FLAGS 10

class Instance {
private:
    int iID;                                    // The specific ID for the instance.
    int iActive;                                // Whether or not the instance should updated.
    int iHidden;                                // Whether or not the instance should be drawn.
    int iDeletion;                              // Whether or not the instance needs to be deleted.
    int drawOrder;                              // What “layer"/order the instance is drawn in.
    int xpos, ypos;                             // The current position of the instance.
    int lastx, lasty;                           // The previous position of the instance.
    vector<bool> iFlags;                        // Set of flags indicating different contexts for the Behaviors.
    list<vector<int> > eventQueue;              // Queue of events being recieved by the Instance.
    GameData* instanceData;                     // The data associated with the instance.
public:

    // Create statement for text file:
    // GameData_name xpos ypos iActive
    // So, player 100 200 1; would create a player at 100 x 200, and would start them as active.
    Instance(int _iID, int _iActive, int _drawOrder, int _xpos, int _ypos, GameData* _instanceData) {
        iID = _iID;
        iActive = _iActive;
        iDeletion = 0;
        drawOrder = _drawOrder;
        xpos = _xpos;
        ypos = _ypos;
        instanceData = _instanceData;
        iFlags.resize(MAX_FLAGS);
    }

    int ID()                                    // Return iID.
        { return iID; }

    int active()                                // Return iActive.
        { return iActive; }

    void setActive(bool n);                     // Toggle whether the Instance is active.

    int hidden()                                // Return iHidden.
        { return iHidden; }

    void setHidden(bool n)                      // Set the hidden state for the Instance.
        { iHidden = n; }

    int deletion()                              // Return iDeletion.
        { return iDeletion; }

    void setDeletion(bool n)                    // Toggle whether the Instance should be deleted.
        { setActive(0);
          iDeletion = n; }

    void coordinates(int &xval, int &yval)      // Sets the arguments to the Instance object's
        { xval = xpos; yval = ypos; }           // current coordinates.

    void setpos(int xval, int yval)             // Set coordinates of the Instance object.
        { this->lastx = xpos; this->lasty = ypos; // to the value of the arguments.
          this->xpos = xval; this->ypos = yval;}

    void moveback()                             // Reset the Instance object to its last position.
        { this->xpos = this->lastx; this->ypos = this->lasty; }

    void setCollision(bool n)                   // Enables or disables collision detection.
        { if(n) { globalCollisionMap.add(iID, xpos, ypos); }
          else { globalCollisionMap.remove(iID); } }

    void recieveEvent(int agentID, int eventID, int arg = 0); // Recieve an event ID and the ID
                                                              // of the agent of this event, then
                                                              // add them to the eventQueue.

    int readEvent(int &agentID, int &eventID, int &arg); // Show the event properties for the first
                                                         // item in the eventQueue. Then, remove
                                                         // that event and return the remaining
                                                         // number of events.

    void clearEvents()                          // Empty the eventqueue.
    { eventQueue.clear(); }

    const vector<bool>& flags()                 // Returns a reference to the flags.
        { return iFlags; }                      // to some outside function.

    void setFlags(vector<bool> newFlags)        // Sets the Instance's flags.
        { iFlags = newFlags; }

    int update();                               // Calls instanceData's update method, which takes a
                                                // pointer to the containing Instance.

    void draw();                                // Calls the instanceData's draw method, as per above.

    bool operator==(int test)                   // Operator==. Used in find algorithm.
    { if (this->iID == test) {
        return true; }
      else { return false; }
    }

    bool operator<(const Instance &rhs)         // Operator<.
    { if (this->drawOrder < rhs.drawOrder) {
        return true; }
      else { return false; }
    }

    bool operator>(const Instance &rhs)         // Operator<.
    { if (this->drawOrder > rhs.drawOrder) {
        return true; }
      else { return false; }
    }

    //
    // DEBUG FUNCTIONS - DO NOT CALL DURING ACTUAL GAME.
    //

    string mySheetName() const;
};

#endif



Instance draw code:

void Instance::draw() {
    instanceData->draw(this);
}



Advertisement
Okay, it looks like the destructor for every object I'm loading gets called at some point, and I'm not entirely sure why. Anyway, I had an SDL_FreeSurface call in the SpriteSheet destructor, so...

Here's my load function. Any idea why the destructor is getting invoked?

int loadArtAssets(list<SpriteSheet> &artAssets, const string &filename) { // Done.    ifstream manifest;    string readin;    string params[ART_PARAMS];    SDL_Surface* loadFile;    // Open the file.    manifest.open(filename.c_str());    // Make sure file opened correctly.    if(!manifest) {        printf("Error opening filename: %s\n", filename.c_str());        return 1;    }    // While not at the end of the file...    while(getline(manifest, readin)) {        // Parse the string into its subcomponents        tokenize(readin, params, ART_PARAMS);        printf("Creating art asset: %s...\n", readin.c_str());        loadFile = IMG_Load(params[artFILENAME].c_str());        if(loadFile == NULL) {            printf("Error loading file: %s\n", params[artFILENAME].c_str());            return 1;        }        // Add the tileset to the art asset vector.        artAssets.push_back(            SpriteSheet(                params[artNAME],                IMG_Load(params[artFILENAME].c_str()),                atoi(params[artS_HEIGHT].c_str()),                atoi(params[artS_WIDTH].c_str()),                atoi(params[artF_HEIGHT].c_str()),                atoi(params[artF_WIDTH].c_str())            )        );    }    // Confirm successful creation of asset.    printf("Art assets successfully loaded.\n");    return 0;}


[Edited by - MeshGearFox on December 6, 2009 9:55:39 PM]
It's hard to read all that code. It's better to try to cut it down. You probably could have just posted SpriteSheet::draw with short snippets of the rest, and a short description of how it code fits in.

I didn't do an in-depth analysis, but there are a couple things that jumped out at me. For one thing, none of the sprite subclasses are actually overriding Sprite::draw(). They all have different argument lists, and thus are different functions, thus making your Sprite class basically pointless.

The problem seems to center around SpriteSheet::draw. The way I do sprite-sheets is to pre-calculate all the source rectangles and put them in a list indexed by the frame number. There might be something wrong with your algorithm for "clipRect"'s. If there was, I'm not sure how that would manifest. Have you gone into this section of code with a debugger and looked at the values of the SDL_Rects?

Also, if what changed is your loading code, there might be something wrong with how the SpriteSheet in the GameData is generated. Do you know for sure what's on that surface? Is there transparency involved? How exactly is the "not drawing" manifesting itself? Is the screen just staying blank? What other infrastructure is there?

I noticed that SpriteSheet frees its surface when it didn't allocate it. Does the rest of your code "know" that SpriteSheet takes ownership of the surface like that? Usually, the best idea is to free stuff in about the same place as it was allocated, and to make it very, very clear when this is not the case, and to have a very good reason. Ownership issues might not have to do with your current problem, but they will bite you eventually.

Anyway, sorry for the OT. Make sure, with a debugger, if you haven't already:
- draw() is called on every Instance. A ridiculous number of hard bugs are ultimately caused by something dumb like a loop whose condition starts out false or something like that. I know from experience. [rolleyes]
- In GameData::draw, reasonable values are being passed to SpriteSheet::draw. When I first saw that function I thought, "What the crud, undefined variables? Oh, references." [grin]. But make sure anyway.
- In SpriteSheet::draw, reasonable values are being assigned all around. Honestly, though, I would recommend the pre-calculation approach. As long as spritesheets are shared, the storage overhead is negligible, and it makes everything simpler. I'll post my implementation (or links to it) if you want.
Quote:They all have different argument lists, and thus are different functions, thus making your Sprite class basically pointless.


I noticed this last night. I originally wrote the Sprite classes for something else completely, and my original intent was to have a couple different groups of sprites with properties and functions associated with them.

Quote:The problem seems to center around SpriteSheet::draw. The way I do sprite-sheets is to pre-calculate all the source rectangles and put them in a list indexed by the frame number. There might be something wrong with your algorithm for "clipRect"'s.


No, draw works fine. Actually...

Quote:I noticed that SpriteSheet frees its surface when it didn't allocate it. Does the rest of your code "know" that SpriteSheet takes ownership of the surface like that? Usually, the best idea is to free stuff in about the same place as it was allocated, and to make it very, very clear when this is not the case, and to have a very good reason. Ownership issues might not have to do with your current problem, but they will bite you eventually.


This was the EXACT problem, actually, although I'm not sure why the destructor was getting called. I have a decent idea why, though. I'm assuming that the change I made to the loading code is doing it.

Originally I was storing pointers of objects and pushing back objects that were explicitly declared new. The way I'm doing it now, I THINK what's happening is that the list is creating some temporary object, copying it, and deleting the temporary object I passed in. Or something along those lines.

Anyway, so essentially, since I had the SpriteSheet freeing its surface in the destructor, the destructor was getting called, freeing the surface, and when I tried to draw from the surface... well, nothing.

Also why the program was crashing on exit. It was trying to delete the SDL_Surfaces that had already been freed.
Cool. I guess I still managed to misunderstand your main problem [lol]. Anyway, I'm glad I could help.

By the way, are you familiar with the Rule of Three? You might just want to privatize the copy constructor and operator= for SpriteSheet, or even just for Sprite. If your theory about temporaries is right, that will at least point you to the problem.

This topic is closed to new replies.

Advertisement