Back to For Beginners

Game design dilemma

For Beginners

Started by sheep19 February 11, 2008 08:11 AM

5 comments, last by sheep19 16 years, 2 months ago

sheep19

494

Author

February 11, 2008 08:11 AM

Hi, I'm making a game and I'm not sure how to store things that are on the screen. At the moment I'm doing this:


std::vector <Fruit*> fruits;
Fruit aFruit(200, 200, APPLE); //Fruit is an object, the parameters are integers
fruits.push_back( aFruit );
    
    if( fruits.size() > 0 )
    {
        for( std::vector <Fruit*>::const_iterator iter = fruits.begin(); iter < fruits.end(); ++iter )
        {
             applySurface( (*iter)->ReturnX(), (*iter)->ReturnY(), (*iter)->ReturnImage(), screen );
        }
    }

So, I'm adding their addresses in a vector of pointers use the above code to draw them. Is it a better idea to store them on the heap?


std::vector <Fruit*> fruits;
fruits.push_back( new Fruit( 100, 100, APPLE ) );
// blah blah blah

//edit:Here's the function that's supposed to clean up the memory on the heap
void cleanUp( std::vector <Fruit*>* const fruits)
{
     if( (*fruits).size() > 0 )
    {
        for( std::vector <Fruit*>::const_iterator iter = (*fruits).begin(); iter < (*fruits).end(); ++iter )
        {
             delete *iter;
        }
    }
}   

//it doesn't seem to work though.. (when I check if fruits[0] == NULL, it's not

Thanks in advance. [Edited by - sheep19 on February 11, 2008 8:56:30 AM]

My Portfolio

dmatter

4,872

February 11, 2008 11:17 AM

In the first example you should be pushing the address of the fruit:

fruits.push_back( &aFruit );

In the second example you ought to pass by reference instead of a constant pointer:

void cleanUp(const std::vector <Fruit*>& fruits)

The decision between heap and stack allocations in C++ is usually largely down to scope and lifetimes. In the first example if aFruit goes out of scope then any pointers to it will be dangling. In the second example individual fruit instances are allowed to exceed the lifetime of the scope that they were constructed in, which is frequently an advantageous thing.

In this example I don't see why the vector needs to be storing pointers at all, you could simply have:

std::vector<Fruit> fruits;fruits.push_back( Fruit(200, 200, APPLE) );

David Gill :: GitHub :: twitter .

sheep19

494

Author

February 11, 2008 12:10 PM

Quote:Original post by dmatter
In this example I don't see why the vector needs to be storing pointers at all, you could simply have:
std::vector<Fruit> fruits;fruits.push_back( Fruit(200, 200, APPLE) );

You are right, I didn't think of that. Thanks a lot!

-----------

Actually there's an error. If I do this:

std::vector<Fruit> fruits;fruits.push_back( Fruit(200, 200, APPLE) );

then the program flashes quickly and exits.

std::vector<Fruit> fruits;    fruits.push_back( Fruit(200, 200, APPLE) );        if( fruits.size() > 0 )    {        for( std::vector <Fruit>::const_iterator iter = fruits.begin(); iter < fruits.end(); ++iter )        {             applySurface( iter->ReturnX(), iter->ReturnY(), iter->ReturnImage(), screen );        }    }

----

If I do this:

std::vector<Fruit*> fruits;    fruits.push_back( new Fruit(200, 200, APPLE) );        if( fruits.size() > 0 )    {        for( std::vector <Fruit*>::const_iterator iter = (*fruits).begin(); iter < (*fruits).end(); ++iter )        {             applySurface( (*iter)->ReturnX(), (*iter)->ReturnY(), (*iter)->ReturnImage(), screen );        }    }/*...*/void cleanUp( std::vector <Fruit*>& fruits){     if( (fruits).size() > 0 )    {        for( std::vector <Fruit*>::const_iterator iter = fruits.begin(); iter < fruits.end(); ++iter )        {             delete *iter;        }    }}

Are the surfaces actually deleted?

[Edited by - sheep19 on February 11, 2008 12:10:35 PM]

My Portfolio

dmatter

4,872

February 11, 2008 01:02 PM

Quote:Original post by sheep19
Actually there's an error. If I do this:
std::vector<Fruit> fruits;fruits.push_back( Fruit(200, 200, APPLE) );
then the program flashes quickly and exits.

*** Source Snippet Removed ***

The code looks fine-ish (iter < fruits.end() will only work for vectors, you'd typically have iter != fruits.end() instead).
The problem must be elsewhere. If it worked before then it's likely to do with your Fruit class which must be properly and safely copy-constructable! If for example Fruit(200, 200, APPLE) allocates memory and deallocates it the destructor and Fruit doesn't have a suitable copy constructor then you can't safely insert it into an STL container directly.

Quote:If I do this:
*** Source Snippet Removed ***

Are the surfaces actually deleted?

Assuming a Fruit is a surface then yes they will be deleted.
In that snippet you're not correctly accessing the vector in the top-most for-loop - you're deferencing it like as if it's a pointer when it isn't.

David Gill :: GitHub :: twitter .

sheep19

494

Author

February 11, 2008 01:21 PM

Quote:The code looks fine-ish (iter < fruits.end() will only work for vectors, you'd typically have iter != fruits.end() instead).
The problem must be elsewhere. If it worked before then it's likely to do with your Fruit class which must be properly and safely copy-constructable! If for example Fruit(200, 200, APPLE) allocates memory and deallocates it the destructor and Fruit doesn't have a suitable copy constructor then you can't safely insert it into an STL container directly.

I haven't declared a copy constructor. I thought I had to do it when I use the heap inside the class. (Which I'm not doing.)
--

Quote:
Assuming a Fruit is a surface then yes they will be deleted.
In that snippet you're not correctly accessing the vector in the top-most for-loop - you're deferencing it like as if it's a pointer when it isn't.

Why Am I not accessing it correctly? How should it be?

That's the Fruit class:

class Fruit{public:      Fruit( int x = 0, int y = 0, const int type = 0 );   ~Fruit();   SDL_Surface* ReturnImage() const;   int ReturnX() const;   int ReturnY() const;   private:   SDL_Surface* m_Image;   int m_X, m_Y, m_Type;};#include "SDL/SDL.h"#include "declarations.h"Fruit::Fruit( int x, int y, const int type ): m_X(x), m_Y(y), m_Type(type){     if( type == APPLE )     {         m_Image = loadImage( "apple.bmp" );     }}Fruit::~Fruit(){     SDL_FreeSurface(m_Image);}SDL_Surface* Fruit::ReturnImage() const{    return m_Image;}int Fruit::ReturnX() const{    return m_X;}int Fruit::ReturnY() const{    return m_Y;}

Thanks a lot for your help :)

My Portfolio

dmatter

4,872

February 11, 2008 01:49 PM

Quote:Original post by sheep19
I haven't declared a copy constructor. I thought I had to do it when I use the heap inside the class. (Which I'm not doing.)

But you are:

m_Image = loadImage( "apple.bmp" );

You also delete the image in the class' destructor. This means that the following line:

fruits.push_back( Fruit(200, 200, APPLE) );

will allocate an image, copy a Fruit instance along with a pointer to that image into the vector and then destroy the memory referenced by that pointer leaving a dangling m_Image pointer - hence the crash.
Simply put, your Fruit class isn't copy-safe.

I don't know SDL but a quick google reveals that SDL_Surface has a reference counter which is used for managing situations like this. I'm not sure if you're meant to fiddle with the reference count yourself or whether there's an SDL smart-pointer as part of the library.

The thing to note here is that you're not making proper use of the RAII idiom, which is almost practically required for putting things into a vector. Now the fact is that this Fruit class can make use of RAII properly (using, for example an SDL smart-pointer). Alternatively you can side-step the issue by using a vector of pointers, as you have done already.

Quote:Why Am I not accessing it correctly? How should it be?

The code you posted was like this:

std::vector<Fruit*> fruits;/*  ...*/        for( std::vector <Fruit*>::const_iterator iter = (*fruits).begin(); iter < (*fruits).end(); ++iter )

It ought to be like this:

std::vector<Fruit*> fruits;/*  ...*/        for( std::vector <Fruit*>::const_iterator iter = fruits.begin(); iter != fruits.end(); ++iter )

Nothing major, I think you knew this already, I think it was just a casual error.

David Gill :: GitHub :: twitter .

sheep19

494

Author

February 11, 2008 03:08 PM

I understand. I'm going to use the vector of pointers to avoid this problem, because my class will need more tweaking otherwise.
I can't believe I learned about constructors, copy constructors and destructors three days ago. :-/

Thanks a lot for your help, I REALLY appreciate it. :)

My Portfolio

Game design dilemma

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Game design dilemma

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines