diff --git a/src/game_events/handlers.cpp b/src/game_events/handlers.cpp index 12ecf9a324493..3a528fad2f824 100644 --- a/src/game_events/handlers.cpp +++ b/src/game_events/handlers.cpp @@ -47,11 +47,11 @@ namespace game_events { /* ** event_handler ** */ -event_handler::event_handler(const config& cfg, bool imi, handler_vec::size_type index, manager& man) +event_handler::event_handler(const config& cfg, bool imi, handler_vec::size_type index) : first_time_only_(cfg["first_time_only"].to_bool(true)) , is_menu_item_(imi) + , disabled_(false) , index_(index) - , man_(&man) , cfg_(cfg) { } @@ -66,31 +66,29 @@ event_handler::event_handler(const config& cfg, bool imi, handler_vec::size_type */ void event_handler::disable() { - assert(man_); - assert(man_->event_handlers_); + assert(!disabled_);; + disabled_ = true; + + //assert(man_); + //assert(man_->event_handlers_); // Handlers must have an index after they're created. - assert(index_ < man_->event_handlers_->size()); + //assert(index_ < man_->event_handlers_->size()); // Disable this handler. - (*man_->event_handlers_)[index_].reset(); + //(*man_->event_handlers_)[index_].reset(); } /** * Handles the queued event, according to our WML instructions. - * WARNING: *this may be destroyed at the end of this call, unless - * the caller maintains a handler_ptr to this. * * @param[in] event_info Information about the event that needs handling. - * @param[in,out] handler_p The caller's smart pointer to *this. It may be - * reset() during processing. */ -void event_handler::handle_event(const queued_event& event_info, handler_ptr& handler_p, game_lua_kernel& lk) +void event_handler::handle_event(const queued_event& event_info, game_lua_kernel& lk) { - // We will need our config after possibly self-destructing. Make a copy now. - // TODO: instead of copying possibly huge config objects we should use shared things and only increase a refcount - // here. - vconfig vcfg(cfg_, true); + if(disabled_) { + return; + } if(is_menu_item_) { DBG_NG << cfg_["name"] << " will now invoke the following command(s):\n" << cfg_; @@ -101,11 +99,10 @@ void event_handler::handle_event(const queued_event& event_info, handler_ptr& ha disable(); // Also remove our caller's hold on us. - handler_p.reset(); + // handler_p.reset(); } - // *WARNING*: At this point, dereferencing this could be a memory violation! - lk.run_wml_action("command", vcfg, event_info); + lk.run_wml_action("command", vconfig(cfg_, false), event_info); sound::commit_music_changes(); } diff --git a/src/game_events/handlers.hpp b/src/game_events/handlers.hpp index ef117e0db9017..51c9c94f7f688 100644 --- a/src/game_events/handlers.hpp +++ b/src/game_events/handlers.hpp @@ -48,7 +48,12 @@ typedef std::vector handler_vec; class event_handler { public: - event_handler(const config& cfg, bool is_menu_item, handler_vec::size_type index, manager&); + event_handler(const config& cfg, bool is_menu_item, handler_vec::size_type index); + + bool disabled() const + { + return disabled_; + } /** The index of *this should only be of interest when controlling iterations. */ handler_vec::size_type index() const @@ -63,9 +68,10 @@ class event_handler return is_menu_item_; } - /** Disables *this, removing it from the game. */ + /** Flag this as disabled. */ void disable(); - void handle_event(const queued_event& event_info, handler_ptr& handler_p, game_lua_kernel&); + + void handle_event(const queued_event& event_info, game_lua_kernel&); const config& get_config() const { @@ -75,8 +81,8 @@ class event_handler private: bool first_time_only_; bool is_menu_item_; + bool disabled_; handler_vec::size_type index_; - manager* man_; config cfg_; }; diff --git a/src/game_events/manager.cpp b/src/game_events/manager.cpp index 59e6aa575e0eb..30e2e29780f2d 100644 --- a/src/game_events/manager.cpp +++ b/src/game_events/manager.cpp @@ -44,7 +44,7 @@ namespace game_events /** Create an event handler. */ void manager::add_event_handler(const config& handler, bool is_menu_item) { - event_handlers_->add_event_handler(handler, *this, is_menu_item); + event_handlers_->add_event_handler(handler, is_menu_item); } /** Removes an event handler. */ @@ -108,15 +108,6 @@ 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(); } - //); } /** @@ -182,6 +173,7 @@ handler_ptr manager::iteration::operator*() // Which list? (Index ties go to the main list.) current_is_known_ = main_index < end_ || var_index < end_; + //current_is_known_ = main_ptr != nullptr || var_ptr != nullptr; main_is_current_ = main_index <= var_index; if(!current_is_known_) { @@ -233,6 +225,9 @@ void manager::execute_on_events(const std::string& event_id, manager::event_func func(*this, hand); ++iter; } + + // Clean up expired ptrs. This saves us effort later since it ensures every ptr is valid. + event_handlers_->drop_expired_handlers(event_id); } game_events::wml_event_pump& manager::pump() diff --git a/src/game_events/manager_impl.cpp b/src/game_events/manager_impl.cpp index a666f86b89303..016ab24426419 100644 --- a/src/game_events/manager_impl.cpp +++ b/src/game_events/manager_impl.cpp @@ -95,7 +95,7 @@ handler_list& event_handlers::get(const std::string& name) * An event with a nonempty ID will not be added if an event with that * ID already exists. */ -void event_handlers::add_event_handler(const config& cfg, manager& man, bool is_menu_item) +void event_handlers::add_event_handler(const config& cfg, bool is_menu_item) { const std::string name = cfg["name"]; std::string id = cfg["id"]; @@ -111,7 +111,7 @@ void event_handlers::add_event_handler(const config& cfg, manager& man, bool is_ // Create a new handler. DBG_EH << "inserting event handler for name=" << name << " with id=" << id << "\n"; - handler_ptr new_handler(new event_handler(cfg, is_menu_item, active_.size(), man)); + handler_ptr new_handler(new event_handler(cfg, is_menu_item, active_.size())); active_.push_back(new_handler); @@ -162,6 +162,44 @@ void event_handlers::remove_event_handler(const std::string& id) log_handlers(); } +namespace +{ +bool is_expired(weak_handler_ptr ptr) +{ + return ptr.expired(); +} + +} // end anon namespace + +void event_handlers::drop_expired_handlers(const std::string& event_name) +{ +#if 0 + std::vector to_remove; + + // First, clean up the 'owning' list. + for(auto iter = active_.begin(); iter != active_.end(); ++iter) { + // if a handler is disabled... + if((*iter)->disabled()) { + // ...reset the shared_ptr that owns it... + iter->reset(); + + // ...and note this entry for removal. + to_remove.push_back(iter); + } + } +#endif + + auto to_remove = std::remove_if(active_.begin(), active_.end(), [](handler_ptr p) { return p->disabled(); }); + active_.erase(to_remove, active_.end()); + + get(event_name).remove_if(is_expired); + dynamic_.remove_if(is_expired); + + //for(auto& i : to_remove) { + // active_.erase(i); + //} +} + const handler_ptr event_handlers::get_event_handler_by_id(const std::string& id) { auto find_it = id_map_.find(id); diff --git a/src/game_events/manager_impl.hpp b/src/game_events/manager_impl.hpp index 978f60581b28d..17fcb7f06d1d6 100644 --- a/src/game_events/manager_impl.hpp +++ b/src/game_events/manager_impl.hpp @@ -77,11 +77,14 @@ class event_handlers 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); + void add_event_handler(const config& cfg, bool is_menu_item = false); /** Removes an event handler, identified by its ID. */ void remove_event_handler(const std::string& id); + /** Removes all expired event handlers and any weak_ptrs to them. */ + void drop_expired_handlers(const std::string& event_name); + /** Gets an event handler, identified by its ID. */ const handler_ptr get_event_handler_by_id(const std::string& id); diff --git a/src/game_events/pump.cpp b/src/game_events/pump.cpp index 0fad7b1244437..46c7f53998c0b 100644 --- a/src/game_events/pump.cpp +++ b/src/game_events/pump.cpp @@ -303,7 +303,7 @@ void wml_event_pump::process_event(handler_ptr& handler_p, const queued_event& e ++impl_->internal_wml_tracking; context::scoped evc(impl_->contexts_); assert(resources::lua_kernel != nullptr); - handler_p->handle_event(ev, handler_p, *resources::lua_kernel); + handler_p->handle_event(ev, *resources::lua_kernel); // NOTE: handler_p may be null at this point! if(ev.name == "select") {