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: allow holstering to use a weapon_category instead of a flag #5632

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ratmice
Copy link
Contributor

@ratmice ratmice commented Oct 30, 2024

Checklist

Required

Optional

Purpose of change

In #1709 it was mentioned:

hopefully in the future can be used to aid with holsters and magazines, to define what can and cannot be stored without writing a full thesis.

This is an experiment in that regard

Describe the solution

Add weapon_category as a optional use_action field, this allows both flags and weapon_category to be used so it should be backwards compatible with existing mods/json data without change.

Describe alternatives you've considered

The weapon_category field in a use_action object could be an array or some such instead of just a string?

Testing

The KNIVES field was added to the bandolier_knife, and the SHEATH_KNIFE field was removed from the folding_knife.
It was tested that the folding knife, and another knife still can be sheathed and drawn from the bandolier.

Additional context

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

ratmice commented Oct 30, 2024

Haven't figured out how to run the translation template extraction script locally...

@ratmice ratmice marked this pull request as draft October 30, 2024 23:17
@ratmice
Copy link
Contributor Author

ratmice commented Oct 30, 2024

I'm currently marking this as draft, for 2 reasons:

  • We can remove a lot of flags from various JSON...
  • We need to audit things that have the weapon_category but don't have the respective flag.

e.g. scissors have the KNIVES category, but not the SHEATH_KNIFE flag, so without fixing that that scissors would become sheathable in knife bandoliers, when before this patch they were not. My opinion here is because scissors really aren't knives, they should probably be removed from that weapon category.

Edit: looking through the cdda data for scissors, it kind of has the opposite data,
there it is in the SHIV rather than in the KNIVES category, but has SHEATH_KNIFE, while BM we have it in the KNIVES category without SHEATH_KNIFE.

https://github.com/CleverRaven/Cataclysm-DDA/blob/master/data/json/items/tool/stationery.json#L68

I guess the 2 important questions are:

  • Whether scissors should be sheathable
  • Whether they really belong in the knives weapon cat

@scarf005 scarf005 changed the title refactor: allow holstering to use a weapon_category instead of a flag refactor: allow holstering to use a weapon_category instead of a flag Oct 31, 2024
@scarf005 scarf005 changed the title refactor: allow holstering to use a weapon_category instead of a flag feat: allow holstering to use a weapon_category instead of a flag Oct 31, 2024
@RoyalFox2140
Copy link
Collaborator

Haven't figured out how to run the translation template extraction script locally...

Ignore translation template failure. We don't know why it's failing but it should be unrelated.

@RoyalFox2140
Copy link
Collaborator

I guess the 2 important questions are:

* Whether scissors should be sheathable

* Whether they really belong in the `knives` weapon cat

I'm ok with both of these. Because of their low weight they aren't a bad starting knife weapon, they can be swung fast and thus can keep up in the DPS curve.
image
image

@ratmice
Copy link
Contributor Author

ratmice commented Oct 31, 2024

Sounds like I should just leave that one as-is then, I will try and do an audit of all the other things in the knives category,
and attempt to make a complete list including any more oddities that I run across I'll try and do that soon.

@ratmice
Copy link
Contributor Author

ratmice commented Oct 31, 2024

Here is a list of the oddball knives/flags combos, most of the things in the knives category without sheathing seem like oversights.
There are a few things in the KNIVES category which use SHEATH_SWORD, but they seem to large of volume for any knife sheaths anyways, so we can likely add KNIVES to the scabbard and let min_volume/max_volume figure out what fits in a sheath or scabbard.

name FLAGS W. CATEGORY COMMENTS
scissors KNIVES Sheathable on dda, very little harm in becoming sheathable by this patch
knife_swissarmy fine as is IMO
kris SHEATH_SWORD KNIVES add knives to scabbards, and let volume figure it out?
bayonette " " "
knife_rambo " " "
multitool SHEATH_KNIFE this usage of SHEATH_KNIFE should continue to work
chaos_dagger KNIVES lack of SHEATH_KNIFE seems an oversight?
bee_sting KNIVES oversight (compare with wasp_sting & fungal fighter sting) which have SHEAF_KNIFE

@RoyalFox2140
Copy link
Collaborator

chaos_dagger is a dev item.

@ratmice
Copy link
Contributor Author

ratmice commented Oct 31, 2024

Moving on to look at more holsters than the bandolier,

In the alternatives solutions I had said:

The weapon_category field in a use_action object could be an array or some such instead of just a string?

It looks like this is going to be required for items like survivor_belt_notools which have both ["SHEATH_KNIFE", "SHEATH_SWORD"].

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