Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disorder sledgehammer reload_eject flag not working correctly #57683

Closed
Nerezza opened this issue May 14, 2022 · 1 comment · Fixed by #59323
Closed

Disorder sledgehammer reload_eject flag not working correctly #57683

Nerezza opened this issue May 14, 2022 · 1 comment · Fixed by #59323

Comments

@Nerezza
Copy link
Contributor

Nerezza commented May 14, 2022

Describe the bug

#55820
This PR implemented the code for ammo-consuming melee weapons to work, right now it's only used by the Disorder which consumes .22 blanks (sold at hardware stores irl, believe it or not) to drive a piston for extra impact on good hits.

When the piston fires the ammo is consumed correctly, and right after that in the code block it checks for the reload_eject flag and attempts to place a .22 casing in the sledgehammer that will be removed when the player reloads it.

Something isn't working right when the .22 casing is placed into the magazine pocket, because it's not actually going into the magazine pocket as expected. If you reload it after it fires off, you don't get a .22 casing back like you should. Instead the casing will drop to the ground next time you pick something up.

Honestly this makes me scratch my head and leaves me confused. I think something is different about how guns handle their magazines and reloading them, and how everything else handles their magazines and reloading them.

Steps to reproduce

Get 2 .22 blanks and 1 the Disorder.

  1. Load the Disorder and smack zeds until it fires off
  2. Reload the Disorder with a new .22 blank, check your inventory and surroundings for a .22 casing.
  3. Find something to pick up (might only work if you use the advanced inventory menu) and the missing .22 casing should appear on the ground nearby.

Expected behavior

The implementation should make the spent casing eject into your inventory when you reload the Disorder like if you'd reloaded a revolver with spent casing.

Screenshots

No response

Versions and configuration

  • OS: Windows
    • OS Version: 10.0.19042.1415 (20H2)
  • Game Version: 7a89bbe [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],
    SpeedyDex [speedydex],
    Stats Through Skills [StatsThroughSkills],
    Bionic Slots [cbm_slots]
    ]

Additional context

No response

@catdach
Copy link
Contributor

catdach commented May 14, 2022

  1. Find something to pick up (might only work if you use the advanced inventory menu) and the missing .22 casing should appear on the ground nearby

In my testing this actually happens with all the "reload to eject" revolvers too (if you don't get the empty casings out of them before picking something up), so it's not exclusive to the Pulverizer/Disorder.

22 casing is placed into the magazine pocket, because it's not actually going into the magazine pocket as expected.

Actually a casing is placed into the magazine pocket, it can be seen in the "Contents of this item:" bit. It just isn't ejecting them when you reload. You can even retrieve them by reloading and immediately unloading the Pulverizer/Disorder.

I think I might know what's happening actually... edit: I was wrong.

edit2: ok, ok, I think I finally found the source of the problem
here's what handles casings in item.cpp :

void item::casings_handle( const std::function<bool( item & )> &func )
{
    if( !is_gun() ) {
        return;
    }
    contents.casings_handle( func );
}

due to that "if( !is_gun() ){ return; }" it just flat out doesn't even bother doing anything unless the item is a type of gun.(probably to help performance I guess?) sigh well I'm exhausted now, I'll try to fix it in a few days if no one else does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants