Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reversible bullet recipes can have different uncraft requirements #4656

Closed
wants to merge 3 commits into from

Conversation

chaosvolt
Copy link
Member

Purpose of change

This at long last follows up on fixing all the jank in #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

@github-actions github-actions bot added src changes related to source code. JSON related to game datas in JSON format. labels May 15, 2024
Copy link
Contributor

autofix-ci bot commented May 15, 2024

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.

@Coolthulhu
Copy link
Member

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.

@chaosvolt
Copy link
Member Author

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
Copy link
Member Author

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 chaosvolt closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants