erk and knives

[Kamejeir]

Summary

Features "Various knives now using the variant system"

Purpose of change

@I-am-Erk wanted this

Describe the solution

Move knives_kitchen.json contents to knives.json

Make the kitchen knives into variants as determined below

Changed a mission needing a knife

Changed around tests to check for the base knives instead of the variants

migrate the knives to their respective variants

Made some mods that need the knives now use any of the knives in their category (sorry crazycata, your bread sword is now no longer just a bread sword. rather chef- and carving sword too)

Describe alternatives you've considered

Testing

Small kitchen knife: Loads no fault, spawns in with separate variants, the items get their own ASCII art

further testing: no fault when loading or throughout testing spawning/recipes

found one of each knife and made this beauty

all knives disassemble successfully

a practice recipe could successfully utilize a knife

I'm not sure how to see about checking the mod changes over just seeing that the unit tests passed

Additional context

[I-am-Erk]

You can also keep them in the current knife file, it really doesn't matter much

[Kamejeir]

Done with the small kitchen knives. I don't know which itemgroups would deserve the extra knives such as the "tomato knife". I mean obviously yes wherever there are all variants (like a kitchen), but scanning for exceptions is the tricky part. Also I'm not too keen on adding descriptions of the extra knives, am not a knife enthusiast.

[Kantonine]

Also, quick thing to add, both cleavers should have the knife proficiencies

[I-am-Erk]

This is looking good so far. I would not bother with adding new knife variants in this PR, that's just something we can think about later.

[Kamejeir]

Here includes all variants of the large knife. Is this the best way to spawn these three? Anything like taking out the variant definitions might not be future-proof, though in case more large knives are added...

Cataclysm-DDA/data/json/itemgroups/collections_domestic.json

Lines 101 to 103 in 3b3b739

	[ "knife_bread", 20 ],
	[ "knife_chef", 20 ],
	[ "knife_carving", 20 ],

This becomes rather silly. One can potentially substitute in a bread knife for use in butchery, which hardly seems appropriate.
"Kit" items are so last year anyway.

Cataclysm-DDA/data/json/recipes/tools/tool.json

Line 231 in 3b3b739

[ [ "knife_carving", 1 ] ],

[I-am-Erk]

You could keep the bread knife as a separate item if it makes you more comfortable. I don't personally think it's all that significant a problem, but it is noticeably different from the others

[Kamejeir]

My only concern is this. It's not letting me use variants, so a new huge knife variant would make it not necessarily be giving this NPC a butcher knife. Maybe the description of the mission can be spiced up as more generic, assuming future huge knives all have function similar to a butcher knife?

Cataclysm-DDA/data/json/npcs/godco/godco_missions.json

Line 60 in de4ec39

"item": "knife_butcher",

[Kamejeir]

@I-am-Erk I'll go with this. I'll test it now and once you're good with how the mission turned out, I'll put it as ready for review.

[Kamejeir]

still waiting for erk to get in his comment, for the merge reading this. disclaimer: did not test the mission, for I could not figure out how. disclaimer 2: after the reviewbot, only tested again by loading into the game, not encountering any load errors

[Kamejeir]

oh no, tests:
crafting_test - missing a cutting tool
effective_dps_test - make it use knife_small instead of knife_butcher. knife_cleaver instead of knife_meat_cleaver
npc_attack_test - make it use knife_large instead of knife_chef. knife_huge instead of knife_butcher

also to remove str_pl from cleavers

npc_talk_test - something about not having enough starting items

[github-actions]

Auto-requesting reviews from non-collaborators: @Light-Wave

[I-am-Erk]

Ah yeah. The magiclysm conflicts and resulting explosion are a good case study of why it might have been better to do just steak/paring knives, then just bigger knives, in separate PRs. It looks like you have it mostly sorted now though, if it passes these tests I'll merge it in a bit.

I'm not too worreid about the godco mission, if we add another type of knife we could either adjust the mission to specifically ask for a butcher knife or we could change the dialogue.

[Kamejeir]

fixed handling migration. I start to understand that the commit log needs to both serve as a changelog while also being readable. good experience for the future

[Maleclypse]

Please rebase to resolve conflicts