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 stacks of ammo giving one round of components #37494

Closed
wants to merge 47 commits into from
Closed

Fix stacks of ammo giving one round of components #37494

wants to merge 47 commits into from

Conversation

Rogal-Dorn
Copy link
Contributor

@Rogal-Dorn Rogal-Dorn commented Jan 29, 2020

Summary

SUMMARY: Bugfixes "Multiply components received from disassembling ammunition by its stack size so that 29/30 cases don't disappear"

Purpose of change

Fixes #37345
A change to ammunition disassembly required a stack to disassemble any ammunition but did not increase the resulting resources gained.

Describe the solution

This PR is a series of .json edits to make the results what was expected. For example

[
  {
    "result": "10mm_fmj",
    "type": "uncraft",
    "time": "5 s",
    "qualities": [ { "id": "PULL", "level": 1 } ],
    "components": [ [ [ "lead", 2 ] ], [ [ "10mm_casing", 1 ] ], [ [ "smpistol_primer", 1 ] ], [ [ "gunpowder_magnum_pistol", 5 ] ] ],
    "flags": [ "UNCRAFT_SINGLE_CHARGE" ]
  }
]

was changed to

[
  {
    "result": "10mm_fmj",
    "type": "uncraft",
    "time": "5 s",
    "qualities": [ { "id": "PULL", "level": 1 } ],
    "components": [ [ [ "lead", 80 ] ], [ [ "10mm_casing", 40 ] ], [ [ "smpistol_primer", 40 ] ], [ [ "gunpowder_magnum_pistol", 200 ] ] ],
    "flags": [ "UNCRAFT_SINGLE_CHARGE" ]
  }
]

as the stack_size value is 40.

Describe alternatives you've considered

Multiplying the results by the stack size, although this would require c++ changes and I don't understand the systems involved.

Testing

Spawned several stacks of different ammunition and a kinetic bullet puller and disassembled them, resulting in the correct amount of components.

Additional context

changed to reflect stacksize of 40
changed for stack size of 150 for CB and 100 for all other
changed for stack size of 47 (why 47?)
changed for stack size of 20
changed for stack size of 20
changed for stack size of 15
changed for stack size of 47
changed for stack size of 20
changed for stack size of 80
changed for stack size of 50
changed for stack size of 50
changed for stack size of 40
changed for stack size of 50 (notice no casing or primer is given, uncertain as to why)
changed for stack size of 40
changed for stack size of 40
changed for stack size of 20
changed for stack size of 20
changed for stack size of 30
Changed for stack size of 15
changed for stack size of 20
changed for stack size of 40
changed for stack size of 20
changed for stack size of 80
changed for stack size of 10
changed for stack size of 10
changed for stack size of 30
changed for stack size of 30
changed for stack size of 60
changed for stack size of 45
changed for stack size of 5
changed for stack size of 40
changed for stack size of 20
changed for stack size of 20
changed for stack size of 20
changed for stack size of 50
changed for stack size of 50
changed for stack size of 20
Edited again for 40 stack size
edited again for stack size of 50
edited again for stack size of 50
Copy link
Contributor

@Soup-de-Loop Soup-de-Loop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack commented wrong. Please ignore this.

@@ -6,7 +6,7 @@
"difficulty": 5,
"time": "5 s",
"qualities": [ { "id": "PULL", "level": 1 } ],
"components": [ [ [ "lead", 2 ] ], [ [ "38super_casing", 1 ] ], [ [ "smpistol_primer", 1 ] ], [ [ "gunpowder_magnum_pistol", 3 ] ] ],
"components": [ [ [ "lead", 80 ] ], [ [ "38super_casing", 40 ] ], [ [ "smpistol_primer", 40 ] ], [ [ "gunpowder_magnum_pistol", 120 ] ] ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it's just expecting you to put it through the linter because the line has become too long with the numbers increasing. It's expecting

[
  {
    "result": "38super_fmj",
    "type": "uncraft",
    "skill_used": "gun",
    "difficulty": 5,
    "time": "5 s",
    "qualities": [ { "id": "PULL", "level": 1 } ],
    "components": [
      [ [ "lead", 80 ] ],
      [ [ "38super_casing", 40 ] ],
      [ [ "smpistol_primer", 40 ] ],
      [ [ "gunpowder_magnum_pistol", 120 ] ]
    ],
    "flags": [ "UNCRAFT_SINGLE_CHARGE" ]
  }
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for letting me know! I used your correction there and I'll make sure to follow any further errors when I wake up.

edited to fit lint requirements
Edited to fit lint requirements
Fixed brackets
@ifreund ifreund added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Jan 29, 2020
@Rogal-Dorn
Copy link
Contributor Author

I've found out .454 Casull behaves as if it has a stack_size value of 20, despite the 454.json file under items/ammo showing it's 15.

I don't believe this is directly related to this PR, and changing the uncraft 454.json to reflect a stack_size of 20 seems like a bad move since the two files would be based off different numbers.

@Rogal-Dorn
Copy link
Contributor Author

I've found out .454 Casull behaves as if it has a stack_size value of 20, despite the 454.json file under items/ammo showing it's 15.

I don't believe this is directly related to this PR, and changing the uncraft 454.json to reflect a stack_size of 20 seems like a bad move since the two files would be based off different numbers.

Apparently the "count" variable is the one that should be used, and not the stack_size. I will correct this in cases where they differ.

@Rogal-Dorn
Copy link
Contributor Author

I've been informed this fix won't be acceptable due to changes in the count variable not being considered. Closing.

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) Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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