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

faction camp: menial labor uses the zone autosort feature #28571

Merged
merged 5 commits into from
Mar 25, 2019

Conversation

mlangsdorf
Copy link
Contributor

@mlangsdorf mlangsdorf commented Mar 8, 2019

Summary

SUMMARY: Features "faction camp: menial labor uses the zone autosort feature"

Purpose of change

Fixes #25397
Fixes #28880
Fix faction camp menial labor sorting by making it use the existing zone autosort feature. As a side effect, add the infrastructure for NPCs performing player activities on an ongoing basis.

Describe the solution

Add a function to clzones to determine if there are any loot zones nearby.

Alter the npc *_destination functions so it is clear that deal with omt destinations.

Add infrastructure to npcmove to let friendly NPCs perform player activities using the NPC_MISSION_ACTIVITY mission. NPCs gain bonus moves from elapsed time during npc::on_load() if they have an activity, and lose those bonus moves when the activity is complete and their activity backlog is empty

Update do_turn_move_loot to support NPCs instead of the player performing the activity. Handle a weird NPC only corner case where the NPC is still active but not in the bounds of map by unloading them.

Change validate_sort_points to check if there is an unsorted loot zone, sorted loot zone, and camp food zone nearby. Change the Distribute Food pseudo mission to take food from the camp food zone. Change the Menial Labor mission to have the NPC perform the loot sorting activity using the infrastructure set up earlier in this PR.

Describe alternatives you've considered

Leaving the NPC in the overmap buffer and copy-pasting the do_activity_move_turn code into menial_return is easier but not the best solution.

Additional context

@mlangsdorf mlangsdorf added <Enhancement / Feature> New features, or enhancements on existing NPC / Factions NPCs, AI, Speech, Factions, Ownership [C++] Changes (can be) made in C++. Previously named `Code` Player Faction Base / Camp All about the player faction base/camp/site labels Mar 8, 2019
@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch from 6409278 to ec6d52a Compare March 9, 2019 18:21
@mlangsdorf
Copy link
Contributor Author

Current status: the NPC does some sorting, taking up time and freezing PC activity. I don't think that's right.

I think the issue is that activity_on_turn_move_loot is doing too much per cycle. I want it to do exactly 1 turn's worth of work per invocation, and I don't think it's doing that. I think it's sorting one entire tile and then getting invoked again.

@ifreund
Copy link
Contributor

ifreund commented Mar 9, 2019

The problem is that move_item here modifies the player's moves not the npc's, so this check never succeeds

Also, letting npcs sort may cause some strange behavior if the player defines new zones during sorting. I'm pretty sure that because of the caching any new LOOT_UNSORTED zones will simply be ignored, and new destination zones will be properly sorted to if they are closest, but it would be good to investigate.

Edit: Looks like new unsorted zones will be sorted from but not make use of the caching. zone_manager::get_num_processed() could be modified to add any new zones to the cache.

@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch 4 times, most recently from 82e99c7 to f96cf58 Compare March 11, 2019 00:50
@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch 3 times, most recently from e20e588 to e7b01b1 Compare March 16, 2019 14:47
@mlangsdorf
Copy link
Contributor Author

@ifreund if you have a chance, please review or playtest. thanks!

Copy link
Contributor

@ifreund ifreund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, still need to fetch and test though.

Changing zone_manager::get_num_processed() to

int zone_manager::get_num_processed( const tripoint &src )
{
    auto it = num_processed.find( src );
    if( it != num_processed.end() ) {
        return it->second;
    }
    // A zone was added while sorting, so add it to the cache.
    return num_processed[src] = 0;
}

should fix the minor caching issue I brought up.

Edit: Need to de-constify get_num_processed then of course.

@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch from e7b01b1 to 57284ed Compare March 16, 2019 17:41
@ifreund
Copy link
Contributor

ifreund commented Mar 16, 2019

Shoot, somehow it took me until now to realize that this actually totally breaks caching if the player decides to remove items from the unsorted zone. What we need to do is clear the cache then restart the activity one extra time the first time we think we are done. There's a low chance that could still leave some items unsorted, but I think that's not a huge deal.

@mlangsdorf
Copy link
Contributor Author

I will address your caching concerns tomorrow when I am slightly less brain-fried.

@mlangsdorf mlangsdorf changed the title [WIP] faction camp: menial labor uses the zone autosort feature faction camp: menial labor uses the zone autosort feature Mar 16, 2019
@ifreund
Copy link
Contributor

ifreund commented Mar 16, 2019

Alright just to flesh out what I said about caching a bit:

  • Adding items to a zone while sorting is no problem, as items are added to the top and sorting checks from bottom to top in order.
  • Removing items that had been cached as unsortable will cause an equal number of sortable items on that tile to be skipped.
  • The most complete fix is to always restart the activity using mgr.start_sort() and mgr.end_sort() to clear the cache unless we iterated through all the items and reached the end of the sort without restarting the activity. If that happens then all remaining items are unsortable and we have completed the activity.

Hopefully that all makes sense.

@ZhilkinSerg ZhilkinSerg self-assigned this Mar 18, 2019
@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch from 57284ed to 7129f0c Compare March 22, 2019 23:53
@mlangsdorf mlangsdorf changed the title faction camp: menial labor uses the zone autosort feature [WIP] faction camp: menial labor uses the zone autosort feature Mar 23, 2019
mlangsdorf and others added 4 commits March 23, 2019 09:20
Add a function to check that there is at least one zone nearby that is
a loot destination.
player:: uses has_destination to indicate a local destination while
npc:: uses has_destination to indicate an omt destination.  This is
confusing, so rename the npc:: functions to *_omt_destination.
Add support for allied NPCs performing player_activity by calling
activity.do_turn() and having the ability to navigate through
activity destinations.

NPCs that are unloaded while performing a player_activity get
the delta time bonus moves when they are reloaded.  They can
then perform their player_activity backlog is empty.  When they
have completed all player_activity, their remaining moves are
zeroed and they return to normal operation.
pass player &p to the underlying functions and adjust that player's moves,
not g->u.  Handle the new corner case of player &p not being inbounds
of g->m, which can't happen if p is g->u but can happen if it is an NPC.

Some minor incidental cleanups of the code.
Get rid of the confusing sort point interface and replace it with the
slightly better supported zone interface.  Have NPCs assigned to
menial labor stay on the map and sort loot in real time.  Adjust the
distribute food mission to use a new zone "camp food."
@mlangsdorf mlangsdorf force-pushed the faction_camp_sort_fix branch from 7129f0c to d5c53a8 Compare March 23, 2019 14:41
@mlangsdorf mlangsdorf changed the title [WIP] faction camp: menial labor uses the zone autosort feature faction camp: menial labor uses the zone autosort feature Mar 23, 2019
@mlangsdorf
Copy link
Contributor Author

jenkins rebuild

@mlangsdorf
Copy link
Contributor Author

Resolved the caching issue by calling end_sort() on any exit from the function.

Added logic to handle the NPC leaving the reality bubble by unloading them and then giving them a lot of bonus moves when they're loaded back into the reality bubble. It seems to work in my tests but could use more testing.

@kevingranade kevingranade merged commit e1bafee into CleverRaven:master Mar 25, 2019
@ZhilkinSerg ZhilkinSerg removed their assignment Mar 25, 2019
@mlangsdorf mlangsdorf deleted the faction_camp_sort_fix branch March 25, 2019 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing NPC / Factions NPCs, AI, Speech, Factions, Ownership Player Faction Base / Camp All about the player faction base/camp/site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants