Unify NPC weapon evaluation (#82151)

* Simplify calls to evaluate_weapon() by giving it a helper

* Move all npc<-->npc evaluation of weapons to evaluate_weapon()

* ...and then move evaluate_weapon() to Character

* Remove the last vestiges of weapon_value() outside of char creation

* Move the existing cache

* Replace character creation's offense summary with evaluate_weapon() values

* Various clang-demanded fixes

Author: Maleclypse

src/avatar_action.cpp

@@ -694,7 +694,7 @@ static float rate_critter( const Creature &c )
     if( np != nullptr ) {
         item_location wielded = np->get_wielded_item();
         if( wielded ) {
-            return np->weapon_value( *wielded );
+            return np->evaluate_weapon( *wielded );
         } else {
             return np->unarmed_value();
         }

src/bionics.cpp

@@ -683,9 +683,7 @@ void npc::check_or_use_weapon_cbm( const bionic_id &cbm_id )
             return;
         }
 
-        const int cbm_ammo = free_power / cbm_weapon.get_gun_energy_drain();
-
-        if( weapon_value( weap, ammo_count ) < weapon_value( cbm_weapon, cbm_ammo ) ) {
+        if( evaluate_weapon( weap ) < evaluate_weapon( cbm_weapon ) ) {
             if( real_weapon.is_null() ) {
                 // Prevent replacing real weapon when migrating saves
                 real_weapon = weap;
@@ -705,7 +703,7 @@ void npc::check_or_use_weapon_cbm( const bionic_id &cbm_id )
 
         const item cbm_weapon = bio.get_weapon();
 
-        if( weapon_value( weap, ammo_count ) < weapon_value( cbm_weapon, 0 ) ) {
+        if( evaluate_weapon( weap ) < evaluate_weapon( cbm_weapon ) ) {
             if( is_armed() ) {
                 stow_item( *weapon );
             }

src/character.h

@@ -1197,7 +1197,7 @@ class Character : public Creature, public visitable
         bool can_attack_high() const override;
 
         /** NPC-related item rating functions */
-        double weapon_value( const item &weap, int ammo = 10 ) const; // Evaluates item as a weapon
+        double evaluate_weapon( const item &maybe_weapon, bool pretend_have_ammo = false ) const;
         double gun_value( const item &weap, int ammo = 10 ) const; // Evaluates item as a gun
         double melee_value( const item &weap ) const; // As above, but only as melee
         double unarmed_value() const; // Evaluate yourself!
@@ -1220,6 +1220,8 @@ class Character : public Creature, public visitable
         bool is_dead_state() const override;
 
     private:
+        double evaluate_weapon_internal( const item &maybe_weapon, bool can_use_gun,
+                                         bool use_silent, int pretend_ammo = 0 ) const;
         mutable std::optional<bool> cached_dead_state;
     public:
         void set_part_hp_cur( const bodypart_id &id, int set ) override;

src/melee.cpp

@@ -2726,37 +2726,72 @@ int Character::attack_speed( const item &weap ) const
     return std::round( move_cost );
 }
 
-double Character::weapon_value( const item &weap, int ammo ) const
+double Character::evaluate_weapon( const item &maybe_weapon, const bool pretend_have_ammo ) const
 {
-    if( is_wielding( weap ) || ( !get_wielded_item() && weap.is_null() ) ) {
+    bool can_use_gun = true;
+    bool use_silent = false;
+    const npc *me_as_npc = dynamic_cast<const npc *>( this );
+    if( me_as_npc ) {
+        if( me_as_npc->is_player_ally() && !me_as_npc->rules.has_flag( ally_rule::use_guns ) ) {
+            can_use_gun = false;
+        }
+        if( me_as_npc->is_player_ally() && me_as_npc->rules.has_flag( ally_rule::use_silent ) ) {
+            use_silent = true;
+        }
+    }
+    // ABSOLUTELY disgusting fake gun assembly for character creation
+    int pretend_ammo = 0;
+    if( pretend_have_ammo && maybe_weapon.is_gun() ) {
+        itype_id ammo_id = itype_id::NULL_ID();
+        if( maybe_weapon.ammo_default().is_null() ) {
+            ammo_id = item( maybe_weapon.magazine_default() ).ammo_default();
+        } else {
+            ammo_id = maybe_weapon.ammo_default();
+        }
+        const ammotype &type_of_ammo = item::find_type( ammo_id )->ammo->type;
+        if( maybe_weapon.magazine_integral() ) {
+            pretend_ammo = maybe_weapon.ammo_capacity( type_of_ammo );
+        } else {
+            pretend_ammo = item( maybe_weapon.magazine_default() ).ammo_capacity( type_of_ammo );
+        }
+    }
+    return evaluate_weapon_internal( maybe_weapon, can_use_gun, use_silent, pretend_ammo );
+}
+
+double Character::evaluate_weapon_internal( const item &maybe_weapon, bool can_use_gun,
+        bool use_silent, const int pretend_ammo ) const
+{
+    if( is_wielding( maybe_weapon ) || ( !get_wielded_item() && maybe_weapon.is_null() ) ) {
         auto cached_value = cached_info.find( "weapon_value" );
         if( cached_value != cached_info.end() ) {
             return cached_value->second;
         }
     }
-    double val_gun = gun_value( weap, ammo );
-    val_gun = val_gun /
-              5.0f; // This is an emergency patch to get melee and ranged in approximate parity, if you're looking at it in 2025 or later and it's still here... I'm sorry.  Kill it with fire.  Tear it all down, and rebuild a glorious castle from the ashes.
+    // Needed because evaluation includes electricity via linked cables.
+    const map &here = get_map();
+
+    bool allowed = can_use_gun && maybe_weapon.is_gun() && ( !use_silent || maybe_weapon.is_silent() );
+    // According to unmodified evaluation score, NPCs almost always prioritize wielding guns if they have one.
+    // This is relatively reasonable, as players can issue commands to NPCs when we do not want them to use ranged weapons.
+    // Conversely, we cannot directly issue commands when we want NPCs to prioritize ranged weapons.
+    // Note that the scoring method here is different from the 'weapon_value' used elsewhere.
+    double val_gun = allowed ? gun_value( maybe_weapon,
+                                          std::max( maybe_weapon.shots_remaining( here, this ), pretend_ammo )
+                                        ) : 0;
     add_msg_debug( debugmode::DF_NPC_ITEMAI,
-                   "<color_magenta>weapon_value</color>%s %s valued at <color_light_cyan>%1.2f as a ranged weapon</color>.",
-                   disp_name( true ), weap.type->get_id().str(), val_gun );
-    double val_melee = melee_value( weap );
-    val_melee *=
-        val_melee; // Same emergency patch.  Same purple prose descriptors, you already saw them above.
+                   "%s %s valued at <color_light_cyan>%1.2f as a ranged weapon to wield</color>.",
+                   disp_name( true ), maybe_weapon.type->get_id().str(), val_gun );
+    double val_melee = melee_value( maybe_weapon );
     add_msg_debug( debugmode::DF_NPC_ITEMAI,
-                   "%s %s valued at <color_light_cyan>%1.2f as a melee weapon</color>.", disp_name( true ),
-                   weap.type->get_id().str(), val_melee );
-    const double more = std::max( val_gun, val_melee );
-    const double less = std::min( val_gun, val_melee );
-
-    // A small bonus for guns you can also use to hit stuff with (bayonets etc.)
-    const double my_val = more + ( less / 2.0 );
-    add_msg_debug( debugmode::DF_MELEE, "%s (%ld ammo) sum value: %.1f", weap.type->get_id().str(),
-                   ammo, my_val );
-    if( is_wielding( weap ) || ( !get_wielded_item() && weap.is_null() ) ) {
-        cached_info.emplace( "weapon_value", my_val );
-    }
-    return my_val;
+                   "%s %s valued at <color_light_cyan>%1.2f as a melee weapon to wield</color>.", disp_name( true ),
+                   maybe_weapon.type->get_id().str(), val_melee );
+    double val = std::max( val_gun, val_melee );
+
+
+    if( is_wielding( maybe_weapon ) || ( !get_wielded_item() && maybe_weapon.is_null() ) ) {
+        cached_info.emplace( "weapon_value", val );
+    }
+    return val;
 }
 
 double Character::melee_value( const item &weap ) const

src/npc.cpp

@@ -1622,7 +1622,7 @@ npc_opinion npc::get_opinion_values( const Character &you ) const
         } else {
             npc_values.fear += 6;
         }
-    } else if( you.weapon_value( *weapon ) > 20 ) {
+    } else if( you.evaluate_weapon( *weapon ) > 20 ) {
         npc_values.fear += 2;
     }
 
@@ -1852,7 +1852,7 @@ void npc::decide_needs()
     }
 
     const item &weap = weapon ? *weapon : null_item_reference();
-    needrank[need_weapon] = weapon_value( weap );
+    needrank[need_weapon] = evaluate_weapon( weap );
     needrank[need_food] = 15 - get_hunger();
     needrank[need_drink] = 15 - get_thirst();
     cache_visit_items_with( "is_food", &item::is_food, [&]( const item & it ) {
@@ -2231,8 +2231,8 @@ double npc::value( const item &it, double market_price ) const
     float ret = 1;
     if( it.is_maybe_melee_weapon() || it.is_gun() ) {
         // todo: remove when weapon_value takes an item_location
-        double wield_val = weapon ? weapon_value( *weapon ) : weapon_value( null_item_reference() );
-        double weapon_val = weapon_value( it ) - wield_val;
+        double wield_val = weapon ? evaluate_weapon( *weapon ) : evaluate_weapon( null_item_reference() );
+        double weapon_val = evaluate_weapon( it ) - wield_val;
 
         if( weapon_val > 0 ) {
             ret += weapon_val * 0.0002;

src/npc.h

@@ -1131,8 +1131,6 @@ class npc : public Character
         int evaluate_sleep_spot( tripoint_bub_ms p );
         // Returns true if did something and we should end turn
         bool scan_new_items();
-        // Returns score for how well this weapon might kill things
-        double evaluate_weapon( item &maybe_weapon, bool can_use_gun, bool use_silent ) const;
         // Returns best weapon. Can return null (fists)
         item *evaluate_best_weapon() const;
         // Returns true if did wield it

src/npcmove.cpp

@@ -512,7 +512,7 @@ float npc::evaluate_character( const Character &candidate, bool my_gun, bool ene
     bool candidate_gun = candidate.get_wielded_item() && candidate.get_wielded_item()->is_gun();
     const item &candidate_weap = candidate.get_wielded_item() ? *candidate.get_wielded_item() :
                                  null_item_reference();
-    double candidate_weap_val = candidate.weapon_value( candidate_weap );
+    double candidate_weap_val = candidate.evaluate_weapon( candidate_weap );
     float candidate_health =  candidate.hp_percentage() / 100.0f;
     float armour = estimate_armour( candidate );
     float speed = std::max( 0.25f, candidate.get_speed() / 100.0f );
@@ -1317,7 +1317,7 @@ void npc::regen_ai_cache()
     ai_cache.danger = 0.0f;
     ai_cache.total_danger = 0.0f;
     item &weapon = get_wielded_item() ? *get_wielded_item() : null_item_reference();
-    ai_cache.my_weapon_value = weapon_value( weapon );
+    ai_cache.my_weapon_value = evaluate_weapon( weapon );
     ai_cache.dangerous_explosives = find_dangerous_explosives();
     mem_combat.formation_distance = -1;
 
@@ -3897,7 +3897,7 @@ bool npc::wants_take_that( const item &it )
 
     item &weap = get_wielded_item() ? *get_wielded_item() : null_item_reference();
     if( ( ( !whitelisting && value( it ) > min_value ) || item_whitelisted( it ) ) ||
-        weapon_value( it ) > weapon_value( weap ) ) {
+        evaluate_weapon( it ) > evaluate_weapon( weap ) ) {
         good = true;
     }
 
@@ -4133,62 +4133,36 @@ bool npc::do_player_activity()
     return moves != old_moves;
 }
 
-double npc::evaluate_weapon( item &maybe_weapon, bool can_use_gun, bool use_silent ) const
-{
-    // Needed because evaluation includes electricity via linked cables.
-    const map &here = get_map();
-
-    bool allowed = can_use_gun && maybe_weapon.is_gun() && ( !use_silent || maybe_weapon.is_silent() );
-    // According to unmodified evaluation score, NPCs almost always prioritize wielding guns if they have one.
-    // This is relatively reasonable, as players can issue commands to NPCs when we do not want them to use ranged weapons.
-    // Conversely, we cannot directly issue commands when we want NPCs to prioritize ranged weapons.
-    // Note that the scoring method here is different from the 'weapon_value' used elsewhere.
-    double val_gun = allowed ? gun_value( maybe_weapon, maybe_weapon.shots_remaining( here,
-                                          this ) ) : 0;
-    add_msg_debug( debugmode::DF_NPC_ITEMAI,
-                   "%s %s valued at <color_light_cyan>%1.2f as a ranged weapon to wield</color>.",
-                   disp_name( true ), maybe_weapon.type->get_id().str(), val_gun );
-    double val_melee = melee_value( maybe_weapon );
-    add_msg_debug( debugmode::DF_NPC_ITEMAI,
-                   "%s %s valued at <color_light_cyan>%1.2f as a melee weapon to wield</color>.", disp_name( true ),
-                   maybe_weapon.type->get_id().str(), val_melee );
-    double val = std::max( val_gun, val_melee );
-    return val;
-}
-
 item *npc::evaluate_best_weapon() const
 {
-    bool can_use_gun = !is_player_ally() || rules.has_flag( ally_rule::use_guns );
-    bool use_silent = is_player_ally() && rules.has_flag( ally_rule::use_silent );
-
     item_location weapon = get_wielded_item();
     item &weap = weapon ? *weapon : null_item_reference();
 
     // Check if there's something better to wield
     item *best = &weap;
-    double best_value = evaluate_weapon( weap, can_use_gun, use_silent );
+    double best_value = evaluate_weapon( weap );
 
     // To prevent changing to barely better stuff
     best_value *= std::max<float>( 1.0f, ai_cache.danger_assessment / 10.0f );
 
     // Fists aren't checked below
-    double fist_value = evaluate_weapon( null_item_reference(), can_use_gun, use_silent );
+    double fist_value = evaluate_weapon( null_item_reference() );
 
     if( fist_value > best_value ) {
         best = &null_item_reference();
         best_value = fist_value;
     }
 
     //Now check through the NPC's inventory for melee weapons, guns, or holstered items
-    visit_items( [this, can_use_gun, use_silent, &weap, &best_value, &best]( item * node, item * ) {
+    visit_items( [this, &weap, &best_value, &best]( item * node, item * ) {
         if( can_wield( *node ).success() ) {
             double weapon_value = 0.0;
             bool using_same_type_bionic_weapon = is_using_bionic_weapon()
                                                  && node != &weap
                                                  && node->type->get_id() == weap.type->get_id();
 
             if( node->is_melee() || node->is_gun() ) {
-                weapon_value = evaluate_weapon( *node, can_use_gun, use_silent );
+                weapon_value = evaluate_weapon( *node );
                 if( weapon_value > best_value && !using_same_type_bionic_weapon ) {
                     best = const_cast<item *>( node );
                     best_value = weapon_value;
@@ -4207,10 +4181,6 @@ item *npc::evaluate_best_weapon() const
 
 bool npc::wield_better_weapon()
 {
-    // These are also assigned here so npc::evaluate_best_weapon() can be called by itself
-    bool can_use_gun = !is_player_ally() || rules.has_flag( ally_rule::use_guns );
-    bool use_silent = is_player_ally() && rules.has_flag( ally_rule::use_silent );
-
     item_location weapon = get_wielded_item();
     item &weap = weapon ? *weapon : null_item_reference();
     item *best = &weap;
@@ -4220,12 +4190,12 @@ bool npc::wield_better_weapon()
     if( best == better_weapon ) {
         add_msg_debug( debugmode::DF_NPC, "Wielded %s is best at %.1f, not switching",
                        best->type->get_id().str(),
-                       evaluate_weapon( *better_weapon, can_use_gun, use_silent ) );
+                       evaluate_weapon( *better_weapon ) );
         return false;
     }
 
     add_msg_debug( debugmode::DF_NPC, "Wielding %s at value %.1f", better_weapon->type->get_id().str(),
-                   evaluate_weapon( *better_weapon, can_use_gun, use_silent ) );
+                   evaluate_weapon( *better_weapon ) );
 
     // Always returns true, but future proof
     bool wield_success = wield( *better_weapon );

src/player_difficulty.cpp

@@ -30,6 +30,9 @@ static const damage_type_id damage_bash( "bash" );
 static const itype_id itype_bat( "bat" );
 static const itype_id itype_knife_combat( "knife_combat" );
 static const itype_id itype_machete( "machete" );
+static const itype_id itype_mossberg_500( "mossberg_500" );
+static const itype_id itype_ruger_1022( "ruger_1022" );
+static const itype_id itype_swbodyguard( "swbodyguard" );
 
 static const profession_id profession_unemployed( "unemployed" );
 
@@ -205,14 +208,23 @@ double player_difficulty::calc_dps_value( const Character &u )
     item early_cutting = item( itype_machete );
     item early_bashing = item( itype_bat );
 
-    double baseline = std::max( u.weapon_value( early_piercing ),
-                                u.weapon_value( early_cutting ) );
-    baseline = std::max( baseline, u.weapon_value( early_bashing ) );
+    // and some firearms, based on availability.
+    item most_common_pistol = item( itype_swbodyguard );
+    item most_common_rifle = item( itype_ruger_1022 );
+    item most_common_shotgun = item( itype_mossberg_500 );
+
+    std::vector<item> weapons_to_test = {early_piercing, early_cutting, early_bashing, most_common_pistol, most_common_rifle, most_common_shotgun};
+    double baseline = 0.0;
+
+    for( item &weapon : weapons_to_test ) {
+        const double new_weapon_value = u.evaluate_weapon( weapon, true );
+        baseline = std::max( baseline, new_weapon_value );
+    }
 
     // check any other items the character has on them
     if( u.prof ) {
         for( const item &i : u.prof->items( true, std::vector<trait_id>() ) ) {
-            baseline = std::max( baseline, u.weapon_value( i ) );
+            baseline = std::max( baseline, u.evaluate_weapon( i, true ) );
         }
     }
 

src/talker_npc.cpp

@@ -433,15 +433,13 @@ std::string talker_npc::give_item_to( const bool to_use )
     bool taken = false;
     std::string reason = me_npc->chat_snippets().snip_give_nope.translated();
     const item_location weapon = me_npc->get_wielded_item();
-    int our_ammo = me_npc->ammo_count_for( weapon );
-    int new_ammo = me_npc->ammo_count_for( loc );
-    const double new_weapon_value = me_npc->weapon_value( given, new_ammo );
+    const double new_weapon_value = me_npc->evaluate_weapon( given );
     const item &weap = weapon ? *weapon : null_item_reference();
-    const double cur_weapon_value = me_npc->weapon_value( weap, our_ammo );
-    add_msg_debug( debugmode::DF_TALKER, "NPC evaluates own %s (%d ammo): %0.1f",
-                   weap.typeId().str(), our_ammo, cur_weapon_value );
-    add_msg_debug( debugmode::DF_TALKER, "NPC evaluates your %s (%d ammo): %0.1f",
-                   given.typeId().str(), new_ammo, new_weapon_value );
+    const double cur_weapon_value = me_npc->evaluate_weapon( weap );
+    add_msg_debug( debugmode::DF_TALKER, "NPC evaluates own %s: %0.1f",
+                   weap.typeId().str(), cur_weapon_value );
+    add_msg_debug( debugmode::DF_TALKER, "NPC evaluates your %s: %0.1f",
+                   given.typeId().str(), new_weapon_value );
     if( to_use ) {
         // Eating first, to avoid evaluating bread as a weapon
         const consumption_result consume_res = try_consume( *me_npc, given, reason );