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

NO_UNLOAD containers cannot be unloaded, even if liquid #61759

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

RenechCDDA
Copy link
Member

Summary

Bugfixes "Containers with NO_UNLOAD cannot unload liquids"

Purpose of change

You could do things like dump pressurized liquid ammonia on the ground. This shouldn't be possible, and that's why NO_DROP exists - but while looking up WHY it was happening, I learned that liquids can always be removed from NO_UNLOAD containers.

Describe the solution

Don't return a check saying you can unload if there's a no unload flag! The flag check needs to be in the first return otherwise it short-circuits before it gets to checking for the flag.

Describe alternatives you've considered

I considered changing the order of the logic so no unload is always checked first, but I tried this solution first and it worked during testing, so...

Testing

Compiled locally and confirmed that this change prevented liquid ammonia from being removed from its pressure tank container. I also crafted with it and (after applying the fix for #59539) confirmed that crafting interactions worked as intended. I did not test with any other liquids.

Additional context

Ammonia was just a useful test item - it needs other PRs to address its ability to migrate outside of pressurized containers.

This changes some pretty old game logic so I won't be surprised if it breaks something. This definitely needs to pass all automated tests before merging, if nothing else.

There may be a much smarter way to code this. My C++ is still novice, at best.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 17, 2022
@dseguin dseguin merged commit 842769c into CleverRaven:master Oct 19, 2022
@RenechCDDA RenechCDDA deleted the NO_unloading branch October 20, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants