From 0fbb01e2192398a5dd9f1d0444832dc68a36dc54 Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Wed, 11 Apr 2018 13:08:27 +1100 Subject: [PATCH] Game Events/Manager: skip disabled assertion if write_events is called mid-event This reverts afaa75842c8914406bed75d3557ec241b0de9b6a and replaces it with a more general no-assert-during-events check. This is because there are other instances besides [inspect] where this function can be called mid-event, and it wouldn't be convenient to add nested 'strict' parameters to multiple functions just to handle them. Fixes #2750. (cherry-picked from commit a1810bde32a95ae00827b5db5ac08f63d9907acb) --- changelog.md | 1 + src/game_events/manager.cpp | 22 ++++++++++++++++------ src/game_events/manager.hpp | 2 +- src/gui/dialogs/gamestate_inspector.cpp | 2 +- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/changelog.md b/changelog.md index 1a2987d40aa1..08a3c30cc491 100644 --- a/changelog.md +++ b/changelog.md @@ -71,6 +71,7 @@ * Fixed images with no alpha channel rendering incorrectly. * Fixed unit selection not persisting between uses of Create Unit. * Fixed assertion when undoing actions in a synced context. + * Fixed assertion when saving game events mid-event. ## Version 1.13.12 ### Security fixes diff --git a/src/game_events/manager.cpp b/src/game_events/manager.cpp index 4e99a12cc930..c8c8f2e454dc 100644 --- a/src/game_events/manager.cpp +++ b/src/game_events/manager.cpp @@ -132,16 +132,26 @@ void manager::add_events(const config::const_child_itors& cfgs, const std::strin } } -void manager::write_events(config& cfg, bool strict) const +void manager::write_events(config& cfg) const { for(const handler_ptr& eh : event_handlers_->get_active()) { - if(eh && !eh->is_menu_item()) { - if(strict) { - assert(!eh->disabled()); - } + if(!eh || eh->is_menu_item()) { + continue; + } - cfg.add_child("event", eh->get_config());; + // This function may be invoked mid-event, such as via [inspect] (the inspector writes + // the events to a local config) or if an out-of-sync error happens in MP. In that case, + // it's possible for the currently running event is already disabled. That would happen + // if it's a first-time-only event; those are disabled before their actions are run. In + // that case, skip disabled events. If invoked from outside an event, however, there + // should be no disabled events in the list, so assert if one is found. + if(eh->disabled() && is_event_running()) { + continue; + } else { + assert(!eh->disabled()); } + + cfg.add_child("event", eh->get_config());; } cfg["unit_wml_ids"] = utils::join(unit_wml_ids_); diff --git a/src/game_events/manager.hpp b/src/game_events/manager.hpp index 20bc0e37dfa3..af2e39f3eae0 100644 --- a/src/game_events/manager.hpp +++ b/src/game_events/manager.hpp @@ -68,7 +68,7 @@ class manager void add_events(const config::const_child_itors& cfgs, const std::string& type = std::string()); - void write_events(config& cfg, bool strict = true) const; + void write_events(config& cfg) const; using event_func_t = std::function; void execute_on_events(const std::string& event_id, event_func_t func); diff --git a/src/gui/dialogs/gamestate_inspector.cpp b/src/gui/dialogs/gamestate_inspector.cpp index ee9f64341fa0..f63db1ffe794 100644 --- a/src/gui/dialogs/gamestate_inspector.cpp +++ b/src/gui/dialogs/gamestate_inspector.cpp @@ -509,7 +509,7 @@ const display_context& single_mode_controller::dc() const { event_mode_controller::event_mode_controller(gamestate_inspector::controller& c) : single_mode_controller(c) { - single_mode_controller::events().write_events(events, false); + single_mode_controller::events().write_events(events); } void variable_mode_controller::show_list(tree_view_node& node)