Smart or dumb? Event handling

Started by
9 comments, last by Durfy 16 years, 9 months ago
This is for a pong game... I have a vector of strings vector<string> Events; In my LRESULT CALLBACK WndProc function I add different events based on keypresses such as:

LRESULT CALLBACK WndProc(HWND hwnd, UINT iMsg, WPARAM wParam, LPARAM lParam) {
	switch(iMsg) {
		case WM_DESTROY:
			PostQuitMessage(0);
			gameover = true;
			break;
		case WM_KEYDOWN:
			switch(LOWORD(wParam)) {
				case VK_LEFT:
					Events.push_back("MOVELEFT");
					break;
				case VK_RIGHT:
					Events.push_back("MOVERIGHT");
					break;
			}
			break;

	}
	return DefWindowProc(hwnd, iMsg, wParam, lParam);
}


Then in my main loop (equipped with handling framerate (sort of in my dumb eyes)

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hpInstance, PSTR cmdLine, int iCMD) {
	WNDCLASS wc;
	HWND     hwnd;
	MSG      msg;
	Ball     ball(300, 300, 50, 50, 10, 315);
	Paddle   paddle(0, SCREEN_HEIGHT - 60, 125, 10, 10, 0);
	wc.cbClsExtra = 0;
	wc.cbWndExtra = 0;
	wc.hbrBackground = (HBRUSH)GetStockObject(WHITE_BRUSH);
	wc.hCursor  = LoadCursor(NULL, IDC_ARROW);
	wc.hIcon = LoadIcon(NULL, IDI_APPLICATION);
	wc.hInstance = hInstance;
	wc.lpfnWndProc = WndProc;
	wc.lpszClassName = "GameClass";
	wc.lpszMenuName  = NULL;
	wc.style = CS_VREDRAW| CS_HREDRAW;
	RegisterClass(&wc);

	hwnd = CreateWindowEx(NULL, "GameClass", "Game Window", WS_OVERLAPPEDWINDOW, 0, 0, 800, 600, NULL, NULL, hInstance, NULL);
	HDC hdc = GetDC(hwnd);
	UpdateWindow(hwnd);
	ShowWindow(hwnd, iCMD);
	DWORD start_tick = GetTickCount();
	while (!gameover) {
		if (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
			TranslateMessage(&msg);
			DispatchMessage(&msg);
		}
		else {
			if (GetTickCount() - start_tick > .000000000001) {
				RECT rect;
				GetWindowRect(hwnd, &rect);
				FillRect(hdc, &rect, (HBRUSH)GetStockObject(WHITE_BRUSH));
				HBRUSH hbrush = (HBRUSH)CreateSolidBrush(RGB(255,0,0));
				SelectObject(hdc, hbrush);
				Ellipse(hdc, ball.getX(), ball.getY(), ball.getX() + ball.getWidth(), ball.getY() + ball.getHeight());
				hbrush = (HBRUSH)CreateSolidBrush(RGB(0,0,255));
				SelectObject(hdc, hbrush);
				Ellipse(hdc, paddle.getX(), paddle.getY(), paddle.getX() + paddle.getWidth(), paddle.getY() + paddle.getHeight());
				
				for (int i = 0; i <= Events.size()-1; i++ ){
					string myevent = (string)Events.at(i);
					if (myevent == "MOVELEFT") {
						paddle.setDirection(1);
					}
					else if(myevent == "MOVERIGHT") {
						paddle.setDirection(2);
					}
				}
				
				paddle.move();
				ball.move();
				if (ball.getX() + ball.getWidth() >= SCREEN_WIDTH || ball.getX() <= SCREEN_X) {
					ball.setDirection(180 - ball.getDirection());
				}
				else if (ball.getY() + ball.getHeight() >= SCREEN_HEIGHT || ball.getY() <= SCREEN_Y) {
					ball.setDirection(360 - ball.getDirection());
				}
				start_tick = GetTickCount();
			}
		}
	}
	ReleaseDC(hwnd, hdc);
	return 0;
}


specific detail on this:

for (int i = 0; i <= Events.size()-1; i++ ){
					string myevent = (string)Events.at(i);
					if (myevent == "MOVELEFT") {
						paddle.setDirection(1);
					}
					else if(myevent == "MOVERIGHT") {
						paddle.setDirection(2);
					}
				}


Thanks for any comments/ideas thrown my way. EDIT: was thinking about adding constants for the different events -durfy
Advertisement
Smart-ish.

You will eventually need parameters from the original events, something which you cannot provide right now.

Using strings is also quite inefficient, since you don't really use the string information.

This approach is missing one last component - automatic binding. Right now, you need to manually resolve what you want to do.

A better way would be something like this (the Command pattern):
class Command{public:  virtual void execute();}class PaddleMoveCommand : public Command{public:  PaddleModeCommand( int direction )    : m_paddle( p )    , m_direction( direction )  {}  virtual void execute()  {    paddle.setDirection( m_direction );  }private:  int m_direction;}


This requires a change in event queue definition:
typedef std::deque< Command * > CommandQueue;CommandQueue queue;...case VK_LEFT:  queue.push_back( new PaddleMoveCommand(1) );  break;case VK_RIGHT:  queue.push_back( new PaddleMoveCommand(2) );  break;


And then, your event handler becomes this:
  while (! queue.empty() ) {    queue.front()->execute();    delete queue.front();    queue.pop();  }


The beauty of this is de-coupling. When you need to add a new command, you merely define a new class, and implement its execute() method.

The game loop doesn't need to be changed when this happens, all the parameters required for the Command are safely encapsulated in one place. If you later need to pass wparam or lparam or some other values, you don't need to change anything.


Note: You need pointers here for polymorphism to work, a thesis could be written on this, there are even some ways to avoid them, but regardless, this is about design, not about intricacies of optimization.

Also, std::deque or std::queue classes are, well, queues. So use those instead of vector, since they are design for such tasks.
I was going to use that idea (mainly) but someone told me i was stupid to create a new class for every event/command lol! But apparently at least you agree with me... THank you sooo much :-)
Ooops.. one more question
how is your version of the command class able to access an instance of paddle that it doesnt know about?
-durfy
Quote:Original post by Durfy
I was going to use that idea (mainly) but someone told me i was stupid to create a new class for every event/command lol! But apparently at least you agree with me... THank you sooo much :-)
Ooops.. one more question
how is your version of the command class able to access an instance of paddle that it doesnt know about?
-durfy


That's up to you.

In pure Command pattern you pass all the parameters required for execution during creation of the Command object.

If you don't want to polute your windows related message loop with game code, you could structure your command like this:
class Command{public:  virtual void execute( GameState &gs );};// and then:class MoveCommand : public Command{public:  virtual void execute( GameState &gs )  {    gs.getPlayer().getPaddle().setDirection( m_direction );  }}


In your game loop you then just pass the game state representation to the command you're executing.

This results in complete separation of the game from main loop, should you choose to.

Otherwise, you'd simply pass the reference to paddle in MoveCommand constructor.
Personally, I'd use a function object (like boost::function) over the common base class. Though to be fair, event handling of any sort may be a bit overkill for pong. Make the game work.
Quote:Original post by Telastyn
Personally, I'd use a function object (like boost::function) over the common base class. Though to be fair, event handling of any sort may be a bit overkill for pong. Make the game work.


But wouldn't this be a good practice to start early?
-durfy
Quote:Original post by Durfy
Quote:Original post by Telastyn
Personally, I'd use a function object (like boost::function) over the common base class. Though to be fair, event handling of any sort may be a bit overkill for pong. Make the game work.


But wouldn't this be a good practice to start early?
-durfy


Sure, but wouldn't it be a better practice to get stuff working? There are innumerable people across these forums who get caught up in trying to make things 'right' at the expense of getting things done.
This approach requires the CommandQueue to be global. Is there a way to avoid this? In a couple of other threads it was mentioned that using something like boost's signal/slot system is better for this sort of thing. What do you guys think about this?

Quote:Original post by Gage64
This approach requires the CommandQueue to be global. Is there a way to avoid this? In a couple of other threads it was mentioned that using something like boost's signal/slot system is better for this sort of thing. What do you guys think about this?


Why does it need to be global? All you need to do is make sure that both source and sink get a reference to the queue.

Nothing ever *NEEDS* to be global. Not even resources that encapsulate static entities or static calls.

Signals are alternative design. They provide dynamic function binding.

What is better or worse depends solely on experience, requirements and budget. Signals are better for different type of tasks, where you have a set of events that you want your listeners to respond to - usually in purely asynchronous manner.


Command pattern is a very nice and useful pattern. I've written several applications based exclusively using Commands. Apart from a few basic classes, the entire logic was written entirely and exclusively within Command classes, making the applications very maintainable and extensible. And not a single global or static instance anywhere. And if done in Java, you have the luxury of using interfaces to express dependent functionality (paddle in this example).

Overkill for Pong? Possibly. But one needs to start small.
This is how I'm doing event handling, although it makes heavy use of boost smart pointers.
// in cpp filevoid Window::AddListener(renwin::event_t e, renwin::ALPtr_t a){	actMap[e].push_back(ALWeakPtr_t(a));}void Window::FireEvent(renwin::event_t e){	typedef ALList_t::iterator it_t;	ALPtr_t p;	for (it_t i=actMap[e].begin();i!=actMap[e].end();i++)	{		// get shared_ptr to action listener		p=i->lock();                // if the object still exists then call it's () operator		if (!i->expired())			p->operator ()();		else // else-wise remove it from the action listener map			actMap[e].erase(i);	}}// in header filenamespace renwin // render window namespace{	class ActionListener	{	public:		virtual ~ActionListener() = 0;		// event trigger function, called on firing of event		virtual void operator()() = 0;	};};// sample action listener for quit messages, sets isAlive to false on messageclass QuitListener : public ActionListener{public:	QuitListener() : isAlive(true) {};	~QuitListener() {};	void operator()() { isAlive=false; };	// returns true if associated message has not yet fired	bool IsAlive();private:	volatile bool isAlive;};typedef boost::shared_ptr<ActionListener> ALPtr_t; // action listener pointer typetypedef boost::weak_ptr<ActionListener> ALWeakPtr_t; // action listener weak pointer typetypedef std::list<ALWeakPtr_t> ALList_t; // action listener list typetypedef std::vector<ALList_t> ALLstMap_t; // action listener map type// EVENTSenum event_t{	FOCUS=0,	BLUR,	QUIT,	RESIZE,	//...	NUMBER_OF_EVENTS // number of event types};// inside the Window class definitionclass Window{//...	ALLstMap_t actMap; // map of action listeners};


When an event occurs my Window class calls "FireEvent(event)," for example when the quit message occurs it calls "FireEvent(QUIT)."

Action listeners that need to be informed of events being fired are added by calling "Window.AddListener(event,actionListernObjectSharedPoitner)" and they are removed auto-magically by boost shared_ptr when all external references go out of scope. This is because the shared_ptr is reference counted and the weak_ptr becomes null when all shared_ptr's to the same object do. That way I can keep a reference to the action listener and ensure not only that it gets deleted when it goes out of scope, but also ensure that the window's reference automatically becomes null when that happens.

I left out most multi-threading code although I believe that "p=i->lock();" can be replaced with "p=i;" if you don't need thread safety.

I like this approach for the following reasons:
1) The most complex parts are handled by a very stable and proven library, boost.
2) The exposed interface is dead simple
AddListener(event,actionListenerObjectSharedPoitner)
3) Removal of action listerners (you could call them even listeners) is autmatic
4) The user can define how they retrieve notification by the definition of the "()" operator function and the custom actionListener object.
5) NO STRINGS are used anywhere in this system, just define enums. If you need strings for scripts to work, then just make a function that translates the strings into numbers. Numeric ID's are far more efficient than strings.

It does have short comings, namely:
1) Each custom actionListener object must have a thread safe "()" operator (if multi-threading is used)
2) No parameters are passed to the actionListener (this could be changed but my Window class can be queried for all sorts of data, so I didn't bother)
3) Requires boost libraries
4) Depends on dynamically bound polymorphic types
*edit*

I just took the time to look at boost singals and I think it's a better solution although there still work involved in implementing it in a manner equivalent to my example.

[Edited by - T1Oracle on July 10, 2007 9:44:08 AM]
Programming since 1995.

This topic is closed to new replies.

Advertisement