From 714f411a6894a69fb5fe81dc6772f1c2971ff5dc Mon Sep 17 00:00:00 2001 From: Celtic Minstrel Date: Sat, 13 Mar 2021 01:09:14 -0500 Subject: [PATCH] WFL: Fix a crash if a formula object goes out of scope before the formula has been fully evaluated This removes the old system that was supposed to prevent this from happening and replaces it with enable_shared_from_this coupled with a system to automatically copy the object if it's not already tracked by a shared_ptr (which usually means it was allocated on the stack). --- src/ai/default/attack.cpp | 8 ++-- src/ai/default/contexts.hpp | 1 + src/ai/formula/ai.cpp | 12 ++--- src/ai/formula/ai.hpp | 5 ++- src/ai/formula/callable_objects.cpp | 18 ++++---- src/ai/formula/callable_objects.hpp | 18 ++++++-- src/ai/formula/candidates.cpp | 6 +-- src/ai/formula/stage_side_formulas.cpp | 2 +- src/ai/formula/stage_unit_formulas.cpp | 6 +-- src/formula/callable.hpp | 61 ++++++++++++++------------ src/formula/callable_fwd.hpp | 5 --- src/formula/callable_objects.cpp | 2 +- src/formula/callable_objects.hpp | 18 ++++++++ src/formula/formula.cpp | 5 +++ src/formula/function.cpp | 1 + src/formula/function.hpp | 1 + src/formula/variant_value.cpp | 9 ---- src/formula/variant_value.hpp | 5 +-- src/gui/core/canvas.cpp | 2 +- src/image_modifications.cpp | 1 + src/scripting/lua_formula_bridge.cpp | 1 + src/scripting/lua_terrainfilter.cpp | 2 +- src/terrain/filter.cpp | 2 +- src/units/filter.cpp | 2 +- 24 files changed, 112 insertions(+), 81 deletions(-) diff --git a/src/ai/default/attack.cpp b/src/ai/default/attack.cpp index 4e6d1c6faab5..33e2359f8190 100644 --- a/src/ai/default/attack.cpp +++ b/src/ai/default/attack.cpp @@ -429,7 +429,7 @@ wfl::variant attack_analysis::execute_self(wfl::variant ctxt) { //check if target is still valid unit = units.find(att_dst); if(unit == units.end()) { - return wfl::variant(std::make_shared(fake_ptr(), attack_result::E_EMPTY_DEFENDER, move_from)); + return wfl::variant(std::make_shared(*this, attack_result::E_EMPTY_DEFENDER, move_from)); } //check if we need to move @@ -437,14 +437,14 @@ wfl::variant attack_analysis::execute_self(wfl::variant ctxt) { //now check if location to which we want to move is still unoccupied unit = units.find(att_src); if(unit != units.end()) { - return wfl::variant(std::make_shared(fake_ptr(), move_result::E_NO_UNIT, move_from)); + return wfl::variant(std::make_shared(*this, move_result::E_NO_UNIT, move_from)); } ai::move_result_ptr result = get_ai_context(ctxt.as_callable()).execute_move_action(move_from, att_src); if(!result->is_ok()) { //move part failed LOG_AI << "ERROR #" << result->get_status() << " while executing 'attack' formula function\n" << std::endl; - return wfl::variant(std::make_shared(fake_ptr(), result->get_status(), result->get_unit_location())); + return wfl::variant(std::make_shared(*this, result->get_status(), result->get_unit_location())); } } @@ -453,7 +453,7 @@ wfl::variant attack_analysis::execute_self(wfl::variant ctxt) { if(!result->is_ok()) { //attack failed LOG_AI << "ERROR #" << result->get_status() << " while executing 'attack' formula function\n" << std::endl; - return wfl::variant(std::make_shared(fake_ptr(), result->get_status())); + return wfl::variant(std::make_shared(*this, result->get_status())); } } return wfl::variant(true); diff --git a/src/ai/default/contexts.hpp b/src/ai/default/contexts.hpp index b5b27954df7b..f88ac13d7571 100644 --- a/src/ai/default/contexts.hpp +++ b/src/ai/default/contexts.hpp @@ -47,6 +47,7 @@ struct target { class attack_analysis : public wfl::action_callable { + attack_analysis* clone() const override {return new attack_analysis(*this);} public: attack_analysis() : wfl::action_callable(), diff --git a/src/ai/formula/ai.cpp b/src/ai/formula/ai.cpp index c19298012605..0fb6861a9f91 100644 --- a/src/ai/formula/ai.cpp +++ b/src/ai/formula/ai.cpp @@ -160,13 +160,13 @@ std::string formula_ai::evaluate(const std::string& formula_str) formula f(formula_str, &function_table_); - map_formula_callable callable(fake_ptr()); + map_formula_callable callable(*this); //formula_debugger fdb; const variant v = f.evaluate(callable,nullptr); if (ai_ptr_) { - variant var = variant(this->fake_ptr()).execute_variant(v); + variant var = this->query_value("self").execute_variant(v); if ( !var.is_empty() ) { return "Made move: " + var.to_debug_string(); @@ -192,7 +192,7 @@ wfl::variant formula_ai::make_action(wfl::const_formula_ptr formula_, const wfl: variant res; if (ai_ptr_) { - res = variant(this->fake_ptr()).execute_variant(var); + res = this->query_value("self").execute_variant(var); } else { ERR_AI << "skipped execution of action because ai context is not set correctly" << std::endl; } @@ -518,7 +518,7 @@ variant formula_ai::get_value(const std::string& key) const } else if(key == "my_attacks") { - return variant(attacks_callable.fake_ptr()); + return attacks_callable.query_value("self"); } else if(key == "enemy_moves") { return variant(std::make_shared(get_enemy_srcdst(), get_enemy_dstsrc(), units)); @@ -543,7 +543,7 @@ variant formula_ai::get_value(const std::string& key) const } else if(key == "vars") { - return variant(vars_.fake_ptr()); + return vars_.query_value("self"); } else if(key == "keeps") { return get_keeps(); @@ -694,7 +694,7 @@ void formula_ai::evaluate_candidate_action(ca_ptr fai_ca) bool formula_ai::execute_candidate_action(ca_ptr fai_ca) { - map_formula_callable callable(fake_ptr()); + map_formula_callable callable(*this); fai_ca->update_callable_map( callable ); const_formula_ptr move_formula(fai_ca->get_action()); return !make_action(move_formula, callable).is_empty(); diff --git a/src/ai/formula/ai.hpp b/src/ai/formula/ai.hpp index 9ff6b46cb26f..27515c39cd1f 100644 --- a/src/ai/formula/ai.hpp +++ b/src/ai/formula/ai.hpp @@ -68,9 +68,9 @@ struct plain_route; namespace ai { class formula_ai : public readonly_context_proxy, public wfl::formula_callable { + formula_ai(const formula_ai&) = default; + formula_ai& operator=(const formula_ai&) = default; public: - formula_ai(const formula_ai&) = delete; - formula_ai& operator=(const formula_ai&) = delete; explicit formula_ai(readonly_context &context, const config &cfg); virtual ~formula_ai() {} @@ -149,6 +149,7 @@ class formula_ai : public readonly_context_proxy, public wfl::formula_callable { wfl::variant make_action(wfl::const_formula_ptr formula_, const wfl::formula_callable& variables); private: + formula_ai* clone() const override {return new formula_ai(*this);} ai_context *ai_ptr_; const config cfg_; recursion_counter recursion_counter_; diff --git a/src/ai/formula/callable_objects.cpp b/src/ai/formula/callable_objects.cpp index fad34416ce4d..f18eb6f4998e 100644 --- a/src/ai/formula/callable_objects.cpp +++ b/src/ai/formula/callable_objects.cpp @@ -92,7 +92,7 @@ variant move_callable::execute_self(variant ctxt) { if(!move_result->is_ok()) { LOG_AI << "ERROR #" << move_result->get_status() << " while executing 'move' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), move_result->get_status(), move_result->get_unit_location())); + return variant(std::make_shared(*this, move_result->get_status(), move_result->get_unit_location())); } return variant(move_result->is_gamestate_changed()); @@ -121,7 +121,7 @@ variant move_partial_callable::execute_self(variant ctxt) { if(!move_result->is_ok()) { LOG_AI << "ERROR #" << move_result->get_status() << " while executing 'move_partial' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), move_result->get_status(), move_result->get_unit_location())); + return variant(std::make_shared(*this, move_result->get_status(), move_result->get_unit_location())); } return variant(move_result->is_gamestate_changed()); @@ -160,8 +160,8 @@ void outcome_callable::get_inputs(formula_input_vector& inputs) const { attack_callable::attack_callable(const map_location& move_from, const map_location& src, const map_location& dst, int weapon) : move_from_(move_from), src_(src), dst_(dst), - bc_(resources::gameboard->units(), src, dst, weapon, -1, 1.0, nullptr, - resources::gameboard->units().find(move_from).get_shared_ptr()) + bc_(std::make_shared(resources::gameboard->units(), src, dst, weapon, -1, 1.0, nullptr, + resources::gameboard->units().find(move_from).get_shared_ptr())) { type_ = ATTACK_C; } @@ -224,7 +224,7 @@ variant attack_callable::execute_self(variant ctxt) { if(!move_result->is_ok()) { //move part failed LOG_AI << "ERROR #" << move_result->get_status() << " while executing 'attack' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), move_result->get_status(), move_result->get_unit_location())); + return variant(std::make_shared(*this, move_result->get_status(), move_result->get_unit_location())); } } @@ -235,7 +235,7 @@ variant attack_callable::execute_self(variant ctxt) { if(!attack_result->is_ok()) { //attack failed LOG_AI << "ERROR #" << attack_result->get_status() << " while executing 'attack' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), attack_result->get_status())); + return variant(std::make_shared(*this, attack_result->get_status())); } } @@ -309,7 +309,7 @@ variant recall_callable::execute_self(variant ctxt) { recall_result->execute(); } else { LOG_AI << "ERROR #" << recall_result->get_status() << " while executing 'recall' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), recall_result->get_status())); + return variant(std::make_shared(*this, recall_result->get_status())); } return variant(recall_result->is_gamestate_changed()); @@ -338,7 +338,7 @@ variant recruit_callable::execute_self(variant ctxt) { recruit_result->execute(); } else { LOG_AI << "ERROR #" << recruit_result->get_status() << " while executing 'recruit' formula function\n" << std::endl; - return variant(std::make_shared(fake_ptr(), recruit_result->get_status())); + return variant(std::make_shared(*this, recruit_result->get_status())); } //is_gamestate_changed()==true means that the game state was somehow changed by action. @@ -385,7 +385,7 @@ variant set_unit_var_callable::execute_self(variant ctxt) { } ERR_AI << "ERROR #" << status << " while executing 'set_unit_var' formula function" << std::endl; - return variant(std::make_shared(fake_ptr(), status)); + return variant(std::make_shared(*this, status)); } variant fallback_callable::execute_self(variant) { diff --git a/src/ai/formula/callable_objects.hpp b/src/ai/formula/callable_objects.hpp index c779116887c2..3c8963a50bf0 100644 --- a/src/ai/formula/callable_objects.hpp +++ b/src/ai/formula/callable_objects.hpp @@ -36,6 +36,7 @@ class attack_map_callable : public formula_callable { const unit_map& units_; const ai::formula_ai& ai_; + attack_map_callable* clone() const override {return new attack_map_callable(*this);} variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; @@ -46,10 +47,12 @@ class attack_map_callable : public formula_callable { class attack_callable : public action_callable { map_location move_from_, src_, dst_; - battle_context bc_; + // Make this shared so we can be coped... + std::shared_ptr bc_; variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + attack_callable* clone() const override {return new attack_callable(*this);} public: attack_callable(const map_location& move_from, const map_location& src, const map_location& dst, int weapon); @@ -57,8 +60,8 @@ class attack_callable : public action_callable { const map_location& move_from() const { return move_from_; } const map_location& src() const { return src_; } const map_location& dst() const { return dst_; } - int weapon() const { return bc_.get_attacker_stats().attack_num; } - int defender_weapon() const { return bc_.get_defender_stats().attack_num; } + int weapon() const { return bc_->get_attacker_stats().attack_num; } + int defender_weapon() const { return bc_->get_defender_stats().attack_num; } /** Compare two attacks in deterministic way or compare pointers * (nondeterministic in consequent game runs) if method argument is not @@ -84,6 +87,7 @@ class move_callable : public action_callable { } int do_compare(const formula_callable* callable) const override; + move_callable* clone() const override {return new move_callable(*this);} public: move_callable(const map_location& src, const map_location& dst) : src_(src), dst_(dst) @@ -113,6 +117,7 @@ class move_partial_callable : public action_callable { } int do_compare(const formula_callable* callable) const override; + move_partial_callable* clone() const override {return new move_partial_callable(*this);} public: move_partial_callable(const map_location& src, const map_location& dst) : src_(src), dst_(dst) @@ -132,6 +137,7 @@ class recall_callable : public action_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + recall_callable* clone() const override {return new recall_callable(*this);} public: recall_callable(const map_location& loc, const std::string& id) : loc_(loc), id_(id) @@ -149,6 +155,7 @@ class recruit_callable : public action_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + recruit_callable* clone() const override {return new recruit_callable(*this);} public: recruit_callable(const map_location& loc, const std::string& type) : loc_(loc), type_(type) @@ -166,6 +173,7 @@ class set_unit_var_callable : public action_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + set_unit_var_callable* clone() const override {return new set_unit_var_callable(*this);} public: set_unit_var_callable(const std::string& key, const variant& value, const map_location& loc) : key_(key), value_(value), loc_(loc) @@ -179,6 +187,7 @@ class set_unit_var_callable : public action_callable { class fallback_callable : public action_callable { variant get_value(const std::string& /*key*/) const override { return variant(); } + fallback_callable* clone() const override {return new fallback_callable(*this);} public: explicit fallback_callable() { } @@ -193,6 +202,7 @@ class move_map_callable : public formula_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + move_map_callable* clone() const override {return new move_map_callable(*this);} public: move_map_callable(const move_map& srcdst, const move_map& dstsrc, const unit_map& units) : srcdst_(srcdst), dstsrc_(dstsrc), units_(units) @@ -210,6 +220,7 @@ class position_callable : public formula_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + position_callable* clone() const override {return new position_callable(*this);} public: position_callable(/*unit_map* units,*/ int chance) : //units_(), @@ -240,6 +251,7 @@ class outcome_callable : public formula_callable { variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + outcome_callable* clone() const override {return new outcome_callable(*this);} public: outcome_callable( const std::vector& hitLeft, const std::vector& prob, diff --git a/src/ai/formula/candidates.cpp b/src/ai/formula/candidates.cpp index 952043b1d9cb..e7551b0600c4 100644 --- a/src/ai/formula/candidates.cpp +++ b/src/ai/formula/candidates.cpp @@ -76,7 +76,7 @@ candidate_action_with_filters::candidate_action_with_filters( variant candidate_action_with_filters::do_filtering(ai::formula_ai* ai, variant& input, const_formula_ptr formula) { - map_formula_callable callable(ai->fake_ptr()); + map_formula_callable callable(*ai); callable.add("input", input); return formula::evaluate(formula, callable); @@ -121,7 +121,7 @@ void move_candidate_action::evaluate(ai::formula_ai* ai, unit_map& units) for(variant_iterator i = filtered_units.begin() ; i != filtered_units.end() ; ++i) { - map_formula_callable callable(ai->fake_ptr()); + map_formula_callable callable(*ai); callable.add("me", *i); int res = execute_formula(eval_, callable, ai); @@ -225,7 +225,7 @@ void attack_candidate_action::evaluate(ai::formula_ai* ai, unit_map& units) auto enemy_unit_callable = enemy_units_flt[enemy_unit].convert_to(); if(ai->can_reach_unit(my_unit_callable->get_location(), enemy_unit_callable->get_location())) { - map_formula_callable callable(ai->fake_ptr()); + map_formula_callable callable(*ai); callable.add("me", filtered_my_units[my_unit]); callable.add("target", filtered_enemy_units[enemy_unit]); diff --git a/src/ai/formula/stage_side_formulas.cpp b/src/ai/formula/stage_side_formulas.cpp index 9fb905bd7488..5b4e0883c6ca 100644 --- a/src/ai/formula/stage_side_formulas.cpp +++ b/src/ai/formula/stage_side_formulas.cpp @@ -43,7 +43,7 @@ stage_side_formulas::~stage_side_formulas() bool stage_side_formulas::do_play_stage() { - wfl::map_formula_callable callable(fai_.fake_ptr()); + wfl::map_formula_callable callable(fai_); try { if (move_formula_) { while( !fai_.make_action(move_formula_,callable).is_empty() ) { } diff --git a/src/ai/formula/stage_unit_formulas.cpp b/src/ai/formula/stage_unit_formulas.cpp index 830cf31902eb..5cf735f412ab 100644 --- a/src/ai/formula/stage_unit_formulas.cpp +++ b/src/ai/formula/stage_unit_formulas.cpp @@ -66,7 +66,7 @@ bool stage_unit_formulas::do_play_stage() try { wfl::const_formula_ptr priority_formula(fai_.create_optional_formula(i->formula_manager().get_priority_formula())); if (priority_formula) { - wfl::map_formula_callable callable(fai_.fake_ptr()); + wfl::map_formula_callable callable(fai_); callable.add("me", wfl::variant(std::make_shared(*i))); priority = (wfl::formula::evaluate(priority_formula, callable)).as_int(); } else { @@ -99,7 +99,7 @@ bool stage_unit_formulas::do_play_stage() try { wfl::const_formula_ptr formula(fai_.create_optional_formula(i->formula_manager().get_formula())); if (formula) { - wfl::map_formula_callable callable(fai_.fake_ptr()); + wfl::map_formula_callable callable(fai_); callable.add("me", wfl::variant(std::make_shared(*i))); fai_.make_action(formula, callable); } else { @@ -121,7 +121,7 @@ bool stage_unit_formulas::do_play_stage() try { wfl::const_formula_ptr loop_formula(fai_.create_optional_formula(i->formula_manager().get_loop_formula())); if (loop_formula) { - wfl::map_formula_callable callable(fai_.fake_ptr()); + wfl::map_formula_callable callable(fai_); callable.add("me", wfl::variant(std::make_shared(*i))); while ( !fai_.make_action(loop_formula, callable).is_empty() && i.valid() ) { diff --git a/src/formula/callable.hpp b/src/formula/callable.hpp index 5145526d88ef..46dc93bc6911 100644 --- a/src/formula/callable.hpp +++ b/src/formula/callable.hpp @@ -25,31 +25,21 @@ namespace wfl { // Interface for objects that can have formulae run on them -class formula_callable +class formula_callable : public std::enable_shared_from_this { + // Don't expose the enable_shared_from_this API to the public; + // it's only public inheritance so that its own internals can work. + using std::enable_shared_from_this::shared_from_this; + using std::enable_shared_from_this::weak_from_this; public: explicit formula_callable(bool has_self = true) : type_(FORMULA_C), has_self_(has_self) {} - virtual ~formula_callable() { - for(auto& d : dtor_notify) { - if(d) { - d->notify_dead(); - } - } - } - - formula_callable_ptr fake_ptr() { - return formula_callable_ptr(this, [](const formula_callable*){}); - } - - const_formula_callable_ptr fake_ptr() const { - return const_formula_callable_ptr(this, [](const formula_callable*){}); - } + virtual ~formula_callable() = default; variant query_value(const std::string& key) const { if(has_self_ && key == "self") { - return variant(fake_ptr()); + return variant(to_ptr()); } return get_value(key); } @@ -90,14 +80,6 @@ class formula_callable serialize_to_string(str); } - void subscribe_dtor(callable_die_subscriber* d) const { - dtor_notify.insert(d); - } - - void unsubscribe_dtor(callable_die_subscriber* d) const { - dtor_notify.erase(d); - } - protected: template static variant convert_map(const std::map& input_map) @@ -172,11 +154,20 @@ class formula_callable TYPE type_; - mutable std::set dtor_notify; - private: + virtual formula_callable* clone() const = 0; + const_formula_callable_ptr to_ptr() const + { + if(weak_from_this().use_count() > 0) { + return shared_from_this(); + } + return const_formula_callable_ptr(clone()); + } virtual variant get_value(const std::string& key) const = 0; bool has_self_; + // These two need special access to to_ptr()... or should it just be public? + friend class map_formula_callable; + friend class safe_call_result; }; class action_callable : public formula_callable @@ -187,6 +178,7 @@ class action_callable : public formula_callable class formula_callable_with_backup : public formula_callable { + formula_callable_with_backup* clone() const override {return new formula_callable_with_backup(*this);} public: formula_callable_with_backup(const formula_callable& main, const formula_callable& backup) : formula_callable(false), main_(main), backup_(backup) @@ -215,6 +207,7 @@ class formula_callable_with_backup : public formula_callable class formula_variant_callable_with_backup : public formula_callable { + formula_variant_callable_with_backup* clone() const override {return new formula_variant_callable_with_backup(*this);} public: formula_variant_callable_with_backup(const variant& var, const formula_callable& backup) : formula_callable(false), var_(var), backup_(backup) @@ -242,12 +235,20 @@ class formula_variant_callable_with_backup : public formula_callable class map_formula_callable : public formula_callable { + map_formula_callable* clone() const override {return new map_formula_callable(*this);} public: explicit map_formula_callable(const_formula_callable_ptr fallback = nullptr) : formula_callable(false) , values_() , fallback_(fallback) {} + + // May copy the passed callable + explicit map_formula_callable(const formula_callable& fallback) + : formula_callable(false) + , values_() + , fallback_(fallback.to_ptr()) + {} map_formula_callable& add(const std::string& key, const variant& value) { @@ -255,10 +256,16 @@ class map_formula_callable : public formula_callable return *this; } + // May copy the passed callable void set_fallback(const_formula_callable_ptr fallback) { fallback_ = fallback; } + + void set_fallback(const formula_callable& fallback) + { + fallback_ = fallback.to_ptr(); + } bool empty() const { return values_.empty(); } diff --git a/src/formula/callable_fwd.hpp b/src/formula/callable_fwd.hpp index 24f92f3ec387..da4be261d593 100644 --- a/src/formula/callable_fwd.hpp +++ b/src/formula/callable_fwd.hpp @@ -23,11 +23,6 @@ namespace wfl class formula_callable; class formula_debugger; -struct callable_die_subscriber { - virtual void notify_dead() {} - virtual ~callable_die_subscriber() {} -}; - enum FORMULA_ACCESS_TYPE { FORMULA_READ_ONLY, FORMULA_WRITE_ONLY, FORMULA_READ_WRITE }; struct formula_input { diff --git a/src/formula/callable_objects.cpp b/src/formula/callable_objects.cpp index d00772e385d5..38912f23b554 100644 --- a/src/formula/callable_objects.cpp +++ b/src/formula/callable_objects.cpp @@ -777,7 +777,7 @@ variant set_var_callable::execute_self(variant ctxt) //too many calls in a row - possible infinite loop ERR_SF << "ERROR #" << 5001 << " while executing 'set_var' formula function" << std::endl; - return variant(std::make_shared(fake_ptr(), 5001)); + return variant(std::make_shared(*this, 5001)); } variant safe_call_callable::get_value(const std::string& key) const diff --git a/src/formula/callable_objects.hpp b/src/formula/callable_objects.hpp index 3c4b4a35e8fd..ebb7e983757c 100644 --- a/src/formula/callable_objects.hpp +++ b/src/formula/callable_objects.hpp @@ -40,6 +40,7 @@ class terrain_callable : public formula_callable int do_compare(const formula_callable* callable) const override; private: + terrain_callable* clone() const override {return new terrain_callable(*this);} const map_location loc_; const terrain_type& t_; const int owner_; @@ -57,6 +58,7 @@ class gamemap_callable : public formula_callable const gamemap& get_gamemap() const; private: + gamemap_callable* clone() const override {return new gamemap_callable(*this);} const display_context& board_; }; @@ -79,6 +81,7 @@ class location_callable : public formula_callable void get_inputs(formula_input_vector& inputs) const override; int do_compare(const formula_callable* callable) const override; + location_callable* clone() const override {return new location_callable(*this);} }; class attack_type_callable : public formula_callable @@ -94,6 +97,7 @@ class attack_type_callable : public formula_callable const attack_type& get_attack_type() const { return *att_; } private: + attack_type_callable* clone() const override {return new attack_type_callable(*this);} const_attack_ptr att_; }; @@ -116,6 +120,7 @@ class unit_callable : public formula_callable const map_location& get_location() const { return loc_; } private: + unit_callable* clone() const override {return new unit_callable(*this);} const map_location& loc_; const unit& u_; }; @@ -136,6 +141,7 @@ class unit_type_callable : public formula_callable const unit_type& get_unit_type() const { return u_; } private: + unit_type_callable* clone() const override {return new unit_type_callable(*this);} const unit_type& u_; }; @@ -151,6 +157,7 @@ class config_callable : public formula_callable const config& get_config() const { return cfg_; } private: + config_callable* clone() const override {return new config_callable(*this);} const config& cfg_; }; @@ -165,6 +172,7 @@ class team_callable : public formula_callable const team& get_team() const { return team_; } private: + team_callable* clone() const override {return new team_callable(*this);} const team& team_; }; @@ -185,6 +193,7 @@ class set_var_callable : public action_callable variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + set_var_callable* clone() const override {return new set_var_callable(*this);} }; class safe_call_callable : public action_callable @@ -214,6 +223,7 @@ class safe_call_callable : public action_callable variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + safe_call_callable* clone() const override {return new safe_call_callable(*this);} }; class safe_call_result : public formula_callable @@ -224,6 +234,13 @@ class safe_call_result : public formula_callable , current_unit_location_(loc) , status_(status) {} + + // May copy the passed callable + safe_call_result(const formula_callable& callable, int status, const map_location& loc = map_location()) + : failed_callable_(callable.to_ptr()) + , current_unit_location_(loc) + , status_(status) + {} private: const_formula_callable_ptr failed_callable_; @@ -233,6 +250,7 @@ class safe_call_result : public formula_callable variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + safe_call_result* clone() const override {return new safe_call_result(*this);} }; } // namespace wfl diff --git a/src/formula/formula.cpp b/src/formula/formula.cpp index 7823fc2899ec..1be0320407b2 100644 --- a/src/formula/formula.cpp +++ b/src/formula/formula.cpp @@ -443,6 +443,7 @@ class unary_operator_expression : public formula_expression class string_callable : public formula_callable { + string_callable* clone() const override {return new string_callable(*this);} public: explicit string_callable(const variant& string) : string_(string) {} @@ -500,6 +501,7 @@ class string_callable : public formula_callable class list_callable : public formula_callable { + list_callable* clone() const override {return new list_callable(*this);} public: explicit list_callable(const variant& list) : list_(list) {} @@ -540,6 +542,7 @@ class list_callable : public formula_callable class map_callable : public formula_callable { + map_callable* clone() const override {return new map_callable(*this);} public: explicit map_callable(const variant& map) : map_(map) {} @@ -591,6 +594,7 @@ class map_callable : public formula_callable class dot_callable : public formula_callable { + dot_callable* clone() const override {return new dot_callable(*this);} public: dot_callable(const formula_callable &global, const formula_callable& local) : global_(global), local_(local) @@ -807,6 +811,7 @@ class operator_expression : public formula_expression class where_variables: public formula_callable { + where_variables* clone() const override {return new where_variables(*this);} public: where_variables(const formula_callable &base, expr_table_ptr table, formula_debugger* fdb) : formula_callable(false) diff --git a/src/formula/function.cpp b/src/formula/function.cpp index fe070bcf322d..2849157531f1 100644 --- a/src/formula/function.cpp +++ b/src/formula/function.cpp @@ -685,6 +685,7 @@ namespace { class variant_comparator : public formula_callable { + variant_comparator* clone() const override {return new variant_comparator(*this);} public: variant_comparator(const expression_ptr& expr, const formula_callable& fallback) : expr_(expr) diff --git a/src/formula/function.hpp b/src/formula/function.hpp index 35f24fc4055d..422a79cfe6c8 100644 --- a/src/formula/function.hpp +++ b/src/formula/function.hpp @@ -142,6 +142,7 @@ class key_value_pair : public formula_callable variant get_value(const std::string& key) const override; void get_inputs(formula_input_vector& inputs) const override; + key_value_pair* clone() const override {return new key_value_pair(*this);} }; class formula_function_expression : public function_expression diff --git a/src/formula/variant_value.cpp b/src/formula/variant_value.cpp index 31d7e68be880..e7af3869e57c 100644 --- a/src/formula/variant_value.cpp +++ b/src/formula/variant_value.cpp @@ -78,15 +78,6 @@ std::string variant_decimal::to_string_impl(const bool sign_value) const variant_callable::variant_callable(const_formula_callable_ptr callable) : callable_(callable) { - if(callable_) { - callable_->subscribe_dtor(this); - } -} - -variant_callable::~variant_callable() { - if(callable_) { - callable_->unsubscribe_dtor(this); - } } std::string variant_callable::get_serialized_string() const diff --git a/src/formula/variant_value.hpp b/src/formula/variant_value.hpp index 0acc8a72103d..5f98702d02f8 100644 --- a/src/formula/variant_value.hpp +++ b/src/formula/variant_value.hpp @@ -310,11 +310,10 @@ class variant_decimal : public variant_numeric }; -class variant_callable : public variant_value_base, private callable_die_subscriber +class variant_callable : public variant_value_base { public: explicit variant_callable(const_formula_callable_ptr callable); - ~variant_callable(); virtual bool as_bool() const override { @@ -360,8 +359,6 @@ class variant_callable : public variant_value_base, private callable_die_subscri } private: - void notify_dead() override {callable_.reset();} - mutable formula_input_vector inputs; // for iteration const_formula_callable_ptr callable_; }; diff --git a/src/gui/core/canvas.cpp b/src/gui/core/canvas.cpp index f6528e6d6409..58dd92925c7f 100644 --- a/src/gui/core/canvas.cpp +++ b/src/gui/core/canvas.cpp @@ -596,7 +596,7 @@ void image_shape::draw(surface& canvas, local_variables.add("clip_y", wfl::variant(clip_y)); // Execute the provided actions for this context. - wfl::variant(variables.fake_ptr()).execute_variant(actions_formula_.evaluate(local_variables)); + variables.query_value("self").execute_variant(actions_formula_.evaluate(local_variables)); // Copy the data to local variables to avoid overwriting the originals. SDL_Rect src_clip = src_clip_; diff --git a/src/image_modifications.cpp b/src/image_modifications.cpp index 987a88f57794..3a6dc81e041f 100644 --- a/src/image_modifications.cpp +++ b/src/image_modifications.cpp @@ -224,6 +224,7 @@ surface wipe_alpha_modification::operator()(const surface& src) const // TODO: Is this useful enough to move into formula/callable_objects? class pixel_callable : public wfl::formula_callable { + pixel_callable* clone() const override {return new pixel_callable(*this);} public: pixel_callable(SDL_Point p, color_t clr, uint32_t w, uint32_t h) : p(p), clr(clr), w(w), h(h) diff --git a/src/scripting/lua_formula_bridge.cpp b/src/scripting/lua_formula_bridge.cpp index c2c02bcfe6dc..edc06efe2d86 100644 --- a/src/scripting/lua_formula_bridge.cpp +++ b/src/scripting/lua_formula_bridge.cpp @@ -37,6 +37,7 @@ variant luaW_tofaivariant(lua_State* L, int i); class lua_callable : public formula_callable { lua_State* mState; int table_i; + lua_callable* clone() const override {return new lua_callable(*this);} public: lua_callable(lua_State* L, int i) : mState(L), table_i(lua_absindex(L,i)) {} variant get_value(const std::string& key) const { diff --git a/src/scripting/lua_terrainfilter.cpp b/src/scripting/lua_terrainfilter.cpp index 26e2162c5695..a644e60b5ae7 100644 --- a/src/scripting/lua_terrainfilter.cpp +++ b/src/scripting/lua_terrainfilter.cpp @@ -583,7 +583,7 @@ class formula_filter : public filter_impl LOG_MATCHES(formula); try { const wfl::location_callable callable1(l); - wfl::map_formula_callable callable(callable1.fake_ptr()); + wfl::map_formula_callable callable(callable1); return (formula_.get() != nullptr) && formula_->evaluate(callable).as_bool(); } catch(const wfl::formula_error& e) { ERR_LMG << "Formula error: " << e.type << " at " << e.filename << ':' << e.line << ")\n"; diff --git a/src/terrain/filter.cpp b/src/terrain/filter.cpp index 256f165f0708..dcb2caeb9549 100644 --- a/src/terrain/filter.cpp +++ b/src/terrain/filter.cpp @@ -333,7 +333,7 @@ bool terrain_filter::match_internal(const map_location& loc, const unit* ref_uni if(cfg_.has_attribute("formula")) { try { const wfl::terrain_callable main(fc_->get_disp_context(), loc); - wfl::map_formula_callable callable(main.fake_ptr()); + wfl::map_formula_callable callable(main); if(ref_unit) { auto ref = std::make_shared(*ref_unit); callable.add("teleport_unit", wfl::variant(ref)); diff --git a/src/units/filter.cpp b/src/units/filter.cpp index 80420452e8fa..343e36f75299 100644 --- a/src/units/filter.cpp +++ b/src/units/filter.cpp @@ -643,7 +643,7 @@ void unit_filter_compound::fill(vconfig cfg) { try { const wfl::unit_callable main(args.loc, args.u); - wfl::map_formula_callable callable(main.fake_ptr()); + wfl::map_formula_callable callable(main); if (args.u2) { std::shared_ptr secondary(new wfl::unit_callable(*args.u2)); callable.add("other", wfl::variant(secondary));