Skip to content

Commit

Permalink
Game Events: ensure event names are properly standardized in the config
Browse files Browse the repository at this point in the history
This ensures all names are always valid to use in by-name lookup and are stored consistently.

Also fixes a potential issue in 2c12d13. 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.
  • Loading branch information
Vultraz committed Nov 30, 2017
1 parent fb5fd3f commit e5cea35
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/game_events/handlers.cpp
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/game_events/handlers.hpp
Expand Up @@ -46,7 +46,7 @@ using handler_list = std::list<weak_handler_ptr>;
class event_handler
{
public:
event_handler(const config& cfg, bool is_menu_item);
event_handler(config&& cfg, bool is_menu_item);

bool disabled() const
{
Expand Down
28 changes: 24 additions & 4 deletions src/game_events/manager_impl.cpp
Expand Up @@ -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()) {
Expand All @@ -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<std::string> 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());
}
}

Expand Down

3 comments on commit e5cea35

@gfgtdf
Copy link
Contributor

@gfgtdf gfgtdf commented on e5cea35 Dec 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lookis like it might break spaces in inline formula (name = "turn $($num_turns - 1)") or an even easier example turn $num_turns end" which would become turn_$num_turns_end"

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @gfgtdf has a point.

@Vultraz
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

God dammit

Please sign in to comment.