Jump to content
  • Advertisement
Sign in to follow this  
squashed_bug

c++ std::list

This topic is 4132 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

	std::list<IWorldObject*>::iterator it;
	for(it = objects.begin(); it != objects.end(); it++){
		(*it)->Update();
	
	}

The problem is (*it)->Update() may remove itself from the objects list, if this occurs on the last element in the list then the iterator becomes invalid and when the iterator is incremented the "list iterator not incrementable" assertion fails. Anyway to work around this?

Share this post


Link to post
Share on other sites
Advertisement
Quote:
Original post by squashed_bug
*** Source Snippet Removed ***

The problem is (*it)->Update() may remove itself from the objects list, if this occurs on the last element in the list then the iterator becomes invalid and when the iterator is incremented the "list iterator not incrementable" assertion fails.

Anyway to work around this?
How exactly do the objects remove themselves from the list? Perhaps you could post that code so we could see what it is exactly that you're doing.

In any case, that's probably not the best way to go about it. An alternative you might consider is to have the Update() function return a Boolean value indicating whether the object on which it's called is still active after the update (that is, whether it's still 'alive').

Using this method you can write something like this (note: not tested or compiled):
for (...::iterator it = objects.begin(); it != objects.end(); ) {
if ((*it)->Update()) {
++it;
} else {
delete *it;
it = objects.erase(it);
}
}
An even better alternative would be to make use of the 'erase-remove' idiom. However, we begin to see a problem with your current method; the 'raw' pointers that you're storing are not self-managing, and having to delete them manually complicates things and leaves a great deal of room for error. Fortunately, there's a solution to that problem too :)

Share this post


Link to post
Share on other sites
I have been using this..

std::list<IWorldObject*>::iterator it, jt;
for(it = objects.begin(), jt = it, jt++; it != objects.end(); it = jt++){
(*it)->Update();
if(jt == objects.end()) break;
}


which does solve the imediate problem.


The objects themselves don't know about the lists they're in, the removal on the list is in the IWorldObject deconstructor.

Ill give you some code pick over..


class IWorldObject;

class World : public Engine::ITask{
private:
std::list<IDrawable*> drawables;
std::list<IWorldObject*> objects;
public:
void Update();
ICamera *camera;
void AddDrawable(IDrawable *d);
void AddObject(IWorldObject *object);
void RemoveObject(IWorldObject *object);
void RemoveDrawable(IDrawable *d);



};

class IWorldObject : public Physics::IPhysicalObject{
public:
World *world;
IWorldObject(World *world);
~IWorldObject();
};

#include "engine.h"


void World::Update(){

std::list<IWorldObject*>::iterator it, jt;
for(it = objects.begin(), jt = it, jt++; it != objects.end(); it = jt++){
(*it)->Update();
if(jt == objects.end()) break;
}

// Draw the objects
std::list<IDrawable*>::iterator kt;
glLoadIdentity();
camera->SetView();
for(kt = drawables.begin(); kt != drawables.end(); kt++){
glPushMatrix();
(*kt)->Draw();
glPopMatrix();
}
}

void World::AddDrawable(IDrawable *d){
drawables.push_back(d);
}
void World::AddObject(IWorldObject *object){
AddDrawable(object);
objects.push_back(object);
}

void World::RemoveObject(IWorldObject *object){
RemoveDrawable(object);
objects.remove(object);
}

void World::RemoveDrawable(IDrawable *d){
drawables.remove(d);
}

IWorldObject::IWorldObject(World *world){
this->world = world;
world->AddObject(this);
}
IWorldObject::~IWorldObject(){
world->RemoveObject(this);
}



Do you think it would be better to set a 'isAlive' attribute in IWorldObject then calling delete(this).
eg

// note: conc is a sub class of IWorldObject
void Conc::Update(){
if(isExploded){
delete(this); // or isAlive = true;
} else {
if(timer.Expired()) Explode();
}
}



hmm maybe it would be best just to implement some memory management.

Thanks for the help.

Share this post


Link to post
Share on other sites
Using:
delete(this)
on the member function of an object that is stored in a container would cause a disastrous double-delete if it were not that you are storing only pointers, not objects.

Just do what jyk suggests. In fact I have used this exact same approach for this exact same problem (updating objects in my world list), and it works nicely!

[Edited by - iMalc on March 23, 2007 11:52:18 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by squashed_bug
The problem is (*it)->Update() may remove itself from the objects list


"Doctor, it hurts when I do that."

Objects shouldn't know about their containers. After all, you might some day want to use that object outside of a container :s

A standard solution is to make Update() communicate back whether or not the object should be removed, and then use the update function as a predicate for removing objects via the erase-remove idiom. Another problem is storing raw pointers for polymorphism; you'll have a much more fun time with some kind of smart pointer.


// return true if alive after the update.
bool IWorldObject::Update();

typedef boost::shared_ptr<IWorldObject> objhandle;
bool aliveObject(objhandle& handle) {
return handle->Update();
}

std::list<objhandle> objects;
objects.erase(std::remove_if(objects.begin(), objects.end(), std::not1(aliveObject)), objects.end());

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!