Skip to content

Commit

Permalink
Rule of Five for event::sdl_handler
Browse files Browse the repository at this point in the history
This fixes the default implementations of widget's copy constructor and copy
assignment, which was root cause of the regression in 4139b43 (#4215).

This reverts commit f46ed66.
  • Loading branch information
stevecotton authored and Vultraz committed Aug 14, 2019
1 parent 8c01004 commit c664d72
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 24 deletions.
54 changes: 52 additions & 2 deletions src/events.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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_) {
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions src/events.hpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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<sdl_handler*> handler_members()
{
Expand Down
21 changes: 0 additions & 21 deletions src/widgets/widget.cpp
Expand Up @@ -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),
Expand Down
1 change: 0 additions & 1 deletion src/widgets/widget.hpp
Expand Up @@ -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();

Expand Down

0 comments on commit c664d72

Please sign in to comment.