From 09b8fceb9e20c5e9994c386a4fe8a536e7569076 Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Thu, 30 Nov 2017 11:44:06 +1100 Subject: [PATCH] Game Events: ensure event names are properly standardized in the config This ensures all names are always valid to use in by-name lookup and are stored consistently. Also fixes a potential issue in 2c12d1328ba6. I should have called standardize_name on each name entry in the cleanup phase like when done in the registration phase, but this commit ensures that's not needed anymore. --- src/game_events/handlers.cpp | 2 +- src/game_events/handlers.hpp | 2 +- src/game_events/manager_impl.cpp | 28 ++++++++++++++++++++++++---- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/game_events/handlers.cpp b/src/game_events/handlers.cpp index 4673966f43bd9..f7411f8d7b65b 100644 --- a/src/game_events/handlers.cpp +++ b/src/game_events/handlers.cpp @@ -47,7 +47,7 @@ namespace game_events { /* ** event_handler ** */ -event_handler::event_handler(const config& cfg, bool imi) +event_handler::event_handler(config&& cfg, bool imi) : first_time_only_(cfg["first_time_only"].to_bool(true)) , is_menu_item_(imi) , disabled_(false) diff --git a/src/game_events/handlers.hpp b/src/game_events/handlers.hpp index acdba02e32190..b44135331e05d 100644 --- a/src/game_events/handlers.hpp +++ b/src/game_events/handlers.hpp @@ -46,7 +46,7 @@ using handler_list = std::list; class event_handler { public: - event_handler(const config& cfg, bool is_menu_item); + event_handler(config&& cfg, bool is_menu_item); bool disabled() const { diff --git a/src/game_events/manager_impl.cpp b/src/game_events/manager_impl.cpp index 17420035e30a4..6824e4494d13c 100644 --- a/src/game_events/manager_impl.cpp +++ b/src/game_events/manager_impl.cpp @@ -97,7 +97,7 @@ handler_list& event_handlers::get(const std::string& name) */ void event_handlers::add_event_handler(const config& cfg, bool is_menu_item) { - const std::string name = cfg["name"]; + std::string name = cfg["name"]; std::string id = cfg["id"]; if(!id.empty()) { @@ -109,18 +109,38 @@ void event_handlers::add_event_handler(const config& cfg, bool is_menu_item) } } + // Make a copy of the event cfg here in order to do some standardization on the + // name field. Will be moved into the handler. + config event_cfg = cfg; + + // Split the name field... + std::vector standardized_names = utils::split(cfg["name"]); + + // ...and standardize each one individually. This ensures they're all valid for by-name lookup. + for(std::string& single_name : standardized_names) { + single_name = standardize_name(single_name); + } + + // Write the new name back to the config. + name = utils::join(standardized_names); + event_cfg["name"] = name; + // Create a new handler. // Do note active_ holds the main shared_ptr, and the other three containers // construct weak_ptrs from the shared one. DBG_EH << "inserting event handler for name=" << name << " with id=" << id << "\n"; - active_.emplace_back(new event_handler(cfg, is_menu_item)); + active_.emplace_back(new event_handler(std::move(event_cfg), is_menu_item)); + + // + // !! event_cfg is invalid past this point! DO NOT USE! + // // File by name. if(utils::might_contain_variables(name)) { dynamic_.emplace_back(active_.back()); } else { - for(const std::string& single_name : utils::split(name)) { - by_name_[standardize_name(single_name)].emplace_back(active_.back()); + for(const std::string& single_name : standardized_names) { + by_name_[single_name].emplace_back(active_.back()); } }