Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Refactor how game events are pumped.

game_events::pump() now records the events in the event queue at
the time it is called. This isolates those events from any that
might be fired within one of the events (e.g. with [fire_event]).
For an example of what was happening, see bug #21031.

Fixes bug #21031.
  • Loading branch information...
commit b4ce6a6b3a0696698772c4e659805abcadb121d9 1 parent af3a323
Ja-MiT authored August 17, 2013

Showing 1 changed file with 81 additions and 28 deletions. Show diff stats Hide diff stats

  1. 109  src/game_events/pump.cpp
109  src/game_events/pump.cpp
@@ -73,33 +73,33 @@ namespace game_events {
73 73
 namespace { // Types
74 74
 	class pump_manager {
75 75
 	public:
76  
-		pump_manager() :
77  
-			x1_(resources::gamedata->get_variable("x1")),
78  
-			x2_(resources::gamedata->get_variable("x2")),
79  
-			y1_(resources::gamedata->get_variable("y1")),
80  
-			y2_(resources::gamedata->get_variable("y2"))
81  
-		{
82  
-			++instance_count;
83  
-		}
84  
-		~pump_manager() {
85  
-			resources::gamedata->get_variable("x1") = x1_;
86  
-			resources::gamedata->get_variable("x2") = x2_;
87  
-			resources::gamedata->get_variable("y1") = y1_;
88  
-			resources::gamedata->get_variable("y2") = y2_;
89  
-			--instance_count;
90  
-		}
  76
+		pump_manager();
  77
+		~pump_manager();
  78
+
  79
+		/// Allows iteration through the queued events.
  80
+		queued_event & next() { return queue_[pumped_count_++]; }
  81
+		/// Indicates the iteration is over.
  82
+		bool done() const { return pumped_count_ >= queue_.size(); }
  83
+
91 84
 		static unsigned count() {
92 85
 			return instance_count;
93 86
 		}
  87
+
94 88
 	private:
95 89
 		static unsigned instance_count;
96 90
 		int x1_, x2_, y1_, y2_;
  91
+		/// Tracks the events to process.
  92
+		/// This isolates these events from any events that might be generated
  93
+		/// during the processing.
  94
+		std::vector<queued_event> queue_;
  95
+		/// Tracks how many events have been processed.
  96
+		size_t pumped_count_;
97 97
 	};
98 98
 	unsigned pump_manager::instance_count=0;
99 99
 } // end anonymous namespace (types)
100 100
 
101 101
 namespace { // Variables
102  
-	std::deque<queued_event> events_queue;
  102
+	std::vector<queued_event> events_queue;
103 103
 
104 104
 	/// The value returned by wml_tracking();
105 105
 	size_t internal_wml_tracking = 0;
@@ -109,6 +109,42 @@ namespace { // Variables
109 109
 
110 110
 namespace { // Support functions
111 111
 
  112
+	pump_manager::pump_manager() :
  113
+		x1_(resources::gamedata->get_variable("x1")),
  114
+		x2_(resources::gamedata->get_variable("x2")),
  115
+		y1_(resources::gamedata->get_variable("y1")),
  116
+		y2_(resources::gamedata->get_variable("y2")),
  117
+		queue_(), // Filled later with a swap().
  118
+		pumped_count_(0)
  119
+	{
  120
+		queue_.swap(events_queue);
  121
+		++instance_count;
  122
+	}
  123
+
  124
+	pump_manager::~pump_manager() {
  125
+		--instance_count;
  126
+
  127
+		// Not sure what the correct thing to do is here. In princple,
  128
+		// discarding all events (i.e. clearing events_queue) seems like
  129
+		// the right thing to do in the face of an exeption. However, the
  130
+		// previous functionality preserved the queue, so for now we will
  131
+		// restore it.
  132
+		if ( !done() ) {
  133
+			// The remainig events get inserted at the beginning of events_queue.
  134
+			std::vector<queued_event> temp;
  135
+			events_queue.swap(temp);
  136
+			events_queue.insert(events_queue.end(), queue_.begin() + pumped_count_, queue_.end());
  137
+			events_queue.insert(events_queue.end(), temp.begin(), temp.end());
  138
+		}
  139
+
  140
+		// Restore the old values of the game variables.
  141
+		resources::gamedata->get_variable("y2") = y2_;
  142
+		resources::gamedata->get_variable("y1") = y1_;
  143
+		resources::gamedata->get_variable("x2") = x2_;
  144
+		resources::gamedata->get_variable("x1") = x1_;
  145
+	}
  146
+
  147
+
112 148
 	inline bool events_init()
113 149
 	{
114 150
 		return resources::screen != NULL;
@@ -418,21 +454,19 @@ void raise(const std::string& event,
418 454
 
419 455
 bool pump()
420 456
 {
421  
-	//ensure the whiteboard doesn't attempt to build its future unit map
422  
-	//for the duration of this method
423  
-	wb::real_map real_unit_map;
424  
-
  457
+	// Quick aborts:
425 458
 	assert(manager::running());
426 459
 	if(!events_init())
427 460
 		return false;
428  
-
429  
-	pump_manager pump_instance;
  461
+	if ( events_queue.empty() ) {
  462
+		DBG_EH << "Processing queued events, but none found.\n";
  463
+		return false;
  464
+	}
430 465
 	if(pump_manager::count() >= game_config::max_loop) {
431 466
 		ERR_NG << "game_events::pump() waiting to process new events because "
432  
-			<< "recursion level would exceed maximum " << game_config::max_loop << '\n';
  467
+		       << "recursion level would exceed maximum: " << game_config::max_loop << '\n';
433 468
 		return false;
434 469
 	}
435  
-
436 470
 	if(!lg::debug.dont_log("event_handler")) {
437 471
 		std::stringstream ss;
438 472
 		BOOST_FOREACH(const queued_event& ev, events_queue) {
@@ -442,13 +476,21 @@ bool pump()
442 476
 	}
443 477
 
444 478
 	const size_t old_wml_track = internal_wml_tracking;
445  
-
446 479
 	bool result = false;
447  
-	while(events_queue.empty() == false) {
  480
+	// Ensure the whiteboard doesn't attempt to build its future unit map
  481
+	// while events are being processed.
  482
+	wb::real_map real_unit_map;
  483
+
  484
+	{ // Scope limitation for pump_manager
  485
+	pump_manager pump_instance;
  486
+	
  487
+	// Loop through the events we need to process.
  488
+	while ( !pump_instance.done() )
  489
+	{
448 490
 		if(pump_manager::count() <= 1)
449 491
 			manager::start_buffering();
450  
-		queued_event ev = events_queue.front();
451  
-		events_queue.pop_front();	// pop now for exception safety
  492
+
  493
+		queued_event & ev = pump_instance.next();
452 494
 		const std::string& event_name = ev.name;
453 495
 
454 496
 		// Clear the unit cache, since the best clearing time is hard to figure out
@@ -488,12 +530,23 @@ bool pump()
488 530
 		// Only commit new handlers when finished iterating over event_handlers.
489 531
 		commit();
490 532
 	}
  533
+	} // Scope limitation for pump_manager
491 534
 
492 535
 	if ( old_wml_track != internal_wml_tracking )
493 536
 		// Notify the whiteboard of any event.
494 537
 		// This is used to track when moves, recruits, etc. happen.
495 538
 		resources::whiteboard->on_gamestate_change();
496 539
 
  540
+	// The previous version of this would iterate through all events, including
  541
+	// those raised, but not pumped, while this function is executing. This
  542
+	// should not actually occur, but to preserve this behavior, initiate a
  543
+	// recursion.
  544
+	// (I think I'll remove this bit shortly, but at least this will allow
  545
+	// that to be done in a separate source code commit.)
  546
+	if ( !events_queue.empty() )
  547
+		if ( pump() )
  548
+			result = true;
  549
+
497 550
 	return result;
498 551
 }
499 552
 

0 notes on commit b4ce6a6

Please sign in to comment.
Something went wrong with that request. Please try again.