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 tilling #25125

Merged
merged 7 commits into from Aug 26, 2018

Conversation

3 participants
@Robik81
Copy link
Contributor

commented Aug 25, 2018

Summary

SUMMARY: Features "adds new Farm: Plot zone and an activity to till it"

Purpose of change

As outlined in #24826 , this PR aims at reducing tedium of farming. Tilling is first activity for Farm: Plot zone, planting etc. will follow in separate PRs.

Describe the solution

  1. Designate new zone of Farm: Plot type in Zone manager.
  2. Perform Zone activity - formerly Sort out loot, was renamed to Zone activities (shortcut Shift+O).
  3. Choose Till farm plots, if more than one activity is available.
  4. Watch your character do all the work automatically.

Additional context

Also contains code infrastructure for future fixing of #25116

Robik81 added some commits Aug 13, 2018

player destination activity
you can set activity to start after the player auto moves to his destination
support for starting till plot activity
and other futer activities from one shortcut
keybinding rename
Sourt out loot -> Zone activities

as it is now used for more zone related activities, not just for loot sorting
gorgon fix
to make compiler happy
@Brambor

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2018

What you do is amazing. Haven't had time yet to test automatic-sorting #24488 and I don't want to get into compiling, so haven't tested that yet as well, but it sounds amazing!

@Brambor

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2018

Btw, this should resolve all the "hoe is now worse since it can dig" type of issues, if there are any.

@ZhilkinSerg ZhilkinSerg self-assigned this Aug 26, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2018

Looks really great :)

@ZhilkinSerg ZhilkinSerg merged commit a58b380 into CleverRaven:master Aug 26, 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 increased (+0.03%) to 24.18%
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Aug 26, 2018

@Robik81

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2018

Damn, you people are on the roll. That was what, 50 PRs merged in last two days or something?

@Brambor Thank you, I sincerely hope the feature will serve well all aspiring farmers in this great game.

@Robik81 Robik81 deleted the Robik81:farm_till branch Aug 27, 2018

return;
} else { // we are at destination already
p->add_msg_if_player( _( "You churn up the earth here." ) );
p->moves = -300;

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Aug 30, 2018

Contributor

I believe this line should be:

p->moves -= 300; - i.e. subtract 300 from player moves.

Now it sets player moves to -300 which makes following check (if( p->moves <= 0 ) {) redundant as this expression will always be true.

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 30, 2018

Author Contributor

Yeah, I think you are correct, that was my intended behavior... not that I came with the formula, I stole these 3 lines from iuse::makemound and I did not check them properly I guess. I think it should be changed in both places. I would normally not duplicate the code, but I did, as this was only 3 lines.

Should I make a PR to add a cost to game_constants.h and use the value in both places? I think it is better than making extra function with just 3 lines of code.

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Aug 30, 2018

Contributor

No new function is necessary right now and you can simply change values in existing places.

Btw, you can also use following syntax: p->mod_moves( -300 );

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 30, 2018

Author Contributor

Fix is in #25253 , tested both manual and automatic tilling.

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.