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

Show ammo_restriction info for non-MAGAZINE items #41532

Merged
merged 5 commits into from
Jun 26, 2020

Conversation

wapcaplet
Copy link
Contributor

@wapcaplet wapcaplet commented Jun 23, 2020

Summary

SUMMARY: Bugfixes "Show ammo_restriction info for non-MAGAZINE items"

Purpose of change

Non-magazine items with non-magazine pockets (including quivers and ammo pouches), yet having an ammo_restriction defined, were not indicating their capacity in the item info panel.

Fixes #40521

Describe the solution

Allows non-MAGAZINE item contents/pockets to indicate their ammo_restriction-based capacity in the pocket section, labeled "Holds:"

Existing MAGAZINE-type containers (as well as GUN and TOOL items with MAGAZINE pockets) continue to indicate their magazine capacity info in the magazine section of the item info.

Change the quiver and ammo pouch item descriptions to say "Use insert to store" rather than "Activate to store".

In the inventory screen, show the quantity of arrows/bolts in a quiver.

Add a test case covering related item ammo_restriction info display, both for the main item info / description panel, and for the display_name of the quiver with arrows loaded in it.

Describe alternatives you've considered

Previous attempt #41122 changed quivers to be MAGAZINES; this attempt aims to make ammo_restriction work for non-MAGAZINES as well.

Testing

Ran new unit tests with tests/cata_test [ammo_restriction]

In-game, spawned a quiver and added some wooden bolts. Quiver ammo_restriction is indicated in the pocket section under "Total capacity:"

image

Ammo pouch now indicates its many ammo types:

image

In the inventory, quivers and ammo pouches will now indicate the total number of ammo items they contain. Prior to my changes, these did not show the (nn) counts. For example a large quiver with 51 of the same kind of arrow:

image

Large quiver with 57 of different kinds of arrow/bolt:

image

Ammo pouch with pebbles:

image

When the quiver has some arrows or bolts loaded into it, the total volume/weight of those items is displayed in the pocket section, but the total capacity (which is zero for ammo_restriction pockets) is not displayed. Thus, as a normal (non-ammo) container pockets, it would be formatted like this:

image

But since the zeros are misleading, they are now hidden for ammo_restriction pockets:

image

Quivers may contain multiple bolts and arrows, although there are some quirks with how the insert menu works - it may disallow explicitly loading multiple kinds into the quiver, though if you pick the arrows or bolts off the ground (or spawn them from the debug menu), they may go directly into a quiver that already contains other bolts or arrows:

image

Additional context

There are some inconsistencies and messaging problems with the insert menu, as noted above (and as described in the comments on #41122) but those are probably best addressed in a separate PR.

@wapcaplet
Copy link
Contributor Author

I ran make astyle to fix some styling bugs in src/item_contents.cpp, though these came in from upstream (not me).

@anothersimulacrum anothersimulacrum added Info / User Interface Game - player communication, menus, etc. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves labels Jun 24, 2020
@wapcaplet
Copy link
Contributor Author

wapcaplet commented Jun 24, 2020

CI failures appear to be caused by known problems in the comestible test (recipe kcals), not related to my changes.

@wapcaplet wapcaplet changed the title [WIP] Show ammo_restriction info for non-MAGAZINE items Show ammo_restriction info for non-MAGAZINE items Jun 24, 2020
@wapcaplet wapcaplet changed the title Show ammo_restriction info for non-MAGAZINE items [WIP] Show ammo_restriction info for non-MAGAZINE items Jun 24, 2020
Non-magazine items with non-magazine pockets (including quivers), yet
having an ammo_restriction defined, were not indicating their capacity
in the item info panel.

This commit allows non-MAGAZINE item contents/pockets to indicate their
ammo_restriction-based capacity in the pocket section, labeled "Holds:"

Existing MAGAZINE-type containers (as well as GUN and TOOL items with
MAGAZINE pockets) continue to indicat their magazine capacity info in
the magazine section of the item info.

Add a test case covering this behavior using existing test items.
@wapcaplet
Copy link
Contributor Author

CI failure appears unrelated, and one I have seen pop up before:

mutation_test.cpp:122: FAILED:
  CHECK( breach_chance <= BREACH_CHANCE_MAX )
with expansion:
  0.53608f <= 0.4f
with message:
  MUTATIONS: MANA_REGEN2 MANA_MULT_MANATOUCHED ROT2 ALBINO WEAK
  MANA_ADD_MANATOUCHED MANA_LUM 

@wapcaplet wapcaplet changed the title [WIP] Show ammo_restriction info for non-MAGAZINE items Show ammo_restriction info for non-MAGAZINE items Jun 26, 2020
@ZhilkinSerg ZhilkinSerg merged commit 4ad6757 into CleverRaven:master Jun 26, 2020
@wapcaplet wapcaplet deleted the quivering branch June 26, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Info / User Interface Game - player communication, menus, etc. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quiver item info display shows no storage and does not show that it can store arrows
3 participants