[WAIT] Remove ammo_modifier

[alanbrady]

Summary

SUMMARY: Infrastructure "Remove ammo_modifier"

Purpose of change

#23078 basically made ammo_modifier obsolete but didn't actually remove the code using it.

#31766 fixed UPS mods to not need ammo_modifier

#31800 removed the ammo_modifier from UPS mod because it causes segfault. After #23078 nothing should be using ammo_modifier anymore. The code is very apparently broken and not needed.

Describe the solution

Remove code references to ammo_modifier and remove it from all json entries. Essentially all json entries with this are already obsolete (except one which is moved to obsolete with this patch), but this removes the field from the item definitions.

There's a bunch of functions that had a conversion parameter just for checking for ammo_modifier. Removing that enables removing the parameter from all of those functions.

This also moves the 'flare gun caliber mod' to obsolete since it appears it should have been included in #23078.

[foulman]

Er, I actually just used this today to create something, how is changing an item from one form of ammo to another supposed to be accomplished if not with ammo_modifier?

[alanbrady]

Adding a mod_target and magazine_adaptor allows specifying different ammo magazines. See https://github.com/CleverRaven/Cataclysm-DDA/blob/master/data/json/items/gunmod/magazine.json

[foulman]

Adding a mod_target and magazine_adaptor allows specifying different ammo magazines. See https://github.com/CleverRaven/Cataclysm-DDA/blob/master/data/json/items/gunmod/magazine.json

Ahh.. I see. I was actually using that mod as a reference, but magazine_adaptor wasn't explained in the docs and I saw ammo_modifier there and used that instead because it was.

[alanbrady]

Yeah it doesn't look like whoever added this magazine_adaptor stuff documented it. Not your fault. I had to go digging through git history to figure out what happened here.

[foulman]

Well maybe I'm doing something wrong, but when I just have that in there it seems to be indicating that there are no "compatible ammo types", I'm trying to change from firing 9mm bullets to not firing bullets at all, it's not just a matter of magazines.

[foulman]

Specifically, I was making a magazine adaptor to convert from firing bullets, 9mm in this case, to firing "mana" bolts, for the Magiclysm mod. I can of course just make a recipe for each and every gun that someone might come across to do that, and convert the whole gun over permanently, but a converting the ammo seemed a lot simpler in that I could just do one for each type or round at most.

[alanbrady]

I'm not super on the up and up on how the magiclysm stuff specifically works but it looks like you want your gunmod to have accept "acceptable_ammo": [ "crystallized_mana" ] and have "magazine_adaptor": [ [ "crystallized_mana", [ "small_mana_crystal" ] ] ]

@KorGgenT does this sound right?

[foulman]

I'm not super on the up and up on how the magiclysm stuff specifically works but it looks like you want your gunmod to have accept "acceptable_ammo": [ "crystallized_mana" ] and have "magazine_adaptor": [ [ "crystallized_mana", [ "small_mana_crystal" ] ] ]

@KorGgenT does this sound right?

That's exactly what I did and got that error.
"magazine_adaptor": [ [ "mana", [ "light_manastone", "small_manastone" ] ] ],
However, my reading of "acceptable_ammo" is that it's a pre-modification filter to control what guns it can be used on, in this case 9mm.
"acceptable_ammo": [ "9mm" ], // Optional filter restricting mod to guns with those base (before modifiers) ammo types

This seems to just allow the use of specified magazines of the same ammo type, as the name implies, and that there is now no way of changing actual ammo types.

[foulman]

In fact, if I take that "acceptable_ammo" filter out, it will install on the glock 17, so that error arises from acceptable_ammo not (or possibly and) magazine_adaptor, but the gun is still demanding 9mm ammo loaded from a magazine that can't hold 9mm ammo, when I include the ammo_modifier it worked just fine and fired mana. If there's some other way to accomplish this I don't know what it is.

[alanbrady]

It looks like #30874 changed the acceptable_ammo check and introduced a logic bug. Will need another patch for that. I'm fairly sure

Cataclysm-DDA/src/item.cpp

Line 6160 in dd1dfac

if( compat_ammo ) {

is supposed to be if( !compat_ammo )

[alanbrady]

I'll play with it some more and see if I can figure out more clearly what's happening. Maintainers please wait on merging this for the moment.

[KorGgenT]

I'm not super on the up and up on how the magiclysm stuff specifically works but it looks like you want your gunmod to have accept "acceptable_ammo": [ "crystallized_mana" ] and have "magazine_adaptor": [ [ "crystallized_mana", [ "small_mana_crystal" ] ] ]

@KorGgenT does this sound right?

sorry, i'm not super familiar with guns, and in fact try to avoid json if i can help it.

[foulman]

'mana' is basically no different than having 'battery' there, it has no real "ammo" characteristics in and of itself assigned by default as it's a power source, not a projectile, but still ammo as the game sees it. I faked the whole thing by using damage_modifier & range_modifier up to 9mm levels.

[alanbrady]

There's some goofy stuff going on with magazine_adaptor and this series apparently broke some tests so this is going to take more work than I hoped. It still seems like I can safely remove ammo_modifier but I have no intention of breaking modability if this is actually enabling that somehow. It might help if you post a sample of one of your gunmods so I can use it just as a reference to see exactly what you're trying to do.

[foulman]

Thankfully I only started on it this afternoon, and came up with this one mod after toying around with various settings. It worked as intended as long as ammo_modifier was in there, without it, the gun still expects 9mm ammo but no magazines can feed it that anymore.

{
"id": "tmconv_pistol",
"type": "GUNMOD",
"name": "Technomagic 9mm pistol converter",
"description": "An adapter for permanently converting a firearm to accept manastones and fire magic bolts instead of bullets. Guaranteed to void your warranty.",
"weight": 140,
"volume": 1,
"integral_volume": 1,
"price": 48000,
"material": "steel",
"symbol": ":",
"color": "green",
"location": "magazine",
"mod_targets": [ "pistol" ],
"acceptable_ammo": [ "9mm" ],
"magazine_adaptor": [ [ "mana", [ "light_manastone", "small_manastone" ] ] ],
"ammo_modifier": [ "mana" ],
"ammo": "mana",
"damage_modifier": 26,
"dispersion_modifier": 50,
"range_modifier": 14,
"recoil_modifier": 250,
"min_skills": [ [ "spellcraft", 3 ] ]
}

[foulman]

It's not a huge deal to me if you go ahead and break this, I can do something else or just make up full conversion recipes for individual guns, or any other number of things, though I would obviously like that functionality documented and working at some point, the mod was intended as a more portable quick-fix to converting guns and I only tried it after noticing the lw adaptor.

[alanbrady]

After some review it seems like we actually do need ammo_modifier to enable what you're trying to do and there's not a super good reason to break that so I'm closing this PR. But the acceptable_ammo thing still looks like a bug there and I think I found some other stuff. Will submit a new PR for those.