Cataclysm-BN
Cataclysm-BN copied to clipboard
feat: reversible bullet recipes can have different uncraft requirements
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:
- In flag.cpp and flag.h, defined two new flags:
UNCRAFT_CONVERT_TO_CUTTING
andUNCRAFT_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. - 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 findsUNCRAFT_CONVERT_TO_CUTTING
tacked onto the hand press, it adds cutting quality of 1 instead of bullet pulling. Meanwhile if it findsUNCRAFT_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 checkingtool_comp
for the flags, I think?
JSON changes:
- Added
UNCRAFT_CONVERT_TO_CUTTING
to the hand press in theshot_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. - Defined a new uncraft,
bullet_forming_rifle
. This is identical tobullet_forming
except its hand press is flagged withUNCRAFT_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:
- Get it working blyat
- 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
Autofix has formatted code style violation in this PR.
I edit commits locally (e.g: git, github desktop) and want to keep autofix
- Run
git pull
. this will merge the automated commit into your local copy of the PR branch. - Continue working.
I do not want the automated commit
- Format your code locally, then commit it.
- Run
git push --force
to force push your branch. This will overwrite the automated commit on remote with your local one. - Continue working.
If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.
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.
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.
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.