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

Preserve ammo linkage on zone unload #62543

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

robob27
Copy link
Contributor

@robob27 robob27 commented Dec 3, 2022

Summary

Bugfixes "Preserve ammo belt linkages when unloading with zones"

Purpose of change

Fixes #60310

Describe the solution

As a quick fix for stable, and as suggested by @RenechCDDA, copies the code that is responsible for preserving ammo belt linkages from unload_activity_actor::unload (the player manually unloading) to other code that is responsible for unloading using zones so that the linkages will also be preserved there. An ideal fix would be more DRY.

The copied code is from:

// remove the belt linkage
if( it.is_ammo_belt() ) {
if( it.type->magazine->linkage ) {
item link( *it.type->magazine->linkage, calendar::turn, qty );
if( who.add_or_drop_with_msg( link, true ) ) {
actually_unloaded = true;
}
}

I made some small modifications to spawn the linkages on the tile the unloaded item is on and not show the disassembly message so it doesn't spam them if many belts get unloaded back to back.

Describe alternatives you've considered

Testing

  • Spawn some ammo belts
  • Put them in an unload zone
  • Unload them using SHIFT+O
  • Check that the ammo linkages don't disappear and that the number of linkages created = the number of ammo unloaded
  • The linkages should be unloaded on the tile that the ammo belt was on
  • The ammo belt should be removed if it's empty

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. <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 labels Dec 3, 2022
@robob27 robob27 force-pushed the preserve-ammo-linkages branch 3 times, most recently from 6686d54 to 4a116aa Compare December 3, 2022 11:28
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 3, 2022
@robob27
Copy link
Contributor Author

robob27 commented Dec 3, 2022

After I got the test setup somewhat properly it didn't fail a single time locally but of course it fails in CI and I can't replicate the failure. It seems like after

complete_activity( dummy, unload_loot_activity_actor() );

the items are not always available on the map right away, so it can fail. Not sure what the best fix is for that but it's probably real simple. I gotta sleep but this gets this issue most of the way there, I think!

tests/clzones_test.cpp Outdated Show resolved Hide resolved
tests/clzones_test.cpp Outdated Show resolved Hide resolved
@robob27 robob27 force-pushed the preserve-ammo-linkages branch 2 times, most recently from 4bb75a9 to 0dba352 Compare December 3, 2022 23:23
@robob27
Copy link
Contributor Author

robob27 commented Dec 3, 2022

The test is still passing 100% of the time locally and failing 100% of the time on the "GCC 9, Curses, LTO" check (I assume my latest commit won't change this). I don't understand 🙃. Otherwise, everything seems to be working as described in the testing steps. I'll probably have to leave this as is, I'm out of ideas for now. Please feel free to ping me here or on the discord if anyone has any suggestions to fix the test, or drop it or whatever works best for stable release 🚀

tests/clzones_test.cpp Outdated Show resolved Hide resolved
@robob27 robob27 force-pushed the preserve-ammo-linkages branch 2 times, most recently from f8af4bc to ab71d4a Compare December 4, 2022 18:34
@robob27
Copy link
Contributor Author

robob27 commented Dec 4, 2022

Current failure is from an unrelated test, I think this is probably good to go.

@dseguin dseguin merged commit bd43f7b into CleverRaven:master Dec 6, 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` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using the unload zone on ammo belts destroys the linkages.
4 participants