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

[WIP] Allow quivers to contain arrows and bolts again #41122

Closed
wants to merge 3 commits into from

Conversation

wapcaplet
Copy link
Contributor

@wapcaplet wapcaplet commented Jun 6, 2020

Summary

SUMMARY: Bugfixes "Allow quivers to contain arrows and bolts again"

Purpose of change

Fix #40521

Four of the quiver items (quiver, quiver_large, quiver_birchbark, quiver_large_birchbark) had their capacity defined in terms of ammo_restriction, but that only works when the pocket has type MAGAZINE, so it was impossible to store any arrows or bolts in any of the quivers.

Describe the solution

Add MAGAZINE type to the quiver pockets, so they can hold ammo. Also make them rigid, so their encumbrance rating does not vary.

Finally, update the quiver descriptions to indicate (a) they can contain bolts as well as arrows, and (b) they can be unloaded or reloaded, rather than "activated".

Describe alternatives you've considered

The ammo_pouch item in this file needs a similar update, but holds many more kinds of ammo and could not be solved as neatly. Saving that for a future PR seemed wise.

Testing

Spawned quivers, bolts, and arrows in game. Stored bolts and arrows in quivers, wore quivers, checked encumbrances.

Additional context

WIP - When firing a bow, I am getting a message like "You don't have any arrows to reload your || self bow", despite having quivers full of arrows. With a backpack full of arrows, however, I am able to fire them.

wapcaplet added 2 commits June 6, 2020 09:56
"Encumbrance when full" was shown incorrectly for quivers. Since they
are magazines with volume determined by their ammo restriction, their
encumbrance does not actually increase when full.

Setting them to explicitly be "rigid" allows the info display to
accurately reflect the encumbrance of these items.
@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Items: Magazines Ammo holding items and objects. labels Jun 6, 2020
The quiver descriptions claimed they could be "activated" to store
arrows, but now they are technically "magazine" items and must be
unloaded/reloaded instead. They may contain bolts as well.
@wapcaplet wapcaplet changed the title [WIP] Allow quivers to contain arrows and bolts again Allow quivers to contain arrows and bolts again Jun 6, 2020
@wapcaplet wapcaplet changed the title Allow quivers to contain arrows and bolts again [WIP] Allow quivers to contain arrows and bolts again Jun 6, 2020
@KorGgenT
Copy link
Member

KorGgenT commented Jun 7, 2020

but that only works when the pocket has type MAGAZINE, so it was impossible to store any arrows or bolts in any of the quivers.

This should be false, why is this happening? quivers should not be a MAGAZINE they should be a container

basically, ammo_restriction should work for any pocket type. if it doesn't, that's a pretty big problem.

@wapcaplet
Copy link
Contributor Author

wapcaplet commented Jun 7, 2020

basically, ammo_restriction should work for any pocket type. if it doesn't, that's a pretty big problem.

It does, but only partially. When the pocket is not a MAGAZINE, the total capacity and pocket capacity is displayed as 0:

image

There appears to be no way to direcly reload more arrows into the quiver - the "reload" action is disabled, "activate" says I can't do anything interesting, and "insert" does not list any of the arrows or bolts in my inventory. When I pick up arrows or bolts, they go into the quiver - but I cannot put them there myself.

The zero volume and weight indicators are due to these two lines in item_pocket::general_info, which prints the pocket volume capacity (which is 0 because of the ammo_resriction), and the pocket's total weight capacity (also 0, because the quiver with ammo_restriction has no weight capacity):

info.push_back( vol_to_info( "CONTAINER", _( "Volume: " ), volume_capacity() ) );
info.push_back( weight_to_info( "CONTAINER", _( " Weight: " ), weight_capacity() ) );

@wapcaplet
Copy link
Contributor Author

wapcaplet commented Jun 7, 2020

This bug is far more multi-faceted than I realized. First, functionally, the existing quivers can in fact contain arrows and bolts. There is no bug with that part.

However there are at least two display-related problems with having "ammo_restriction" defined for an item with a non-MAGAZINE pocket, both related to the item info display being misleading:

  • Those ammo restrictions are not displayed unless the item is also a MAGAZINE (being in item::magazine_info, which quits immediately if the item is not a magazine). This could be solved by refactoring the item info functions so they can display "ammo_restrictions" even when the pocket is not a MAGAZINE.
  • Volume and Weight capacity are displayed as 0.0 for non-MAGAZINE pockets (all pockets with "ammo_restriction" have their volume set to 0, because their capacity is limited only by what kind of ammo is put into it). This can be solved by completely omitting volume/weight capacity from the item info display for "ammo_restriction" pockets, regardless of whether the pocket is a MAGAZINE or not.

Related to putting items in the quivers, another misleading-display issue concerns the "Insert" menu. When using the "insert" popup menu for a quiver or ammo pouch, it may show "Your inventory is empty", when it really means any of several things, including:

  • "This pocket is already full"
  • "This pocket is partly full, but you have no more of the same item"
  • "You don't have any compatible items that can be inserted"
  • "You have compatible items that can be inserted, but they need to be divided into smaller stacks"

How to distinguish and simplify these cases is another issue; they are already fairly cleanly handled for the "reload" action with MAGAZINE types, but extending them to work with "inserting" with non-MAGAZINEs may need to be a separate PR.

In short, all of this has to do with making non-magazine pockets behave more like magazines, to have some of the features of magazines without actually being magazines.

@wapcaplet
Copy link
Contributor Author

I have a better solution on the way with #41532 - closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Items: Magazines Ammo holding items and objects. [JSON] Changes (can be) made in JSON
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