wrap pollEvent() to guarantee all loops handle modifiers (#477)

src/dialogxml/dialogs/dialog.cpp

@@ -552,7 +552,7 @@ void cDialog::run(std::function<void(cDialog&)> onopen){
 	handle_events();
 
 	win.setVisible(false);
-	while(parentWin->pollEvent(currentEvent));
+	while(pollEvent(parentWin, currentEvent));
 	set_cursor(former_curs);
 	topWindow = formerTop;
 	makeFrontWindow(*parentWin);
@@ -584,7 +584,7 @@ void cDialog::handle_events() {
 			cTextField& text_field = dynamic_cast<cTextField&>(getControl(currentFocus));
 			text_field.replay_selection(pop_next_action());
 		}else{
-			while(win.pollEvent(currentEvent)){
+			while(pollEvent(win, currentEvent)){
 				handle_one_event(currentEvent, fps_limiter);
 			}
 		}
@@ -618,8 +618,6 @@ void cDialog::handle_one_event(const sf::Event& currentEvent, cFramerateLimiter&
 	std::string itemHit = "";
 	location where;
 
-	if(kb.handleModifier(currentEvent)) return;
-
 	switch(currentEvent.type) {
 		case sf::Event::KeyPressed:
 			switch(currentEvent.key.code){

src/dialogxml/widgets/control.cpp

@@ -19,6 +19,7 @@
 #include "tools/cursors.hpp"
 #include "replay.hpp"
 #include <boost/lexical_cast.hpp>
+#include "winutil.hpp"
 
 void cControl::setText(std::string l){
 	lbl = l;
@@ -228,7 +229,7 @@ bool cControl::handleClick(location, cFramerateLimiter& fps_limiter){
 	depressed = true;
 	while(!done){
 		redraw();
-		while(inWindow->pollEvent(e)){
+		while(pollEvent(inWindow, e)){
 			if(e.type == sf::Event::MouseButtonReleased){
 				done = true;
 				location clickPos(e.mouseButton.x, e.mouseButton.y);

src/dialogxml/widgets/field.cpp

@@ -163,7 +163,7 @@ bool cTextField::handleClick(location clickLoc, cFramerateLimiter& fps_limiter)
 	int initial_ip = insertionPoint, initial_sp = selectionPoint;
 	while(!done) {
 		redraw();
-		while(inWindow->pollEvent(e)){
+		while(pollEvent(inWindow, e)){
 			if(e.type == sf::Event::MouseButtonReleased){
 				done = true;
 			} else if(e.type == sf::Event::MouseMoved){

src/dialogxml/widgets/scrollbar.cpp

@@ -15,6 +15,7 @@
 #include "tools/cursors.hpp"
 #include "replay.hpp"
 #include <boost/lexical_cast.hpp>
+#include "winutil.hpp"
 
 std::string cScrollbar::scroll_textures[NUM_STYLES] = {
 	"dlogscrollwh",
@@ -319,7 +320,7 @@ bool cScrollbar::handleClick(location where, cFramerateLimiter& fps_limiter) {
 	int diff = clickPos - thumbPos;
 	while(!done){
 		redraw();
-		while(inWindow->pollEvent(e)){
+		while(pollEvent(inWindow, e)){
 			location mouseLoc = sf::Mouse::getPosition(*inWindow);
 			mouseLoc = inWindow->mapPixelToCoords(mouseLoc);
 			int mousePos = vert ? mouseLoc.y : mouseLoc.x;

src/game/boe.actions.cpp

@@ -408,7 +408,7 @@ void handle_rest(bool& need_redraw, bool& need_reprint) {
 				i = 200;
 				add_string_to_buf("  Monsters nearby.");
 			}
-			while(mainPtr.pollEvent(dummy_evt));
+			while(pollEvent(mainPtr, dummy_evt));
 			redraw_screen(REFRESH_NONE);
 			i++;
 		}
@@ -1068,7 +1068,7 @@ static void handle_town_wait(bool& need_redraw, bool& need_reprint) {
 			i = 200;
 			add_string_to_buf("  Monster sighted!");
 		}
-		while(mainPtr.pollEvent(dummy_evt));
+		while(pollEvent(mainPtr, dummy_evt));
 		redraw_screen(REFRESH_NONE);
 	}
 	put_pc_screen();
@@ -3648,7 +3648,7 @@ bool check_for_interrupt(){
 		pop_next_action();
 		interrupt = true;
 	}
-	else if(mainPtr.pollEvent(evt) && evt.type == sf::Event::KeyPressed) {
+	else if(pollEvent(mainPtr, evt) && evt.type == sf::Event::KeyPressed) {
 		// TODO: I wonder if there are other events we should handle here? Resize maybe?
 #ifdef __APPLE__
 		if(evt.key.code == kb::Period && evt.key.system)

src/game/boe.main.cpp

@@ -827,12 +827,12 @@ void handle_events() {
 				fake_event_queue.pop_front();
 				handle_one_event(next_event, fps_limiter);
 			}
-			while(mainPtr.pollEvent(currentEvent)) handle_one_event(currentEvent, fps_limiter);
+			while(pollEvent(mainPtr, currentEvent)) handle_one_event(currentEvent, fps_limiter);
 
 			// It would be nice to have minimap inside the main game window (we have lots of screen space in fullscreen mode).
 			// Alternatively, minimap could live on its own thread.
 			// But for now we just handle events from both windows on this thread.
-			while(map_visible && mini_map.pollEvent(currentEvent)) handle_one_minimap_event(currentEvent);
+			while(map_visible && pollEvent(mini_map, currentEvent)) handle_one_minimap_event(currentEvent);
 		}
 
 		if(changed_display_mode) {
@@ -893,9 +893,6 @@ void handle_one_event(const sf::Event& event, cFramerateLimiter& fps_limiter) {
 	// What does this do and should it be here?
 	clear_sound_memory();
 
-	// If it's just a modifier key, update the state
-	if(kb.handleModifier(event)) return;
-
 	// Check if any of the event listeners want this event.
 	for(auto & listener : event_listeners) {
 		if(listener.second->handle_event(event)) return;

src/game/boe.specials.cpp

@@ -33,6 +33,7 @@
 #include "boe.menus.hpp"
 #include "replay.hpp"
 #include <boost/lexical_cast.hpp>
+#include "winutil.hpp"
 
 extern sf::RenderWindow mainPtr;
 extern eGameMode overall_mode;
@@ -2017,7 +2018,7 @@ void run_special(eSpecCtx which_mode, eSpecCtxType which_type, spec_num_t start_
 				if(replaying && has_next_action("step_through_continue")){
 					pop_next_action();
 					break;
-				}else if(mainPtr.pollEvent(evt) && (evt.type == sf::Event::KeyPressed || evt.type == sf::Event::MouseButtonPressed)){
+				}else if(pollEvent(mainPtr, evt) && (evt.type == sf::Event::KeyPressed || evt.type == sf::Event::MouseButtonPressed)){
 					if(recording){
 						record_action("step_through_continue", "");
 					}

src/game/boe.startup.cpp

@@ -126,7 +126,7 @@ bool handle_startup_press(location the_point) {
 
 static void handle_splash_events(cFramerateLimiter& fps_limiter) {
 	sf::Event event;
-	while(mainPtr.pollEvent(event)) {
+	while(pollEvent(mainPtr, event)) {
 		if(event.type == sf::Event::GainedFocus || event.type == sf::Event::MouseMoved)
 			set_cursor(sword_curs);
 	}

src/game/boe.ui.cpp

@@ -18,6 +18,7 @@
 #include "fileio/resmgr/res_image.hpp"
 #include "mathutil.hpp"
 #include "sounds.hpp"
+#include "winutil.hpp"
 
 namespace UI {
 	cToolbar toolbar;
@@ -62,7 +63,7 @@ eToolbarButton cToolbar::button_hit(sf::RenderWindow& win, location click, cFram
 				active = i;
 				while(!done){
 					redraw_screen(REFRESH_NONE);
-					while(win.pollEvent(e)) {
+					while(pollEvent(win, e)) {
 						if(e.type == sf::Event::MouseButtonReleased){
 							done = true;
 							location clickPos(e.mouseButton.x, e.mouseButton.y);

src/pcedit/pc.main.cpp

@@ -214,7 +214,7 @@ void handle_events() {
 			menuChoiceId=-1;
 		}
 #endif
-		while(mainPtr.pollEvent(currentEvent)) handle_one_event(currentEvent);
+		while(pollEvent(mainPtr, currentEvent)) handle_one_event(currentEvent);
 
 		redraw_everything();
 
@@ -225,9 +225,6 @@ void handle_events() {
 
 void handle_one_event (const sf::Event& event) {
 
-	// If it's just a modifier key, update the state
-	if(kb.handleModifier(event)) return;
-
 	// Check if the menubar wants to handle this event.
 	if(menuBarProcessEvent(event)) return;
 

src/scenedit/scen.graphics.cpp

@@ -16,6 +16,7 @@
 #include "mathutil.hpp"
 #include "tools/drawable_manager.hpp"
 #include "tools/cursors.hpp"
+#include "tools/winutil.hpp"
 
 #include "dialogxml/dialogs/dialog.hpp"
 
@@ -366,7 +367,7 @@ void run_startup_g() {
 	sf::Clock timer;
 	while(sound_going(95) || timer.getElapsedTime() < delay) {
 		draw_splash(pict_to_draw, mainPtr, dest_rect);
-		if(!mainPtr.pollEvent(event)) continue;
+		if(!pollEvent(mainPtr, event)) continue;
 		if(event.type == sf::Event::GainedFocus || event.type == sf::Event::MouseMoved)
 			set_cursor(watch_curs);
 		if(event.type == sf::Event::KeyPressed || event.type == sf::Event::MouseButtonPressed)

src/scenedit/scen.main.cpp

@@ -273,7 +273,7 @@ void handle_events() {
 			menuChoiceId=-1;
 		}
 #endif
-		while(mainPtr.pollEvent(currentEvent)) handle_one_event(currentEvent);
+		while(pollEvent(mainPtr, currentEvent)) handle_one_event(currentEvent);
 
 		// Why do we have to set this to false after handling every event?
 		ae_loading = false;
@@ -287,9 +287,6 @@ void handle_events() {
 
 void handle_one_event(const sf::Event& event) {
 
-	// If it's just a modifier key, update the state
-	if(kb.handleModifier(event)) return;
-
 	// Check if any of the event listeners want this event.
 	for (auto& listener : event_listeners) {
 		if(listener.second->handle_event(event)) return;

src/tools/winutil.cpp

@@ -1,5 +1,7 @@
 #include "winutil.hpp"
 
+#include "keymods.hpp"
+
 // The default scale should be the largest that the user's screen can fit all three
 // BoE application windows (because they should probably default to match each other).
 double fallback_scale() {
@@ -28,4 +30,22 @@ double fallback_scale() {
 	}
 
 	return scale;
+}
+
+// We use many nested event loops in this codebase. Each one of them
+// calls pollEvent() and they each need to remember to call handleModifier()
+// or else modifier keys will claim to be held forever.
+// The best solution for this is to wrap pollEvent() so that it calls
+// handleModifier for us every time.
+bool pollEvent(sf::Window& win, sf::Event& event){
+    if(win.pollEvent(event)) {
+        if(kb.handleModifier(event)) return false;
+        return true;
+    }
+
+    return false;
+}
+
+bool pollEvent(sf::Window* win, sf::Event& event){
+    return pollEvent(*win, event);
 }

src/tools/winutil.hpp

@@ -23,6 +23,10 @@ char keyToChar(sf::Keyboard::Key key, bool isShift);
 void makeFrontWindow(sf::Window& win);
 void setWindowFloating(sf::Window& win, bool floating);
 
+// Necessary wrapper for sf::Window.pollEvent()
+bool pollEvent(sf::Window& win, sf::Event& event);
+bool pollEvent(sf::Window* win, sf::Event& event);
+
 void init_fileio();
 void launchURL(std::string url);