From 965b5e164836090772eb1b141bf9463cc3ab9c1e 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. (cherry-picked from commit 00e58f12f9a57b2e25dd4f67efb1255b177e173b) --- 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 d7e9d8bb941c..62ab350a5b5c 100644 --- a/changelog.md +++ b/changelog.md @@ -241,6 +241,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.13.12 ### Security fixes diff --git a/src/gui/dialogs/unit_attack.cpp b/src/gui/dialogs/unit_attack.cpp index d584688d6b00..236bc4f88699 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 3531561e132a..4a94cf03d184 100644 --- a/src/mouse_events.cpp +++ b/src/mouse_events.cpp @@ -957,6 +957,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();