Skip to main content
gamedev.net
🔒 Locked

validating objects in an observer pattern (stl list remove function)

Started by DrZoidberg 5 replies 1.3k views
Original Post
DrZoidberg
DrZoidberg
hello, i'm using the observer pattern and i'm wondering why this code fails to remove NULL pointers from a list. m_pObjects.remove(NULL); this is called for each class that may have objects that another class has references to, so if i delete an object before one of these validation functions is called (which happens) the object should be removed from each class that has references to this object and it's functions should not be called ... but this is not the case. why doesnt remove(NULL) remove objects that point to NULL?
Woop woop woop woop!
Fruny
Fruny
Should work. Broken library (VC6)?
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
DrZoidberg
DrZoidberg
i'm using visual studio express beta, im not sure if that include vc7 or vc6 compiler.
Woop woop woop woop!
lucky_monkey
lucky_monkey
Quote:
Original post by DrZoidberg
this is called for each class that may have objects that another class has references to, so if i delete an object before one of these validation functions is called (which happens) the object should be removed from each class that has references to this object and it's functions should not be called ... but this is not the case.
I'm assuming you're not doing something along the lines of the following (which is simplified greatly)...
std::list< some_object* > my_list;some_object* my_object = new some_object();my_list.push_back( my_object );delete my_object;my_object = 0;my_list.remove( 0 );
because that would just be silly, so I'll assume you're not doing it. In which case it should work, so I have no idea what's wrong.

[Edited by - lucky_monkey on February 23, 2005 10:07:04 PM]
DrZoidberg
DrZoidberg
:o im sorta doing it like that ... if i am i feel dumb becasue then i cannot see what's wrong with it.

this is sorta what im doing.

struct IRender {
virtual void render() = 0;
}

class CEntity : public IRender {
// blah
public:
// blah
void render() { // draw }
};

class CRenderer {
private:
list objects;
public:
void addVisObj(IRender*);
void drawAll();
void validate() { objects.remove(NULL); }
};

// somewhere along the way
CRenderer *renderer = new CRenderer;
CEntity *entity = new CEntity;
renderer->addVisObj(entity);

// then eventually entity will be deleted
// and the there is seg. fault. when the renderer comes along.

... i guess that really is what im doing, :( since im doing this all completely wrong, can you explain to me why it doesnt work?
Woop woop woop woop!
leiavoia
leiavoia
When you delete your entity, the renderer doesn't know that. It still has what it thinks is a pointer to a perfectly valid object. It doesn't. If you delete a pointer, it does not automatically update all the pointers you went and distributed throughout your system.

Your Subject (rendering object) needs to fire back a message of some sort to the Observer (renderer), that basically says "I'm dead. Remove me from your list". The actual removal is done by the Observer, but you at least need the message. That's the whole point of the system really, that communication there. Your design you posted looks "shaky" if that means anything. Prone to break under stress.

Another way you could solve the problem, which i currently am using for my core object processor, is to let the manager deal with the memory. For instance, instead of:

Object* obj = new Object(bla);
Manager->Register( obj );
// do stuph with obj
delete obj;


You would just do this:


// Fire and forget!
Manager->Register( new Object(bla) );


I can do this because each of my core objects has an Update() type function that returns a bool. If it returns true, it's still good for another round. If it returns false, that means "I'm dead, delete me." It's a great system. In fact, i'm feeling so generous right now (in a truly shameless attempt to up my rating), i'll even post my whole core system for ya. It's got a few extra bells and whistles, but you can take it apart however you want. I believe it does exactly what you are trying to do with your code (update and draw objects registered with it). You are more than welcome to use it if it looks appealing to you.

PollingObject HEADER:
#ifndef POLLINGOBJECT_H#define POLLINGOBJECT_H#include "PollingEngine.h"/*PollingObjects have two main functions: Poll() and Draw(). This is used to update the object and draw it every frame.PollingObjects are specifically designed to work with PollingEngines and are a very tightly coupled pair of classes.The object's Poll and Draw functions are defined by some inheritedclass and must be defined by you. The good news is that you can definethose functions however you want. You can put absolutely ANYTHING in the Poll() function including movement, AI, physics, updating internalmechanisms, whatever. You can even leave it completely blank. Draw()obviously is supposed to draw something, but how you do that in yoursystem is also up to you. If you need an object to update and draw, simply inherit from this classand feed the object into a PollingEngine. See the header for PollingEngine for more details.*/class PollingObject {public:	enum Management {		DELETE, // delete and erase the object when it stops polling		PROTECT, // erase but do not delete		PRESERVE // do not erase or delete the object ever. 		};			PollingObject( Management mode = DELETE ) :				killme(false),		mode(mode),		engine(NULL),		birth_tick(0)		{ }	virtual ~PollingObject() {}	/** Performs an action on polling. Returns FALSE if the object is finished	    with it's lifespan and should be deleted. Returns TRUE if the object is	    still active and requires further polling. */	virtual bool Poll() = 0;		/** */	virtual void Draw() = 0;		/** See above mode for management options */	inline void SetPollingMode( Management m ) { mode = m; }	/** Returns the object's protected status. */	inline bool Protected() { return mode == PROTECT; }		/** Returns the object's preserved status. */	inline bool Preserved() { return mode == PRESERVE; }						/** Forces an object to return FALSE on it's next poll. */	inline void StopPolling() { killme = true; }	/** Returns TRUE if an object is flagged with StopPolling() */	inline bool StoppedPolling() { return killme; }	/** Assigns an engine as an "observer subject" to get times and info from. */	void AssignEngine( PollingEngine* pe );					/** */	inline const float BirthTick() { return birth_tick; }		/** */	inline const float TimeFromBirth() { return (engine) ? engine->Time() - birth_tick : 0; }	/** */	inline const float Time() { return (engine) ? engine->Time() : 0 ; }	/** */	inline const float LastTime() { return (engine) ? engine->LastTime() : 0; }	/** */	inline const float TimeFromLast() { return (engine) ? engine->TimeFromLast() : 0; }	private:	bool killme;	Management mode;		PollingEngine* engine;		float birth_tick;		}; #endif


PollingObject BODY (not much :-):
#include "PollingObject.h"#include "PollingEngine.h"/** Assigns an engine as an "observer subject" to get times and info from. */void PollingObject::AssignEngine( PollingEngine* pe ) { 	engine = pe; 	birth_tick = pe->Time();	}


PollingEngine HEADER:
#ifndef POLLINGENGINE_H#define POLLINGENGINE_H#include <list>/**INSTRUCTIONS FOR USE:1) Create an object. It should be a class the inherits from PollingObject:		SomeClass* thing = new SomeClass( params );2) Register the object with the polling engine. This makes it update every frame:		PollingEngine.Register( thing );3) Make sure you poll the engine every frame. Somewhere in   Your game loop, you need to call:   		PollingEngine.PollAll( time_delta  );		PollingEngine.DrawAll();		That's it! "Thing" will now update every frame and draw too. When doingthe polling, you need to supply the engine with a float that representsthe time past from the last frame. The engine will do all the number crunchingon it's own. Management: Because there will be some special objects (like the player, GUI elements, etc)that need finer manual control, there are three management modes provided (documented below). If you want the engine to update the object but do not want itdeleted when it returns false, you can set the object to be either PROTECTED or PRESERVED. Important things to remember:- The polling engine only accepts pointers to objects.  It does this to preserve polymorphism. Beware that the engine will  attempt to delete the object if it returns false ("is finished").  If the object is not dynamically allocated with the NEW keyword, it  will segfault on deletion.- You should override the Poll() function for any inherited object. This  Does the actual updating. What you do in Poll() is completely up to you.  It makes no difference to the engine.- Polling objects return TRUE from Poll() if they are still alive. They  Return FALSE if they are finished and need to be deleted. DO NOT  DELETE OBJECTS YOURSELF. The engine will do that for you.- Most of the time, once the engine is set up, you'll want to follow a  "fire and forget" policy for adding new objects to the engine. Suppose  you want to fire a missile. You would do that like so:  		PollingEngine.Register(   new Missile( whatever )   );  Then just forget about it! It will take care of itself after that. You  do not need to keep track of "Missile" in any way, shape, or form.- You can register ANYTHING that needs time based updating. It need not  be a graphical object. It can be a timer, a player or enemy, a world  trigger, a time bomb, anything with delayed reactions, animations, etc.  What you do in the Poll() function is completely up to you.*/// pre-decclass PollingObject;class PollingEngine {public:	PollingEngine();	virtual ~PollingEngine();	/** Performs a Poll() on all associated objects */	virtual bool PollAll( float time_diff );	virtual void DrawAll();			/** Registers an object with the polling engine.	    NOTE: To "unregister", just have the object return FALSE in it's Poll() function.	    It will be erased and deleted on the engine's next polling cycle.	    WARNING: Objects WILL be deleted, so they must be pointers to dynamically allocated memory.	*/	void Register( PollingObject* p );		/** */	void Freeze() { frozen = true; }		/** */	void Thaw() { frozen = false; }		/**  	level (1) will only erase and delete regular objects.	level (2) will erase PROTECTED objects and erase+delete DELETE objects.	level (3) will erase all objects and delete DELETE objects.		*/	void Flush( int level=1 );		/** */	void ModSpeed( float x ) { time_mod = x; }			/** */	inline const float Time() { return time; }	/** */	inline const float LastTime() { return last_time; }	/** */	inline const float TimeFromLast() { return time_from_last; }		private:	std::list<PollingObject*> registry;	// the current frame time	float time;	// difference between this frame and the last frame	float time_from_last;	// the last frame time	float last_time;		// time modifier ( 0.0 = frozen, 1.0 = normal time, 1+ = speedup)	float time_mod;		bool frozen;			};		#endif


PollingEngine BODY:
#include "../utilities/LogManager.h"#include "PollingEngine.h"#include "PollingObject.h"PollingEngine::PollingEngine( )  :	time(0),	time_from_last(0),	last_time(0),	time_mod(1.0),	frozen(false)	{}/*Cycle through and Poll() all objects. The basic idea here is to iteratethrough each layer list, and any objects that return FALSE on their Poll(),( the "kill me" signal ) are deleted immediately and the next object is polled.*/bool PollingEngine::PollAll( float time_diff ) {		if (frozen) { return false; }	// recalc time for everybody		time_diff *= time_mod;	last_time = time;	time += time_diff;	time_from_last = time_diff;		// poll all objects and kill FALSE returners	for (std::list<PollingObject*>::iterator iter = registry.begin(); iter != registry.end();) {		if (  (*iter)->StoppedPolling()  ||  !(*iter)->Poll()  ) {			if ( (*iter)->Preserved() ) { continue; }			if ( !(*iter)->Protected() ) { delete *iter; }			iter = registry.erase(iter);			}		else { iter++; }		}	if ( registry.size() == 0 ) { return false; }		return true;	}	void PollingEngine::DrawAll() {	for (std::list<PollingObject*>::iterator iter = registry.begin(); iter != registry.end(); iter++) {		(*iter)->Draw();		}	}	void PollingEngine::Register( PollingObject* p ) {	if (!p) { return; }	p->AssignEngine( this );	registry.push_back(p);	}	PollingEngine::~PollingEngine() {	Flush();	if ( registry.size() > 0 ) {	     LogMgr::Write( LOGMSG_WARNING, "PollingEngine had [%d] protected or preserved objects upon destruction.", registry.size() );		}	}	void PollingEngine::Flush( int level ) {	for (std::list<PollingObject*>::iterator iter = registry.begin(); iter != registry.end();) {		if ( (*iter)->Preserved() && level == 3 ) {			iter = registry.erase(iter);			}		else if ( (*iter)->Protected() && (level != 1)  ) { 			iter = registry.erase(iter);			}				else if ( !(*iter)->Protected() && !(*iter)->Preserved() ) {						delete *iter;			iter = registry.erase(iter);			}		else { iter++; }		}		}


[Edited by - leiavoia on February 25, 2005 12:00:34 AM]
DrZoidberg
DrZoidberg
wow thanks you for your help, now that i read your post and looked back at what i was doing it seems silly ... becasue all i was doing was deleteing the object at that location but the list still referenced the deleted position. so by doing remove(NULL) nothing would be removed.

yes i like your idea of the manager system, as it would be annoying to send a message to each system when deleting an object (this is orginally why i thought remove(NULL) would work).

i guess it may even be possible to create a class that overloads new and delete and derive any objects that will be 'observerd' from that class. so something like.

CObject *object = new CObject;

would automatically use the manager system to keep track of it. but in the end that may be worth more trouble than its worth.

thanks again for the help and code.
Woop woop woop woop!

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.