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

cans spawn sealed #41745

Merged
merged 2 commits into from
Jul 2, 2020
Merged

Conversation

KorGgenT
Copy link
Member

@KorGgenT KorGgenT commented Jul 2, 2020

Summary

SUMMARY: None
No need to hit the 0.F changelog but this is a pretty significant bugfix for those who play experimental.

Purpose of change

Fixes #40127
Fixes #40658

Describe the solution

When the item is being modified in item_group.cpp, make sure to seal it when adding things to the CONTAINER pocket

Testing

wander around a town and look for aluminum cans of beer and small tin cans of cat food and mashed pumpkins

Additional context

image

Also, i left in the minor code changes i made to help me find the bug. they are minor organizational things but make it a lot easier to put breakpoints in.

@KorGgenT KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) Items: Containers Things that hold other things labels Jul 2, 2020
@Stephen2
Copy link
Contributor

Stephen2 commented Jul 2, 2020

Should we be adding or modifying a test for this, to ensure it never comes back?

@Stephen2
Copy link
Contributor

Stephen2 commented Jul 2, 2020

(Just learning the policies of this repository, so please excuse my question if wrong)

@anothersimulacrum
Copy link
Member

It'd be great to have a test for this, if you're volunteering to write it :)!

@@ -596,7 +596,9 @@ bool item_pocket::seal()

void item_pocket::unseal()
{
_sealed = false;
if( _sealed ) {
_sealed = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this change? The behaviour seems the be exactly the same as before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not exactly married to keeping it, but my logic was that it made setting breakpoints at that part much easier if you only want to find out when it actually matters that it sets it false

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set a conditional breakpoint then.

@Stephen2
Copy link
Contributor

Stephen2 commented Jul 2, 2020

@anothersimulacrum I'm gonna try and figure out how to write C++ so I can contribute here. Kind of daunting to jump into :p

src/item_pocket.cpp Outdated Show resolved Hide resolved
@kevingranade kevingranade merged commit 7194cdc into CleverRaven:master Jul 2, 2020
@KorGgenT KorGgenT deleted the cans-spawn-sealed branch July 3, 2020 01:18
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` Items: Containers Things that hold other things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot stash cans Canned items are spawning opened again
5 participants