CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.71k stars 4.2k forks source link

Gun's displayed weight doesn't account for magazine/ammo weight #45490

Closed Salty-Panda closed 4 years ago

Salty-Panda commented 4 years ago

Describe the bug

Gun with and without magazine (or ammo in case of some shotguns) display the same weight

Steps To Reproduce

Debug gun, magazine, ammo, load and unload to your heart's content while inspecting the weapon

Expected behavior

The gun to display increased weight that includes all its nested pockets

Versions and configuration

J-Cieplinski commented 4 years ago

@Mrzjadacz what do you mean by "some shotguns"? Do shotguns that are not using magazine display the correct weight with ammo loaded?

Is magazine considered a gunmod in the code or is it a separate entity?

J-Cieplinski commented 4 years ago

Ok I see now that it's a separate entity. I just need confirmation if weapons that are not using magazine also displaying incorrect weight

Salty-Panda commented 4 years ago

Yes, problem was the same for shotguns using ammo directly

J-Cieplinski commented 4 years ago

I'm not completely sure but the issue possibly stems from

bool item::is_magazine() const
{
    return !!type->magazine || contents.has_pocket_type( item_pocket::pocket_type::MAGAZINE );
} 
bool item::magazine_integral() const
{
    return contents.has_pocket_type( item_pocket::pocket_type::MAGAZINE );
}
bool item_contents::has_pocket_type( const item_pocket::pocket_type pk_type ) const
{
    for( const item_pocket &pocket : contents ) {
        if( pocket.is_type( pk_type ) ) {
            return true;
        }
    }
    return false;
}
if( magazine_integral() && !is_magazine() )

I'm not sure what is the function of !! in is_magazine, but let's just say it's double negation which would yield a positive(I'm probably wrong, dunno why it's done that way). So the if( magazine_integral() && !is_magazine() ) from item.cpp:5005 which I believe is responsible for adding ammo weight to gun total weight, depends on both magazine_integral being true and it not being a magazine. However the item is a magazine only if it's type is a magazine OR it has pocket type of MAGAZINE. And magazine_integral is true only if item has a pocket type of MAGAZINE.

So the

if( magazine_integral() && !is_magazine() )

is true only if an item has a pocket type of MAGAZINE and item does not have a pocket type of MAGAZINE which is never true.

I would like for someone with more knowledge about the code base to chime in, and maybe someone who knowys why !! is there :)

J-Cieplinski commented 4 years ago
diff --git a/src/item.cpp b/src/item.cpp
index 95d16a6ff1..0ff6d6e0c0 100644
--- a/src/item.cpp
+++ b/src/item.cpp
@@ -5002,7 +5002,7 @@ units::mass item::weight( bool, bool integral ) const
             ret *= 0.85;
         }

-    } else if( magazine_integral() && !is_magazine() ) {
+    } else if( magazine_integral() && (!is_magazine() || is_gun()) ) {
         if( ammo_current() == itype_plut_cell ) {
             ret += ammo_remaining() * find_type( ammotype(
                     *ammo_types().begin() )->default_ammotype() )->weight / PLUTONIUM_CHARGES;

Seem to fix it for weapon not using a magazine. I will try to figure out the fix for magazine weapons later in the day