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

Fix ammo disassembly and allow for disassembly of multiple rounds #37709

Merged

Conversation

SeventhSandwich
Copy link
Contributor

SUMMARY: Bugfixes "Fix ammo disassembly and allow for disassembly of multiple rounds"

Purpose of change

Fixes #37345
The current build of the game has a two-pronged issue with disassembling ammo. Firstly, you can only disassemble in multiples of whatever the 'count' of the ammo's JSON entry is. This varies from round to round. Secondly, disassembling that amount of ammo (for instance, 50 rounds of 9mm FMJ) only returns the materials needed for one round. This is not intended behavior because the uncraft recipe for each ammo type is designed to return one round's worth of materials.

Describe the solution

This PR adds a popup window when the player chooses to disassemble ammo. It prompts the player to enter an amount of ammo to disassemble, and then starts a disassembly activity with a length of time scaled to the amount of ammo being deconstructed. After it finishes, it drops the correct number of materials.

To keep track of the number of rounds the player wants to disassemble, I store the selected quantity of rounds in the 'position' attribute of the player_activity class (which is unused during item disassembly). This is a little bit hacky, but I didn't want to add an entirely new attribute just for a single purpose like ammo disassembly. If devs would prefer that instead, I'm open to revisions.

Describe alternatives you've considered

I thought of doing something like #37494 but ruled it out because of the difficulty of upkeep. It is also nice to have the ability to disassemble fewer than 50 rounds of ammunition.

I also thought of implementing a new JSON flag to disassemble things by charge count, but I couldn't think of any use cases besides ammunition.

Testing

I built CDDA in Visual Studio 2019 and tested various types of ammo (default-kind and crafted) and tried different quantities. Everything seems to work perfectly.

Additional context

None.

fixes ammo disassembly and allow players to input number of rounds
@Brian-Otten
Copy link
Contributor

I'm a big fan of the small extra prompt to decide how much to disassemble, so the player is fully aware of what's happening.

bugfix: added check to disassembly time to prevent instant disassembly of non-ammo items
@I-am-Erk I-am-Erk added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Feb 4, 2020
src/crafting.cpp Outdated Show resolved Hide resolved
src/crafting.cpp Outdated Show resolved Hide resolved
incorporate BevapDin's suggested revisions
src/crafting.cpp Outdated Show resolved Hide resolved
src/crafting.cpp Outdated Show resolved Hide resolved
src/crafting.cpp Outdated Show resolved Hide resolved
src/crafting.cpp Outdated Show resolved Hide resolved
@ZhilkinSerg
Copy link
Contributor

You need to style your changes - see #24006 for instruction for VS.

SeventhSandwich and others added 3 commits February 5, 2020 19:57
Co-Authored-By: ZhilkinSerg <ZhilkinSerg@users.noreply.github.com>
Co-Authored-By: ZhilkinSerg <ZhilkinSerg@users.noreply.github.com>
Co-Authored-By: ZhilkinSerg <ZhilkinSerg@users.noreply.github.com>
@SeventhSandwich
Copy link
Contributor Author

I did run it through Astyle - I'm not sure why it didn't catch the spacing on that if statement

Co-Authored-By: ZhilkinSerg <ZhilkinSerg@users.noreply.github.com>
@Rivet-the-Zombie Rivet-the-Zombie merged commit 977bca1 into CleverRaven:master Feb 6, 2020
@ZhilkinSerg
Copy link
Contributor

Nope, it is still not-astyled properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disassembling 30 bullets gives the components of a single round
6 participants