Skip to content

Commit

Permalink
WFL: Fix a crash if a formula object goes out of scope before the for…
Browse files Browse the repository at this point in the history 
…mula 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).
  • Loading branch information
@CelticMinstrel
CelticMinstrel committed Mar 13, 2021
1 parent ce67e78 commit 714f411
Show file tree
Hide file tree
Showing 24 changed files with 112 additions and 81 deletions.
8 changes: 4 additions & 4 deletions src/ai/default/attack.cpp
View file
Expand Up @@ -429,22 +429,22 @@ 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<wfl::safe_call_result>(fake_ptr(), attack_result::E_EMPTY_DEFENDER, move_from));
return wfl::variant(std::make_shared<wfl::safe_call_result>(*this, attack_result::E_EMPTY_DEFENDER, move_from));
}

//check if we need to move
if(move_from != att_src) {
//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<wfl::safe_call_result>(fake_ptr(), move_result::E_NO_UNIT, move_from));
return wfl::variant(std::make_shared<wfl::safe_call_result>(*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<wfl::safe_call_result>(fake_ptr(), result->get_status(), result->get_unit_location()));
return wfl::variant(std::make_shared<wfl::safe_call_result>(*this, result->get_status(), result->get_unit_location()));
}
}

Expand All @@ -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<wfl::safe_call_result>(fake_ptr(), result->get_status()));
return wfl::variant(std::make_shared<wfl::safe_call_result>(*this, result->get_status()));
}
}
return wfl::variant(true);
Expand Down
1 change: 1 addition & 0 deletions src/ai/default/contexts.hpp
View file
Expand Up @@ -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(),
Expand Down
12 changes: 6 additions & 6 deletions src/ai/formula/ai.cpp
View file
Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<move_map_callable>(get_enemy_srcdst(), get_enemy_dstsrc(), units));
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions src/ai/formula/ai.hpp
View file
Expand Up @@ -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() {}
Expand Down Expand Up @@ -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_;
Expand Down
18 changes: 9 additions & 9 deletions src/ai/formula/callable_objects.cpp
View file
Expand Up @@ -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<safe_call_result>(fake_ptr(), move_result->get_status(), move_result->get_unit_location()));
return variant(std::make_shared<safe_call_result>(*this, move_result->get_status(), move_result->get_unit_location()));
}

return variant(move_result->is_gamestate_changed());
Expand Down Expand Up @@ -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<safe_call_result>(fake_ptr(), move_result->get_status(), move_result->get_unit_location()));
return variant(std::make_shared<safe_call_result>(*this, move_result->get_status(), move_result->get_unit_location()));
}

return variant(move_result->is_gamestate_changed());
Expand Down Expand Up @@ -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<battle_context>(resources::gameboard->units(), src, dst, weapon, -1, 1.0, nullptr,
resources::gameboard->units().find(move_from).get_shared_ptr()))
{
type_ = ATTACK_C;
}
Expand Down Expand Up @@ -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<safe_call_result>(fake_ptr(), move_result->get_status(), move_result->get_unit_location()));
return variant(std::make_shared<safe_call_result>(*this, move_result->get_status(), move_result->get_unit_location()));
}
}

Expand All @@ -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<safe_call_result>(fake_ptr(), attack_result->get_status()));
return variant(std::make_shared<safe_call_result>(*this, attack_result->get_status()));
}
}

Expand Down Expand Up @@ -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<safe_call_result>(fake_ptr(), recall_result->get_status()));
return variant(std::make_shared<safe_call_result>(*this, recall_result->get_status()));
}

return variant(recall_result->is_gamestate_changed());
Expand Down Expand Up @@ -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<safe_call_result>(fake_ptr(), recruit_result->get_status()));
return variant(std::make_shared<safe_call_result>(*this, recruit_result->get_status()));
}

//is_gamestate_changed()==true means that the game state was somehow changed by action.
Expand Down Expand Up @@ -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<safe_call_result>(fake_ptr(), status));
return variant(std::make_shared<safe_call_result>(*this, status));
}

variant fallback_callable::execute_self(variant) {
Expand Down
18 changes: 15 additions & 3 deletions src/ai/formula/callable_objects.hpp
View file
Expand Up @@ -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;
Expand All @@ -46,19 +47,21 @@ 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<battle_context> 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);

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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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() {
}
Expand All @@ -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)
Expand All @@ -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_(),
Expand Down Expand Up @@ -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<variant>& hitLeft,
const std::vector<variant>& prob,
Expand Down
6 changes: 3 additions & 3 deletions src/ai/formula/candidates.cpp
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<unit_callable>();
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]);

Expand Down
2 changes: 1 addition & 1 deletion src/ai/formula/stage_side_formulas.cpp
View file
Expand Up @@ -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() ) { }
Expand Down
6 changes: 3 additions & 3 deletions src/ai/formula/stage_unit_formulas.cpp
View file
Expand Up @@ -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<wfl::unit_callable>(*i)));
priority = (wfl::formula::evaluate(priority_formula, callable)).as_int();
} else {
Expand Down Expand Up @@ -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<wfl::unit_callable>(*i)));
fai_.make_action(formula, callable);
} else {
Expand All @@ -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<wfl::unit_callable>(*i)));
while ( !fai_.make_action(loop_formula, callable).is_empty() && i.valid() )
{
Expand Down

0 comments on commit 714f411

Please sign in to comment.