Merge pull request #251 from x-qq/fix_scrollbar_segfaults

fix for scrollbar segfaults

* fixes #206
* fixes broken mousewheel scrolling of the scenedit palette
* removed boost threads dependency
* added foundation for further refactoring of the drawing and event handling code: interfaces and drawable manager with layering
* removed a bunch of unneeded redraw calls
* removed some repeated recalculation of effectively constant values (boe.actions)
* removed recalculation of effectively constant scrollbar and button positions (boe.graphics)

Closes #251

src/game/boe.actions.cpp

@@ -72,7 +72,6 @@ cOutdoors::cWandering store_wandering_special;
 short store_selling_values[8] = {0,0,0,0,0,0,0,0};
 extern cShop active_shop;
 
-extern rectangle shop_frame;
 extern short cen_x, cen_y;//,pc_moves[6];
 extern eGameMode overall_mode;
 extern eItemWinMode stat_window;
@@ -2114,29 +2113,19 @@ bool handle_keystroke(const sf::Event& event){
 }
 
 bool handle_scroll(const sf::Event& event) {
-	rectangle status_panel_rect = {0,0,144,271}, text_panel_rect = {0,0,138,271};
 	rectangle world_screen = win_to_rects[WINRECT_TERVIEW];
 	world_screen.inset(13, 13);
-	status_panel_rect.offset(win_to_rects[WINRECT_INVEN].topLeft());
-	text_panel_rect.offset(win_to_rects[WINRECT_TRANSCRIPT].topLeft());
 	fill_rect(mainPtr, world_screen, sf::Color::Magenta);
+	
+	// TODO: centralize this translation somewhere
 	location pos(event.mouseWheel.x, event.mouseWheel.y);
 	pos = mainPtr.mapPixelToCoords(pos, mainView);
+	
 	int amount = event.mouseWheel.delta;
-	if(item_sbar->isVisible() && pos.in(status_panel_rect)) {
-		item_sbar->setPosition(item_sbar->getPosition() - amount);
-		redraw_screen(REFRESH_INVEN);
-	} else if(text_sbar->isVisible() && pos.in(text_panel_rect)) {
-		text_sbar->setPosition(text_sbar->getPosition() - amount);
-		redraw_screen(REFRESH_TRANS);
-	} else if(shop_sbar->isVisible() && pos.in(shop_frame)) {
-		shop_sbar->setPosition(shop_sbar->getPosition() - amount);
-		redraw_screen(REFRESH_DLOG);
-	} else if(scrollableModes.count(overall_mode) && pos.in(world_screen)) {
+	if(scrollableModes.count(overall_mode) && pos.in(world_screen)) {
 		if(sf::Keyboard::isKeyPressed(sf::Keyboard::LControl) || sf::Keyboard::isKeyPressed(sf::Keyboard::RControl))
 			center.x = minmax(4, univ.town->max_dim - 5, center.x - amount);
 		else center.y = minmax(4, univ.town->max_dim - 5, center.y - amount);
-		redraw_screen(REFRESH_TERRAIN);
 	}
 	return true;
 }

src/game/boe.consts.hpp

@@ -158,8 +158,8 @@ const int MENUBAR_HEIGHT = 20;
 const int NUM_MAGE_SPELLS = 62;
 const int NUM_PRIEST_SPELLS = 62;
 
-// spell levels
-const int MAGE_SPELL_LEVEL_MAX = 7;
-const int PRIEST_SPELL_LEVEL_MAX = 7;
+// UI layers
+const int UI_LAYER_DEFAULT = 1000;
+const int UI_LAYER_MENUBAR = 1200;
 
 #endif

src/game/boe.graphics.cpp

@@ -2,6 +2,9 @@
 #include <cstring>
 #include <cstdio>
 #include <list>
+#include <unordered_map>
+#include <string>
+#include <memory>
 
 #include "boe.global.hpp"
 
@@ -19,6 +22,7 @@
 #include "mathutil.hpp"
 #include "button.hpp"
 #include "enum_map.hpp"
+#include "drawable_manager.hpp"
 
 #include "boe.party.hpp"
 #include "boe.town.hpp"
@@ -56,7 +60,8 @@ extern location spell_targets[8];
 extern std::shared_ptr<cScrollbar> text_sbar,item_sbar,shop_sbar;
 extern std::shared_ptr<cButton> done_btn, help_btn;
 extern sf::Texture bg_gworld;
-extern rectangle sbar_rect,item_sbar_rect,shop_sbar_rect,startup_top;
+extern rectangle const sbar_rect,item_sbar_rect,shop_sbar_rect;
+extern rectangle startup_top;
 extern rectangle talk_area_rect, word_place_rect;
 extern location store_anim_ul;
 extern long register_flag;
@@ -68,6 +73,7 @@ extern cCustomGraphics spec_scen_g;
 extern sf::RenderWindow mini_map;
 bool map_visible = false;
 extern std::string save_talk_str1, save_talk_str2;
+extern cDrawableManager drawable_mgr;
 
 rectangle		menuBarRect;
 Region originalGrayRgn, newGrayRgn, underBarRgn;
@@ -166,13 +172,7 @@ void adjust_window_mode() {
 	const ImageRsrc& icon = ResMgr::graphics.get("icon", true);
 	mainPtr.setIcon(icon->getSize().x, icon->getSize().y, icon->copyToImage().getPixelsPtr());
 #endif
-	if(text_sbar) {
-		text_sbar->relocate({560,285});
-		item_sbar->relocate({560,148});
-		shop_sbar->relocate({272,69});
-		done_btn->relocate({231,395});
-		help_btn->relocate({273,12});
-	}
+
 	init_menubar();
 	showMenuBar();
 }
@@ -538,13 +538,10 @@ void redraw_screen(int refresh) {
 			draw_targeting_line(sf::Mouse::getPosition(mainPtr));
 		refresh_stat_areas(0);
 	}
-	text_sbar->draw();
-	item_sbar->draw();
-	shop_sbar->draw();
 	done_btn->draw();
 	help_btn->draw();
 
-	drawMenuBar();
+	drawable_mgr.draw_all();
 
 	mainPtr.display();
 }

src/game/boe.main.cpp

@@ -2,9 +2,10 @@
 #include "boe.global.hpp"
 #include "universe.hpp"
 
-#define BOOST_NO_CXX11_NUMERIC_LIMITS // Because my libc++ is old and not quite standard-compliant, which breaks Boost.Thread
-#include <boost/thread.hpp>
 #include <boost/filesystem/operations.hpp>
+#include <unordered_map>
+#include <string>
+#include <memory>
 #include "boe.graphics.hpp"
 #include "boe.newgraph.hpp"
 #include "boe.fileio.hpp"
@@ -17,6 +18,8 @@
 #include "boe.dlgutil.hpp"
 #include "boe.infodlg.hpp"
 #include "boe.main.hpp"
+#include "boe.consts.hpp"
+#include "boe.ui.hpp"
 #include "winutil.hpp"
 #include "sounds.hpp"
 #include "render_image.hpp"
@@ -32,7 +35,8 @@
 #include "button.hpp"
 #include "enum_map.hpp"
 #include "framerate_limiter.hpp"
-
+#include "event_listener.hpp"
+#include "drawable_manager.hpp"
 bool All_Done = false;
 sf::RenderWindow mainPtr;
 short had_text_freeze = 0,num_fonts;
@@ -41,9 +45,10 @@ bool first_sound_played = false,spell_forced = false;
 bool party_in_memory = false;
 std::shared_ptr<cScrollbar> text_sbar, item_sbar, shop_sbar;
 std::shared_ptr<cButton> done_btn, help_btn;
-rectangle sbar_rect = {283,546,421,562};
-rectangle shop_sbar_rect = {67,258,357,274};
-rectangle item_sbar_rect = {146,546,253,562};
+// TODO: move these 3 to boe.ui.cpp ?
+extern rectangle const sbar_rect      = {285,560,423,576};
+extern rectangle const shop_sbar_rect = {69,272,359,288};
+extern rectangle const item_sbar_rect = {148,560,255,576};
 bool bgm_on = false,bgm_init = false;
 location store_anim_ul;
 cUniverse univ;
@@ -57,6 +62,9 @@ short on_monst_menu[256];
 extern bool map_visible;
 extern sf::View mainView;
 
+extern rectangle shop_frame;
+extern enum_map(eGuiArea, rectangle) win_to_rects;
+
 std::string scenario_temp_dir_name = "scenario";
 
 /* Display globals */
@@ -91,6 +99,10 @@ effect_pat_type current_pat;
 short missile_firer,current_monst_tactic;
 short store_current_pc = 0;
 
+// TODO: these should be members of some global entity instead of being here
+std::unordered_map <std::string, std::shared_ptr <iEventListener>> event_listeners;
+cDrawableManager drawable_mgr;
+
 sf::Clock animTimer;
 extern long anim_ticks;
 
@@ -135,21 +147,65 @@ int main(int argc, char* argv[]) {
 	}
 }
 
-static void init_sbar(std::shared_ptr<cScrollbar>& sbar, rectangle rect, int max, int pgSz, int start = 0) {
+static void init_sbar(std::shared_ptr<cScrollbar>& sbar, std::string const name, rectangle rect, rectangle events_rect, int max, int pgSz, int start = 0) {
 	sbar.reset(new cScrollbar(mainPtr));
 	sbar->setBounds(rect);
 	sbar->setMaximum(max);
 	sbar->setPosition(start);
 	sbar->setPageSize(pgSz);
+	sbar->set_wheel_event_rect(events_rect);
 	sbar->hide();
+	
+	drawable_mgr.add_drawable(UI_LAYER_DEFAULT, name, sbar);
+	event_listeners[name] = std::dynamic_pointer_cast <iEventListener> (sbar);
 }
 
-static void init_btn(std::shared_ptr<cButton>& btn, eBtnType type) {
+static void init_scrollbars() {
+
+	// Cover entire transcript + scrollbar
+	rectangle const transcript_events_rect {
+		win_to_rects[WINRECT_TRANSCRIPT].top,
+		win_to_rects[WINRECT_TRANSCRIPT].left,
+		sbar_rect.bottom,
+		sbar_rect.right
+	};
+	
+	// Cover entire inventory + scrollbar
+	rectangle const inventory_events_rect {
+		win_to_rects[WINRECT_INVEN].top,
+		win_to_rects[WINRECT_INVEN].left,
+		item_sbar_rect.bottom,
+		item_sbar_rect.right
+	};
+	
+	// MAGIC NUMBERS: max size, page size, initial position - all in abstract "step" units
+	init_sbar(text_sbar, "transcript-scrollbar", sbar_rect, transcript_events_rect, 58, 11, 58);
+	init_sbar(item_sbar, "inventory-scrollbar", item_sbar_rect, inventory_events_rect, 16, 8);
+	init_sbar(shop_sbar, "shop-scrollbar", shop_sbar_rect, shop_frame, 16, 8);	
+}
+
+static void init_btn(std::shared_ptr<cButton>& btn, eBtnType type, location loc) {
 	btn.reset(new cButton(mainPtr));
 	btn->setBtnType(type);
+	btn->relocate(loc);
 	btn->hide();
 }
 
+static void init_buttons() {
+	
+	// MAGIC NUMBERS: move to boe.ui.cpp ?
+	
+	init_btn(done_btn, BTN_DONE, {231,395});
+	init_btn(help_btn, BTN_HELP, {273,12});
+}
+
+// NOTE: this should possibly be moved to boe.ui.cpp at some point
+static void init_ui() {
+	cDialog::init();
+	init_scrollbars();
+	init_buttons();
+}
+
 void init_boe(int argc, char* argv[]) {
 	set_up_apple_events(argc, argv);
 	init_directories(argv[0]);
@@ -161,12 +217,7 @@ void init_boe(int argc, char* argv[]) {
 	init_tiling();
 	init_snd_tool();
 	
-	cDialog::init();
-	init_sbar(text_sbar, sbar_rect, 58, 11, 58);
-	init_sbar(item_sbar, item_sbar_rect, 16, 8);
-	init_sbar(shop_sbar, shop_sbar_rect, 16, 8);
-	init_btn(done_btn, BTN_DONE);
-	init_btn(help_btn, BTN_HELP);
+	init_ui();
 	
 	adjust_window_mode();
 	// If we don't do this now it'll flash white to start with
@@ -236,8 +287,10 @@ void handle_one_event(const sf::Event& event) {
 	// What does this do and should it be here?
 	clear_sound_memory();
 
-	// Check if the menubar wants to handle this event.
-	if(menuBarProcessEvent(event)) return;
+	// Check if any of the event listeners want this event.
+	for(auto & listener : event_listeners) {
+		if(listener.second->handle_event(event)) return;
+	}
 
 	switch(event.type) {
 		case sf::Event::KeyPressed:
@@ -264,6 +317,7 @@ void handle_one_event(const sf::Event& event) {
 			change_cursor({event.mouseMove.x, event.mouseMove.y});
 			return;
 
+		// TODO: EVENT TYPE DEPRECATED IN SFML 2.5.1
 		case sf::Event::MouseWheelMoved:
 			if(flushingInput) return;
 			handle_scroll(event);
@@ -348,14 +402,6 @@ void redraw_everything() {
 	if(map_visible) draw_map(false);
 }
 
-static void handleUpdateWhileScrolling(volatile bool& doneScrolling, int refresh) {
-	while(!doneScrolling) {
-		sf::sleep(sf::milliseconds(10));
-		redraw_screen(refresh);
-	}
-	mainPtr.setActive(false);
-}
-
 void Mouse_Pressed(sf::Event const & event) {
 
 	// What is this stuff? Why is it here?
@@ -364,37 +410,14 @@ void Mouse_Pressed(sf::Event const & event) {
 		return;
 	}
 	
-	if(overall_mode != MODE_STARTUP) {
-		location mousePos(event.mouseButton.x, event.mouseButton.y);
-		mousePos = mainPtr.mapPixelToCoords(mousePos, mainView);
-		volatile bool doneScrolling = false;
-		if(mousePos.in(text_sbar->getBounds())) {
-			mainPtr.setActive(false);
-			boost::thread updater(std::bind(handleUpdateWhileScrolling, std::ref(doneScrolling), REFRESH_TRANS));
-			text_sbar->handleClick(mousePos);
-			doneScrolling = true;
-			updater.join();
-			redraw_screen(REFRESH_TRANS);
-		} else if(mousePos.in(item_sbar->getBounds())) {
-			mainPtr.setActive(false);
-			boost::thread updater(std::bind(handleUpdateWhileScrolling, std::ref(doneScrolling), REFRESH_INVEN));
-			item_sbar->handleClick(mousePos);
-			doneScrolling = true;
-			updater.join();
-			redraw_screen(REFRESH_INVEN);
-		} else if(overall_mode == MODE_SHOPPING && mousePos.in(shop_sbar->getBounds())) {
-			mainPtr.setActive(false);
-			boost::thread updater(std::bind(handleUpdateWhileScrolling, std::ref(doneScrolling), REFRESH_DLOG));
-			shop_sbar->handleClick(mousePos);
-			doneScrolling = true;
-			updater.join();
-			redraw_screen(REFRESH_DLOG);
-		} else All_Done = handle_action(event);
-	} else All_Done = handle_startup_press({event.mouseButton.x, event.mouseButton.y});
+	if(overall_mode == MODE_STARTUP) {
+		All_Done = handle_startup_press({event.mouseButton.x, event.mouseButton.y});
+	} else {
+		All_Done = handle_action(event);
+	}
 	
 	// Why does every mouse click activate a menu?
 	menu_activate();
-	
 }
 
 void close_program() {

src/game/boe.menu.cpp

@@ -10,7 +10,6 @@
 
 OpenBoEMenu::OpenBoEMenu(sf::RenderWindow& window, cUniverse& universe)
 	: tgui { window }
-	, mainPtr { window }
 	, univ { universe } {
 
 	// Build a menubar and store it in tgui with a known name
@@ -26,7 +25,7 @@ OpenBoEMenu::OpenBoEMenu(sf::RenderWindow& window, cUniverse& universe)
 tgui::MenuBar::Ptr OpenBoEMenu::build_menubar() const {
 	auto menubar = tgui::MenuBar::create();
 
-	menubar->setSize(this->mainPtr.getSize().x, MENUBAR_HEIGHT);
+	menubar->setSize("100%", MENUBAR_HEIGHT);
 
 	this->add_menu_placeholders(menubar);
 	this->add_persistent_menu_items(menubar);

src/game/boe.menu.hpp

@@ -10,14 +10,16 @@
 #include "boe.consts.hpp"
 #include "spell.hpp"
 #include "skills_traits.hpp"
+#include "event_listener.hpp"
+#include "drawable.hpp"
 
-class OpenBoEMenu {
+class OpenBoEMenu : public iEventListener, public iDrawable {
 public:
 
 	OpenBoEMenu(sf::RenderWindow&, cUniverse&);
 
-	bool handle_event(const sf::Event&);
-	void draw();
+	virtual bool handle_event(const sf::Event&) override;
+	virtual void draw() override;
 	void update_for_game_state(eGameMode overall_mode, bool party_in_memory);
 	void update_spell_menus();
 	void update_monsters_menu();
@@ -27,7 +29,6 @@ private:
 	using MenuHierarchy = std::vector<sf::String>;
 
 	tgui::Gui tgui;
-	sf::RenderWindow& mainPtr;
 	cUniverse& univ;
 	const sf::String internal_menubar_widget_name { "openboe-menu" };
 	std::vector<unsigned int> spell_menus_connection_ids;

src/game/boe.menus.hpp

@@ -22,12 +22,6 @@ void menu_activate();
 void hideMenuBar();
 void showMenuBar();
 
-// If a menubar runs inside our own event loop, we need ways to supply it with events and to draw it.
-namespace sf { class Event; };
-// Return true if event has been consumed by the menubar.
-bool menuBarProcessEvent(const sf::Event&);
-void drawMenuBar();
-
 enum class eMenu {
 	NONE, ABOUT, PREFS, QUIT,
 	FILE_NEW, FILE_OPEN, FILE_ABORT, FILE_SAVE, FILE_SAVE_AS,

src/game/boe.menus.linux.cpp

@@ -1,28 +1,31 @@
 #include "boe.menus.hpp"
 #include "boe.consts.hpp"
 
+#include <unordered_map>
+#include <string>
 #include <memory>
 #include <SFML/Graphics.hpp>
 
 #include "boe.menu.hpp"
+#include "boe.consts.hpp"
+
+#include "event_listener.hpp"
+#include "drawable_manager.hpp"
 
 extern sf::RenderWindow mainPtr;
 extern cUniverse univ;
 extern bool party_in_memory;
 extern eGameMode overall_mode;
+extern std::unordered_map<std::string, std::shared_ptr<iEventListener>>event_listeners;
+extern cDrawableManager drawable_mgr;
 
 std::shared_ptr<OpenBoEMenu> menu_ptr;
 
 void init_menubar() {
 	menu_ptr.reset(new OpenBoEMenu(mainPtr, univ));
-}
-
-bool menuBarProcessEvent(const sf::Event& event) {
-	return menu_ptr->handle_event(event);
-}
-
-void drawMenuBar() {
-	menu_ptr->draw();
+	
+	event_listeners["menubar"] = std::dynamic_pointer_cast<iEventListener>(menu_ptr); 
+	drawable_mgr.add_drawable(UI_LAYER_MENUBAR, "menubar", menu_ptr); 
 }
 
 void adjust_monst_menu() {

src/game/boe.menus.mac.mm

@@ -278,13 +278,6 @@ void menu_activate() {
 	[[file_menu itemWithTitle: @"Save As…"] setEnabled: YES];
 }
 
-bool menuBarProcessEvent(const sf::Event&) {
-	return false;
-}
-
-void drawMenuBar() {
-}
-
 @implementation MenuHandler
 -(void) monstMenu:(id) sender {
 	cMonster* monst = [[sender representedObject] monst];

src/game/boe.menus.win.cpp

@@ -334,10 +334,3 @@ void set_up_apple_events(int argc, char* argv[]) {
 			post_load();
 	}
 }
-
-bool menuBarProcessEvent(const sf::Event&) {
-	return false;
-}
-
-void drawMenuBar() {
-}