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

map: update active items cache when removing contents #62916

Conversation

andrei8l
Copy link
Contributor

@andrei8l andrei8l commented Dec 28, 2022

Summary

None

Purpose of change

Removing active items from an otherwise inactive container does not properly update the active item cache because the container's processing speed changes.

Fixes the place_player_can_safely_move_multiple_submaps failure that has been plaguing CI jobs lately, for example:
https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/3793690000/jobs/6451047981#step:17:349
https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/3789774728/jobs/6445913140#step:17:305
https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/3773072901/jobs/6414342348#step:17:264

Describe the solution

Update the active item cache when removing items with item_location::impl::item_in_container::remove_item() (used by shop restock code, for example)
If the container of the item-to-be-removed isn't active (anymore), remove it from all active queues.

Describe alternatives you've considered

N/A

Testing

New test unit

Also ran while :; do ../build_lto/cata_test --rng-seed time --min-duration 0.2 --use-colour yes "npc does healing",place_player_can_safely_move_multiple_submaps;ret=$?;if [[ "$ret" != '0' ]];then break;fi;done for a couple of hours with no issues

Additional context

The test unit place_player_can_safely_move_multiple_submaps is really bad because it doesn't set up anything and can only catch issues by happenstance. However, it caught this (almost) completely unrelated issue so I'll leave it alone for now. I've only updated it a little bit so it's a bit more clear what's failing.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Dec 28, 2022
@andrei8l andrei8l force-pushed the inactive_container_with_active_contents branch 3 times, most recently from bc77ca8 to dd21eac Compare December 28, 2022 14:12
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 28, 2022
@andrei8l andrei8l marked this pull request as ready for review December 28, 2022 17:56
@andrei8l andrei8l force-pushed the inactive_container_with_active_contents branch from dd21eac to 2970e33 Compare December 28, 2022 17:56
@Rivet-the-Zombie Rivet-the-Zombie merged commit 0eea824 into CleverRaven:master Dec 31, 2022
@andrei8l andrei8l deleted the inactive_container_with_active_contents branch December 31, 2022 10:16
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 [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.

2 participants