From 28028a8fd53f00e1b036134d77cc0dbaee78fab5 Mon Sep 17 00:00:00 2001 From: PatrikLundell Date: Wed, 6 Nov 2024 11:13:42 +0100 Subject: [PATCH 1/5] Don't try to shoot with empty gun --- src/npc_attack.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/npc_attack.cpp b/src/npc_attack.cpp index b14b5e06649c9..7b9f4d9c120db 100644 --- a/src/npc_attack.cpp +++ b/src/npc_attack.cpp @@ -6,6 +6,7 @@ #include "dialogue.h" #include "flag.h" #include "item.h" +#include "itype.h" #include "line.h" #include "magic.h" #include "magic_spell_effect_helpers.h" @@ -465,8 +466,9 @@ void npc_attack_gun::use( npc &source, const tripoint_bub_ms &location ) const bool npc_attack_gun::can_use( const npc &source ) const { - // can't attack with something you can't wield - return source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success(); + // can't attack with something you can't wield or which lacks ammo. + return ( source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success() + && gun.ammo_data() != nullptr && gun.ammo_data()->ammo != nullptr ); } int npc_attack_gun::base_time_penalty( const npc &source ) const From 0d1ce452fe5786e9f34cc8465177ef6a2f51ce8c Mon Sep 17 00:00:00 2001 From: PatrikLundell Date: Wed, 6 Nov 2024 12:15:36 +0100 Subject: [PATCH 2/5] Moved misplaced parenthesis --- src/npc_attack.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/npc_attack.cpp b/src/npc_attack.cpp index 7b9f4d9c120db..c0163a220204b 100644 --- a/src/npc_attack.cpp +++ b/src/npc_attack.cpp @@ -467,8 +467,8 @@ void npc_attack_gun::use( npc &source, const tripoint_bub_ms &location ) const bool npc_attack_gun::can_use( const npc &source ) const { // can't attack with something you can't wield or which lacks ammo. - return ( source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success() - && gun.ammo_data() != nullptr && gun.ammo_data()->ammo != nullptr ); + return ( source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success() ) + && gun.ammo_data() != nullptr && gun.ammo_data()->ammo != nullptr; } int npc_attack_gun::base_time_penalty( const npc &source ) const From a09750503128d82d8624878cbe5883b37929ce7d Mon Sep 17 00:00:00 2001 From: PatrikLundell Date: Sat, 9 Nov 2024 19:56:40 +0100 Subject: [PATCH 3/5] Cheaper operations for checking presence of ammo and ammo_data --- src/avatar_action.cpp | 2 +- src/game_inventory.cpp | 2 +- src/item.cpp | 66 +++++++++++++++++++++++++++++----- src/item.h | 6 ++++ src/npc_attack.cpp | 2 +- src/ranged.cpp | 10 +++--- tests/reload_magazine_test.cpp | 18 +++++----- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/src/avatar_action.cpp b/src/avatar_action.cpp index 451ad7909090f..976c6c692e99b 100644 --- a/src/avatar_action.cpp +++ b/src/avatar_action.cpp @@ -755,7 +755,7 @@ void avatar_action::fire_wielded_weapon( avatar &you ) return; } else if( !weapon->is_gun() ) { return; - } else if( weapon->ammo_data() && + } else if( weapon->has_ammo_data() && !weapon->ammo_types().count( weapon->loaded_ammo().ammo_type() ) ) { add_msg( m_info, _( "The %s can't be fired while loaded with incompatible ammunition %s" ), weapon->tname(), weapon->ammo_current()->nname( 1 ) ); diff --git a/src/game_inventory.cpp b/src/game_inventory.cpp index e03589e9f500a..2d0d4232a24bd 100644 --- a/src/game_inventory.cpp +++ b/src/game_inventory.cpp @@ -1688,7 +1688,7 @@ class weapon_inventory_preset: public inventory_selector_preset const int total_damage = loc->gun_damage( true ).total_damage(); - if( loc->ammo_data() && loc->ammo_remaining() ) { + if( loc->has_ammo_data() && loc->ammo_remaining() ) { const int basic_damage = loc->gun_damage( false ).total_damage(); const damage_instance &damage = loc->ammo_data()->ammo->damage; if( !damage.empty() ) { diff --git a/src/item.cpp b/src/item.cpp index 183bb6f880c9b..551cc7c002ab1 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -2962,7 +2962,7 @@ void item::ammo_info( std::vector &info, const iteminfo_query *parts, bool /* debug */ ) const { // Skip this section for guns and items without ammo data - if( is_gun() || !ammo_data() || + if( is_gun() || !has_ammo_data() || !parts->test( iteminfo_parts::AMMO_REMAINING_OR_TYPES ) ) { return; } @@ -3416,7 +3416,7 @@ void item::gun_info( const item *mod, std::vector &info, const iteminf } } - if( mod->ammo_data() && parts->test( iteminfo_parts::AMMO_REMAINING ) ) { + if( mod->has_ammo_data() && parts->test( iteminfo_parts::AMMO_REMAINING ) ) { info.emplace_back( "AMMO", _( "Ammunition: " ), string_format( "%s", mod->ammo_data()->nname( mod->ammo_remaining() ) ) ); } @@ -4838,7 +4838,7 @@ void item::tool_info( std::vector &info, const iteminfo_query *parts, _( "* This tool runs on bionic power." ) ); } else if( has_flag( flag_BURNOUT ) && parts->test( iteminfo_parts::TOOL_BURNOUT ) ) { int percent_left = 0; - if( ammo_data() != nullptr ) { + if( has_ammo_data() ) { // Candle items that use ammo instead of charges // The capacity should never be zero but clang complains about it percent_left = 100 * ammo_remaining() / std::max( ammo_capacity( ammo_data()->ammo->type ), 1 ); @@ -10590,7 +10590,7 @@ int item::gun_dispersion( bool with_ammo, bool with_scaling ) const int dispPerDamage = get_option< int >( "DISPERSION_PER_GUN_DAMAGE" ); dispersion_sum += damage_level() * dispPerDamage; dispersion_sum = std::max( dispersion_sum, 0 ); - if( with_ammo && ammo_data() ) { + if( with_ammo && has_ammo_data() ) { dispersion_sum += ammo_data()->ammo->dispersion_considering_length( barrel_length() ); } if( !with_scaling ) { @@ -10643,7 +10643,7 @@ damage_instance item::gun_damage( bool with_ammo, bool shot ) const ret.add( mod->type->gunmod->damage.di_considering_length( bl ) ); } - if( with_ammo && ammo_data() ) { + if( with_ammo && has_ammo_data() ) { if( shot ) { ret.add( ammo_data()->ammo->shot_damage.di_considering_length( bl ) ); } else { @@ -10728,7 +10728,7 @@ int item::gun_recoil( const Character &p, bool bipod, bool ideal_strength ) cons handling = std::pow( wt, 0.8 ) * std::pow( handling, 1.2 ); int qty = type->gun->recoil; - if( ammo_data() ) { + if( has_ammo_data() ) { qty += ammo_data()->ammo->recoil; } @@ -10763,7 +10763,7 @@ int item::gun_range( bool with_ammo ) const ret += mod->type->gunmod->range; range_multiplier *= mod->type->gunmod->range_multiplier; } - if( with_ammo && ammo_data() ) { + if( with_ammo && has_ammo_data() ) { ret += ammo_data()->ammo->range; range_multiplier *= ammo_data()->ammo->range_multiplier; } @@ -11179,6 +11179,56 @@ int item::activation_consume( int qty, const tripoint &pos, Character *carrier ) return ammo_consume( qty * ammo_required(), pos, carrier ); } +const bool item::has_ammo() const +{ + const item *mag = magazine_current(); + if( mag ) { + return mag->has_ammo(); + } + + if( is_ammo() ) { + return true; + } + + if( is_magazine() ) { + return !contents.empty(); + } + + auto mods = is_gun() ? gunmods() : toolmods(); + for( const item *e : mods ) { + if( !e->type->mod->ammo_modifier.empty() && e->has_ammo() ) { + return true; + } + } + + return false; +} + +const bool item::has_ammo_data() const +{ + const item *mag = magazine_current(); + if( mag ) { + return mag->has_ammo_data(); + } + + if( is_ammo() ) { + return true; + } + + if( is_magazine() ) { + return !contents.empty(); + } + + auto mods = is_gun() ? gunmods() : toolmods(); + for( const item *e : mods ) { + if( !e->type->mod->ammo_modifier.empty() && e->has_ammo_data() ) { + return true; + } + } + + return false; +} + const itype *item::ammo_data() const { const item *mag = magazine_current(); @@ -11350,7 +11400,7 @@ std::set item::ammo_effects( bool with_ammo ) const } std::set res = type->gun->ammo_effects; - if( with_ammo && ammo_data() ) { + if( with_ammo && has_ammo_data() ) { res.insert( ammo_data()->ammo->ammo_effects.begin(), ammo_data()->ammo->ammo_effects.end() ); } diff --git a/src/item.h b/src/item.h index 7102fa971685e..860d97e3240f6 100644 --- a/src/item.h +++ b/src/item.h @@ -2564,6 +2564,12 @@ class item : public visitable */ int activation_consume( int qty, const tripoint &pos, Character *carrier ); + // Returns whether the item has ammo in it, either directly or via a selected magazine, which + // contrasts with ammo_data(), which just returns the magazine data if a magazine is selected, + // regardless of whether that magazine is empty or not. + const bool has_ammo() const; + // Cheaper way to just check if ammo_data exists if the data is to be just discarded afterwards. + const bool has_ammo_data() const; /** Specific ammo data, returns nullptr if item is neither ammo nor loaded with any */ const itype *ammo_data() const; /** Specific ammo type, returns "null" if item is neither ammo nor loaded with any */ diff --git a/src/npc_attack.cpp b/src/npc_attack.cpp index c0163a220204b..95ae5c3cf0e56 100644 --- a/src/npc_attack.cpp +++ b/src/npc_attack.cpp @@ -468,7 +468,7 @@ bool npc_attack_gun::can_use( const npc &source ) const { // can't attack with something you can't wield or which lacks ammo. return ( source.is_wielding( *gunmode ) || source.can_wield( *gunmode ).success() ) - && gun.ammo_data() != nullptr && gun.ammo_data()->ammo != nullptr; + && gun.has_ammo(); } int npc_attack_gun::base_time_penalty( const npc &source ) const diff --git a/src/ranged.cpp b/src/ranged.cpp index d7c5fc276bc67..7f1b0af98b716 100644 --- a/src/ranged.cpp +++ b/src/ranged.cpp @@ -807,8 +807,8 @@ bool Character::handle_gun_damage( item &it ) it.inc_damage(); } if( !it.has_flag( flag_PRIMITIVE_RANGED_WEAPON ) ) { - if( it.ammo_data() != nullptr && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) || - ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) && + if( it.has_ammo_data() && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) || + ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) && it.faults_potential().count( fault_gun_chamber_spent ) ) { add_msg_player_or_npc( m_bad, _( "Your %s fails to cycle!" ), _( "'s %s fails to cycle!" ), @@ -2172,12 +2172,12 @@ static projectile make_gun_projectile( const item &gun ) auto &fx = proj.proj_effects; - if( ( gun.ammo_data() && gun.ammo_data()->phase == phase_id::LIQUID ) || + if( ( gun.has_ammo_data() && gun.ammo_data()->phase == phase_id::LIQUID ) || fx.count( ammo_effect_SHOT ) || fx.count( ammo_effect_BOUNCE ) ) { fx.insert( ammo_effect_WIDE ); } - if( gun.ammo_data() ) { + if( gun.has_ammo_data() ) { const auto &ammo = gun.ammo_data()->ammo; // Some projectiles have a chance of being recoverable @@ -2312,7 +2312,7 @@ item::sound_data item::gun_noise( const bool burst ) const } int noise = type->gun->loudness; - if( ammo_data() ) { + if( has_ammo_data() ) { noise += ammo_data()->ammo->loudness; } for( const item *mod : gunmods() ) { diff --git a/tests/reload_magazine_test.cpp b/tests/reload_magazine_test.cpp index d1cb4249e9ca3..19b22b5f078ed 100644 --- a/tests/reload_magazine_test.cpp +++ b/tests/reload_magazine_test.cpp @@ -72,7 +72,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re CHECK( mag->ammo_types().count( gun_ammo ) ); CHECK( mag->ammo_capacity( gun_ammo ) == mag_cap ); CHECK( mag->ammo_current().is_null() ); - CHECK( mag->ammo_data() == nullptr ); + CHECK( !mag->has_ammo_data() ); GIVEN( "An empty magazine" ) { CHECK( mag->ammo_remaining() == 0 ); @@ -96,7 +96,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re AND_THEN( "the current ammo is updated" ) { REQUIRE( mag->ammo_current() == ammo_id ); - REQUIRE( mag->ammo_data() ); + REQUIRE( mag->has_ammo_data() ); } AND_THEN( "the magazine is filled to capacity" ) { REQUIRE( mag->remaining_ammo_capacity() == 0 ); @@ -126,7 +126,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re AND_THEN( "the current ammo is updated" ) { REQUIRE( mag->ammo_current() == ammo_id ); - REQUIRE( mag->ammo_data() ); + REQUIRE( mag->has_ammo_data() ); } AND_THEN( "the magazine is filled with the correct quantity" ) { REQUIRE( mag->ammo_remaining() == mag_cap - 2 ); @@ -206,7 +206,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re CHECK( gun->ammo_capacity( gun_ammo ) == 0 ); CHECK( gun->ammo_remaining() == 0 ); CHECK( gun->ammo_current().is_null() ); - CHECK( gun->ammo_data() == nullptr ); + CHECK( !gun->has_ammo_data() ); WHEN( "the gun is reloaded with an incompatible magazine" ) { item_location mag = player_character.i_add( item( bad_mag ) ); @@ -237,7 +237,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re AND_THEN( "the gun contains no ammo" ) { REQUIRE( gun->ammo_current().is_null() ); REQUIRE( gun->ammo_remaining() == 0 ); - REQUIRE( gun->ammo_data() == nullptr ); + REQUIRE( !gun->has_ammo_data() ); } } } @@ -264,7 +264,7 @@ TEST_CASE( "reload_magazine", "[magazine] [visitable] [item] [item_location] [re AND_THEN( "the gun contains the correct amount and type of ammo" ) { REQUIRE( gun->ammo_remaining() == mag_cap - 2 ); REQUIRE( gun->ammo_current() == ammo_id ); - REQUIRE( gun->ammo_data() ); + REQUIRE( gun->has_ammo_data() ); } AND_WHEN( "the guns magazine is further reloaded with compatible but different ammo" ) { @@ -359,7 +359,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" ) CHECK( gun->ammo_capacity( gun_ammo ) == mag_cap ); CHECK( gun->ammo_remaining() == 0 ); CHECK( gun->ammo_current().is_null() ); - CHECK( gun->ammo_data() == nullptr ); + CHECK( !gun->has_ammo_data() ); WHEN( "the gun is reloaded with incompatible ammo" ) { item_location ammo = player_character.i_add( item( bad_ammo ) ); @@ -380,7 +380,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" ) AND_THEN( "the current ammo is updated" ) { REQUIRE( gun->ammo_current() == ammo_id ); - REQUIRE( gun->ammo_data() ); + REQUIRE( gun->has_ammo_data() ); } AND_THEN( "the gun is filled to capacity" ) { REQUIRE( gun->remaining_ammo_capacity() == 0 ); @@ -410,7 +410,7 @@ TEST_CASE( "reload_revolver", "[visitable] [item] [item_location] [reload]" ) AND_THEN( "the current ammo is updated" ) { REQUIRE( gun->ammo_current() == ammo_id ); - REQUIRE( gun->ammo_data() ); + REQUIRE( gun->has_ammo_data() ); } AND_THEN( "the gun is filled with the correct quantity" ) { REQUIRE( gun->ammo_remaining() == mag_cap - 2 ); From 9588f663e51ec63cefbfdf2fe633a5eb28b353c3 Mon Sep 17 00:00:00 2001 From: PatrikLundell Date: Sat, 9 Nov 2024 20:08:04 +0100 Subject: [PATCH 4/5] Less const --- src/item.cpp | 4 ++-- src/item.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/item.cpp b/src/item.cpp index 551cc7c002ab1..0f1d0aa74ab86 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -11179,7 +11179,7 @@ int item::activation_consume( int qty, const tripoint &pos, Character *carrier ) return ammo_consume( qty * ammo_required(), pos, carrier ); } -const bool item::has_ammo() const +bool item::has_ammo() const { const item *mag = magazine_current(); if( mag ) { @@ -11204,7 +11204,7 @@ const bool item::has_ammo() const return false; } -const bool item::has_ammo_data() const +bool item::has_ammo_data() const { const item *mag = magazine_current(); if( mag ) { diff --git a/src/item.h b/src/item.h index 860d97e3240f6..79f63ee4675ff 100644 --- a/src/item.h +++ b/src/item.h @@ -2567,9 +2567,9 @@ class item : public visitable // Returns whether the item has ammo in it, either directly or via a selected magazine, which // contrasts with ammo_data(), which just returns the magazine data if a magazine is selected, // regardless of whether that magazine is empty or not. - const bool has_ammo() const; + bool has_ammo() const; // Cheaper way to just check if ammo_data exists if the data is to be just discarded afterwards. - const bool has_ammo_data() const; + bool has_ammo_data() const; /** Specific ammo data, returns nullptr if item is neither ammo nor loaded with any */ const itype *ammo_data() const; /** Specific ammo type, returns "null" if item is neither ammo nor loaded with any */ From 24cff6871427c08bcf1b04099e7966c3b74a653b Mon Sep 17 00:00:00 2001 From: PatrikLundell Date: Sun, 10 Nov 2024 11:11:17 +0100 Subject: [PATCH 5/5] Adjusted has_ammo_data() => has_ammo() where subsequent use requires ammo pointer to be valid --- src/game_inventory.cpp | 2 +- src/item.cpp | 10 +++++----- src/ranged.cpp | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/game_inventory.cpp b/src/game_inventory.cpp index 2d0d4232a24bd..7d2b8ada9fa92 100644 --- a/src/game_inventory.cpp +++ b/src/game_inventory.cpp @@ -1688,7 +1688,7 @@ class weapon_inventory_preset: public inventory_selector_preset const int total_damage = loc->gun_damage( true ).total_damage(); - if( loc->has_ammo_data() && loc->ammo_remaining() ) { + if( loc->has_ammo() ) { const int basic_damage = loc->gun_damage( false ).total_damage(); const damage_instance &damage = loc->ammo_data()->ammo->damage; if( !damage.empty() ) { diff --git a/src/item.cpp b/src/item.cpp index 0f1d0aa74ab86..95908c2c5e2b5 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -10590,7 +10590,7 @@ int item::gun_dispersion( bool with_ammo, bool with_scaling ) const int dispPerDamage = get_option< int >( "DISPERSION_PER_GUN_DAMAGE" ); dispersion_sum += damage_level() * dispPerDamage; dispersion_sum = std::max( dispersion_sum, 0 ); - if( with_ammo && has_ammo_data() ) { + if( with_ammo && has_ammo() ) { dispersion_sum += ammo_data()->ammo->dispersion_considering_length( barrel_length() ); } if( !with_scaling ) { @@ -10643,7 +10643,7 @@ damage_instance item::gun_damage( bool with_ammo, bool shot ) const ret.add( mod->type->gunmod->damage.di_considering_length( bl ) ); } - if( with_ammo && has_ammo_data() ) { + if( with_ammo && has_ammo() ) { if( shot ) { ret.add( ammo_data()->ammo->shot_damage.di_considering_length( bl ) ); } else { @@ -10728,7 +10728,7 @@ int item::gun_recoil( const Character &p, bool bipod, bool ideal_strength ) cons handling = std::pow( wt, 0.8 ) * std::pow( handling, 1.2 ); int qty = type->gun->recoil; - if( has_ammo_data() ) { + if( has_ammo() ) { qty += ammo_data()->ammo->recoil; } @@ -10763,7 +10763,7 @@ int item::gun_range( bool with_ammo ) const ret += mod->type->gunmod->range; range_multiplier *= mod->type->gunmod->range_multiplier; } - if( with_ammo && has_ammo_data() ) { + if( with_ammo && has_ammo() ) { ret += ammo_data()->ammo->range; range_multiplier *= ammo_data()->ammo->range_multiplier; } @@ -11400,7 +11400,7 @@ std::set item::ammo_effects( bool with_ammo ) const } std::set res = type->gun->ammo_effects; - if( with_ammo && has_ammo_data() ) { + if( with_ammo && has_ammo() ) { res.insert( ammo_data()->ammo->ammo_effects.begin(), ammo_data()->ammo->ammo_effects.end() ); } diff --git a/src/ranged.cpp b/src/ranged.cpp index 7f1b0af98b716..b94cffddc65d7 100644 --- a/src/ranged.cpp +++ b/src/ranged.cpp @@ -807,8 +807,8 @@ bool Character::handle_gun_damage( item &it ) it.inc_damage(); } if( !it.has_flag( flag_PRIMITIVE_RANGED_WEAPON ) ) { - if( it.has_ammo_data() && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) || - ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) && + if( it.has_ammo() && ( ( it.ammo_data()->ammo->recoil < it.min_cycle_recoil() ) || + ( it.has_fault_flag( "BAD_CYCLING" ) && one_in( 16 ) ) ) && it.faults_potential().count( fault_gun_chamber_spent ) ) { add_msg_player_or_npc( m_bad, _( "Your %s fails to cycle!" ), _( "'s %s fails to cycle!" ), @@ -2177,7 +2177,7 @@ static projectile make_gun_projectile( const item &gun ) fx.insert( ammo_effect_WIDE ); } - if( gun.has_ammo_data() ) { + if( gun.has_ammo() ) { const auto &ammo = gun.ammo_data()->ammo; // Some projectiles have a chance of being recoverable @@ -2312,7 +2312,7 @@ item::sound_data item::gun_noise( const bool burst ) const } int noise = type->gun->loudness; - if( has_ammo_data() ) { + if( has_ammo() ) { noise += ammo_data()->ammo->loudness; } for( const item *mod : gunmods() ) {