Cataclysm-BN icon indicating copy to clipboard operation
Cataclysm-BN copied to clipboard

feat: reversible bullet recipes can have different uncraft requirements

Open chaosvolt opened this issue 9 months ago • 3 comments

Purpose of change

This at long last follows up on fixing all the jank in https://github.com/cataclysmbnteam/Cataclysm-BN/pull/886 by properly implementing a way for reversible recipes to automatically decide whether it requires cutting 1, pulling 1, or pulling 2 to take the ammo apart. Without this you have to define explicit uncrafts for factory ammo, something we can now axe. Conversely, this fixes the issue of the few existing reversible ammo recipes all demanding pulling 1 no matter what their factory counterparts need to dismantle.

Currently WIP as it's gonna take a while to add to all the recipes, plus right now I'm having a problem actually finding how to get the code to correctly check for flags in the tool it finds. Was given the go-ahead to post it up here so others can look at it.

Describe the solution

C++ changes:

  1. In flag.cpp and flag.h, defined two new flags: UNCRAFT_CONVERT_TO_CUTTING and UNCRAFT_CONVERT_TO_PULLING_2. These are intended to be used in tool entries, to affect what that recipe does when reversed, if marked as reversible.
  2. In requirements.cpp, changed requirement_data::disassembly_requirements so that the handling of converting hand press recipes into bullet pulling when reversed is at long last sanity-checked: if it finds UNCRAFT_CONVERT_TO_CUTTING tacked onto the hand press, it adds cutting quality of 1 instead of bullet pulling. Meanwhile if it finds UNCRAFT_CONVERT_TO_PULLING_2 it instead upgrades the required pulling requirement to level 2. At least, it's supposed to but currently I need to get it checking tool_comp for the flags, I think?

JSON changes:

  1. Added UNCRAFT_CONVERT_TO_CUTTING to the hand press in the shot_forming crafting requirement. This makes it so that shotshells that use this crafting requirement can now be set to be reverisible while still converting them to use cutting instead of pulling quality, where previously this only worked right if you set aside an explicit uncraft for that.
  2. Defined a new uncraft, bullet_forming_rifle. This is identical to bullet_forming except its hand press is flagged with UNCRAFT_CONVERT_TO_PULLING_2. Intended for rifle bullet recipes where we want them to require a bullet puller to take apart and not just mere pliers, this ensures they will get their pulling quality escalated from 1 to 2 during recipe reversal.

TODO:

  1. Get it working blyat
  2. Finish updating all ammo recipes to be reversible, and phase out the old uncrafts.

Describe alternatives you've considered

Unending fucking screaming at how janky the hardcoded uncraft generation is.

Testing

Testing to continue as implementation is completed. Right now I'm at an impasse: the actual check is a cursed-ass for loop buried in another for loop, which is why it's so hard to effectively vary up the conversions here. Far as I can tell, the check needs to be looking to see if the tool_comp that it found the tool in has the flag, and right now it's checking the itype of the tool it found for presumably item flags instead. Hence, it currently compiles without issue but completely fails to trigger either of the if statements, so 00 shot as of right now demands pliers to reverse its recipe instead of cutting quality.

Additional context

Speaking of, the ability to axe explicit uncrafts for ammo and use purely reversible recipes will come in handy if developed further later on, to convert all those wacky conversions into a more JSONized form.

Checklist

chaosvolt avatar May 15 '24 00:05 chaosvolt

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

autofix-ci[bot] avatar May 15 '24 00:05 autofix-ci[bot]

Is it visible to players? As in, does it show up replaced in item description? If yes, it's hacky, but tolerable. If no, it has to be made visible before merging.

Ideally, we'd have a proper jsonized requirement replacement system. It wouldn't be THAT hard to write. But at the same time, without properly informing the players how will we replace things, it may be better to avoid replacing.

Coolthulhu avatar May 15 '24 20:05 Coolthulhu

Problem right now is even getting this to work at all, which...well, it's not flagging correctly because I need to check the tool entry's flags like with NO_RECOVER, but instead it's checking what I assume is the flags on the item it found.

chaosvolt avatar May 15 '24 21:05 chaosvolt

Closing for now as an easy way to get this function doing what I intend doesn't come to mind. In the future it'll likely need a more in-depth overhaul of the underlying logic instead.

chaosvolt avatar May 16 '24 19:05 chaosvolt