0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor battle_context

Browse source 
this has multiple advantages:;
1) it fixes #3594, an assertion failue that happend when a unit with
only one attack attacked when that one attack was disabled.
2) It now creates each pair or battle_context_unit_stats only once for
each pauir of attacks, previsouly the coud would create multiple
temporary battle_context_unit_stats objects to for exampel check
"disabled"
3) It simplifies the code by removing special handling of some cases.
This commit is contained in:
gfgtdf 2018-10-06 02:34:56 +02:00
parent bc9953f0f2
commit b4196af8c0
5 changed files with 134 additions and 192 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

298
src/actions/attack.cpp
View file

@ -349,6 +349,42 @@ battle_context_unit_stats::battle_context_unit_stats(const unit_type* u_type,
// BATTLE CONTEXT
// ==================================================================================
battle_context::battle_context(
const unit& attacker,
const map_location& a_loc,
int a_wep_index,
const unit& defender,
const map_location& d_loc,
int d_wep_index,
const unit_map& units)
: attacker_stats_()
, defender_stats_()
, attacker_combatant_()
, defender_combatant_()
{
size_t a_wep_uindex = static_cast<size_t>(a_wep_index);
size_t d_wep_uindex = static_cast<size_t>(d_wep_index);
const_attack_ptr a_wep(a_wep_uindex < attacker.attacks().size() ? attacker.attacks()[a_wep_index].shared_from_this() : nullptr);
const_attack_ptr d_wep(d_wep_uindex < defender.attacks().size() ? defender.attacks()[d_wep_index].shared_from_this() : nullptr);
attacker_stats_.reset(new battle_context_unit_stats(attacker, a_loc, a_wep_index, true , defender, d_loc, d_wep, units));
defender_stats_.reset(new battle_context_unit_stats(defender, d_loc, d_wep_index, false, attacker, a_loc, a_wep, units));
}
void battle_context::simulate(const combatant* prev_def)
{
assert((attacker_combatant_.get() != nullptr) == (defender_combatant_.get() != nullptr));
assert(attacker_stats_);
assert(defender_stats_);
if(!attacker_combatant_) {
attacker_combatant_.reset(new combatant(*attacker_stats_));
defender_combatant_.reset(new combatant(*defender_stats_, prev_def));
attacker_combatant_->fight(*defender_combatant_);
}
}
// more like a factory method than a constructor, always calls one of the other constructors.
battle_context::battle_context(const unit_map& units,
const map_location& attacker_loc,
const map_location& defender_loc,
@ -367,43 +403,19 @@ battle_context::battle_context(const unit_map& units,
const double harm_weight = 1.0 - aggression;
if(attacker_weapon == -1) {
attacker_weapon = choose_attacker_weapon(
attacker, defender, units, attacker_loc, defender_loc, harm_weight, &defender_weapon, prev_def
*this = choose_attacker_weapon(
attacker, defender, units, attacker_loc, defender_loc, harm_weight, prev_def
);
} else if(defender_weapon == -1) {
defender_weapon = choose_defender_weapon(
}
else if(defender_weapon == -1) {
*this = choose_defender_weapon(
attacker, defender, attacker_weapon, units, attacker_loc, defender_loc, prev_def
);
}
// If those didn't have to generate statistics, do so now.
if(!attacker_stats_) {
const_attack_ptr adef = nullptr;
const_attack_ptr ddef = nullptr;
if(attacker_weapon >= 0) {
VALIDATE(attacker_weapon < static_cast<int>(attacker.attacks().size()),
_("An invalid attacker weapon got selected."));
adef = attacker.attacks()[attacker_weapon].shared_from_this();
}
if(defender_weapon >= 0) {
VALIDATE(defender_weapon < static_cast<int>(defender.attacks().size()),
_("An invalid defender weapon got selected."));
ddef = defender.attacks()[defender_weapon].shared_from_this();
}
assert(!defender_stats_ && !attacker_combatant_ && !defender_combatant_);
attacker_stats_.reset(new battle_context_unit_stats(
attacker, attacker_loc, attacker_weapon, true, defender, defender_loc, ddef, units));
defender_stats_.reset(new battle_context_unit_stats(
defender, defender_loc, defender_weapon, false, attacker, attacker_loc, adef, units));
else {
*this = battle_context(attacker, attacker_loc, attacker_weapon, defender, defender_loc, defender_weapon, units);
}
// There have been various bugs where only one of these was set
assert(attacker_stats_);
assert(defender_stats_);
}
@ -446,34 +458,18 @@ battle_context& battle_context::operator=(const battle_context& other)
const combatant& battle_context::get_attacker_combatant(const combatant* prev_def)
{
// We calculate this lazily, since AI doesn't always need it.
if(!attacker_combatant_) {
assert(!defender_combatant_);
attacker_combatant_.reset(new combatant(*attacker_stats_));
defender_combatant_.reset(new combatant(*defender_stats_, prev_def));
attacker_combatant_->fight(*defender_combatant_);
}
simulate(prev_def);
return *attacker_combatant_;
}
const combatant& battle_context::get_defender_combatant(const combatant* prev_def)
{
// We calculate this lazily, since AI doesn't always need it.
if(!defender_combatant_) {
assert(!attacker_combatant_);
attacker_combatant_.reset(new combatant(*attacker_stats_));
defender_combatant_.reset(new combatant(*defender_stats_, prev_def));
attacker_combatant_->fight(*defender_combatant_);
}
simulate(prev_def);
return *defender_combatant_;
}
// Given this harm_weight, are we better than this other context?
// Given this harm_weight, are we better than that other context?
bool battle_context::better_attack(class battle_context& that, double harm_weight)
{
return better_combat(
@ -485,6 +481,18 @@ bool battle_context::better_attack(class battle_context& that, double harm_weigh
);
}
// Given this harm_weight, are we better than that other context?
bool battle_context::better_defense(class battle_context& that, double harm_weight)
{
return better_combat(
get_defender_combatant(),
get_attacker_combatant(),
that.get_defender_combatant(),
that.get_attacker_combatant(),
harm_weight
);
}
// Does combat A give us a better result than combat B?
bool battle_context::better_combat(const combatant& us_a,
const combatant& them_a,
@ -528,124 +536,60 @@ bool battle_context::better_combat(const combatant& us_a,
return them_a.average_hp() < them_b.average_hp();
}
int battle_context::choose_attacker_weapon(const unit& attacker,
battle_context battle_context::choose_attacker_weapon(const unit& attacker,
const unit& defender,
const unit_map& units,
const map_location& attacker_loc,
const map_location& defender_loc,
double harm_weight,
int* defender_weapon,
const combatant* prev_def)
{
std::vector<unsigned int> choices;
std::vector<battle_context> choices;
// What options does attacker have?
unsigned int i;
for(i = 0; i < attacker.attacks().size(); ++i) {
for(size_t i = 0; i < attacker.attacks().size(); ++i) {
const attack_type& att = attacker.attacks()[i];
if(att.attack_weight() > 0) {
choices.push_back(i);
if(att.attack_weight() <= 0) {
continue;
}
battle_context bc = choose_defender_weapon(attacker, defender, i, units, attacker_loc, defender_loc, prev_def);
//choose_defender_weapon will always choose the weapon that disabels the attackers weapon if possible.
if(bc.attacker_stats_->disable) {
continue;
}
choices.emplace_back(std::move(bc));
}
if(choices.empty()) {
return -1;
return battle_context(attacker, attacker_loc, -1, defender, defender_loc, -1, units);
}
if(choices.size() == 1) {
*defender_weapon
= choose_defender_weapon(attacker, defender, choices[0], units, attacker_loc, defender_loc, prev_def);
const_attack_ptr def_weapon
= *defender_weapon >= 0 ? defender.attacks()[*defender_weapon].shared_from_this() : nullptr;
attacker_stats_.reset(new battle_context_unit_stats(
attacker, attacker_loc, choices[0], true, defender, defender_loc, def_weapon, units));
if(attacker_stats_->disable) {
attacker_stats_.reset();
return -1;
}
const attack_type& att = attacker.attacks()[choices[0]];
defender_stats_.reset(new battle_context_unit_stats(
defender, defender_loc, *defender_weapon, false, attacker, attacker_loc, att.shared_from_this(), units));
return choices[0];
return std::move(choices[0]);
}
// Multiple options: simulate them, save best.
std::unique_ptr<battle_context_unit_stats> best_att_stats(nullptr);
std::unique_ptr<battle_context_unit_stats> best_def_stats(nullptr);
battle_context* best_choice = nullptr;
for(auto& choice : choices) {
// If choose_defender_weapon didn't simulate, do so now.
choice.simulate(prev_def);
std::unique_ptr<combatant> best_att_comb(nullptr);
std::unique_ptr<combatant> best_def_comb(nullptr);
for(i = 0; i < choices.size(); ++i) {
const attack_type& att = attacker.attacks()[choices[i]];
int def_weapon =
choose_defender_weapon(attacker, defender, choices[i], units, attacker_loc, defender_loc, prev_def);
// If that didn't simulate, do so now.
if(!attacker_combatant_) {
const_attack_ptr def = nullptr;
if(def_weapon >= 0) {
def = defender.attacks()[def_weapon].shared_from_this();
}
attacker_stats_.reset(new battle_context_unit_stats(
attacker, attacker_loc, choices[i], true, defender, defender_loc, def, units));
if(attacker_stats_->disable) {
continue;
}
defender_stats_.reset(new battle_context_unit_stats(
defender, defender_loc, def_weapon, false, attacker, attacker_loc, att.shared_from_this(), units));
attacker_combatant_.reset(new combatant(*attacker_stats_));
defender_combatant_.reset(new combatant(*defender_stats_, prev_def));
attacker_combatant_->fight(*defender_combatant_);
} else {
if(attacker_stats_ != nullptr && attacker_stats_->disable) {
continue;
}
if(!best_choice || choice.better_attack(*best_choice, harm_weight)) {
best_choice = &choice;
}
if(!best_att_comb ||
better_combat(*attacker_combatant_, *defender_combatant_, *best_att_comb, *best_def_comb, harm_weight)
) {
best_att_comb = std::move(attacker_combatant_);
best_def_comb = std::move(defender_combatant_);
best_att_stats = std::move(attacker_stats_);
best_def_stats = std::move(defender_stats_);
}
attacker_combatant_.reset();
defender_combatant_.reset();
attacker_stats_.reset();
defender_stats_.reset();
}
attacker_combatant_ = std::move(best_att_comb);
defender_combatant_ = std::move(best_def_comb);
attacker_stats_ = std::move(best_att_stats);
defender_stats_ = std::move(best_def_stats);
// These currently mean the same thing, but assumptions like that have been broken before
if(!defender_stats_ || !attacker_stats_) {
return -1;
if(best_choice) {
return std::move(*best_choice);
}
else {
return battle_context(attacker, attacker_loc, -1, defender, defender_loc, -1, units);
}
*defender_weapon = defender_stats_->attack_num;
return attacker_stats_->attack_num;
}
/** @todo FIXME: Hand previous defender unit in here. */
int battle_context::choose_defender_weapon(const unit& attacker,
battle_context battle_context::choose_defender_weapon(const unit& attacker,
const unit& defender,
unsigned attacker_weapon,
const unit_map& units,
@ -656,28 +600,35 @@ int battle_context::choose_defender_weapon(const unit& attacker,
VALIDATE(attacker_weapon < attacker.attacks().size(), _("An invalid attacker weapon got selected."));
const attack_type& att = attacker.attacks()[attacker_weapon];
std::vector<unsigned int> choices;
auto no_weapon = [&]() { return battle_context(attacker, attacker_loc, attacker_weapon, defender, defender_loc, -1, units); };
std::vector<battle_context> choices;
// What options does defender have?
unsigned int i;
for(i = 0; i < defender.attacks().size(); ++i) {
for(size_t i = 0; i < defender.attacks().size(); ++i) {
const attack_type& def = defender.attacks()[i];
if(def.range() == att.range() && def.defense_weight() > 0) {
choices.push_back(i);
if(def.range() != att.range() || def.defense_weight() <= 0) {
//no need to calculate the battle_context here.
continue;
}
battle_context bc(attacker, attacker_loc, attacker_weapon, defender, defender_loc, i, units);
if(bc.defender_stats_->disable) {
continue;
}
if(bc.attacker_stats_->disable) {
//the defenders attack disables the attakers attack: always choose this one.
return bc;
}
choices.emplace_back(std::move(bc));
}
if(choices.empty()) {
return -1;
return no_weapon();
}
if(choices.size() == 1) {
const battle_context_unit_stats def_stats(
defender, defender_loc, choices[0], false, attacker, attacker_loc, att.shared_from_this(), units);
return (def_stats.disable) ? -1 : choices[0];
//only one usable weapon, don't simulate
return std::move(choices[0]);
}
// Multiple options:
@ -689,16 +640,11 @@ int battle_context::choose_defender_weapon(const unit& attacker,
{
double max_weight = 0.0;
for(i = 0; i < choices.size(); ++i) {
const attack_type& def = defender.attacks()[choices[i]];
for(const auto& choice : choices) {
const attack_type& def = defender.attacks()[choice.defender_stats_->attack_num];
if(def.defense_weight() >= max_weight) {
const battle_context_unit_stats def_stats(defender, defender_loc, choices[i], false, attacker,
attacker_loc, att.shared_from_this(), units);
if(def_stats.disable) {
continue;
}
const battle_context_unit_stats& def_stats = *choice.defender_stats_;
max_weight = def.defense_weight();
int rating = static_cast<int>(
@ -711,40 +657,24 @@ int battle_context::choose_defender_weapon(const unit& attacker,
}
}
battle_context* best_choice = nullptr;
// Multiple options: simulate them, save best.
for(i = 0; i < choices.size(); ++i) {
const attack_type& def = defender.attacks()[choices[i]];
for(auto& choice : choices) {
const attack_type& def = defender.attacks()[choice.defender_stats_->attack_num];
std::unique_ptr<battle_context_unit_stats> att_stats(new battle_context_unit_stats(
attacker, attacker_loc, attacker_weapon, true, defender, defender_loc, def.shared_from_this(), units));
std::unique_ptr<battle_context_unit_stats> def_stats(new battle_context_unit_stats(
defender, defender_loc, choices[i], false, attacker, attacker_loc, att.shared_from_this(), units));
if(def_stats->disable) {
continue;
}
std::unique_ptr<combatant> att_comb(new combatant(*att_stats));
std::unique_ptr<combatant> def_comb(new combatant(*def_stats, prev_def));
att_comb->fight(*def_comb);
choice.simulate(prev_def);
int simple_rating = static_cast<int>(
def_stats->num_blows * def_stats->damage * def_stats->chance_to_hit * def.defense_weight());
choice.defender_stats_->num_blows * choice.defender_stats_->damage * choice.defender_stats_->chance_to_hit * def.defense_weight());
if(simple_rating >= min_rating &&
(!attacker_combatant_ || better_combat(*def_comb, *att_comb, *defender_combatant_, *attacker_combatant_, 1.0))
) {
attacker_combatant_ = std::move(att_comb);
defender_combatant_ = std::move(def_comb);
attacker_stats_ = std::move(att_stats);
defender_stats_ = std::move(def_stats);
//FIXME: make sure there is no mostake in the better_combat call-
if(simple_rating >= min_rating && (!best_choice || choice.better_defense(*best_choice, 1.0))) {
best_choice = &choice;
}
}
return defender_stats_ ? defender_stats_->attack_num : -1;
return best_choice ? std::move(*best_choice) : no_weapon();
}

18
src/actions/attack.hpp
View file

@ -189,6 +189,7 @@ public:
battle_context(const battle_context_unit_stats& att, const battle_context_unit_stats& def);
battle_context(const battle_context& other);
battle_context(battle_context&& other) = default;
battle_context& operator=(const battle_context& other);
@ -212,6 +213,8 @@ public:
/** Given this harm_weight, is this attack better than that? */
bool better_attack(class battle_context& that, double harm_weight);
/** Given this harm_weight, is this attack better than that? */
bool better_defense(class battle_context& that, double harm_weight);
static bool better_combat(const combatant& us_a,
const combatant& them_a,
@ -219,17 +222,26 @@ public:
const combatant& them_b,
double harm_weight);
void simulate(const combatant* prev_def);
private:
int choose_attacker_weapon(const unit& attacker,
battle_context(
const unit& attacker,
const map_location& attacker_loc,
int attacker_weapon,
const unit& defender,
const map_location& defender_loc,
int defender_weapon,
const unit_map& units);
static battle_context choose_attacker_weapon(const unit& attacker,
const unit& defender,
const unit_map& units,
const map_location& attacker_loc,
const map_location& defender_loc,
double harm_weight,
int* defender_weapon,
const combatant* prev_def);
int choose_defender_weapon(const unit& attacker,
static battle_context choose_defender_weapon(const unit& attacker,
const unit& defender,
unsigned attacker_weapon,
const unit_map& units,

4
src/gui/dialogs/unit_attack.cpp
View file

@ -71,12 +71,12 @@ REGISTER_DIALOG(unit_attack)
unit_attack::unit_attack(const unit_map::iterator& attacker_itor,
const unit_map::iterator& defender_itor,
const std::vector<battle_context>& weapons,
std::vector<battle_context>&& weapons,
const int best_weapon)
: selected_weapon_(-1)
, attacker_itor_(attacker_itor)
, defender_itor_(defender_itor)
, weapons_(weapons)
, weapons_(std::move(weapons))
, best_weapon_(best_weapon)
{
}

2
src/gui/dialogs/unit_attack.hpp
View file

@ -28,7 +28,7 @@ class unit_attack : public modal_dialog
public:
unit_attack(const unit_map::iterator& attacker_itor,
const unit_map::iterator& defender_itor,
const std::vector<battle_context>& weapons,
std::vector<battle_context>&& weapons,
const int best_weapon);
/***** ***** ***** setters / getters for members ***** ****** *****/

4
src/mouse_events.cpp
View file

@ -966,7 +966,7 @@ int mouse_handler::fill_weapon_choices(
best = bc_vector.size();
}
bc_vector.push_back(std::move(bc));
bc_vector.emplace_back(std::move(bc));
}
}
@ -994,7 +994,7 @@ int mouse_handler::show_attack_dialog(const map_location& attacker_loc, const ma
return -1;
}
gui2::dialogs::unit_attack dlg(attacker, defender, bc_vector, best);
gui2::dialogs::unit_attack dlg(attacker, defender, std::move(bc_vector), best);
if(dlg.show()) {
return dlg.get_selected_weapon();