void World::UpdateDecals()
{
// get time between this and last frame
float fDeltaTime = Core::Get()->GetDeltaTime();
vector<Decal *>::iterator pkDecal = m_vpkDecals.begin();
while( pkDecal != m_vpkDecals.end() )
{
// update time
(*pkDecal)->m_fTime += fDeltaTime; // <- occasional crash
// get transparency
const float &fTransparency = (*pkDecal)->GetTransparency();
// check if it's transparent
if( fTransparency < Epsilon )
{
m_vpkDecals.erase( pkDecal );
delete (*pkDecal), (*pkDecal) = 0;
if( m_vpkDecals.size() == 0 ) // <- why do i need this?
break;
}
else if( (*pkDecal)->m_fTime > (*pkDecal)->m_fLifetime )
{
// update transparency
(*pkDecal)->SetTransparency( fTransparency-0.5f*fDeltaTime );
}
pkDecal++;
}
}
fun with std::vector
Author
this function traverses all decals. if a decal's lifetime has expired, it slowly fades. if it's transparent, it's removed from the array and deleted.
this works fine in most cases, but sometimes it crashes at the first line in the loop. why? is there a better way to do this? (yeah i know it'd be best not to remove elements from a vector at all) thanks..!
Once you call erase(pkDecal), the pkDecal iterator becomes invalid and cannot be used. Also, naked pointers are ugly, consider using either boost::ptr_vector or boost::shared_ptr. Using boost::ptr_vector:
bool decal::invisible() const{ return transparency < std::numeric_limits<float>::epsilon();}class update_decal{ float delta;public: update_decal(float delta) : delta(delta) {} void operator()(Decal & d) const { d.time += delta: if (d.time > d.lifetime) d.add_transparency(-0.5f * delta); }};void world::update_decals(){ const float delta = Core::Get()->get_delta_time(); decals.erase( std::remove_if( decals.begin(), decals.end(), std::mem_fun_ref(&decal::invisible)), decals.end()); std::for_each(decals.begin(), decals.end(), update_decal(delta));}
edit: yeah, what ToohrVyk said...
When you use 'erase' on a vector it invalidates all iterators, thus once you erase something and then loop back around your iterator isn't valid.
What you should do is;
As 'erase' returns the next valid iterator you use that next time around the loop, otherwise you increment the iterator.
Although, personally, I'd have probably written this as a combination of std::erase_if and std::foreach instead, if only because erasing in the middle of a vector is going to kill you performance wise as it's reshuffled down.
When you use 'erase' on a vector it invalidates all iterators, thus once you erase something and then loop back around your iterator isn't valid.
What you should do is;
if( fTransparency < Epsilon ){ delete (*pkDecal), (*pkDecal) = 0; pkDecal = m_vpkDecals.erase( pkDecal );}else if( (*pkDecal)->m_fTime > (*pkDecal)->m_fLifetime ){ // update transparency (*pkDecal)->SetTransparency( fTransparency-0.5f*fDeltaTime ); pkDecal++;}else{ pkDecal++;}As 'erase' returns the next valid iterator you use that next time around the loop, otherwise you increment the iterator.
Although, personally, I'd have probably written this as a combination of std::erase_if and std::foreach instead, if only because erasing in the middle of a vector is going to kill you performance wise as it's reshuffled down.
I was typing up something pretty much on the lines of what ToorhVyk posted, but I was interrupted and beaten to it. I will post the only difference, that the logic of updating a decal should be inside the decal class too, IMO. If you had boost, I think boost::bind can be used instead of a custom updater struct.
Building on ToohrVyk's sample code
Or using std::bind2nd
Building on ToohrVyk's sample code
void decal::update(float delta){ time += delta: if (time > lifetime) transparency += -0.5f * delta;}class update_decal{ float delta;public: update_decal(float delta) : delta(delta) {} void operator()(Decal & d) const { d.update(delta) }};void world::update_decals(){ const float delta = Core::Get()->get_delta_time(); decals.erase( std::remove_if( decals.begin(), decals.end(), std::mem_fun_ref(&decal::invisible)), decals.end()); std::for_each(decals.begin(), decals.end(), update_decal(delta));}Or using std::bind2nd
void decal::update(float delta){ time += delta: if (time > lifetime) transparency += -0.5f * delta;}void world::update_decals(){ const float delta = Core::Get()->get_delta_time(); decals.erase( std::remove_if( decals.begin(), decals.end(), std::mem_fun_ref(&decal::invisible)), decals.end()); std::for_each(decals.begin(), decals.end(), std::bind2nd(std::mem_fun_ref(&Decal::update), delta));}Quote:Original post by ToohrVyk
rip-off: I assume you mean d.update(delta).
Yes, with the update function having a parameter too ( I forgot it [embarrass] ). Fixed in (yet another) edit [smile]
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
