List Problem

Started by
44 comments, last by Antonym 15 years, 4 months ago
I made modifications to the code... my brain is about to hit a meltdown.. The same problem has pop up again and I don't know why. Asoon as it compiles it crashes telling me an iterator can't be incremented. I can't see what's wrong with it this time.. help please...

list<object> gameObjects;list<object> removeObjects;void updateObjects(list<object>* gameObjects){		list<object>::iterator it;	list<object>::iterator it2;	for(it = gameObjects->begin(); it != gameObjects->end(); it++){		const object &one = *it;		for(it2 = it + 1 ; it2 != gameObjects->end() ; it2++)		{			const object &two = *it2;			if(isColliding(one,two))			{				removeObjects.push_back(*it);			}		}	}	for(it = removeObjects.begin() ; it != removeObjects.end() ; it++){		gameObjects->erase(it);	}	return;}bool isColliding(const object &one, const object &two){	return(one.pos.x < two.pos.x + two.width &&		   one.pos.x > two.pos.x - two.width &&		   one.pos.y < two.pos.y + two.height &&		   one.pos.y > two.pos.y - two.height);}
Advertisement
You have 3 for loops, you only need two. Also, this is incorrect:
for(it2 = it2++ ; // ...

Read visage's post again, paying careful attention to the variables used.
Oops for that, caught it soon and removed it, the problem is still present.

Edit: How could I miss that bad, bad me..

I tried changing the it2++ to it + 1 and it gave me more error messages :S..

1>c:\documents and settings\david\mis documentos\visual studio 2005\projects\dungeon\dungeon\logic.cpp(116) : error C2784: 'std::_String_iterator<_Elem,_Traits,_Alloc> std::operator +(_String_iterator<_Elem,_Traits,_Alloc>::difference_type,std::_String_iterator<_Elem,_Traits,_Alloc>)' : could not deduce template argument for 'std::_String_iterator<_Elem,_Traits,_Alloc>' from 'int'
1> c:\archivos de programa\microsoft visual studio 8\vc\include\xstring(438) : see declaration of 'std::operator +'
1>c:\documents and settings\david\mis documentos\visual studio 2005\projects\dungeon\dungeon\logic.cpp(116) : error C2784: 'std::_String_const_iterator<_Elem,_Traits,_Alloc> std::operator +(_String_const_iterator<_Elem,_Traits,_Alloc>::difference_type,std::_String_const_iterator<_Elem,_Traits,_Alloc>)' : could not deduce template argument for 'std::_String_const_iterator<_Elem,_Traits,_Alloc>' from 'int'
1> c:\archivos de programa\microsoft visual studio 8\vc\include\xstring(298) : see declaration of 'std::operator +'
1>c:\documents and settings\david\mis documentos\visual studio 2005\projects\dungeon\dungeon\logic.cpp(116) : error C2784: 'std::reverse_iterator<_RanIt> std::operator +(_Diff,const std::reverse_iterator<_RanIt> &)' : could not deduce template argument for 'const std::reverse_iterator<_RanIt> &' from 'int'
1> c:\archivos de programa\microsoft visual studio 8\vc\include\xutility(1809) : see declaration of 'std::operator +'
1>c:\documents and settings\david\mis documentos\visual studio 2005\projects\dungeon\dungeon\logic.cpp(116) : error C2676: binary '+' : 'std::list<_Ty>::_Iterator<_Secure_validation>' does not define this operator or a conversion to a type acceptable to the predefined operator
1> with
1> [
1> _Ty=object,
1> _Secure_validation=true
1> ]
std::list isn't a random access container. This means that you can't use operator + with iterators from std::list. You could change to a different container, like std::vector<>. Alternatively, something like this:
for(it = gameObjects->begin(); it != gameObjects->end(); it++){    const object &one = *it;    it2 = it;    // skip the current element    ++it2;    for(/* nothing here */ ; it2 != gameObjects->end() ; it2++)    {        // ...    }}
Quote:Original post by Antonym
I've found that I learn way better from trial and error rather than reading books.


Don't you think your experience in this thread (and, for that matter, the need to start it in the first place) kind of contradicts that evaluation? :)

Hello I changed from list to vector but well I still have sort of the same problem and have run out of ideas..

It tells me vector erase iterator is out of range.. I compile it and it works until I try shooting with the ship, the bullet shows up for a split second and then the program crashes. I think the problem is still in this part since it mentions vectors and I don't use vectors on any other part of the program..

void updateObjects(vector<object>* gameObjects){		vector<object>::iterator it;	vector<object>::iterator it2;	for(it = gameObjects->begin() ; it != gameObjects->end() ; it++)	{		const object &one = *it;		for(it2 = gameObjects->begin() ; it2 != gameObjects->end() ; it2++)		{			if(it2 != it){				const object &two = *it2;				if(isColliding(one,two))				{					removeObjects.push_back(*it);				}			}		}	}	for(it = removeObjects.begin() ; it != removeObjects.end() ; it++){		gameObjects->erase(it);	}	return;}bool isColliding(const object &one, const object &two){	return(one.pos.x < two.pos.x + two.width &&		   one.pos.x > two.pos.x - two.width &&		   one.pos.y < two.pos.y + two.height &&		   one.pos.y > two.pos.y - two.height);}
Why not just use a for loop?

for(int i = 0; i < gameObjects.size(); i++)
{
if(collide)
{
gameObjects.erase(gameObjects.begin()+i);
i--;
}
}
The problem is that you are calling erase on one vector with an iterator from a different one.

Here is how I would do it:
#include <vector>#include <algorithm>struct collision_functor{	const object *obj;	collision_functor(const object &o)	:		obj(&o)	{	}	bool operator()(const object &other) const	{		return isColliding(*obj,other);	}};typedef std::vector<object> object_list;void updateObjects(object_list &objects){	object_list::iterator end = objects.end();	for(object_list::iterator it = objects.begin() ; it < end ; it++)	{		const object &one = *it;		end = std::remove_if(it + 1,end,collision_functor(one));	}	objects.erase(end,objects.end());}

This code relies on std::remove_if. This function takes a range of objects and a predicate (a function that takes an argument and returns a boolean). The predicate is applied to every element in the range. Elements for which the predicate returns false are retained. An iterator is returned to the (smaller) range in which the predicate doesn't hold. Elements beyond that point are unspecified.

The predicate needed to be stateful, so I used a functor (an object that can be used as a function through an overload of operator()). This code is reasonably efficient, as the erase is only done once, and the elements in the range to be erased are all at the end of the vector.

The only issue left is whether you want both colliding objects to be removed. Typically, you need a little bit of logic to work this out. For example, if a bullet hits a player we don't remove the player if they still have health left.

This revised version shows this. Some functions are mocked up for testing purposes:
#include <string>#include <vector>#include <iostream>#include <algorithm>#include <functional>struct object{	object()	:		lives((rand() % 3) + 1)	{	}	bool dead() const { return lives <= 0; }	static void handleCollision(object &one, object &two)	{		one.handleCollision(two);		two.handleCollision(one);	}private:	void handleCollision(const object &other)	{		if(lives <= other.lives)		{			--lives;		}	}	int lives;};bool isColliding(const object &,const object &){	return rand() < (RAND_MAX / 4);}struct collision_functor{	object *obj;	collision_functor(object &o)	:		obj(&o)	{	}	void operator()(object &other)	{		if(isColliding(*obj,other))		{			object::handleCollision(*obj,other);		}	}};typedef std::vector<object> object_list;void updateObjects(object_list &objects){	object_list::iterator end = objects.end();	for(object_list::iterator it = objects.begin() ; it < end ; it++)	{		object &one = *it;		std::for_each(it + 1,end,collision_functor(one));	}	end = std::remove_if(objects.begin(),end,std::mem_fun_ref(&object::dead));	objects.erase(end,objects.end());}int main(){	object_list objects(100);	while(objects.size() > 1)	{		updateObjects(objects);		std::cout << "Now have " << objects.size() << " objects";		std::string line;		std::getline(std::cin,line);	}}
Yay it worked! Thank you very much. Tho I have a couple of questions, this is all quite advanced for me.

Are you using std:: to show that it belongs to a library that has been added or is there another reason to leave it in?

The other questions are in comment form at the following code.
struct collision_functor{	const object *obj;	//This function's statements are odd, 2 dots, an object with a parameter and empty brackets?	collision_functor(const object &o)	:	obj(&o)	{	}	//When/how is this function(?) called? I don't think this a conventional function is it?	bool operator()(const object &other) const	{		return isColliding(*obj, other);	}};	vector<object>::iterator end = gameObjects->end();	for(vector<object>::iterator it = gameObjects->begin() ; it < end ; it++)	{		const object &one = *it;		//How does this line work again? I didnt quite understood the explanation wouldn't end be reset at each iteration? //Also the way the start of the range is set up(it + 1) won't it end up skipping the first element of the vector? //Testing it that seems to be the case, the ship isnt affected by collision which is the first object to tbe added to the vector.		end = remove_if(it + 1, end, collision_functor(one));	}	gameObjects->erase(end, gameObjects->end());
std:: is used to access the namespace std and unless you add "using namespace std;" to the code you have to do that.

This topic is closed to new replies.

Advertisement