optimize sorting loot activity #79229
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If this commit is applied it will prevent PC running to sorted zones. It was made because PC was running across the map when there was not items to move
| bool unload_corpses = false; | ||
|
|
||
| std::vector<zone_data const *> const zones = mgr.get_zones_at( src, zone_type_UNLOAD_ALL, | ||
| _fac_id( you ) ); |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| _fac_id( you ) ); | |
| _fac_id( you ) ); |
Comment on lines
+2119
to
+2120
| for( zone_data const *zone : zones ) | ||
| { |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| for( zone_data const *zone : zones ) | |
| { | |
| for( zone_data const *zone : zones ) { |
Comment on lines
+2137
to
+2139
| if( vp ) | ||
| { | ||
| for( const item& it : vp->items() ) { |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| if( vp ) | |
| { | |
| for( const item& it : vp->items() ) { | |
| if( vp ) { | |
| for( const item &it : vp->items() ) { |
Comment on lines
+2142
to
+2144
| } else | ||
| { | ||
| for( const item& it : here.i_at( src_loc ) ) { |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| } else | |
| { | |
| for( const item& it : here.i_at( src_loc ) ) { | |
| } else { | |
| for( const item &it : here.i_at( src_loc ) ) { |
Comment on lines
+2151
to
+2152
| for( const item * it : items ) | ||
| { |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| for( const item * it : items ) | |
| { | |
| for( const item *it : items ) { |
Comment on lines
+2221
to
+2222
| if( !has_items_to_work_on ) | ||
| { |
There was a problem hiding this comment.
[JSON & C++ formatters] reported by reviewdog 🐶
Suggested change
| if( !has_items_to_work_on ) | |
| { | |
| if( !has_items_to_work_on ) { |
|
Why close? |
|
@Zireael07 I need to update master first |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If this commit is applied it will prevent PC running to sorted zones.
It was made because PC was running across the map when there was no items to move
Summary
Bugfixes "Sorting loot: stop player running to items that's not going to be moved"
Purpose of change
Player or NPC were travelling all across the map to unsorted loot zone with items that does not have a valid destination stockpile. This was annoying and time consuming. NPCs were perceived as being dumb
Describe the solution
The issue was there because we did not check in the "THINK" stage of activity if unsorted zone tile has any items that require action. I copied all the logic from the "DO" stage carefully to predict if we need to run to the source tile at all.
Describe alternatives you've considered
I thought about extracting the logic to separate function instead of mimicking it but it would require a full rewrite of the DO stage. And I'm not yet comfortable with tasks of this size.
Testing
I did the ./tests/cata_test and a lot of manual testing to prevent any regressions:
I checked cases with disabling source and target zones, stripping corpses, unloading containers, custom loot target zones, removing mods from a gun
Additional context
After this change NPC will think better about his destination and stop wasting his time running between stockpiles. This makes him 'smarter' for the player.