WIP: some refactoring of event handling w/ celticminstrel

src/events.cpp

@@ -46,17 +46,6 @@ void context::add_handler(sdl_handler* ptr)
 	handlers.push_back(ptr);
 }
 
-void context::delete_handler_index(size_t handler)
-{
-	if(focused_handler == static_cast<int>(handler)) {
-		focused_handler = -1;
-	} else if(focused_handler > static_cast<int>(handler)) {
-		--focused_handler;
-	}
-
-	handlers.erase(handlers.begin()+handler);
-}
-
 bool context::remove_handler(sdl_handler* ptr)
 {
 	if(handlers.empty()) {
@@ -66,17 +55,26 @@ bool context::remove_handler(sdl_handler* ptr)
 	static int depth = 0;
 	++depth;
 
-	//the handler is most likely on the back of the events array,
+	// The handler is most likely on the back of the events list,
-	//so look there first, otherwise do a complete search.
+	// so look there first, otherwise do a complete search.
 	if(handlers.back() == ptr) {
-		delete_handler_index(handlers.size()-1);
+		if(*focused_handler == ptr) {
+			--focused_handler;
+		}
+
+		handlers.pop_back();
 	} else {
-		const std::vector<sdl_handler*>::iterator i = std::find(handlers.begin(),handlers.end(),ptr);
+		const handler_list::iterator i = std::find(handlers.begin(), handlers.end(), ptr);
-		if(i != handlers.end()) {
+
-			delete_handler_index(i - handlers.begin());
+		if(i == handlers.end()) {
-		} else {
 			return false;
 		}
+
+		if(i == focused_handler) {
+			focused_handler != handlers.begin() ? --focused_handler : ++focused_handler;
+		}
+
+		handlers.erase(i);
 	}
 
 	--depth;
@@ -84,34 +82,33 @@ bool context::remove_handler(sdl_handler* ptr)
 	if(depth == 0) {
 		cycle_focus();
 	} else {
-		focused_handler = -1;
+		focused_handler = handlers.end();
 	}
 
 	return true;
 }
 
-int context::cycle_focus()
+void context::cycle_focus()
 {
-	int index = focused_handler+1;
+	handler_list::const_iterator temp_focused_handler = focused_handler;
-	for(size_t i = 0; i != handlers.size(); ++i) {
+
-		if(size_t(index) == handlers.size()) {
+	for(++temp_focused_handler; temp_focused_handler != focused_handler; ++temp_focused_handler) {
-			index = 0;
+		if(temp_focused_handler == handlers.end()) {
+			temp_focused_handler = handlers.begin();
 		}
 
-		if(handlers[size_t(index)]->requires_event_focus()) {
+		if((**temp_focused_handler).requires_event_focus()) {
-			focused_handler = index;
+			focused_handler = temp_focused_handler;
 			break;
 		}
 	}
-
-	return focused_handler;
 }
 
 void context::set_focus(const sdl_handler* ptr)
 {
-	const std::vector<sdl_handler*>::const_iterator i = std::find(handlers.begin(),handlers.end(),ptr);
+	const handler_list::const_iterator i = std::find(handlers.begin(),handlers.end(),ptr);
 	if(i != handlers.end() && (**i).requires_event_focus()) {
-		focused_handler = int(i - handlers.begin());
+		focused_handler = i;
 	}
 }
 
@@ -199,14 +196,14 @@ void sdl_handler::join_same(sdl_handler* parent)
 	}
 
 	for(std::deque<context>::reverse_iterator i = event_contexts.rbegin(); i != event_contexts.rend(); ++i) {
-		std::vector<sdl_handler *> &handlers = (*i).handlers;
+		handler_list& handlers = (*i).handlers;
 		if (std::find(handlers.begin(), handlers.end(), parent) != handlers.end()) {
 			join(*i);
 			return;
 		}
 	}
-	join(global_context);
 
+	join(global_context);
 }
 
 void sdl_handler::leave()
@@ -276,32 +273,32 @@ bool has_focus(const sdl_handler* hand, const SDL_Event* event)
 		return true;
 	}
 
-	const int foc_i = event_contexts.back().focused_handler;
+	const handler_list::const_iterator foc = event_contexts.back().focused_handler;
+	auto& handlers = event_contexts.back().handlers;
 
-	//if no-one has focus at the moment, this handler obviously wants
+	// If no-one has focus at the moment, this handler obviously wants
-	//focus, so give it to it.
+	// focus, so give it to it.
-	if(foc_i == -1) {
+	if(foc == handlers.end()) {
 		focus_handler(hand);
 		return true;
 	}
 
-	sdl_handler *const foc_hand = event_contexts.back().handlers[foc_i];
+	sdl_handler* const foc_hand = *foc;
 	if(foc_hand == hand){
 		return true;
 	} else if(!foc_hand->requires_event_focus(event)) {
-		//if the currently focused handler doesn't need focus for this event
+		// If the currently focused handler doesn't need focus for this event
-		//allow the most recent interested handler to take care of it
+		// allow the most recent interested handler to take care of it
-		int back_i = event_contexts.back().handlers.size() - 1;
+		for(auto i = handlers.rbegin(); i != handlers.rend(); ++i) {
-		for(int i=back_i; i>=0; --i) {
+			sdl_handler* const thief_hand = *i;
-			sdl_handler *const thief_hand = event_contexts.back().handlers[i];
+
-			if(i != foc_i && thief_hand->requires_event_focus(event)) {
+			if(thief_hand != foc_hand && thief_hand->requires_event_focus(event)) {
-				//steal focus
+				// Steal focus
 				focus_handler(thief_hand);
-				if(foc_i < back_i) {
+
-					//position the previously focused handler to allow stealing back
+				// Position the previously focused handler to allow stealing back
-					event_contexts.back().delete_handler_index(foc_i);
+				handlers.splice(handlers.end(), handlers, foc);
-					event_contexts.back().add_handler(foc_hand);
+
-				}
 				return thief_hand == hand;
 			}
 		}
@@ -424,14 +421,14 @@ void pump()
 				//make sure this runs in it's own scope.
 				{
 					for( std::deque<context>::iterator i = event_contexts.begin() ; i != event_contexts.end(); ++i) {
-						const std::vector<sdl_handler*>& event_handlers = (*i).handlers;
+						const handler_list& event_handlers = (*i).handlers;
-						for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+						for(auto handler : event_handlers) {
-							event_handlers[i1]->handle_window_event(event);
+							handler->handle_window_event(event);
 						}
 					}
-					const std::vector<sdl_handler*>& event_handlers = global_context.handlers;
+					const handler_list& event_handlers = global_context.handlers;
-					for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+					for(auto handler : event_handlers) {
-						event_handlers[i1]->handle_window_event(event);
+						handler->handle_window_event(event);
 					}
 				}
 
@@ -473,8 +470,8 @@ void pump()
 			{
 				/* iterate backwards as the most recent things will be at the top */
 				for( std::deque<context>::iterator i = event_contexts.begin() ; i != event_contexts.end(); ++i) {
-					const std::vector<sdl_handler*>& event_handlers = (*i).handlers;
+					const handler_list& event_handlers = (*i).handlers;
-					for( std::vector<sdl_handler*>::const_iterator i1 = event_handlers.begin(); i1 != event_handlers.end(); ++i1) {
+					for( handler_list::const_iterator i1 = event_handlers.begin(); i1 != event_handlers.end(); ++i1) {
 						(*i1)->handle_event(event);
 					}
 				}
@@ -512,19 +509,19 @@ void pump()
 			}
 		}
 
-		const std::vector<sdl_handler*>& event_handlers = global_context.handlers;
+		const handler_list& event_handlers = global_context.handlers;
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->handle_event(event);
+			handler->handle_event(event);
 		}
 
 		if(event_contexts.empty() == false) {
 
-			const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+			const handler_list& event_handlers = event_contexts.back().handlers;
 
 			//events may cause more event handlers to be added and/or removed,
 			//so we must use indexes instead of iterators here.
-			for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+			for(auto handler : event_handlers) {
-				event_handlers[i1]->handle_event(event);
+				handler->handle_event(event);
 			}
 		}
 
@@ -540,12 +537,12 @@ void raise_process_event()
 {
 	if(event_contexts.empty() == false) {
 
-		const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+		const handler_list& event_handlers = event_contexts.back().handlers;
 
 		//events may cause more event handlers to be added and/or removed,
 		//so we must use indexes instead of iterators here.
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->process_event();
+			handler->process_event();
 		}
 	}
 }
@@ -558,7 +555,7 @@ void raise_resize_event()
 	event.window.windowID = 0; // We don't check this anyway... I think...
 	event.window.data1 = CVideo::get_singleton().getx();
 	event.window.data2 = CVideo::get_singleton().gety();
-	
+
 	SDL_PushEvent(&event);
 }
 
@@ -566,12 +563,12 @@ void raise_draw_event()
 {
 	if(event_contexts.empty() == false) {
 
-		const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+		const handler_list& event_handlers = event_contexts.back().handlers;
 
 		//events may cause more event handlers to be added and/or removed,
 		//so we must use indexes instead of iterators here.
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->draw();
+			handler->draw();
 		}
 	}
 }
@@ -579,9 +576,9 @@ void raise_draw_event()
 void raise_draw_all_event()
 {
 	for( std::deque<context>::iterator i = event_contexts.begin() ; i != event_contexts.end(); ++i) {
-		const std::vector<sdl_handler*>& event_handlers = (*i).handlers;
+		const handler_list& event_handlers = (*i).handlers;
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->draw();
+			handler->draw();
 		}
 	}
 }
@@ -590,12 +587,12 @@ void raise_volatile_draw_event()
 {
 	if(event_contexts.empty() == false) {
 
-		const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+		const handler_list& event_handlers = event_contexts.back().handlers;
 
 		//events may cause more event handlers to be added and/or removed,
 		//so we must use indexes instead of iterators here.
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->volatile_draw();
+			handler->volatile_draw();
 		}
 	}
 }
@@ -603,9 +600,9 @@ void raise_volatile_draw_event()
 void raise_volatile_draw_all_event()
 {
 	for( std::deque<context>::iterator i = event_contexts.begin() ; i != event_contexts.end(); ++i) {
-		const std::vector<sdl_handler*>& event_handlers = (*i).handlers;
+		const handler_list& event_handlers = (*i).handlers;
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->volatile_draw();
+			handler->volatile_draw();
 		}
 	}
 }
@@ -614,12 +611,12 @@ void raise_volatile_undraw_event()
 {
 	if(event_contexts.empty() == false) {
 
-		const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+		const handler_list& event_handlers = event_contexts.back().handlers;
 
 		//events may cause more event handlers to be added and/or removed,
 		//so we must use indexes instead of iterators here.
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->volatile_undraw();
+			handler->volatile_undraw();
 		}
 	}
 }
@@ -628,11 +625,11 @@ void raise_help_string_event(int mousex, int mousey)
 {
 	if(event_contexts.empty() == false) {
 
-		const std::vector<sdl_handler*>& event_handlers = event_contexts.back().handlers;
+		const handler_list& event_handlers = event_contexts.back().handlers;
 
-		for(size_t i1 = 0, i2 = event_handlers.size(); i1 != i2 && i1 < event_handlers.size(); ++i1) {
+		for(auto handler : event_handlers) {
-			event_handlers[i1]->process_help_string(mousex,mousey);
+			handler->process_help_string(mousex,mousey);
-			event_handlers[i1]->process_tooltip_string(mousex,mousey);
+			handler->process_tooltip_string(mousex,mousey);
 		}
 	}
 }

src/events.hpp

@@ -18,6 +18,7 @@
 #include <SDL_events.h>
 #include <SDL_version.h>
 #include <vector>
+#include <list>
 
 //our user-defined double-click event type
 #define DOUBLE_CLICK_EVENT SDL_USEREVENT
@@ -33,23 +34,23 @@ namespace events
 
 class sdl_handler;
 
+typedef std::list<sdl_handler*> handler_list;
+
 struct context
 {
 	context() :
-			handlers(),
+		handlers(),
-			focused_handler(-1)
+		focused_handler()
 	{
 	}
 
 	void add_handler(sdl_handler* ptr);
 	bool remove_handler(sdl_handler* ptr);
-	int cycle_focus();
+	void cycle_focus();
 	void set_focus(const sdl_handler* ptr);
 
-	std::vector<sdl_handler*> handlers;
+	handler_list handlers;
-	int focused_handler;
+	handler_list::const_iterator focused_handler;
-
-	void delete_handler_index(size_t handler);
 };
 
 //any classes that derive from this class will automatically
@@ -97,7 +98,6 @@ private:
 };
 
 void focus_handler(const sdl_handler* ptr);
-void cycle_focus();
 
 bool has_focus(const sdl_handler* ptr, const SDL_Event* event);