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

Zone activity - farm plot harvesting #27278

Merged
merged 11 commits into from Jan 5, 2019

Conversation

Projects
None yet
3 participants
@prutschman
Copy link
Contributor

commented Dec 23, 2018

Summary

SUMMARY: Features "Adds Zone Activity to harvest plots"

Purpose of change

Addresses a farming QoL issue from #24826

Describe the solution

I refactored the harvesting code from iexamine::aggie_plant into its own function so it could also be called from a zone activity. The implementation for ACT_HARVEST_PLOT ended up sharing a lot of similarities with ACT_TILL_PLOT and ACT_PLANT_PLOT, so I extracted the common code into a driver function, including the pathing logic.

@mlangsdorf

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2018

Can you add fertilizing?

@prutschman

This comment has been minimized.

Copy link
Contributor Author

commented Dec 23, 2018

Fertilization is a tad more complicated, since I'll need to prompt the player for a fertilizer type. It's next on my list.

@prutschman

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

Okay. Fertilization now works. If you only have one fertilizer type, it will use that. If you have more than one fertilizer type, you are prompted for which to use. If you run out of that type, it will ask you if you have more than one type remaining, otherwise it automatically picks.

There are only two item types tagged "FERTILIZER" at the moment. (I did test the above logic by wishing for explicitly fertilizer-tagged objects, so I did confirm that it would prompt if there are two types remaining). If I'm going to bother to ask the user which one to use, it seems like I should maybe give the user the chance to not use up whichever one they didn't pick.

Alternately, maybe it doesn't make sense to ask the user which one to use?

There are also getting to be a fair number of helper functions. I'm not sure at what point it might make sense to break some of this off into its own file/namespace.

@prutschman prutschman referenced this pull request Dec 26, 2018

Open

Farming plot zone - QoL improvement features #24826

4 of 9 tasks complete
@kevingranade

This comment has been minimized.

Copy link
Member

commented Dec 26, 2018

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/hybrid-inventory-2018-12-26/15953/76

@prutschman

This comment has been minimized.

Copy link
Contributor Author

commented Jan 3, 2019

It looks like the mention on discourse was just thanking me for working on it (much appreciated!) but no specific feedback.

Are there changes people would like to see before merging this?

@kevingranade kevingranade merged commit c1b26b1 into CleverRaven:master Jan 5, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
gorgon-ghprb Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.