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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix unloading containers which also have other types of pockets #58315

Merged
merged 2 commits into from Jun 19, 2022

Conversation

maspri
Copy link
Contributor

@maspri maspri commented Jun 9, 2022

Summary

Bugfixes "fix unloading containers which also have other types of pockets"

Purpose of change

fix #55481, fix #54330

Describe the solution

If an item has a CONTAINER pocket it is treated as container by character::unload and nothing else.

Before, containers with additional multiple pocket types, for example rc_car or multi_cooker, still had all their standard pockets unloaded as a side-effect. This included unloading batteries.

Then pr #53250 changed this to only unloading CONTAINER pockets (which makes sense):

@@ -9897,7 +9897,7 @@ bool Character::unload( item_location &loc, bool bypass_activity )
         }

         int moves = 0;
-        for( item *contained : it.all_items_top() ) {
+        for( item *contained : it.all_items_top( item_pocket::pocket_type::CONTAINER, true ) ) {
             moves += this->item_handling_cost( *contained );
         }
         assign_activity( player_activity( unload_activity_actor( moves, loc ) ) );

However, batteries are now no longer unloaded, causing #55481 and #54330.

The fix in this pr restores the old behaviour by explicitly unloading all standard pocket types of a container using the function introduced in pr #53250, maintaining the functionality of the pr.

Describe alternatives you've considered

There could probably be some better refactoring of character::unload, but I feel this changes the code as little as possible avoiding regressions.
There could be unintended side-effects with the originial behaviour, for example if a new gun is added which has a snack compartment. This gun would be treated as container and mods with magazines could no longer be unloaded. But until then this should be fine 馃槂

Testing

batteries of radio_car or multi_cooker could not be unloaded
fixes CleverRaven#55481, fixes CleverRaven#54330
@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 Jun 9, 2022
src/activity_actor.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
@dseguin dseguin merged commit 3091c9f into CleverRaven:master Jun 19, 2022
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.

Unable to unload RC Car Multi Cooker Battery cannot be changed
2 participants