Cataclysm-dda: Add longest_side to JSON data for TOOL items

Created on 27 Jun 2020  路  11Comments  路  Source: CleverRaven/Cataclysm-DDA

Is your feature request related to a problem? Please describe.

Many of the TOOL items in the game do not have an explicit "longest_side" length defined in their JSON. Without this, an item's length is assumed (and displayed) to be the side length of a cube that encloses the item's "volume" (cube root of volume).

This leads to patently silly lengths such as the 9 cm bolt cutters, and the 14 cm banjo:

image

image

Describe the solution you'd like

I would really like all items to have a "longest_side", but for the sake of staying focused, this feature request is just to do the TOOL items. Many of the important melee weapons such as swords and knives already have a longest_side, but hundreds more tools and weapons don't yet have it.

Describe alternatives you've considered

I could take it upon myself to come up with lengths for all these tools, but tasks like this are ideal entry points for new contributors to dip their toes in, so why should I have all the fun?

Additional context

I made a spreadsheet of all the tools, including their weight, volume, description, and longest_side (if they have one).

https://docs.google.com/spreadsheets/d/1OWGFzrqIodwT5BF94hezW2IxAkN897apxMivB7pvhhY/edit?usp=sharing

This is generated from the table.py script in our repository, which scrapes JSON data and generates a Markdown or CSV table with columns you choose, ex.:

tools/json_tools/table.py --type=TOOL --format=csv id weight volume longest_side description

The tools span a long list of files, but most can be found in data/json/items/tool/*.json

For previous (merged) PRs adding longest_side, see #41583, #41199, #41044

To participate I would suggest making small pull requests, just adding "longest_side" to items in one or a few .json files at a time. Mention this issue number in your PRs, and please feel free to comment below to share what tools/files you are working on, to avoid duplication of effort.

In data/json/items/tool:

  • [x] container.json
  • [x] cooking.json
  • [x] deployable.json
  • [x] electronics.json
  • [x] entry_tools.json
  • [x] explosives.json
  • [x] firefighting.json
  • [x] fire.json
  • [x] fishing.json
  • [x] handloading.json
  • [x] knives.json
  • [x] landscaping.json
  • [ ] lighting.json
  • [ ] med.json
  • [ ] metalworking.json
  • [ ] misc.json
  • [x] musical_instruments.json
  • [ ] pets.json
  • [ ] radio_tools.json
  • [ ] raincatchers.json
  • [ ] science.json
  • [ ] shelters.json
  • [ ] smoking.json
  • [ ] stationary.json
  • [ ] tailoring.json
  • [x] toileteries.json
  • [ ] traps.json
  • [ ] woodworking.json
  • [ ] workshop.json
Good First Issue Items / Item Actions / Item Qualities [JSON] stale

All 11 comments

i'm currently planning to tackle this next

I started looking into this with "data/json/items/tool/knives.json", but adding "longest_side"s to the tool knives made the test suite regarding professions go haywire:

For example:

20:45:37.253 ERROR : src/character.cpp:9599 [Character::migrate_items_to_storage(bool)::<lambda(const item*)>] ERROR: Could not put inhaler into inventory.  Check if the profession has enough space.new_character_test.cpp:178: FAILED:
  CHECK( num_items_pre_migration == num_items_post_migration )
with expansion:
  16 == 15
with message:
  g->u.prof->ident().c_str() := "beekeeper"


20:45:37.344 ERROR : src/character.cpp:9599 [Character::migrate_items_to_storage(bool)::<lambda(const item*)>] ERROR: Could not put pocket knife into inventory.  Check if the profession has enough space.new_character_test.cpp:178: FAILED:
  CHECK( num_items_pre_migration == num_items_post_migration )
with expansion:
  13 == 11
with message:
  g->u.prof->ident().c_str() := "mall_cop"

20:45:37.391 ERROR : src/character.cpp:9599 [Character::migrate_items_to_storage(bool)::<lambda(const item*)>] ERROR: Could not put pocket knife into inventory.  Check if the profession has enough space.new_character_test.cpp:178: FAILED:
  CHECK( num_items_pre_migration == num_items_post_migration )
with expansion:
  15 == 10
with message:
  g->u.prof->ident().c_str() := "rancher"

How do we handle cases like these?

i'd imagine that those specific professions need to be investigated and make sure that they have appropriate containers to hold the items that they start with.

seeing as the pocket length for most clothes still auto generated they have quite short max_item_lengths. for example jean pockets are 350 mL and thus have an auto calculated max_item_length of ~10cm, which wouldn't even fit your hand.

Two of your errors are from pocket knife not fitting, which I imagine might be due to the jeans example above.

Thanks for the suggestion. The pocket knife is an interesting example, since by design in real life it has two different lengths (folded or unfolded). Maybe the minimum value would suffice, or it should follow the example of an extendable baton?

IMO, items that are small enough to fit it pockets don't really need longest_side assigned. all it does is cause problems with the auto generated pocket lenghts. I'd say it's mainly important for items that are more than 20-30cm and shouldn't fit in a pocket, where the current auto generated value allows it to fit in tiny pockets

IMO, items that are small enough to fit it pockets don't really need longest_side assigned. all it does is cause problems with the auto generated pocket lenghts. I'd say it's mainly important for items that are more than 20cm and shouldn't fit in a pocket

Cool, sounds like a good assumption.

container.json, explosives.json, and fire.json don't appear to need any specific additions of longest_side and can be checked off the list

container.json, explosives.json, and fire.json don't appear to need any specific additions of longest_side and can be checked off the list

@johnrdconnolly I checked those off along with the others you have submitted. Thanks for continuing to work on these!

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not \'bump\' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not \'bump\' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it

Was this page helpful?
0 / 5 - 0 ratings