From c664d72ed0153cef101a2750bb00e3bc25be2220 Mon Sep 17 00:00:00 2001 From: Steve Cotton Date: Wed, 14 Aug 2019 14:38:20 +0200 Subject: [PATCH] Rule of Five for event::sdl_handler This fixes the default implementations of widget's copy constructor and copy assignment, which was root cause of the regression in 4139b43cc900 (#4215). This reverts commit f46ed66f2c9a8acf8d88338d8433fc35689d4c39. --- src/events.cpp | 54 ++++++++++++++++++++++++++++++++++++++++-- src/events.hpp | 13 ++++++++++ src/widgets/widget.cpp | 21 ---------------- src/widgets/widget.hpp | 1 - 4 files changed, 65 insertions(+), 24 deletions(-) diff --git a/src/events.cpp b/src/events.cpp index e2400fb75e0c..9ef064cbdfa2 100644 --- a/src/events.cpp +++ b/src/events.cpp @@ -87,6 +87,14 @@ void context::add_handler(sdl_handler* ptr) staging_handlers.push_back(ptr); } +bool context::has_handler(const sdl_handler* ptr) const +{ + if(handlers.cend() != std::find(handlers.cbegin(), handlers.cend(), ptr)) { + return true; + } + return staging_handlers.cend() != std::find(staging_handlers.cbegin(), staging_handlers.cend(), ptr); +} + bool context::remove_handler(sdl_handler* ptr) { static int depth = 0; @@ -235,6 +243,49 @@ sdl_handler::sdl_handler(const bool auto_join) } } +sdl_handler::sdl_handler(const sdl_handler &that) + : has_joined_(that.has_joined_) + , has_joined_global_(that.has_joined_global_) +{ + if(has_joined_global_) { + assert(!event_contexts.empty()); + event_contexts.front().add_handler(this); + } else if(has_joined_) { + bool found_context = false; + for(auto &context : boost::adaptors::reverse(event_contexts)) { + if(context.has_handler(&that)) { + found_context = true; + context.add_handler(this); + break; + } + } + + if (!found_context) { + throw std::logic_error("Copy-constructing a sdl_handler that has_joined_ but can't be found by searching contexts"); + } + } +} + +sdl_handler &sdl_handler::operator=(const sdl_handler &that) +{ + if(that.has_joined_global_) { + join_global(); + } else if(that.has_joined_) { + for(auto &context : boost::adaptors::reverse(event_contexts)) { + if(context.has_handler(&that)) { + join(context); + break; + } + } + } else if(has_joined_) { + leave(); + } else if(has_joined_global_) { + leave_global(); + } + + return *this; +} + sdl_handler::~sdl_handler() { if(has_joined_) { @@ -282,8 +333,7 @@ void sdl_handler::join_same(sdl_handler* parent) } for(auto& context : boost::adaptors::reverse(event_contexts)) { - handler_list& handlers = context.handlers; - if(std::find(handlers.begin(), handlers.end(), parent) != handlers.end()) { + if(context.has_handler(parent)) { join(context); return; } diff --git a/src/events.hpp b/src/events.hpp index 621ab8007a21..2dd3f41df5f4 100644 --- a/src/events.hpp +++ b/src/events.hpp @@ -51,6 +51,8 @@ class context context(const context&) = delete; void add_handler(sdl_handler* ptr); + /** Returns true if @a ptr is found in either the handlers or staging_handlers lists */ + bool has_handler(const sdl_handler* ptr) const; bool remove_handler(sdl_handler* ptr); void cycle_focus(); void set_focus(const sdl_handler* ptr); @@ -95,8 +97,19 @@ friend class context; virtual bool has_joined() { return has_joined_;} virtual bool has_joined_global() { return has_joined_global_;} + + /** + * Moving would require two instances' context membership to be handled, + * it's simpler to delete these and require the two instances to be + * separately constructed / destructed. + */ + sdl_handler &operator=(sdl_handler &&) = delete; + sdl_handler(sdl_handler &&) = delete; + protected: sdl_handler(const bool auto_join=true); + sdl_handler(const sdl_handler &); + sdl_handler &operator=(const sdl_handler &); virtual ~sdl_handler(); virtual std::vector handler_members() { diff --git a/src/widgets/widget.cpp b/src/widgets/widget.cpp index 3ab8a8a51595..7d19556101cc 100644 --- a/src/widgets/widget.cpp +++ b/src/widgets/widget.cpp @@ -30,27 +30,6 @@ namespace gui { bool widget::mouse_lock_ = false; -widget::widget(const widget& o) - : events::sdl_handler() - , focus_(o.focus_) - , video_(o.video_) - , restorer_(o.restorer_) - , rect_(o.rect_) - , needs_restore_(o.needs_restore_) - , state_(o.state_) - , hidden_override_(o.hidden_override_) - , enabled_(o.enabled_) - , clip_(o.clip_) - , clip_rect_(o.clip_rect_) - , volatile_(o.volatile_) - , help_text_(o.help_text_) - , tooltip_text_(o.tooltip_text_) - , help_string_(o.help_string_) - , id_(o.id_) - , mouse_lock_local_(o.mouse_lock_local_) -{ -} - widget::widget(CVideo& video, const bool auto_join) : events::sdl_handler(auto_join), focus_(true), video_(&video), rect_(EmptyRect), needs_restore_(false), state_(UNINIT), hidden_override_(false), enabled_(true), clip_(false), diff --git a/src/widgets/widget.hpp b/src/widgets/widget.hpp index 34f457bed82a..a200f0f4507b 100644 --- a/src/widgets/widget.hpp +++ b/src/widgets/widget.hpp @@ -69,7 +69,6 @@ class widget : public events::sdl_handler virtual void process_tooltip_string(int mousex, int mousey); protected: - widget(const widget& o); widget(CVideo& video, const bool auto_join=true); virtual ~widget();