Quote:Original post by Arreis
1- Calls the normal constructor of the Block class to create a new object.
2- Calls the copy constructor on that object, and obtain a duplicate.
3- Adds that copy to the end of the vector.
Close, there's a bit more to it ...
In the code: blocks.push_back( Block(...) ); :
1- Create a temporary Block with parameters ...
2- blocks copy-constructs the temporary object
3- blocks adds the new object to the end of the list
4- The temporary Block is destructed when the function exits.
The problem is that during a Block copy-constructor (that your compiler is filling for you automatically), it's also copying the
pointer to the sprite and collision box, not the actual sprite and collision box. At the beginning of step three, there are two Blocks that point to one sprite and one collision box. As soon as one of those Blocks are destructed, that one collision box and sprite are deleted, turning the other Block's pointers into invalid pointers.
Quote:Original post by Arreis
So I create new objects, and the push_back() function creates copies of them, which should be ok as long as the destructor for the original object is not called.
Yup. As the push_back() function exits, the temporary Block object used to create the copy for blocks is destroyed, deleting the pointers, and thus making the copy's pointers invalid. The simplest solution would be for Sprite to allocate a copy of the data held by the copy:
class Sprite{public: Sprite(int x, int y); Sprite( const Sprite& copy ) { this->x = copy.x; this->y = copy.y; // this->collBox = copy.collBox; // Does not copy the object // this->image = copy.image; // Does not copy the object // Now this Sprite class has a collBox to call its own this->collBox = new CollisionBox( *copy.collBox ); // It'd be safer to create the image here, instead of copying the image directly this->image = selectColor( /* ... */ ); }};
Quote:Original post by Arreis
However, since all the objects are created on local context, they are deleted sooner or later and the copy is left with several inconsistent pointers. Is that correct? Is that what was happening?
If by that you mean that the temporary object passed in to push_back, then yes, that's what happens. Here are a few more things you can try:
1) Provide a copy constructor to deal with those pointers in Sprite (and/or Block, depending if those have pointers as well).
2) Use a level of indirection; use std::vector<Block*> instead of std::vector<Block>. This method would probably be better since is has more of a Java-feel to it:
typedef std::vector<Block*> BlockContainer;BlockContainer blocks;for (int i = 0; i < LINES_PER_LEVEL; i++) for (int j = 0; j < BLOCKS_PER_LINE; j++) blocks.push_back( new Block(...) ); // Push back a *copy of the pointer* to the allocated memory// When accessing:blocks.front()->collBox;blocks[0]->collBox;blocks.at(0)->collBox;// When removing a block:for ( BlockContainerType::iterator it = blocks.begin(), end = blocks.end(); it != end; ++it ){ if ( /* collision */ ) { delete *it; // Delete the memory it = blocks.erase( it ); // Erase the node }}// And on cleanup:for ( BlockContainer::iterator it = blocks.begin(), end = blocks.end(); it != end; ++it ) delete *it; // Delete the rest of them
Hope this helps!
