-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
NPC activity rework - farming and generic multi-activity loop #32730
Conversation
This is awesome. Thank you for taking initiative on it! |
There was some issue I discovered with a farming task to fetch a shovel, with the loot zone being very near the work spot , meaning that spot was discounted, like it should be for construction tasks. So nothing was fetched. also enacted some clang tidy suggestions, but not all, the recommendation about using a reference on the requirement id , im not totally sure about yet, will test further. |
This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there: https://discourse.cataclysmdda.org/t/npc-faction-camp-issues/21020/4 |
infinite loop fix first pass at planting git planting working partially completed fetch_seeds testing seed collecting everything working from talk function astyle and remove some auto etc woops edited in error removed debug messages removed debug include another debug include remove another debug message clangtidy unitiliazed usage travis errors fixed unused variables and lambda captures more clang tidy checks for travis some more clang tidy recommendations gave default initialization to quality ID replaced 'using map' typo test path fix astylin Update src/activity_item_handling.cpp Co-Authored-By: ZhilkinSerg <[email protected]> Update src/activity_item_handling.cpp Co-Authored-By: ZhilkinSerg <[email protected]> Update src/iuse.cpp Co-Authored-By: ZhilkinSerg <[email protected]> astylin silly commit to trigger rebuild typo from review: early return, remove unnecessary itype_id conversions common dangerous field function astyle from review: extraneous comment, shorten lines typo I must have changed add_field() in error from review : removing unnecessary variables and stopping potential infinite loop etc remove NPC path fix merge error partially restructure generic_mutli_act get_zone_set_not() change name of enum to not clash added dig requirement and construction requirement searching finished main loop for construction/farming, started fetch loop removed old fetch activities started on fetch items loop - unpacking requirements added find zones from requiremnts, exclude nopickup zone seperated out the set-from-requirement function plotting points to fetch components from requirements is tricky added fetching - some problems to be iorned out still got multi-farming working first stage of a more generic function to build a mental map of area requirements got construction/fetching working fixed some infinite loops starting on tidy up task got tidying up working folded move_loot activity into new generic loop structure astyle and debug message removal remove requrements.cpp change from review :remove pointer usage, use item weight/volume instead of type->volume/weight clang tidy warning used unitialized fix for farming task fetch near work spot ensure no traps on target tile before adding partial_con trap marker remove const-ness to satisfy clang-tidy which wants consty things to be references? ( which dosnt work ) Update activity_item_handling.cpp take by reference. clang-tidy ignore comments
Tried to start a construction (brick wall) and immediately crashed, I had merged about 20 other PRs at the same time, so it might be because of something else, but yea it's probably this. |
will look into it. |
can't reproduce with player manually building a brick wall, or NPC doing so. |
edited in error
…/Cataclysm-DDA into npc_activity_rework
Summary
SUMMARY: Features "NPC activity rework - farming and generic multi-activity loop"
Purpose of change
Adds a generic multi-activity function that other activities can be plugged into without much duplication.
Describe the solution
In the previous incarnation of this, #31696, I basically just copied the structure of the other activity loops, and did a custom fetching loop to get tools required and so on.
Bevapdin reviewed and correctly suggested that there is too much duplication and a generic fetch_required activity could be used for a number of different activities.
So here it is.
The loop goes like this :
5.a if its a fetch task thats started, go back to 1, search in loot zones for the required items, build a mental map of where all these items are, and how much of each to get from each tile.
The practical upshot of this, and what ive rolled into it so far, is construction , farming, fetching, and loot sorting.
Now, the NPCs do not need the components placed near the construction blueprint, they can go and fetch them from nearby loot zones, they will go and fetch seeds, shovels etc for farming tasks, and place the remainder back afterwards.
This will make it much easier going forward to enable NPC crafting, NPC butchery, log chopping , any thing you can think of, because I can just insert the applicable conditions into the loop, and the requirements for what the NPC needs, and theyll go and fetch and do it.
This will also make it easier to enable a future NPC "job " system, where they periodically scan for things that are possible to do.
Describe alternatives you've considered
Additional context
Current limitations - I tried to test everything I could, though its certainly possible there are edge cases where certain constructions might cause weirdness.
I think I managed to avoid all potential cases of infinite looping, though this kinda thing has a lot of potential for infinite looping to occur, it should fail gracefully and just stop activity, when things arent possible though.
this has a follow-on PRs that relies upon this one.
#33091
A little gif of the NPC getting requirements for a construction from seperate loot tiles.
https://gfycat.com/whichadoredindianelephant