Mech weapons disappear on load

[Maleclypse]

Describe the bug

When you load a game after getting into a mech and saving you will find that it's built in weapon system is gone.

Steps to reproduce

1. Make a character
2. Spawn the two mechs with weapons
3. Jump into one
4. See it's weapon system
5. save
6. quit and reload
7. Receive following error message ` DEBUG : Couldn't find the bionic UID for the current bionic weapon. You will need to reactivate it.

FUNCTION : void Character::load(const JsonObject&) FILE : src/savegame_json.cpp LINE : 1159 VERSION : 25fd53c ` 8. See empty space where mech weapon once was. 9. repeat

Expected behavior

The guns don't disappear.

Screenshots

Scout Marksmmen

Versions and configuration

• OS: Windows
  • OS Version: 10.0.19043.2006 (21H1)
• Game Version: 25fd53c [64-bit]
• Graphics Version: Tiles
• Game Language: System language []
• Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], No Fungal Growth [no_fungal_growth], Bionic Professions [package_bionic_professions] ]

Additional context

Screenshots come from a friend who experienced the bug, and I've confirmed it.

[akrieger]

This feels like it could be my fault. Is there any trickery to spawning a mech / what debug spawn menu do I need to go find it?

[Maleclypse]

Spawn monster, search for mech, only the grunt and specter have weapons I believe.

[akrieger]

Dug into it enough to verify it happens on builds before #50143 so it's at least not my fault the way I was thinking.

[Maleclypse]

Well that’s good to know at least.

[akrieger]

Not trying to be like 'not my problem byeee' but I don't have time to take on fixes other than things I directly broke from that PR and want to indicate to people that they should feel free to investigate this without worry.

[EIIKaO]

I enjoy investigating and fixing small bugs, so I investigated the actual behavior of the program in past versions and the history of changes related of this issue.

The behavior of equipping MECH weapons when riding MECH worked since https://github.com/CleverRaven/Cataclysm-DDA/commit/a5b1baa77ad16df125aefbecc87001d40dad8a0f, and in there, the bug already happened( https://github.com/CleverRaven/Cataclysm-DDA/releases/tag/cdda-experimental-2022-05-02-0624 ).

Therefore, this may be due to a failure to identify Mech and bionic weapons in src/savegame_json.cpp.

    bool has_old_bionic_weapon = !is_using_bionic_weapon() && weapon &&
                                 weapon->has_flag( flag_NO_UNWIELD ) && !weapon->ethereal;

Here are my shortsighted solutions. a. Create and add json flag ( like a "MECHANIC_WEAPON" ) to the item, insert code of checking on loading that flag has or not into has_old_bionic_weapon's expression

b. Assign variables instead of json flag to MECH weapons ( such mechwep.set_var( "MECHANIC_WEAPON", "true" ); ) when riding( void Character::mount_creature( monster &z ) ) and check them during loading.

These are easy to implement, but in my opinion, it would be preferable for someone with a long-term development vision regarding the mech series to come up with a more planned solution.

[irwiss]

I don't think you need any new flags/variables, that code happens after mounted creature and it's type are known, so you could explicitly check for monster gun

diff --git a/src/savegame_json.cpp b/src/savegame_json.cpp
index 79aedf2e14b376aa7888055de56aa105ba2351c8..cfb88a90832f1e9ee637c0ebb34a0272840c4fe4 100644
--- a/src/savegame_json.cpp
+++ b/src/savegame_json.cpp
@@ -1121,7 +1121,10 @@ void Character::load( const JsonObject &data )
     }
 
     item_location weapon = get_wielded_item();
-    bool has_old_bionic_weapon = !is_using_bionic_weapon() && weapon &&
+    const bool wielding_mech_weapon = weapon && mounted_creature &&
+                                      mounted_creature->type->mech_weapon == weapon->typeId();
+    bool has_old_bionic_weapon = !is_using_bionic_weapon() && !wielding_mech_weapon && weapon &&
                                  weapon->has_flag( flag_NO_UNWIELD ) && !weapon->ethereal;
 
     const auto find_parent = [this]( bionic_id & bio_id ) {

The above appears to "fix" this specific bug

Although I think the bionic migration code assuming anything wielded that has NO_UNWIELD and not ethereal is a legacy bionic isn't correct - spawn police bot, let yourself get handcuffed, do a save/load cycle and you'll see the same debugmsg and handcuffs will disappear

quick check says bionic guns/weapons should have their flags: BIONIC_WEAPON/BIONIC_GUN so checking for those would likely work:

diff --git a/src/savegame_json.cpp b/src/savegame_json.cpp
index 79aedf2e14b376aa7888055de56aa105ba2351c8..2576c3a9c33f29f2eb4a53367ac59b5cf49b2b14 100644
--- a/src/savegame_json.cpp
+++ b/src/savegame_json.cpp
@@ -1122,6 +1122,7 @@ void Character::load( const JsonObject &data )
 
     item_location weapon = get_wielded_item();
     bool has_old_bionic_weapon = !is_using_bionic_weapon() && weapon &&
+                                 ( weapon->has_flag( flag_BIONIC_WEAPON ) || weapon->has_flag( flag_BIONIC_GUN ) ) &&
                                  weapon->has_flag( flag_NO_UNWIELD ) && !weapon->ethereal;
 
     const auto find_parent = [this]( bionic_id & bio_id ) {

Didn't check all bionics though

[EIIKaO]

This is just what I was hoping for, expert solution.

[Maleclypse]

Did this get fixed or is it waiting for someone to make use of the diff irwss posted? Trying to determine if it should be closed.