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

Fix cash card stacking and display #31732

Merged
merged 3 commits into from
Jun 23, 2019

Conversation

jbytheway
Copy link
Contributor

@jbytheway jbytheway commented Jun 22, 2019

Summary

SUMMARY: Interface "Fix cash card stacking and display"

Purpose of change

Fixes #31716.

After the recent change (#30874) to allow multiple ammo types, cash cards needed different code to identify them as such. This check was happening in many places, some of which were updated, but not all.

Describe the solution

Add a new function item::is_money() to make factor out this logic and use it in all the places.

Also tweak how the advanced inventory handles stacks from a character inventory. Previously it wasn't rendering mixed-value stacks properly.

This fixes various issues related to cash cards like them not stacking any more or strange messages when you pick them up.

Describe alternatives you've considered

We might want to make stacking of vari-charges items an generic item flag, rather than money-specific.

Additional context

Here's the before-and-after messages in a single screenshot:
cata-cash-card-messages
And here are two differently-valued cash cards stacking in an inventory:
cata-cash-card-stacking

In advanced inventory, when handling a stack from a character inventory,
it was stored only as a pointer-to-first-item-and-count.  Store it
instead as a list-of-item-pointers.  This provides more consistency in
rendering between inventory and map items.  In particular, cash cards
were previously rendering differently.
After the recent change to allow multiple ammo types, cash cards needed
different code to identify them as such.  This check was happening in
many places, some of which were updated, but not all.

Add a new function item::is_money() to make factor out this logic and
use it in all the places.

This fixes various issues related to cash cards like them not stacking
any more or strange messages when you pick them up.
@jbytheway jbytheway changed the title Cash card stacking Fix cash card stacking and display Jun 22, 2019
@ifreund ifreund added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones Items / Item Actions / Item Qualities Items and how they work and interact labels Jun 22, 2019
@EvgenijM86
Copy link
Contributor

EvgenijM86 commented Jun 22, 2019

I have encountered the same bug for "chunks of chitin" item.
image

@jbytheway
Copy link
Contributor Author

I have encountered the same bug for "chunks of chitin" item.

There are various different aspects to this bug. Can you clarify what you mean? Perhaps a screenshot?

@EvgenijM86
Copy link
Contributor

I have encountered the same bug for "chunks of chitin" item.

There are various different aspects to this bug. Can you clarify what you mean? Perhaps a screenshot?

I have posted screenshot above. Is it not visible? Here is direct link - https://user-images.githubusercontent.com/430076/59964949-7ebf6a80-9510-11e9-936a-41f847a9ed70.png

I would assume that "chunks of chitin" should stack together, just like sinew and other non-perishable items. Or maybe I am wrong and they have some sort of durability stat that is different for each chunk of chitin - then this is not a bug in this case.

@jbytheway
Copy link
Contributor Author

jbytheway commented Jun 22, 2019

Screenshot shows now; don't know why it didn't before. This must be caused by some kind of hidden stat; not sure which one. But the fact that you can't see what it is makes it a bug, IMO. But it's almost certainly unrelated to this cash card issue, so out of scope for this PR.

@alanbrady
Copy link
Contributor

This must be caused by some kind of hidden stat

I believe if the items are not exactly alike they will not stack. So different rot/birthday/temperature, almost anything will make an item not stack with each other. This why cash cards need the exception because they're saying it's ok we don't have the same ammo amount we can stack anyway and we'll handle it ourselves by storing a list of ammo amounts such that whenever you pull a cash card off the stack it pops an internally saved value off to make the "new" cash card.

@alanbrady
Copy link
Contributor

Further context for anyone unaware, stacking coalesces multiple items down into one item in such a way that we don't need to track each individual item and just say "I have x number of items exactly like this" so that unstacking them is trivial. Stacking causes them to lose any individual properties, so if they're not exactly alike we do not stack.

@ZhilkinSerg ZhilkinSerg self-assigned this Jun 23, 2019
@ZhilkinSerg ZhilkinSerg merged commit 8880df9 into CleverRaven:master Jun 23, 2019
@ZhilkinSerg ZhilkinSerg removed their assignment Jun 23, 2019
@jbytheway jbytheway deleted the cash_card_stacking branch June 23, 2019 09:14
@Inglonias
Copy link
Contributor

YAAAAAAAAAY

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) [C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones Items / Item Actions / Item Qualities Items and how they work and interact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cash cards not formatted properly when examined on ground
6 participants