diff --git a/src/game_events/handlers.cpp b/src/game_events/handlers.cpp index ff62c621c17bb..12ecf9a324493 100644 --- a/src/game_events/handlers.cpp +++ b/src/game_events/handlers.cpp @@ -45,29 +45,6 @@ static lg::log_domain log_event_handler("event_handler"); // This file is in the game_events namespace. namespace game_events { -/* ** handler_list::iterator ** */ - -/** - * Dereference. - * If the current element has become invalid, we will increment first. - */ -handler_ptr handler_list::iterator::operator*() -{ - // Check for an available handler. - while(iter_.derefable()) { - // Handler still accessible? - if(handler_ptr lock = iter_->lock()) { - return lock; - } else { - // Remove the now-defunct entry. - iter_ = list_t::erase(iter_); - } - } - - // End of the list. - return handler_ptr(); -} - /* ** event_handler ** */ event_handler::event_handler(const config& cfg, bool imi, handler_vec::size_type index, manager& man) diff --git a/src/game_events/handlers.hpp b/src/game_events/handlers.hpp index 4af20f4b60486..ef117e0db9017 100644 --- a/src/game_events/handlers.hpp +++ b/src/game_events/handlers.hpp @@ -24,8 +24,8 @@ #pragma once #include "config.hpp" -#include "utils/smart_list.hpp" +#include #include #include #include @@ -80,104 +80,6 @@ class event_handler config cfg_; }; -/** - * This is a wrapper for a list of weak pointers to handlers. It allows forward - * iterations of the list, with each element returned as a shared pointer. - * (Weak pointers that fail to lock are silently removed from the list.) These - * iterations can be used recursively, even when the innermost iteration might - * erase arbitrary elements from the list. - * - * The interface is not the standard list interface because that would be - * inconvenient. The functionality implemented is that required by Wesnoth. - */ -class handler_list -{ - /// The weak pointers that are used internally. - typedef std::weak_ptr internal_ptr; - - /// The underlying list. - typedef utils::smart_list list_t; - -public: - /** - * Handler list iterators are rather limited. They can be constructed - * from a reference iterator (not default constructed), incremented, - * and dereferenced. Consecutive dereferences are not guaranteed to - * return the same element (if the list mutates between them, the next - * element might be returned). An increment guarantees that the next - * dereference will differ from the previous (unless at the end of the - * list). The end of the list is indicated by dereferencing to a null - * pointer. - */ - class iterator - { - /// The current element. - list_t::iterator iter_; - - public: - /// Initialized constructor (to be called by handler_list). - explicit iterator(const list_t::iterator& base_iter) - : iter_(base_iter) - { - } - - /// Increment. - iterator& operator++() - { - ++iter_; - return *this; - } - /// Dereference. - handler_ptr operator*(); - }; - friend class iterator; - typedef iterator const_iterator; - -public: - /** - * Default constructor. - * Note: This explicit definition is required (by the more pedantic - * compilers) in order to declare a default-constructed, static, - * and const variable in event_handlers::get(), in handlers.cpp. - */ - handler_list() - : data_() - { - } - - const_iterator begin() const - { - return iterator(const_cast(data_).begin()); - } - - // The above const_cast is so the iterator can remove obsolete entries. - const_iterator end() const - { - return iterator(const_cast(data_).end()); - } - - // push_front() is probably unneeded, but I'll leave the code here, just in case. - // (These lists must be maintained in index order, which means pushing to the back.) - void push_front(const handler_ptr& p) - { - data_.push_front(internal_ptr(p)); - } - - void push_back(const handler_ptr& p) - { - data_.push_back(internal_ptr(p)); - } - - void clear() - { - data_.clear(); - } - -private: - /// No implementation of operator=() since smart_list does not support it. - handler_list& operator=(const handler_list&); - - /// The actual list. - list_t data_; -}; +using weak_handler_ptr = std::weak_ptr; +using handler_list = std::list; } diff --git a/src/game_events/manager.cpp b/src/game_events/manager.cpp index 0f76ce5168905..61590b8e98c0e 100644 --- a/src/game_events/manager.cpp +++ b/src/game_events/manager.cpp @@ -108,12 +108,21 @@ manager::iteration::iteration(const std::string& event_name, manager& man) , var_it_(event_name.empty() ? var_list_.end() : var_list_.begin()) , gamedata_(resources::gamedata) { + // Clean up expired ptrs. This saves us effort later since it ensures every ptr is valid. + + main_list_.remove_if( + [](weak_handler_ptr ptr) { return ptr.expired(); } + ); + + var_list_.remove_if( + [](weak_handler_ptr ptr) { return ptr.expired(); } + ); } /** * Increment * Incrementing guarantees that the next dereference will differ from the - * previous derference (unless the iteration is exhausted). However, multiple + * previous dereference (unless the iteration is exhausted). However, multiple * increments between dereferences are allowed to have the same effect as a * single increment. */ @@ -139,6 +148,16 @@ manager::iteration& manager::iteration::operator++() return *this; } +// Small helper function to ensure we don't try to dereference an invalid iterator. +static handler_ptr lock_ptr(const handler_list& list, handler_list::iterator iter) +{ + if(iter != list.end()) { + return iter->lock(); + } + + return nullptr; +} + /** * Dereference * Will return a null pointer when the end of the iteration is reached. @@ -146,15 +165,15 @@ manager::iteration& manager::iteration::operator++() handler_ptr manager::iteration::operator*() { // Get the candidate for the current element from the main list. - handler_ptr main_ptr = *main_it_; + handler_ptr main_ptr = lock_ptr(main_list_, main_it_); handler_vec::size_type main_index = ptr_index(main_ptr); // Get the candidate for the current element from the var list. - handler_ptr var_ptr = *var_it_; + handler_ptr var_ptr = lock_ptr(var_list_, var_it_); // (Loop while var_ptr would be chosen over main_ptr, but the name does not match.) while(var_ptr && var_ptr->index() < main_index && !var_ptr->matches_name(event_name_, gamedata_)) { - var_ptr = *++var_it_; + var_ptr = lock_ptr(var_list_, ++var_it_); } handler_vec::size_type var_index = ptr_index(var_ptr); diff --git a/src/game_events/manager.hpp b/src/game_events/manager.hpp index 60b4df60894df..7c03a4f6cb82f 100644 --- a/src/game_events/manager.hpp +++ b/src/game_events/manager.hpp @@ -78,9 +78,9 @@ class manager private: /// The fixed-name event handlers for this iteration. - const handler_list& main_list_; + handler_list& main_list_; /// The varying-name event handlers for this iteration. - const handler_list& var_list_; + handler_list& var_list_; /// The event name for this iteration. const std::string event_name_; /// The end of this iteration. We intentionally exclude handlers diff --git a/src/game_events/manager_impl.cpp b/src/game_events/manager_impl.cpp index 20b1e3013c301..a666f86b89303 100644 --- a/src/game_events/manager_impl.cpp +++ b/src/game_events/manager_impl.cpp @@ -80,10 +80,10 @@ std::string event_handlers::standardize_name(const std::string& name) /** * Read-only access to the handlers with fixed event names, by event name. */ -const handler_list& event_handlers::get(const std::string& name) const +handler_list& event_handlers::get(const std::string& name) { // Empty list for the "not found" case. - static const handler_list empty_list; + static handler_list empty_list; // Look for the name in the name map. auto find_it = by_name_.find(standardize_name(name)); diff --git a/src/game_events/manager_impl.hpp b/src/game_events/manager_impl.hpp index c8557993ed878..978f60581b28d 100644 --- a/src/game_events/manager_impl.hpp +++ b/src/game_events/manager_impl.hpp @@ -26,7 +26,7 @@ class event_handlers { private: typedef std::unordered_map map_t; - typedef std::unordered_map> id_map_t; + typedef std::unordered_map id_map_t; /** * Active event handlers. Will not have elements removed unless the event_handlers is clear()ed. @@ -61,8 +61,8 @@ class event_handlers { } - /** Read-only access to the handlers with varying event names. */ - const handler_list& get_dynamic() const + /** Access to the handlers with varying event names. */ + handler_list& get_dynamic() { return dynamic_; } @@ -73,8 +73,8 @@ class event_handlers return active_; } - /** Read-only access to the handlers with fixed event names, by event name. */ - const handler_list& get(const std::string& name) const; + /** Access to the handlers with fixed event names, by event name. */ + handler_list& get(const std::string& name); /** Adds an event handler. */ void add_event_handler(const config& cfg, manager& man, bool is_menu_item = false);