Flawed engine, need help getting rid of singleton mania.

Started by
13 comments, last by Mastaba 17 years, 4 months ago
Hello, if you are here. THat means somehow you want to help :D. Lets got straight to the grain. The Problem: I am trying to implement an engine like operation with my classes. My point of focus was using the engine sample created on GDNet ( no links :( ) and using the Task and kernel execution idea. I got it working on my program. Great, but now... how does one task get, set, execute things on another task without singletons? Look for example at some psudo-code: Video Rendernig Task Execute Clear screen Begin scene Singleton of GameStates.render end scene THis looks fine to a point where i need to render something else. I recently got CEGUI working and it is great work, but now it needs rendering too. I made a GUI task which renders GUI related things. The above Videorendering code has to be taken apart and apply localized clear begins. Could you help a fellow programmer? Sam needs you (big mean figer)! Here is some code to show what the problem is::::

class CCEGUITask: public ITask
{
private:
	bool m_bInitialized;
public:
	bool Start(){ return true; };
	void Update(){ 
		if ( m_bInitialized == true )
			CVideoTask::GetSingleton().ClearScene(D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER);
			CVideoTask::GetSingleton().BeginScene();
			CEGUI::System::getSingleton().renderGUI();
			CVideoTask::GetSingleton().EndScene();
	}
	void Stop()
	void CreateGUI(IDirect3DDevice9* pDevice)
};

int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nShowCmd )
{
	// Initialisation
	CLogger::Initialize();
	CSettingsManager* pSettingsMgr = new CSettingsManager();

	CKernel* pEngine = new CKernel();

	CWindowTask* pWindowTask = new CWindowTask();
	CGlobalTimer* pGlobalTimer = new CGlobalTimer();
	pGlobalTimer->m_lPriority = 10;
	CStateMgrTask* pStateMgrTask = new CStateMgrTask();
	pStateMgrTask->m_lPriority = 400;
	CVideoTask* pVideoTask = new CVideoTask();
	pVideoTask->m_lPriority = 5000;
	CCEGUITask* pGUITask = new CCEGUITask();
	pGUITask->m_lPriority = 700;

	// Load the settings
	// Initialize window and dx
	pVideoTask->CreateObject();
	D3DPRESENT_PARAMETERS d3dpp;
        // set up the pp
	d3dpp.hDeviceWindow = pWindowTask->GetHWND();
	pVideoTask->SetClearColor(D3DCOLOR_XRGB(150, 150, 150));
	pVideoTask->SetPresentParams(&d3dpp);
	pVideoTask->CreateDevice(D3DADAPTER_DEFAULT, pWindowTask->GetHWND());
	pGUITask->CreateGUI(pVideoTask->GetDevice());

	TestState* pState = new TestState();
	pStateMgrTask->AddState("introstate", pState);
	pStateMgrTask->SetState("introstate");

	// Add tasks for execution
	pEngine->AddTask( pWindowTask );
	pEngine->AddTask( pGlobalTimer );
	pEngine->AddTask( pStateMgrTask );
	pEngine->AddTask( pVideoTask );
	pEngine->AddTask( pGUITask );

	// Loop
	pEngine->Execute();

	// Free all allocated memory
	
	return 0;
}

class CVideoTask: public ITask, public Singleton<CVideoTask>
{
	CVideoTask();
	virtual ~CVideoTask();

	bool CreateObject();
	bool CreateDevice(UINT Adaptor, HWND hWindow);
	HRESULT CreateMasterSprite()
	void DestroyMasterSprite() { SAFE_RELEASE(m_pMasterSprite); };
	void DestroyDevice() { SAFE_RELEASE(m_pD3dDevice); };
	void DestroyObject() { SAFE_RELEASE(m_pD3dObject); };
	void SetPresentParams(D3DPRESENT_PARAMETERS* d3dpp)
	void SetClearColor(D3DCOLOR kClearColor)
	D3DPRESENT_PARAMETERS* GetPresentParams() { return m_pd3dpp; };
	IDirect3DDevice9* GetDevice() { return m_pD3dDevice; };
	IDirect3D9* GetObject() { return m_pD3dObject; };
	ID3DXSprite* GetMasterSprite() { return m_pMasterSprite; };
	HRESULT ClearScene(DWORD dwFlags);
	HRESULT BeginScene();
	HRESULT EndScene();
	HRESULT PresentScene();
	bool Start(){ return true; };
	void Update(){
		if (CWindowTask::GetSingletonPtr()->IsActive() == false)
			return;

		//ClearScene(D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER);
		//BeginScene();
		//CStateMgrTask::GetSingletonPtr()->RenderState();
		//EndScene();
		PresentScene();
	};
	void Stop(){};
};


--------------------------------------------------------------------- If you understand what i am trying to do, my problem is intercomunication of the tasks, telling when to render and things like this.
Advertisement
Provide more detail please.

I don't see a reason for those tasks to need to communicate with eachother.

Remember, the most common (mis)use of singletons is to enable lines of communication that don't really need to be there in a properly-designed system.

So the apparent solution to singletonitis is to remove the singletons -- this is where people run into a wall, usually. You tell them to remove the singletons, and they say, "but how will I let all my subsystems talk to my formerly-singleton subsystem? I'd have to pass pointers around all over the place!" Well, the real solution to singletonitis is to remove the need for that intercommunication in the first place. Singletons, in that case, are the manifestation of a flawed design and removing them but keeping the fundamental design flaw (the tight coupling via communication lines) doesn't solve anything.

So tell me what tasks need to communicate, and why you think they do?
Jpetrie, i think that the VideoTask ( one that sets up directX ) needs to communicate with all the tasks that require rendering, for ex: StatesManagerTask, CEGUITask. The WindowTask( creates the game window ) needs to let everyone else know about what messages are comming in. The to be created InputTask, needs to send key, mouse, or any input data to the tasks that need input.

VideoTask why: Because then how will the other tasks know when to render?
WindowTask why: Because how will the other tasks find out about the window messages?

InputTask why: Need to send input the the tasks that might need input.

The concept of the tasks, is that i could create a new one and just compile, no other task needs too know about the new one or anyone in particular, but it seems to be harder than i though to achieve this "equilibrium."

Also, extra bit of info the idea was taken for Enginuity article ( http://www.gamedev.net/reference/programming/features/enginuity1/ )
Maybe totally unrelated to your problem, but there are some errors in your code:

Brackets missing for the if:

...	void Update(){ 		if ( m_bInitialized == true )			CVideoTask::GetSingleton().ClearScene(D3DCLEAR_TARGET | D3DCLEAR_ZBUFFER);			CVideoTask::GetSingleton().BeginScene();			CEGUI::System::getSingleton().renderGUI();			CVideoTask::GetSingleton().EndScene();	}...



And way too much semi-colons! ;-)

There is your problem.

If the video task does the rendering, why should any other task have anything to do with the rendering? You're making additional tasks for no benefit, they just duplicate functionality. This is bad.

I've never applied the "task" mentality to my designs, so I can't comment on solutions from practical experience. However, I think you are overusing the design -- your use of the system seems like you'd end up with a large number of tasks all intertwined, causing the kind of trouble you have now.

I would try to reduce the overall number of tasks you have. A task sounds like it should be something that requires periodic processing (graphics, sound, physics, input, possibly game logic states); there's very little reason in my mind to have more tasks than that. Everything else should probably be implemented within a particular task object.

Subsystems (within tasks) responsible for the construction of objects that have visual representations can be provided a means of telling the renderer there is a new visual object, and then procede to forget about that visual representation (since it is the domain of the renderer) until its time to remove that visual object; in the past I've used a render-queue system for this. Anything that stimulated the creation of visual objects (ultimately, only the game-specific logic) would request a new renderable entity be created, any game-specific information was attached to the entity, and the entity was inserted into the queue.

Robert Frunzke, the semi colons are missing cause i did lots of cut and pasting :P, i tried to minimize the code to the maximum.

Jpetrie, so what you are suggesting is to have a renderer kind of task instead of a video initialization task? One that i will be able to send the visible entities to it, and it will render them? Sounds like a nice solution.

My question would be how do i make such a system, and how would i render the GUI, since the code isn't mine... All i know i need to call is the Render() function. I don't know much about entities, but can you tell them what function to call? Or could there be an exception the this problem?
Quote:Original post by chimera007
Robert Frunzke, the semi colons are missing cause i did lots of cut and pasting :P, i tried to minimize the code to the maximum.


There are too many semi colons.
I've run into the singletonmania wall several times and screeched to a halt right before hitting it.

So if anyone can clarify, I'm listening too :)
Working on a fully self-funded project
Quote:Original post by jpetrie
Subsystems (within tasks) responsible for the construction of objects that have visual representations can be provided a means of telling the renderer there is a new visual object, and then procede to forget about that visual representation (since it is the domain of the renderer) until its time to remove that visual object; in the past I've used a render-queue system for this. Anything that stimulated the creation of visual objects (ultimately, only the game-specific logic) would request a new renderable entity be created, any game-specific information was attached to the entity, and the entity was inserted into the queue.

I'd be interested in more information on how to do this too. My present engine design is suffering a bit from overuse from singeltons - understandable maybe since I'm my engine is fairly much in a prototype phase. But one of the subsystems I'm having trouble conceptualising how to "unsingletonise" is the graphics renderer, as presently my design is close to a wrapper over OpenGL (with a sprite resource manager tacked in). I could fairly easily pass pointers around whenever it's time to render something to screen, but that doesn't alter the fundamental structure of the code.

I'm also interested in a method of how to do something like a render-queue that's unsingletoned, but I'm not sure how this could be done without some singleton equivalent or sacrificing the flexibility of a more transparent OpenGL or Direct3D wrapper.

Sorry if this seems like bit of a hijack, but I am interested in what I think is the central question of chimera007 (the original poster) - how do you go about structuring the video display part of a game in a sensible fashion without something that's either a singleton or the functional equivalent of one?

The simplest form of avoiding singletons is to use dependancy injection (DI):

class GUI {public:    void Render() const {        Video::Instance()->DrawSomething();    }};int main () {    GUI gui;    ...    gui.Render();}


Becomes:

class GUI {    Video * video;public:    GUI() : video() {}        void inject( Video * video ) { this->video = video; }    void Render() const {        assert( video );        video->DrawSomething();    }};int main () {    GUI gui;    Video video;    gui.inject( &video );    ...    gui.Render();}


The Singleton pattern's main problem -- that it's forcibly tied to a single instance of Video, application wide -- is immediately dealt with, though, as this change decouples GUIs in general from a specific instance of Video. That is, it's now a trivial operation to have multiple GUIs for multiple Videos (screens?):

int main () {    Video screens[2];    GUI   guis[2];    for ( unsigned i = 0 ; i < 2 ; ++i ) guis.inject( & screens );    ...    for ( unsigned i = 0 ; i < 2 ; ++i ) guis.Render(); //renders to seperate Videos}


There are variations on this pattern as well -- injection via constructor, or "service locator" if you're injecting many dependancies. These range from simple structures by which to easily pass related dependancies together around in, to full blown systems themselves with features like conditional construction/initialization of dependancies, depending on if they're actually used.

This also allows for things like runtime selection of components without trying to stuff everything into the same class. It can now be:

std::auto_ptr< Video > video;switch ( renderer_type ) {case OPENGL_RENDERER_TYPE: video = new OpenGLVideo; break;case DIRECTX_RENDERER_TYPE: video = new DirectXVideo; break;default: throw error( "I hate you" );}GUI gui;gui.inject( video.get() );


Instead of:

OpenGLAndDirectXVideo video; //uses a bunch of conditionals internallyvideo.select_mode( renderer_type );GUI gui; //uses OpenGLAndDirectXVideo::Instance()


Furhter, unit testing is even easier:

void check_gui_renders_correctly() {    PolygonCountingVideoFacade pcount;    GUI gui;    Model model( "example.model" );    gui.inject( &pcount );    gui.render( model );    BOOST_CHECK( pcount.count() == model.polygons() ); //should render every polygon by default    pcount.reset();    gui.enable_polygon_culling();    gui.render( model );    BOOST_CHECK( pcount.count() <= model.polygons() * 2 / 3 ); //assume at least 1/3rd of the polygons should be cullable if polygon culling is enabled}


[Edited by - MaulingMonkey on December 13, 2006 12:25:30 PM]

This topic is closed to new replies.

Advertisement