fix ammo_capacity() and implement item_id_restriction infrastructure #40399

KorGgenT · 2020-05-10T01:04:37Z

Summary

SUMMARY: Infrastructure "fix ammo_capacity() and implement item_id_restriction infrastructure"

Purpose of change

This fixes the bug with not being able to load sewing kits. It also makes it so the magazine code relies more on pockets than the islots. This PR also requires that gun json gets updated first, because copy-from is being very frustratingly broken with the item factory fake pocket loading code.

So, when you get the ammo_capacity() of an item, before you know it could only have 1 type of clip_size whether it's a mag or a gun. however, now, you can have a different clip size (for lack of better term) per ammotype per pocket - so it's imperative that is calculated correctly.

After working on this from the ammo_capacity direction, it became clear to me that i needed to implement item_id_restriction and MAGAZINE_WELL type pockets in order for the gun/reloading/unloading code to work properly.

Describe alternatives you've considered

Testing

TBD - running the tests locally for now is breaking things

Additional Comments

Closes #40382
Fixes #40315
Fixes #40158
Fixes #40307
Fixes #40203
Fixes #40125

Resok · 2020-05-10T21:16:43Z

I'm trying to validate the JSON changes to the jackhammer (gas powered) using this branch and I'm getting an exception in xtree when reloading the Jackhammer. Error reads:

xtree
Line: 241

Expression: map/set iterators in range are from different containers

I have the callstack if it's useful to you.

This was after updating the JSON to have a pocket magazine that can only contain gasoline.

Callstack -

 	Cataclysm-vcpkg-static-Debug-x64.exe!std::_Adl_verify_range<std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>>,std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>>>(const std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> & _First, const std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> & _Last) Line 1044	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!std::distance<std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>>>(std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> _First, std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> _Last) Line 1658	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!enumerate_as_string<std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>>,std::string <lambda>(const string_id<ammunition_type> &)>(std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> first, std::_Tree_const_iterator<std::_Tree_val<std::_Tree_simple_types<string_id<ammunition_type>>>> last, player::select_ammo::__l26::std::string <lambda>(const string_id<ammunition_type> &) string_for, enumeration_conjunction conj) Line 676	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!player::select_ammo(const item & base, bool prompt, bool empty) Line 2478	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!game::reload(item_location & loc, bool prompt, bool empty) Line 8545	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!game::inventory_item_menu(item_location locThisItem, int iStartX, int iWidth, game::inventory_item_menu_positon position) Line 2231	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!game_menus::inv::common(avatar & you) Line 239	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!game::handle_action() Line 1956	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!game::do_turn() Line 1517	C++
 	Cataclysm-vcpkg-static-Debug-x64.exe!WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, char * __formal, int __formal) Line 697	C++
 	[External Code]	```

Json for the jackhammer -

```  {
    "id": "jackhammer",
    "type": "TOOL",
    "name": { "str": "jackhammer" },
    "description": "This is a construction tool for drilling through hard rock or other surfaces.  It runs on gasoline.  Use it (if loaded) to blast a hole in adjacent solid terrain.",
    "weight": "15875 g",
    "volume": "5 L",
    "price": 40000,
    "price_postapoc": 1000,
    "to_hit": -8,
    "bashing": 14,
    "cutting": 6,
    "material": "steel",
    "symbol": ";",
    "color": "light_gray",
    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid":true, "sealed":true, "watertight":true, "ammo_restriction": { "gasoline": 400 } } ],
    "charges_per_use": 10,
    "use_action": "JACKHAMMER",
    "flags": [ "STAB", "DIG_TOOL", "POWERED" ]
  },```

KorGgenT · 2020-05-11T06:09:07Z

ok there might be a bit of scop creep here, but i need to get it to where the bugs i introduced with the adjustments to the foundational functions get reduced. i'm going to keep this in draft for a while so that it's transparent what i'm doing

Resok · 2020-05-11T08:14:58Z

Did some testing with these changes. Was able to add the new "MAGAZINE_WELL" type pocket to the json of a few items to test out. So far it seems to work better at least - though I still see things like this:

01:07:49.721 ERROR : src\item.cpp:600 [ammo_set] Tried to set invalid magazine of batteries for medium plutonium fuel battery
01:07:53.755 ERROR : src\item.cpp:600 [ammo_set] Tried to set invalid magazine of batteries for medium plutonium fuel battery
01:07:53.945 ERROR : src\item.cpp:600 [ammo_set] Tried to set invalid magazine of batteries for light plutonium fuel battery

when trying to reload an item that I set with the following:

"pocket_data": [
  {
    "pocket_type": "MAGAZINE_WELL",
    "holster": true,
    "max_contains_volume": "5 L",
    "max_contains_weight": "5 kg",
    "item_restriction": [
      "light_battery_cell",
      "light_plus_battery_cell",
      "light_minus_battery_cell",
      "light_atomic_battery_cell",
      "light_minus_atomic_battery_cell",
      "light_minus_disposable_cell",
      "light_disposable_cell"          
    ]
  }
],

However when I go to reload them it seems to work fine and only shows me valid items in my list of reload targets.

Resok · 2020-05-11T20:40:48Z

Looks like this unhooked the vehicle tool crafting checks - Save below.

Save.zip

ymber

Looks good. From here the rest of the reloading and ammo consumption work should go much quicker.

ymber · 2020-05-13T11:54:40Z

src/item.cpp

+
+    const itype *loaded_ammo = ammo_data();
+    if( loaded_ammo == nullptr ) {
+        return ammo_capacity( item::find_type( ammo_default() )->ammo->type ) - ammo_remaining();


How can this give meaningful numbers under the new system without a current ammotype?

This function is used in a couple places to ask "can this be reloaded?" and sometimes there's no ammo type available. I thought that it would make sense to have the default ammo as a fallback if there was no ammo loaded in order to calculate some number, instead of just returning 0.

src/item_contents.cpp

src/dump.cpp

kevingranade · 2020-05-15T15:26:09Z

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/psa-nested-containers-is-now-live-in-experimental/23529/62

src/item.cpp

kevingranade · 2020-05-17T03:38:20Z

Wielded gun, hit 'f'ire, nothing happened.

KorGgenT · 2020-05-17T18:38:48Z

I fixed some issues, but I have run into the main crux of the problem:
Item_factory::check_and_create_magazine() is not sufficient anymore. every gun that has copy-from and that changes the magazine will now incorrectly have the previous magazine data, which conflicts with islot_gun and will show this error message

I have already spent too many hours copy-pasting by hand, the rest of them just need some python scripting or something.

data/json/items/tool/tailoring.json

Lamandus · 2020-05-20T19:53:04Z

since it will be handled as a container, is it now possible to load a weapon with different types of ammo (for example FMJ and AP)? And if so, what ammo will it use? The one loaded in last?

KorGgenT · 2020-05-20T20:56:24Z

since it will be handled as a container, is it now possible to load a weapon with different types of ammo (for example FMJ and AP)? And if so, what ammo will it use? The one loaded in last?

not as of this PR, MAGAZINE type pockets when using ammo_restriction only take 1 stack of items. you might be able to fudge it by using item_restriction maybe, but that would be a pretty hacky solution imo and mod-only because of it, for now

* Added magazine pocket data to all relevant items, allowing them to be loaded. I'll need to retest after CleverRaven/Cataclysm-DDA#40399 is merged to confirm that unloading will work afterward, but now at least it actually loads and uses charges properly. * Reworked several transforming items to account for the fact that you can no longer change an item with ammo into an active item with null ammotype. * Hammers of the hunter now call their flashbang via spell, needed at bare minimum to YASD-proof it slightly via retaining the needs_wielding boolean. While I had to engage in some dirty hacks to work around the fact that flashbang spells don't do anything to NPCs, this had the side benefit of letting me ditch the never-actually-worked on-hit effect for a more direct subspell, explicitly burning and further debuffing SUNDEATH monsters. * Veinreavers now have to use 3 charges of blood essence instead of just one. The explosion has been buffed slightly and the timer shortened by two turns, as a direct result of this change. * Due to technical issues, I had to effectively choose between ARTC_PORTAL and being able to load it with crystallized essence, because artifact charging doesn't sanely translate into ammo that actually exists. I decided, after soliciting feedback from various people, that retaining ARTC_PORTAL was the more interesting option. As a minor side effect it no longer has a cutting quality, and can store up to 10 charges. * However, I added a less direct way to translate crystallized essence into charges for the restored ritual blade. Slivers of reality, implied to be relevant for backstory rituals like the Dragonblood Sacrament and other non-standard summoning, its function is basically a disposable, single-use portal generator. Since crafting it requires crystallized essence to provide the raw power-spike needed, this can indirectly be used to fuel the restored ritual blade that way. Lore-wise however, doing that is something He From Beyond The Veil would frown upon, and if ter-transform could cite traps I'd make it a spell item in order to add some more interesting side effects.

kevingranade · 2020-05-23T00:27:34Z

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/psa-nested-containers-is-now-live-in-experimental/23529/94

anothersimulacrum · 2020-05-23T03:55:41Z

src/item.cpp

@@ -6143,7 +6179,7 @@ bool item::is_bionic() const

 bool item::is_magazine() const
 {
-    return !!type->magazine;
+    return !!type->magazine || contents.has_pocket_type( item_pocket::pocket_type::MAGAZINE );


This change means that things that use this to gate accessing type->magazine will no longer work, and will crash on attempting to access non-existent magazine data.

this is true, and i have come across these that do cause crashes and fixed them. islot_magazine is on the chopping block in favor of pocket_data (need to check with yutna for sure but i'm confident that this is true), and if i do not have this bool in here it will backpedal a lot of the work on this pr

Not this one
https://github.com/CleverRaven/Cataclysm-DDA/pull/40399/files#diff-f7ca61cf749eba039da961e8a7be1630L8408
if( !is_magazine() || !type->magazine->protects_contents ) {

src/item_factory.cpp

src/item_pocket.cpp

Co-authored-by: anothersimulacrum <[email protected]>

KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels May 10, 2020

KorGgenT force-pushed the adjust-ammo_capacity() branch from 2c1fe2d to 27eea0f Compare May 10, 2020 03:15

KorGgenT force-pushed the adjust-ammo_capacity() branch from d88a94e to b93d9b0 Compare May 11, 2020 16:57

KorGgenT changed the title ~~fix ammo_capacity()~~ fix ammo_capacity() and implement item_id_restriction infrastructure May 11, 2020

KorGgenT marked this pull request as ready for review May 11, 2020 21:50

Resok mentioned this pull request May 12, 2020

[Bugfixes] Vehicle tanks ui fix for PR #40399 #40473

Closed

KorGgenT force-pushed the adjust-ammo_capacity() branch from 1bebf0b to 39430da Compare May 12, 2020 15:10

KorGgenT mentioned this pull request May 12, 2020

Headlamp Does Not Hold Batteries #40426

Closed

DillonSimeone mentioned this pull request May 13, 2020

Unable to reload anesthetic kit with anesthetic. #40522

Closed

ymber reviewed May 13, 2020

View reviewed changes

KorGgenT mentioned this pull request May 13, 2020

Allow extra magazines for AR-based pistols #40514

Merged

KorGgenT commented May 15, 2020

View reviewed changes

src/dump.cpp Outdated Show resolved Hide resolved

KorGgenT force-pushed the adjust-ammo_capacity() branch from 9fc3ba3 to d0be7ce Compare May 15, 2020 22:16

kevingranade reviewed May 15, 2020

View reviewed changes

src/item.cpp Outdated Show resolved Hide resolved

Wishbringer reviewed May 15, 2020

View reviewed changes

src/item.cpp Outdated Show resolved Hide resolved

KorGgenT mentioned this pull request May 16, 2020

Pistols, Magazines increase volume when loaded, causes problems with professions #40609

Closed

KorGgenT force-pushed the adjust-ammo_capacity() branch from 0891abc to 93118fd Compare May 16, 2020 17:57

KorGgenT force-pushed the adjust-ammo_capacity() branch from 93118fd to 09c475b Compare May 17, 2020 18:37

MechaJeb mentioned this pull request May 18, 2020

Unable to Reload Certain Tools (Sewing Kits, Shaving Kits, Fish Traps) #40666

Closed

ymber mentioned this pull request May 18, 2020

Add magazine wells to items #40668

Merged

CodeBandit mentioned this pull request May 18, 2020

Fix the problem caused by curammo being deleted #40670

Closed

KorGgenT mentioned this pull request May 19, 2020

DO NOT MERGE - Ammo capacity and magazine wells #40707

Closed

protodave reviewed May 19, 2020

View reviewed changes

data/json/items/tool/tailoring.json Show resolved Hide resolved

rigidify batteries

c835426

KorGgenT force-pushed the adjust-ammo_capacity() branch 2 times, most recently from 594fb28 to eef1fcb Compare May 20, 2020 19:33

KorGgenT mentioned this pull request May 20, 2020

assorted Json pocket fixes #40723

Merged

KorGgenT added 3 commits May 20, 2020 16:49

create MAGAZINE_WELL and refactor ammo_capacity()

1e7f5c9

update tests with new logic

efabc25

add ammo restrictions and ammo to things that took charges before

6782ad0

KorGgenT force-pushed the adjust-ammo_capacity() branch from eef1fcb to 6782ad0 Compare May 20, 2020 20:50

add tailor's kit pocket for thread

17c78d7

ghost mentioned this pull request May 23, 2020

Items of type TOOL_ARMOR cannot be reloaded, regardless of correct pocket_data #40765

Closed

anothersimulacrum reviewed May 23, 2020

View reviewed changes

src/item_factory.cpp Outdated Show resolved Hide resolved

anothersimulacrum reviewed May 23, 2020

View reviewed changes

src/item_pocket.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

b1ae32f

Co-authored-by: anothersimulacrum <[email protected]>

KorGgenT mentioned this pull request May 23, 2020

Add tests for item_pocket and pocket_data #40578

Merged

KorGgenT added 5 commits May 23, 2020 00:29

prevent crashing by checking type->magazine

af258a6

fix crash when magazine_current() == nullptr

ad4deb6

fix crash when npcs try to take an empty gun from you

f16bf63

make ammo_consume more clear in item pockets

0840374

fix turrets being able to fire guns

aecbf7b

kevingranade merged commit dc615f0 into CleverRaven:master May 23, 2020

morthein mentioned this pull request May 23, 2020

New issue after fixing #40399, Reloaded Fish Trap error #40778

Closed

KorGgenT deleted the adjust-ammo_capacity() branch May 24, 2020 02:01

ussdefiant60 mentioned this pull request May 25, 2020

"charges" in JSON itemgroups not respected for guns/their magazines? #40817

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ammo_capacity() and implement item_id_restriction infrastructure #40399

fix ammo_capacity() and implement item_id_restriction infrastructure #40399

KorGgenT commented May 10, 2020 •

edited

Loading

Resok commented May 10, 2020 •

edited

Loading

KorGgenT commented May 11, 2020

Resok commented May 11, 2020

Resok commented May 11, 2020

ymber left a comment

ymber May 13, 2020

KorGgenT May 13, 2020

kevingranade commented May 15, 2020

kevingranade commented May 17, 2020

KorGgenT commented May 17, 2020

Lamandus commented May 20, 2020

KorGgenT commented May 20, 2020

kevingranade commented May 23, 2020

anothersimulacrum May 23, 2020

KorGgenT May 23, 2020

anothersimulacrum May 23, 2020

fix ammo_capacity() and implement item_id_restriction infrastructure #40399

fix ammo_capacity() and implement item_id_restriction infrastructure #40399

Conversation

KorGgenT commented May 10, 2020 • edited Loading

Summary

Purpose of change

Describe alternatives you've considered

Testing

Additional Comments

Resok commented May 10, 2020 • edited Loading

KorGgenT commented May 11, 2020

Resok commented May 11, 2020

Resok commented May 11, 2020

ymber left a comment

Choose a reason for hiding this comment

ymber May 13, 2020

Choose a reason for hiding this comment

KorGgenT May 13, 2020

Choose a reason for hiding this comment

kevingranade commented May 15, 2020

kevingranade commented May 17, 2020

KorGgenT commented May 17, 2020

Lamandus commented May 20, 2020

KorGgenT commented May 20, 2020

kevingranade commented May 23, 2020

anothersimulacrum May 23, 2020

Choose a reason for hiding this comment

KorGgenT May 23, 2020

Choose a reason for hiding this comment

anothersimulacrum May 23, 2020

Choose a reason for hiding this comment

KorGgenT commented May 10, 2020 •

edited

Loading

Resok commented May 10, 2020 •

edited

Loading