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 planting #25181

Merged
merged 16 commits into from Aug 31, 2018

Conversation

4 participants
@Robik81
Copy link
Contributor

commented Aug 27, 2018

Summary

SUMMARY: Features "adds new option for Farm: Plot zone to attach specific seed and new activity to plant in nearby plot zones"

Purpose of change

As outlined in #24826 , this PR aims at reducing tedium of farming. Follow up of #25125 . Planting activity for Farm: Plot zone. Watering, weeding and fertilizing will follow in separate PRs.

Describe the solution

  1. Select seed for existing Farm: Plot zone (via Edit->Edit options in zone manager) or select seed while creating a new one
  2. After Tilling farm plots activity, perform Plant seeds activity (shortcut Shift+O).
  3. Watch your character do all the work automatically.

Update

Watering, weeding and fertilizing will follow if / when the improved crop farming is implemented again.

@Robik81 Robik81 force-pushed the Robik81:farm_plant branch Aug 27, 2018

@Vasyan2006

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2018

I'm still planning to separate >Watering, weeding and fertilizing into new functions, so you will be able to use your tumba-jumba zone magic and apply them over every tile. I suppose it will be faster to commit such changes into your branch rather then opening new PR.

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2018

I suppose it will be faster to commit such changes into your branch rather then opening new PR.

I think if you'l make tiny PR only with refactoring of a function into two, @ZhilkinSerg can review and merge something like this quite fast?

src/iexamine.cpp Outdated

// if true, it works fine for normal planting, but closes immediately if called
// from parent uimenu when adding new planting zone
smenu.return_invalid = false;

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 27, 2018

Author Contributor

What comment says.

This comment has been minimized.

Copy link
@Qrox

Qrox Aug 28, 2018

Contributor

I guess it is because there's a timeout set in the input manager (for blinking), so when the timeout is triggered in the submenu, it thinks it is an unbound key, and returns because return_invalid is true.

See here:
https://github.com/CleverRaven/Cataclysm-DDA/blob/c27cf5e2335d55cb13cf97e8b47b485b51b6c5d2/src/game.cpp#L8420

I'll let uimenu handle it properly in my PR. For now you may want to temporarily disable the timeout before calling up the submenu.

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 28, 2018

Author Contributor

Your guess was correct. That was indeed a problem.

Thank you very much for helping me out.

(I see you are going to handle it on the uimenu side, cool.)

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2018

#25204 is merged

@Robik81 Robik81 force-pushed the Robik81:farm_plant branch Aug 28, 2018

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2018

Thank you guys.

On the weeding I go... it will be in separate PR though, this one is big enough as it is.

Robik81 added some commits Aug 14, 2018

subtype -> zone_options
much easier to add more than one extra option to zones this way
zone marks
when zone manager is open, zones which support marking show selected mark in its tiles
refactoring of iexamine::dirtmound
break the code into multiple functions to allow calling the same code from outside
IMarkOption -> mark_option
while I prefer distinct naming convention for interfaces, it looked incredibly odd with naming conventions in this project
gorgon fix
gorgon fix #2

gorgon fix
safe mode check
to prevent possible infinite loop

@Robik81 Robik81 force-pushed the Robik81:farm_plant branch to 252883e Aug 29, 2018

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2018

Given that PR #24291 was rolled back, I updated this PR to work with the old planting system.

@ZhilkinSerg ZhilkinSerg self-assigned this Aug 29, 2018

g->m.add_item_or_charges( examp, used_seed.front() );
g->m.set( examp, t_dirt, f_plant_seed );
p.moves -= 500;
add_msg( _( "Planted %s" ), item::nname( seed_id ).c_str() );

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Aug 29, 2018

Contributor

Should probably add period after Planted %s

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

No feedback for player (in message log or popup) when Plant items action is selected, but plant zone is not defined or plant zone has no seeds selected in its options.

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2018

No feedback for player (in message log or popup) when Plant items action is selected, but plant zone is not defined or plant zone has no seeds selected in its options.

There is feedback if no compatible zone is nearby:
image

There is no feedback if you have only Unsorted loot zone defined but there is nothing to sort (i'l write it down as sort loot issue). You can't pick any farm related action in this case though.

If you have plant zone, but is not tilled, have no seed attached, or you have no seeds, there is also not any feedback. This one is probably the one you are referring to.

I'll fix the issue.

Robik81 added some commits Aug 29, 2018

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2018

New commits changes

  • extra dot in Planted %s message
  • action menu now checks if you have any seeds at all (planting disabled if not)
  • added message "You planted all seeds you could." after planting action is finished (whether you planted anything or not)
fix position of binary operator
damn you, astyling in Visual Studio

@ZhilkinSerg ZhilkinSerg merged commit 0db5290 into CleverRaven:master Aug 31, 2018

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 23.174%
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Aug 31, 2018

@Robik81 Robik81 deleted the Robik81:farm_plant branch Sep 2, 2018

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.