From 477bb5df7d2427df02482d8e9e8d05a0514b9423 Mon Sep 17 00:00:00 2001 From: Nat Quayle Nelson Date: Thu, 21 Nov 2024 11:05:17 -0600 Subject: [PATCH 1/3] wrap pollEvent() to guarantee all loops handle modifiers --- src/dialogxml/dialogs/dialog.cpp | 6 ++---- src/dialogxml/widgets/control.cpp | 3 ++- src/dialogxml/widgets/field.cpp | 2 +- src/dialogxml/widgets/scrollbar.cpp | 3 ++- src/game/boe.actions.cpp | 6 +++--- src/game/boe.main.cpp | 7 ++----- src/game/boe.specials.cpp | 3 ++- src/game/boe.startup.cpp | 2 +- src/game/boe.ui.cpp | 3 ++- src/pcedit/pc.main.cpp | 5 +---- src/scenedit/scen.graphics.cpp | 2 +- src/scenedit/scen.main.cpp | 5 +---- src/tools/winutil.cpp | 20 ++++++++++++++++++++ src/tools/winutil.hpp | 4 ++++ 14 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/dialogxml/dialogs/dialog.cpp b/src/dialogxml/dialogs/dialog.cpp index 397dbc491..256859055 100644 --- a/src/dialogxml/dialogs/dialog.cpp +++ b/src/dialogxml/dialogs/dialog.cpp @@ -552,7 +552,7 @@ void cDialog::run(std::function 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(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){ diff --git a/src/dialogxml/widgets/control.cpp b/src/dialogxml/widgets/control.cpp index d4b681a7f..f77096413 100644 --- a/src/dialogxml/widgets/control.cpp +++ b/src/dialogxml/widgets/control.cpp @@ -19,6 +19,7 @@ #include "tools/cursors.hpp" #include "replay.hpp" #include +#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); diff --git a/src/dialogxml/widgets/field.cpp b/src/dialogxml/widgets/field.cpp index 5e4d54dbd..46c2467f2 100644 --- a/src/dialogxml/widgets/field.cpp +++ b/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){ diff --git a/src/dialogxml/widgets/scrollbar.cpp b/src/dialogxml/widgets/scrollbar.cpp index 045b26d23..7fe1998c3 100644 --- a/src/dialogxml/widgets/scrollbar.cpp +++ b/src/dialogxml/widgets/scrollbar.cpp @@ -15,6 +15,7 @@ #include "tools/cursors.hpp" #include "replay.hpp" #include +#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; diff --git a/src/game/boe.actions.cpp b/src/game/boe.actions.cpp index 1d9810165..34cf72674 100644 --- a/src/game/boe.actions.cpp +++ b/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(); @@ -3644,7 +3644,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) diff --git a/src/game/boe.main.cpp b/src/game/boe.main.cpp index 726cf58b6..fafdf2b57 100644 --- a/src/game/boe.main.cpp +++ b/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; diff --git a/src/game/boe.specials.cpp b/src/game/boe.specials.cpp index 2ec0985ba..9456bbf39 100644 --- a/src/game/boe.specials.cpp +++ b/src/game/boe.specials.cpp @@ -33,6 +33,7 @@ #include "boe.menus.hpp" #include "replay.hpp" #include +#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", ""); } diff --git a/src/game/boe.startup.cpp b/src/game/boe.startup.cpp index 0a68edb67..fde8b77e7 100644 --- a/src/game/boe.startup.cpp +++ b/src/game/boe.startup.cpp @@ -123,7 +123,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); } diff --git a/src/game/boe.ui.cpp b/src/game/boe.ui.cpp index 5b00f1169..d99d4f8d5 100644 --- a/src/game/boe.ui.cpp +++ b/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); diff --git a/src/pcedit/pc.main.cpp b/src/pcedit/pc.main.cpp index 2d6c19c99..b2588ccf1 100644 --- a/src/pcedit/pc.main.cpp +++ b/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; diff --git a/src/scenedit/scen.graphics.cpp b/src/scenedit/scen.graphics.cpp index 8f902d465..6b303fc00 100644 --- a/src/scenedit/scen.graphics.cpp +++ b/src/scenedit/scen.graphics.cpp @@ -366,7 +366,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) diff --git a/src/scenedit/scen.main.cpp b/src/scenedit/scen.main.cpp index 2b886e2bd..de6968e19 100644 --- a/src/scenedit/scen.main.cpp +++ b/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; diff --git a/src/tools/winutil.cpp b/src/tools/winutil.cpp index 0c24ab4ab..140f8ca2f 100644 --- a/src/tools/winutil.cpp +++ b/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); } \ No newline at end of file diff --git a/src/tools/winutil.hpp b/src/tools/winutil.hpp index b6c3de644..cb56fb5eb 100644 --- a/src/tools/winutil.hpp +++ b/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); From dde430c83c790ddcaadd1486633f4d4fde26a2ef Mon Sep 17 00:00:00 2001 From: Nat Quayle Nelson Date: Thu, 21 Nov 2024 11:47:37 -0600 Subject: [PATCH 2/3] add include --- src/scenedit/scen.graphics.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scenedit/scen.graphics.cpp b/src/scenedit/scen.graphics.cpp index 6b303fc00..99888dacb 100644 --- a/src/scenedit/scen.graphics.cpp +++ b/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" From 72767af07edd97ff4163cab8d1129210e823873e Mon Sep 17 00:00:00 2001 From: Nat Quayle Nelson Date: Sat, 23 Nov 2024 13:06:56 -0600 Subject: [PATCH 3/3] Update src/tools/winutil.cpp Co-authored-by: Celtic Minstrel --- src/tools/winutil.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/winutil.cpp b/src/tools/winutil.cpp index 140f8ca2f..ffc291321 100644 --- a/src/tools/winutil.cpp +++ b/src/tools/winutil.cpp @@ -38,7 +38,7 @@ double fallback_scale() { // 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(win.pollEvent(event)) { if(kb.handleModifier(event)) return false; return true; }