From 5f58cd7c6d39c35dcefa6fb9f71777a42ea5cb5e Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Sat, 9 Jun 2018 03:31:52 +1100 Subject: [PATCH] Unit Attack: fixed issues arising from displayed items potentially not equally available items Since there were weapons included that weren't shown, it was possible for the best weapon selection to be a hidden one (in which case, the initial selection would be wrong, and it was possible for the returned index to point to a hidden, disabled weapon. This resolves both issues by excluding these disabled attacks from the weapon choices list altogether. They aren't considered when calculating the best attack either. mouse_handler::fill_weapon_choices is also used in mouse_handler::attack_enemy_, but I don't foresee this change should cause any issues there, since you aren't supposed to be able to actually attack with disabled weapons anyway. --- changelog.md | 4 ++++ src/gui/dialogs/unit_attack.cpp | 12 +++++------- src/mouse_events.cpp | 5 +++++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/changelog.md b/changelog.md index e73057275815..d3c963f9c861 100644 --- a/changelog.md +++ b/changelog.md @@ -35,6 +35,10 @@ custom caption= is specified (fixes #3211). * do_not_list=yes units are no longer excluded from the debug mode-only Create Unit dialog. + * Fixed a rare issue where disabled attacks could cause the wrong attack to + be initially selected in the Unit Attack dialog. This bug also had the + potential to cause units to the wrong attack when engaging or viewing + damage calculations. ## Version 1.14.2 ### Campaigns diff --git a/src/gui/dialogs/unit_attack.cpp b/src/gui/dialogs/unit_attack.cpp index 34f92c757f65..a32e9012fe09 100644 --- a/src/gui/dialogs/unit_attack.cpp +++ b/src/gui/dialogs/unit_attack.cpp @@ -121,11 +121,6 @@ void unit_attack::pre_show(window& window) const attack_type& defender_weapon = defender.weapon ? *defender.weapon : *no_weapon; - // Don't show if the attacker's weapon has at least one active "disable" special. - if(attacker.disable) { - continue; - } - const color_t a_cth_color = game_config::red_to_green(attacker.chance_to_hit); const color_t d_cth_color = game_config::red_to_green(defender.chance_to_hit); @@ -198,8 +193,11 @@ void unit_attack::pre_show(window& window) weapon_list.add_row(data); } - const int last_item = weapon_list.get_item_count() - 1; - weapon_list.select_row(std::min(best_weapon_, last_item)); + // If these two aren't the same size, we can't use list selection incides + // to access to weapons list! + assert(weapons_.size() == weapon_list.get_item_count()); + + weapon_list.select_row(best_weapon_); } void unit_attack::post_show(window& window) diff --git a/src/mouse_events.cpp b/src/mouse_events.cpp index 3950d8fbb6c3..27a38f269b6e 100644 --- a/src/mouse_events.cpp +++ b/src/mouse_events.cpp @@ -958,6 +958,11 @@ int mouse_handler::fill_weapon_choices( if(attacker->attacks()[i].attack_weight() > 0) { battle_context bc(pc_.gamestate().board_.units_, attacker->get_location(), defender->get_location(), i); + // Don't include if the attacker's weapon has at least one active "disable" special. + if(bc.get_attacker_stats().disable) { + continue; + } + if(!bc_vector.empty() && bc.better_attack(bc_vector[best], 0.5)) { // as some weapons can be hidden, i is not a valid index into the resulting vector best = bc_vector.size();