Sign in to follow this  

Vectors won't work: "error C2664"

This topic is 3628 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

For some reason, I can't get push_back to work with vectors at all. I tried this simple code:
std::vector<int> x;
x.push_back(new int);
But I get error C2664: 'std::vector<_Ty>::push_back' : cannot convert parameter 1 from 'int *' to 'const int &' I've looked around and that seems to be the way that vectors are used in tutorials and I can't seem to find anything to fix this. What's happening?

Share this post


Link to post
Share on other sites
Error is pretty much bang on here, the compiler doesn't know how to make an integer from an integer pointer.

For a vector with template T, std::vector<T>::push_back() takes a const T reference.

Either you want a vector of integer pointers (unlikely), or you want to push_back an actual integer:

x.push_back(42);

Share this post


Link to post
Share on other sites
K...

But what I really wanted to do was use it with a user-defined class so I could make an object of that class when something happens and push it back on the vector. How's that done exactly? I saw some tutorials that seemed to be like I was doing :o

Even stranger, when I push_back a number, I get

error LNK2019: unresolved external symbol __imp___CrtDbgReportW referenced in function "public: __thiscall std::_Vector_const_iterator<i...

And it goes on for ages but you get the idea. Must be something fairly simple but I haven't been able to notice anything strange or anything that I know to normally result in an unresolved external...

Share this post


Link to post
Share on other sites
Hmm, checked around and the problem seems to be a build error.

Changing the Code Generation Runtime Library to Multi-Threaded Debug DLL fixes it...

One question though: I'm using SDL and in Lazy Foo's tutorials he says to set it to Multi-Threaded DLL instead. Will the debug DLL cause a problem with SDL?

Share this post


Link to post
Share on other sites
When programming with SDL you should always use the multithreaded dll versions of the runtime library. In a debug build you would use the Multithreaded Debug DLL while in a Release build you would use the Multithreaded DLL (no debug).

Share this post


Link to post
Share on other sites
So I just use the Multithreaded Debug DLL for programming in SDL? K.

As for using new with push_back(), how do you make it work?

EDIT: Would you have to make it a vector of pointers to the objects you want to store?

Share this post


Link to post
Share on other sites
Jep, that's correct. Keep in mind that the vector won't delete the object that is pointed at when you remove the pointer from the vector, it only removes the pointer itself. You are still responsible for delete'ing what you new'ed.

Share this post


Link to post
Share on other sites
Quote:
So I just use the Multithreaded Debug DLL for programming in SDL? K.


No, you use the Debug version when you are making a Debug build, you use the Release version when you are making a Release build.

Share this post


Link to post
Share on other sites
Quote:
Original post by Captain P
Jep, that's correct. Keep in mind that the vector won't delete the object that is pointed at when you remove the pointer from the vector, it only removes the pointer itself. You are still responsible for delete'ing what you new'ed.


Hmm... that seems a bit of work then... well probably just that I haven't really used pointers or dynamic allocation much before ^_^

Basically I wanted to put explosions in my Space Invaders clone that would be created and put into a vector whenever they were needed. Up to now, I'd been using static arrays for such things (like my enemies themselves) but that involved keeping all 55 of my enemies in existence even when they were no longer required or blitted to the screen, which I'm sure is terrible nooby style.

Alright, so if I want to have a vector of explosions like that, I just need to create a vector of pointers to explosions and then use the -> operator with those to change the explosions and when the explosion is finished I remove the explosion from the vector and use delete on the pointer?

Looks like this game'll be much more of a learning exercise than I'd at first anticipated. Yay I suppose :O

Quote:
Original post by CodeMunkie
Quote:
So I just use the Multithreaded Debug DLL for programming in SDL? K.


No, you use the Debug version when you are making a Debug build, you use the Release version when you are making a Release build.


K. As long as SDL won't mess up because of using the Debug DLL... it should all be fine :)

Share this post


Link to post
Share on other sites
Alright, I've actually got the explosions working now. Whenever I kill an enemy, an explosion pops up and animates perfectly.

Yay, thanks for the help all ^_^

Now for the bad part; I need help erasing the pointer from the vector and deleting it. Basically I have 5 frames in the animation, 0-4. When the frame counter gets to 5, I want to remove the explosion.

I've heard of iterators but do I need one for this?

I used normal ints fine for things like this for example:

	for(unsigned int i = 0; i < explosions.size(); i++)
{
explosions.at(i)->animate();
}


But this doesn't work:

void Explosion::manageExpl()
{
std::vector<Explosion>::iterator it;

for(unsigned int i = 0; i < explosions.size(); i++)
{
if( explosions.at(i)->frame == 5 )
{
explosions.erase(explosions.begin());
delete explosions.at(i);
}
}
}


It compiles fine but when that finishes I get an exception:

Unhandled exception at 0x7c812a5b in Spice_Invaders.exe: Microsoft C++ exception: std::out_of_range at memory location 0x0012f7ec..

I'm sure I'm doing something real stupid since I haven't worked with either pointers or vectors a lot, much less both at the same time. Any ideas?

Share this post


Link to post
Share on other sites
EDIT: You're better off using std::list, which is made to allow to easy insertion/deletion.

You shouldn't change the vector/list while you're iterating over it, esp. not deleting. Because if you delete an item from the vector, it will move the other items, and therefor, you might miss items that should animate.

What you can do best, is to create a 2nd list in which you store all the items you wish to remove, and once you're done processing the list, you remove those from the actual vector(Preferably end -> front, because you can delete by index then).

Example code:

std::list<Explosions *> explosions;
std::list<int> deletedEntries;
for(unsigned int i = 0; i < explosions.size(); i++)
{
Explosion *expl = explosion[i]; // Note the use of the index operator
expl.animate();
if (expl.frames == 5)
deletedEntries.push_front(i); // Just store the index
explosions.at(i)->animate();
}

if (deletedEntries.size() != 0)
{
for (std::list<int>::iterator it = deletedEntries.begin(); it != deletedEntries.end(); ++it)
{
explosions.remove(explosions[*it]);
}
}
deletedEntries.clear();




However, I notice you're using std::vector<Explosion>. If you're using explosions.push_back(new Explosion(...)), that might cause problems(As it should be std::vector<Explosion *>). If you don't use new, then there is no need for a delete.

Toolmaker

Share this post


Link to post
Share on other sites
Well, don't know why you want to clear the vector but(won't you use it again?!)... You can call std::vector::clear() method to remove all the elements from the vector but and you said, you need to delete each element first to avoid the memory leak. You can do it with iterators or not:


//without iterators
for(unsigned int i = 0; i<explosions.size(); ++i)
delete explosions[i];//theres no need to use the at() method, you can use this overloaded operator []

explosions.clear();

//or using iterators(if you don't know they have overloaded operators to behave like pointers)
std::vector<Explosion> iterator it;
std::vector<Explosion> iterator end;
it = explosions.begin();
end = explosions.end();

while(it != end)
{
delete *it;
++it;
}

explosions.clear();



hope this helps and hope I didn't speak shits...

Share this post


Link to post
Share on other sites
A little extra addition here:
Which std::list and std::vector basically both offer the same features(More or less, not exactly), they're made for different purposes.

std::vector is in essence a dynamic array: It's easy to walk over, but it's harder to add/remove items. As the vector grows, there's a time the vector needs more memory then it has(Say, it allocated 10 entries, but it needs an 11th). In that case, it will create a new piece of memory which is larger than it needs(Say, 15 items), copies over all the data to the new memory and frees the previous memory.

The same accounts for removing: If you delete item 0 and it has stored 11 items, it will take the remaining 10 items, and move to the front(Overwriting item 0). This is costly. If you have to add data every now and then and not a lot, but want good performance when iterating over the items, you use vector.

std::list is a linked list. In essence, it's like a chain neckless. Each item stored in the array is a chain, with a link(Or pointer in this case) to the next item. Iterating over the list is more costly, as it has to walk all the links in order to find a specific item you requested with list[index]. But adding is simple: Create a new link, find the place where you need to be, and put it in(So, in the worst case, it has to walk n / 2 items).

Therefor, if you need to add/remove data often, you go with std::list, because it's fast in adding/removing. It's still fast when accessing the data, but slower than std::vector.

Hopes this helps in the future.

Toolmaker

Share this post


Link to post
Share on other sites
As others have mentioned, modifying a container class while iterating over it is a bad idea. It's like removing stairs from the staircase you're walking on.

I'd probably settle for something like this:

// A function that returns true when an Explosion object needs to be removed.
// It also deletes that Explosion object. That feels a bit hacky, but it works. *cough*

bool NeedsRemoval(Explosion* explosion)
{
if(explosion->frame == 5)
{
delete explosion;
return true;
}
else
return false;
}

// Elsewhere, we use the remove_if algorithm. We let it run over the explosions vector
// from start to end, and for each object, it runs the NeedsRemoval function.
// If that function returns true for the passed object, it will be removed from the vector.
// However, since this algorithm can't actually erase objects from the vector, it
// relocates them to the end of the vector, and returns an iterator that points at
// the first 'removed' object. Erasing all objects from that iterator to the end
// actually eradicates them from the vector. Phew.

explosions.erase(remove_if(explosions.begin(), explosions.end(), NeedsRemoval), explosions.end());

Share this post


Link to post
Share on other sites
Thanks all, that was very helpful for understanding these container classes :D

I only got around today to actually trying to fix the problem properly, but when I had a bit of a mess around with the code just to see what would happen, I found an amazingly simple solution.

Noting what xissburg said, I just tried to swap the commands around for curiosity so that now I have this:

void Explosion::manageExpl()
{
for(unsigned int i = 0; i < explosions.size(); i++)
{
if( explosions.at(i)->frame == 5 )
{
delete explosions.at(i); //Used to be on the next line
explosions.erase(explosions.begin()); //Used to be above
}
}
}


And now it all works completely perfectly. The explosions appear, animate and disappear when they're finished without any glitches.

Is this a perfectly fine way to code this? Or would it cause problems in other cases or otherwise be bad practice?

Share this post


Link to post
Share on other sites
You're probably just getting lucky.

The erase method returns an iterator to the next valid element, so use a while loop:


void Explosion::manageExpl()
{
std::vector<Explosion*>::iterator it( explosions.begin() );
while ( it != explosions.end() )
{
Explosion* pExp = *it;

if ( pExp->frame == 5 )
{
delete pExp;
it = explosions.erase( it );
}
else
{
++it;
}
}
}


Any performance hints from above still apply though, a vector will copy all elements after the one that's getting erased.

Share this post


Link to post
Share on other sites
To elaborate on the "probably just getting lucky" comment - you're searching through the container looking for one to delete, and when you find it, you delete it properly but then always erase the first pointer in the container. What you really need to do is erase the pointer you just deleted! That's what Endurion's solution does.

Share this post


Link to post
Share on other sites
Yeah, that's something I noticed and was wondering about...

My solution was replacing explosions.begin() with explosions.begin() + i.

That would work fine too, wouldn't it? Since if it's at position 0 it won't change and if it's at 1 it'll go to 1 after the beginning?

I think why it was working though must have been because it's always the oldest explosion that's deleted since the only criteria for it to be deleted is it having gone through its animation sequence, so the one to delete in this case will always actually be the first. Pure luck that it worked I'll admit though ;) Still, it's often confusing at first coming back to code you haven't looked at in a few days so I didn't bother changing that right away even though it seemed wrong...

Share this post


Link to post
Share on other sites

This topic is 3628 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this