From a1f44d8f6448eda20ced1af8ec042b2f9b2f787f Mon Sep 17 00:00:00 2001 From: Celtic Minstrel Date: Fri, 9 Oct 2020 01:15:14 -0400 Subject: [PATCH 1/2] Implement thread-safety in the logging system This chooses a method that minimizes the need to alter the way log messages are written. Only a few places that did unusual things with the logger needed to be updated. --- src/deprecation.cpp | 2 +- src/font/pango/stream_ops.hpp | 4 -- src/formatter.hpp | 25 ++++++++++ src/game_config_manager.cpp | 5 +- src/game_events/pump.cpp | 2 +- src/gui/core/log.hpp | 7 +-- src/log.cpp | 68 ++++++++++++++++++---------- src/log.hpp | 49 +++++++++++++++----- src/server/campaignd/addon_utils.cpp | 2 +- 9 files changed, 116 insertions(+), 48 deletions(-) diff --git a/src/deprecation.cpp b/src/deprecation.cpp index 4702f45e9c28..90ce291b374d 100644 --- a/src/deprecation.cpp +++ b/src/deprecation.cpp @@ -77,7 +77,7 @@ std::string deprecated_message( if(log_ptr && !log_ptr->dont_log(log_deprecate)) { const lg::logger& out_log = *log_ptr; - out_log(log_deprecate) << message << '\n'; + FORCE_LOG_TO(out_log, log_deprecate) << message << '\n'; if(preferences::get("show_deprecation", false)) { lg::wml_error() << message << '\n'; diff --git a/src/font/pango/stream_ops.hpp b/src/font/pango/stream_ops.hpp index 5d63324e3037..6426f9482314 100644 --- a/src/font/pango/stream_ops.hpp +++ b/src/font/pango/stream_ops.hpp @@ -17,12 +17,8 @@ #include #include -namespace font { - inline std::ostream& operator<<(std::ostream& s, const PangoRectangle &rect) { s << rect.x << ',' << rect.y << " x " << rect.width << ',' << rect.height; return s; } - -} // end namespace font diff --git a/src/formatter.hpp b/src/formatter.hpp index af9cec6c6ba3..6afb04b1245f 100644 --- a/src/formatter.hpp +++ b/src/formatter.hpp @@ -67,6 +67,31 @@ class formatter { return stream_.str(); } + + // Support manipulators + formatter& operator<<(std::ostream&(*fn)(std::ostream&)) & + { + fn(stream_); + return *this; + } + + formatter&& operator<<(std::ostream&(*fn)(std::ostream&)) && + { + fn(stream_); + return std::move(*this); + } + + formatter& operator<<(std::ios_base&(*fn)(std::ios_base&)) & + { + fn(stream_); + return *this; + } + + formatter&& operator<<(std::ios_base&(*fn)(std::ios_base&)) && + { + fn(stream_); + return std::move(*this); + } private: std::ostringstream stream_; diff --git a/src/game_config_manager.cpp b/src/game_config_manager.cpp index 8c9c1d8dc02b..f2b84db9e5d0 100644 --- a/src/game_config_manager.cpp +++ b/src/game_config_manager.cpp @@ -134,7 +134,7 @@ void game_config_manager::load_game_config_with_loadscreen(FORCE_RELOAD_CONFIG f boost::optional> active_addons) { if (!lg::info().dont_log(log_config)) { - auto& out = lg::info()(log_config); + auto out = formatter(); out << "load_game_config: defines:"; for(const auto& pair : cache_.get_preproc_map()) { out << pair.first << ","; @@ -149,7 +149,8 @@ void game_config_manager::load_game_config_with_loadscreen(FORCE_RELOAD_CONFIG f out << "\n Everything:"; } out << "\n"; - } + FORCE_LOG_TO(lg::info(), log_config) << out.str(); + } game_config::scoped_preproc_define debug_mode("DEBUG_MODE", diff --git a/src/game_events/pump.cpp b/src/game_events/pump.cpp index cd9332209376..f3b1207e0596 100644 --- a/src/game_events/pump.cpp +++ b/src/game_events/pump.cpp @@ -402,7 +402,7 @@ void wml_event_pump::show_wml_messages() void wml_event_pump::put_wml_message( lg::logger& logger, const std::string& prefix, const std::string& message, bool in_chat) { - logger(log_wml) << message << std::endl; + FORCE_LOG_TO(logger, log_wml) << message << std::endl; if(in_chat) { impl_->wml_messages_stream << prefix << message << std::endl; } diff --git a/src/gui/core/log.hpp b/src/gui/core/log.hpp index a71d81c5c862..e538314a6d11 100644 --- a/src/gui/core/log.hpp +++ b/src/gui/core/log.hpp @@ -43,11 +43,8 @@ extern lg::log_domain log_gui_general; #define ERR_GUI_G LOG_STREAM_INDENT(err, gui2::log_gui_general) extern lg::log_domain log_gui_iterator; -#define TST_GUI_I \ - if(lg::debug().dont_log(gui2::log_gui_iterator)) \ - ; \ - else \ - lg::debug()(gui2::log_gui_iterator, false, false) + +#define TST_GUI_I LOG_STREAM_NAMELESS(debug, gui2::log_gui_iterator) #define DBG_GUI_I LOG_STREAM_INDENT(debug, gui2::log_gui_iterator) #define LOG_GUI_I LOG_STREAM_INDENT(info, gui2::log_gui_iterator) #define WRN_GUI_I LOG_STREAM_INDENT(warn, gui2::log_gui_iterator) diff --git a/src/log.cpp b/src/log.cpp index e658b745f38f..ca8ef9aaeee3 100644 --- a/src/log.cpp +++ b/src/log.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include "global.hpp" @@ -44,6 +45,7 @@ static std::ostream null_ostream(new null_streambuf); static int indent = 0; static bool timestamp = true; static bool precise_timestamp = false; +static std::mutex log_mutex; static boost::posix_time::time_facet facet("%Y%m%d %H:%M:%S%F "); static std::ostream *output_stream = nullptr; @@ -207,7 +209,7 @@ static void print_precise_timestamp(std::ostream & out) noexcept } catch(...) {} } -std::ostream &logger::operator()(const log_domain& domain, bool show_names, bool do_indent) const +log_in_progress logger::operator()(const log_domain& domain, bool show_names, bool do_indent) const { if (severity_ > domain.domain_->second) { return null_ostream; @@ -218,33 +220,58 @@ std::ostream &logger::operator()(const log_domain& domain, bool show_names, bool std::cerr << ss.str() << std::endl; strict_threw_ = true; } - std::ostream& stream = output(); + log_in_progress stream = output(); if(do_indent) { - for(int i = 0; i != indent; ++i) - stream << " "; - } + stream.set_indent(indent); + } if (timestamp) { - if(precise_timestamp) { - print_precise_timestamp(stream); - } else { - stream << get_timestamp(std::time(nullptr)); - } + stream.enable_timestamp(); } if (show_names) { - stream << name_ << ' ' << domain.domain_->first << ": "; + stream.set_prefix(formatter() << name_ << ' ' << domain.domain_->first << ": "); } return stream; } } -void scope_logger::do_log_entry(const log_domain& domain, const std::string& str) noexcept +log_in_progress::log_in_progress(std::ostream& stream) + : stream_(stream) +{} + +void log_in_progress::operator|(formatter&& message) +{ + std::lock_guard lock(log_mutex); + for(int i = 0; i < indent; ++i) + stream_ << " "; + if(timestamp_) { + if(precise_timestamp) { + print_precise_timestamp(stream_); + } else { + stream_ << get_timestamp(std::time(nullptr)); + } + } + stream_ << prefix_ << message.str(); +} + +void log_in_progress::set_indent(int level) { + indent_ = level; +} + +void log_in_progress::enable_timestamp() { + timestamp_ = true; +} + +void log_in_progress::set_prefix(const std::string& prefix) { + prefix_ = prefix; +} + +void scope_logger::do_log_entry(const std::string& str) noexcept { - output_ = &debug()(domain, false, true); str_ = str; try { ticks_ = boost::posix_time::microsec_clock::local_time(); } catch(...) {} - (*output_) << "{ BEGIN: " << str_ << "\n"; + debug()(domain_, false, true) | formatter() << "{ BEGIN: " << str_ << "\n"; ++indent; } @@ -255,15 +282,10 @@ void scope_logger::do_log_exit() noexcept ticks = (boost::posix_time::microsec_clock::local_time() - ticks_).total_milliseconds(); } catch(...) {} --indent; - do_indent(); - if (timestamp) (*output_) << get_timestamp(std::time(nullptr)); - (*output_) << "} END: " << str_ << " (took " << ticks << "ms)\n"; -} - -void scope_logger::do_indent() const -{ - for(int i = 0; i != indent; ++i) - (*output_) << " "; + auto output = debug()(domain_, false, true); + output.set_indent(indent); + if(timestamp) output.enable_timestamp(); + output | formatter() << "} END: " << str_ << " (took " << ticks << "ms)\n"; } std::stringstream& wml_error() diff --git a/src/log.hpp b/src/log.hpp index 4466ac2f6911..02e53939f3fb 100644 --- a/src/log.hpp +++ b/src/log.hpp @@ -58,6 +58,7 @@ #include #include +#include "formatter.hpp" using boost::posix_time::ptime; @@ -112,12 +113,32 @@ void set_strict_severity(int severity); void set_strict_severity(const logger &lg); bool broke_strict(); +// A little "magic" to surround the logging operation in a mutex. +// This works by capturing the output first to a stringstream formatter, then +// locking a mutex and dumping it to the stream all in one go. +// By doing this we can avoid rare deadlocks if a function whose output is streamed +// calls logging of its own. +// We overload operator| only because it has lower precedence than operator<< +// Any other lower-precedence operator would have worked just as well. +class log_in_progress { + std::ostream& stream_; + int indent_ = 0; + bool timestamp_ = false; + std::string prefix_; +public: + log_in_progress(std::ostream& stream); + void operator|(formatter&& message); + void set_indent(int level); + void enable_timestamp(); + void set_prefix(const std::string& prefix); +}; + class logger { char const *name_; int severity_; public: logger(char const *name, int severity): name_(name), severity_(severity) {} - std::ostream &operator()(const log_domain& domain, + log_in_progress operator()(const log_domain& domain, bool show_names = true, bool do_indent = false) const; bool dont_log(const log_domain& domain) const @@ -147,26 +168,25 @@ log_domain& general(); class scope_logger { ptime ticks_; - std::ostream *output_; + const log_domain& domain_; std::string str_; public: scope_logger(const log_domain& domain, const char* str) : - output_(nullptr) + domain_(domain) { - if (!debug().dont_log(domain)) do_log_entry(domain, str); + if (!debug().dont_log(domain)) do_log_entry(str); } scope_logger(const log_domain& domain, const std::string& str) : - output_(nullptr) + domain_(domain) { - if (!debug().dont_log(domain)) do_log_entry(domain, str); + if (!debug().dont_log(domain)) do_log_entry(str); } ~scope_logger() { - if (output_) do_log_exit(); + if (!str_.empty()) do_log_exit(); } - void do_indent() const; private: - void do_log_entry(const log_domain& domain, const std::string& str) noexcept; + void do_log_entry(const std::string& str) noexcept; void do_log_exit() noexcept; }; @@ -186,7 +206,14 @@ std::stringstream& wml_error(); #define log_scope(description) lg::scope_logger scope_logging_object__(lg::general(), description); #define log_scope2(domain,description) lg::scope_logger scope_logging_object__(domain, description); -#define LOG_STREAM(level, domain) if (lg::level().dont_log(domain)) ; else lg::level()(domain) +#define LOG_STREAM(level, domain) if (lg::level().dont_log(domain)) ; else lg::level()(domain) | formatter() + +// Don't prefix the logdomain to messages on this stream +#define LOG_STREAM_NAMELESS(level, domain) if (lg::level().dont_log(domain)) ; else lg::level()(domain, false) | formatter() // When using log_scope/log_scope2 it is nice to have all output indented. -#define LOG_STREAM_INDENT(level,domain) if (lg::level().dont_log(domain)) ; else lg::level()(domain, true, true) +#define LOG_STREAM_INDENT(level,domain) if (lg::level().dont_log(domain)) ; else lg::level()(domain, true, true) | formatter() + +// If you have an explicit logger object and want to ignore the logging level, use this. +// Meant for cases where you explicitly call dont_log to avoid an expensive operation if the logging is disabled. +#define FORCE_LOG_TO(logger, domain) logger(domain) | formatter() diff --git a/src/server/campaignd/addon_utils.cpp b/src/server/campaignd/addon_utils.cpp index 13c5e730fbdb..5cef6881c039 100644 --- a/src/server/campaignd/addon_utils.cpp +++ b/src/server/campaignd/addon_utils.cpp @@ -24,7 +24,7 @@ #include static lg::log_domain log_network("network"); -#define LOG_CS if (lg::err().dont_log(log_network)) ; else lg::err()(log_network, false) +#define LOG_CS LOG_STREAM_NAMELESS(err, log_network) namespace { From 8b1c6cf59d260c68e2d67eb7d4aac840ef278b5c Mon Sep 17 00:00:00 2001 From: Steve Cotton Date: Wed, 14 Oct 2020 00:32:47 +0200 Subject: [PATCH 2/2] Another go at refactoring distributor.cpp (builds and runs successfully) This is part-way though a refactor, but I'm not committing myself to completing it; publishing for the comments and discussion. This solves PR #5198's compile errors about undefined references, but the solution here is to expand the templates into the multiple hardcoded copies block starting with `switch(events_.sdl_button_down_event)`. --- src/gui/core/event/distributor.cpp | 181 +++++++++++++++++++++-------- src/gui/core/event/distributor.hpp | 111 +++++++++--------- 2 files changed, 189 insertions(+), 103 deletions(-) diff --git a/src/gui/core/event/distributor.cpp b/src/gui/core/event/distributor.cpp index 972087f95054..f372fb49b6e4 100644 --- a/src/gui/core/event/distributor.cpp +++ b/src/gui/core/event/distributor.cpp @@ -357,47 +357,92 @@ void mouse_motion::stop_hover_timer() #undef LOG_HEADER #define LOG_HEADER \ - "distributor mouse button " << name_ << " [" << owner_.id() << "]: " + "distributor mouse button " << events_.name << " [" << owner_.id() << "]: " -template -mouse_button::mouse_button(const std::string& name_, widget& owner, +mouse_button::mouse_button(const mouse_button_event_types& events, widget& owner, const dispatcher::queue_position queue_position) : mouse_motion(owner, queue_position) , last_click_stamp_(0) , last_clicked_widget_(nullptr) , focus_(nullptr) - , name_(name_) + , events_(events) , is_down_(false) , signal_handler_sdl_button_down_entered_(false) , signal_handler_sdl_button_up_entered_(false) { - owner_.connect_signal( - std::bind(&mouse_button::signal_handler_sdl_button_down, - this, - _2, - _3, - _5), - queue_position); - owner_.connect_signal( - std::bind(&mouse_button::signal_handler_sdl_button_up, - this, - _2, - _3, - _5), - queue_position); + // The connect_signal framework is currently using SFINAE checking to ensure that we only + // register mouse button signal handlers for mouse buttons. That causes us to need this + // hardcoded (either directly or by making mouse_button a templated class), the manual handling + // of the three cases here is the current progress on refactoring. + switch(events_.sdl_button_down_event) { + case event::SDL_LEFT_BUTTON_DOWN: { + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_down, + this, + _2, + _3, + _5), + queue_position); + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_up, + this, + _2, + _3, + _5), + queue_position); + break; + } + case event::SDL_MIDDLE_BUTTON_DOWN: { + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_down, + this, + _2, + _3, + _5), + queue_position); + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_up, + this, + _2, + _3, + _5), + queue_position); + break; + } + case event::SDL_RIGHT_BUTTON_DOWN: { + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_down, + this, + _2, + _3, + _5), + queue_position); + owner_.connect_signal( + std::bind(&mouse_button::signal_handler_sdl_button_up, + this, + _2, + _3, + _5), + queue_position); + break; + } + default: { + // There's exactly three instances of this class per instance of distributor, so this assert + // will be caught during the build-time-tests. + assert(!"Hardcoded assumption about button being LEFT / MIDDLE / RIGHT failed"); + } + } } -template -void mouse_button::initialize_state(const bool is_down) +void mouse_button::initialize_state(int32_t button_state) { last_click_stamp_ = 0; last_clicked_widget_ = nullptr; focus_ = 0; - is_down_ = is_down; + is_down_ = button_state & events_.mask; } -template -void mouse_button::signal_handler_sdl_button_down(const event::ui_event event, bool& handled, +void mouse_button::signal_handler_sdl_button_down(const event::ui_event event, bool& handled, const point& coordinate) { if(signal_handler_sdl_button_down_entered_) { @@ -420,10 +465,10 @@ void mouse_button::signal_handler_sdl_button_down(const event::ui_event event if(mouse_captured_) { assert(mouse_focus_); focus_ = mouse_focus_; - DBG_GUI_E << LOG_HEADER << "Firing: " << T::sdl_button_down_event << ".\n"; - if(!owner_.fire(T::sdl_button_down_event, *focus_, coordinate)) { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::button_down_event << ".\n"; - owner_.fire(T::button_down_event, *mouse_focus_); + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.sdl_button_down_event << ".\n"; + if(!owner_.fire(events_.sdl_button_down_event, *focus_, coordinate)) { + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.button_down_event << ".\n"; + owner_.fire(events_.button_down_event, *mouse_focus_); } } else { widget* mouse_over = owner_.find_at(coordinate, true); @@ -440,17 +485,16 @@ void mouse_button::signal_handler_sdl_button_down(const event::ui_event event } focus_ = mouse_over; - DBG_GUI_E << LOG_HEADER << "Firing: " << T::sdl_button_down_event << ".\n"; - if(!owner_.fire(T::sdl_button_down_event, *focus_, coordinate)) { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::button_down_event << ".\n"; - owner_.fire(T::button_down_event, *focus_); + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.sdl_button_down_event << ".\n"; + if(!owner_.fire(events_.sdl_button_down_event, *focus_, coordinate)) { + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.button_down_event << ".\n"; + owner_.fire(events_.button_down_event, *focus_); } } handled = true; } -template -void mouse_button::signal_handler_sdl_button_up(const event::ui_event event, bool& handled, +void mouse_button::signal_handler_sdl_button_up(const event::ui_event event, bool& handled, const point& coordinate) { if(signal_handler_sdl_button_up_entered_) { @@ -470,13 +514,17 @@ void mouse_button::signal_handler_sdl_button_up(const event::ui_event event, is_down_ = false; if(focus_) { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::sdl_button_up_event << ".\n"; - if(!owner_.fire(T::sdl_button_up_event, *focus_, coordinate)) { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::button_up_event << ".\n"; - owner_.fire(T::button_up_event, *focus_); + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.sdl_button_up_event << ".\n"; + if(!owner_.fire(events_.sdl_button_up_event, *focus_, coordinate)) { + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.button_up_event << ".\n"; + owner_.fire(events_.button_up_event, *focus_); } } + // FIXME: The block below is strange diamond inheritance - it's code that could be in + // mouse_motion which applies to all three buttons, but it will be run in one of the + // three mouse_button subclasses, and then the other two mouse_button subclasses + // will reach here with mouse_captured_ == false. widget* mouse_over = owner_.find_at(coordinate, true); if(mouse_captured_) { const unsigned mask = SDL_BUTTON_LMASK | SDL_BUTTON_MMASK @@ -503,23 +551,22 @@ void mouse_button::signal_handler_sdl_button_up(const event::ui_event event, handled = true; } -template -void mouse_button::mouse_button_click(widget* widget) +void mouse_button::mouse_button_click(widget* widget) { uint32_t stamp = SDL_GetTicks(); if(last_click_stamp_ + settings::double_click_time >= stamp && last_clicked_widget_ == widget) { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::button_double_click_event << ".\n"; + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.button_double_click_event << ".\n"; - owner_.fire(T::button_double_click_event, *widget); + owner_.fire(events_.button_double_click_event, *widget); last_click_stamp_ = 0; last_clicked_widget_ = nullptr; } else { - DBG_GUI_E << LOG_HEADER << "Firing: " << T::button_click_event << ".\n"; - owner_.fire(T::button_click_event, *widget); + DBG_GUI_E << LOG_HEADER << "Firing: " << events_.button_click_event << ".\n"; + owner_.fire(events_.button_click_event, *widget); last_click_stamp_ = stamp; last_clicked_widget_ = widget; } @@ -530,6 +577,40 @@ void mouse_button::mouse_button_click(widget* widget) #undef LOG_HEADER #define LOG_HEADER "distributor mouse motion [" << owner_.id() << "]: " +namespace +{ + +const auto mouse_button_left_events = mouse_button_event_types { + SDL_LEFT_BUTTON_DOWN, + SDL_LEFT_BUTTON_UP, + LEFT_BUTTON_DOWN, + LEFT_BUTTON_UP, + LEFT_BUTTON_CLICK, + LEFT_BUTTON_DOUBLE_CLICK, + SDL_BUTTON_LMASK, + "left"}; + +const auto mouse_button_middle_events = mouse_button_event_types { + SDL_MIDDLE_BUTTON_DOWN, + SDL_MIDDLE_BUTTON_UP, + MIDDLE_BUTTON_DOWN, + MIDDLE_BUTTON_UP, + MIDDLE_BUTTON_CLICK, + MIDDLE_BUTTON_DOUBLE_CLICK, + SDL_BUTTON_MMASK, + "middle"}; + +const auto mouse_button_right_events = mouse_button_event_types { + SDL_RIGHT_BUTTON_DOWN, + SDL_RIGHT_BUTTON_UP, + RIGHT_BUTTON_DOWN, + RIGHT_BUTTON_UP, + RIGHT_BUTTON_CLICK, + RIGHT_BUTTON_DOUBLE_CLICK, + SDL_BUTTON_RMASK, + "right"}; +} // anonymous namespace + /** * @todo Test whether the state is properly tracked when an input blocker is * used. @@ -537,9 +618,9 @@ void mouse_button::mouse_button_click(widget* widget) distributor::distributor(widget& owner, const dispatcher::queue_position queue_position) : mouse_motion(owner, queue_position) - , mouse_button_left("left", owner, queue_position) - , mouse_button_middle("middle", owner, queue_position) - , mouse_button_right("right", owner, queue_position) + , mouse_button_left(mouse_button_left_events, owner, queue_position) + , mouse_button_middle(mouse_button_middle_events, owner, queue_position) + , mouse_button_right(mouse_button_right_events, owner, queue_position) , keyboard_focus_(nullptr) , keyboard_focus_chain_() { @@ -581,11 +662,11 @@ distributor::~distributor() void distributor::initialize_state() { - const uint8_t button_state = SDL_GetMouseState(nullptr, nullptr); + const uint32_t button_state = SDL_GetMouseState(nullptr, nullptr); - mouse_button_left::initialize_state((button_state & SDL_BUTTON(1)) != 0); - mouse_button_middle::initialize_state((button_state & SDL_BUTTON(2)) != 0); - mouse_button_right::initialize_state((button_state & SDL_BUTTON(3)) != 0); + mouse_button_left::initialize_state(button_state); + mouse_button_middle::initialize_state(button_state); + mouse_button_right::initialize_state(button_state); init_mouse_location(); } diff --git a/src/gui/core/event/distributor.hpp b/src/gui/core/event/distributor.hpp index 31fb5530e4d0..4461d5c02457 100644 --- a/src/gui/core/event/distributor.hpp +++ b/src/gui/core/event/distributor.hpp @@ -155,41 +155,36 @@ class mouse_motion /***** ***** ***** ***** mouse_button ***** ***** ***** ***** *****/ /** - * Small helper metastruct to specialize mouse_button with and provide ui_event type - * aliases without needing to make mouse_button take a million template types. + * Small helper metastruct to configure instances of mouse_button. */ -template< - ui_event sdl_button_down, - ui_event sdl_button_up, - ui_event button_down, - ui_event button_up, - ui_event button_click, - ui_event button_double_click> -struct mouse_button_event_types_wrapper +struct mouse_button_event_types { - static const ui_event sdl_button_down_event = sdl_button_down; - static const ui_event sdl_button_up_event = sdl_button_up; - static const ui_event button_down_event = button_down; - static const ui_event button_up_event = button_up; - static const ui_event button_click_event = button_click; - static const ui_event button_double_click_event = button_double_click; + ui_event sdl_button_down_event; + ui_event sdl_button_up_event; + ui_event button_down_event; + ui_event button_up_event; + ui_event button_click_event; + ui_event button_double_click_event; + /** Bitmask corresponding to this button's bit in SDL_GetMouseState's return value */ + int32_t mask; + /** used for debug messages. */ + std::string name; }; -template class mouse_button : public virtual mouse_motion { public: - mouse_button(const std::string& name_, + mouse_button(const mouse_button_event_types& events, widget& owner, const dispatcher::queue_position queue_position); /** * Initializes the state of the button. * - * @param is_down The initial state of the button, if true down - * else initialized as up. + * @param button_state The initial state of all buttons, in which the bit corresponding to + mouse_button_event_types.mask will be set if the button is down, or unset if it is up. */ - void initialize_state(const bool is_down); + void initialize_state(int32_t button_state); protected: /** The time of the last click used for double clicking. */ @@ -206,8 +201,8 @@ class mouse_button : public virtual mouse_motion widget* focus_; private: - /** used for debug messages. */ - const std::string name_; + /** Which set of SDL events correspond to this button. */ + const mouse_button_event_types events_; /** Is the button down? */ bool is_down_; @@ -226,37 +221,47 @@ class mouse_button : public virtual mouse_motion void mouse_button_click(widget* widget); }; -/***** ***** ***** ***** distributor ***** ***** ***** ***** *****/ +/** + * Three subclasses of mouse_button, so that the distributor class can inherit from them; + * C++ doesn't allow multiple inheritance to directly use more than one instance of a + * superclass. + * + * It's a diamond inheritance, as all of these have virtual base class mouse_motion; + * refactoring that would allow these multiple classes to be replaced with a simple + * (distributor has-a std::array) relationship. + */ +struct mouse_button_left : public mouse_button +{ + mouse_button_left(const mouse_button_event_types& events, + widget& owner, + const dispatcher::queue_position queue_position) + : mouse_motion(owner, queue_position) + , mouse_button(events, owner, queue_position) + { + } +}; +struct mouse_button_middle : public mouse_button +{ + mouse_button_middle(const mouse_button_event_types& events, + widget& owner, + const dispatcher::queue_position queue_position) + : mouse_motion(owner, queue_position) + , mouse_button(events, owner, queue_position) + { + } +}; +struct mouse_button_right : public mouse_button +{ + mouse_button_right(const mouse_button_event_types& events, + widget& owner, + const dispatcher::queue_position queue_position) + : mouse_motion(owner, queue_position) + , mouse_button(events, owner, queue_position) + { + } +}; -using mouse_button_left = mouse_button< - mouse_button_event_types_wrapper< - SDL_LEFT_BUTTON_DOWN, - SDL_LEFT_BUTTON_UP, - LEFT_BUTTON_DOWN, - LEFT_BUTTON_UP, - LEFT_BUTTON_CLICK, - LEFT_BUTTON_DOUBLE_CLICK> - >; - -using mouse_button_middle = mouse_button< - mouse_button_event_types_wrapper< - SDL_MIDDLE_BUTTON_DOWN, - SDL_MIDDLE_BUTTON_UP, - MIDDLE_BUTTON_DOWN, - MIDDLE_BUTTON_UP, - MIDDLE_BUTTON_CLICK, - MIDDLE_BUTTON_DOUBLE_CLICK> - >; - -using mouse_button_right = mouse_button< - mouse_button_event_types_wrapper< - SDL_RIGHT_BUTTON_DOWN, - SDL_RIGHT_BUTTON_UP, - RIGHT_BUTTON_DOWN, - RIGHT_BUTTON_UP, - RIGHT_BUTTON_CLICK, - RIGHT_BUTTON_DOUBLE_CLICK> - >; +/***** ***** ***** ***** distributor ***** ***** ***** ***** *****/ /** The event handler class for the widget library. */