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 crash when stacking items #27828

Merged
merged 1 commit into from
Jan 24, 2019

Conversation

GoLoT
Copy link
Contributor

@GoLoT GoLoT commented Jan 24, 2019

Summary

SUMMARY: Bugfixes "Fix crash related to item stacking introduced by food rebalance"

Purpose of change

#27252

Describe the solution

#27252 introduced a crash that is fixed by this PR, details can be found in the referenced PR. The fix tries not to break the new features of the original PR by not stacking food that has different components, as I believe was the intention of the author. This in turn will make crafted food split into different stacks when crafted from different components (minor issue) and might clutter the inventory a bit. It should only affect comestible items so it shouldn't be a big deal.

The code that caused the crash also duplicated logic that was added by a recent PR so this PR also combines both changes to avoid duplicated code.

Describe alternatives you've considered

Not adding the comestible check so food isn't stacked unless the flag is explicitly set, but that might had broken some of the new features (mainly the different nutrient values based on components).

@GoLoT
Copy link
Contributor Author

GoLoT commented Jan 24, 2019

The crash only happened when the game tried to stack items that were crafted with items that were spawned (through mapgen or debug menu), that is items that had different number of components.

The fix prevents the crash and seems to behave well with the food rebalance, but it's a quick fix and @KorGgenT might want to take a look and possibly test his changes to make sure that the fix doesn't break anything he added.

@KorGgenT
Copy link
Member

if i'm reading this right, no foods will stack with this change? that's a reasonable quickfix for a crash (that i was unaware of, sorry) but maybe we can look into having foods stack like cash cards do?

@GoLoT
Copy link
Contributor Author

GoLoT commented Jan 24, 2019

if i'm reading this right, no foods will stack with this change? that's a reasonable quickfix for a crash (that i was unaware of, sorry) but maybe we can look into having foods stack like cash cards do?

Only food with different components won't stack, which is what I think you intended in the first place. Meaning that if you craft something with fish and then craft it with meat, both batches won't stack and each stack will have different nutrient values. If you then craft a new batch using meat it will stack with the previous stack made with meat but not with the one made with fish. Any non-comestible item will stack as usual.

Does that work for your changes?

@KorGgenT
Copy link
Member

Sounds great, thanks for catching it!

@ZhilkinSerg ZhilkinSerg added Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones [C++] Changes (can be) made in C++. Previously named `Code` labels Jan 24, 2019
@ZhilkinSerg ZhilkinSerg self-assigned this Jan 24, 2019
@ZhilkinSerg ZhilkinSerg merged commit 06ff7da into CleverRaven:master Jan 24, 2019
@ZhilkinSerg ZhilkinSerg removed their assignment Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants