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

Make mining less tedious, part 2 #24787

Merged
merged 16 commits into from Aug 26, 2018

Conversation

@Rail-Runner
Copy link
Contributor

commented Aug 11, 2018

  1. Changed all three ways of mining (burrowing, pickaxes and jackhammers) to be possible to do via walk-and-mine feature.
  2. Burrower mutation was changed to be toggleable: while it's on, it is used to mine/till while moving. Had to remove manual activation to burrow into a single tile. Alternatively, player can activate specific actions instead of toggling dig mode.
  3. Added MINEABLE flag to most terrain and furniture that is tough enough to be unlikely to be broken by hand.
  4. Allowed Burrower mutation to be a substitute for having a pickaxe and a shovel, for all purposes. I had to rewrite everything related to rubble removal, but in fact it seems to be better now.

The only issue I found with this is that once player finishes burrowing (using a mutation), they are queried about whether they should stop or not, because of pain caused by digging.

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Aug 11, 2018

Allowed Burrower mutation to be a substitute for having a pickaxe and a shovel. I had to bring back pit-digging constructions to do that.

So we'll have two ways of digging pits? One, which takes digging quality of the tool into account, and another, which takes fixed 30 minutes no matter if this is performed by a digging stick or by a shovel. What's the point of this?

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2018

It's supposed to be a way for burrower mutants to dig pits. I couldn't figure out a better way to do it.

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Aug 11, 2018

Then I suppose it's better not to allow burrowers dig pits rather than bring back digging pits through the construction menu.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2018

Then I suppose it's better not to allow burrowers dig pits rather than bring back digging pits through the construction menu.

I agree. Digging through item action is way better.

What we can do for Burrowiers is some way to activate mutation and give some pseudo item with DIG quality which can be activated like shovels and other tools. Something similar to Monomolecular blade CBM.

@@ -883,7 +883,8 @@ Melee flags are fully compatible with tool flags, and vice versa.
- ```BOMB``` It can be a remote controlled bomb.
- ```CABLE_SPOOL``` This item is a cable spool and must be processed as such. It has an internal "state" variable which may be in the states "attach_first" or "pay_out_cable" -- in the latter case, set its charges to `max_charges - dist(here, point(vars["source_x"], vars["source_y"]))`. If this results in 0 or a negative number, set its state back to "attach_first".
- ```CHARGEDIM``` If illuminated, light intensity fades with charge, starting at 20% charge left.
- ```DIG_TOOL``` If wielded, item is automatically activated when player tries to move onto map tile with ```MINEABLE``` flag.
- ```DIG_TOOL``` If wielded, digs thorough terrain like rock and walls, as player walks into them.
- ```DIG_TOOL_POWERED``` If wielded, digs thorough terrain like rock and walls, as player walks into them. Faster than ```DIG_TOOL``` and uses up power.

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 11, 2018

Contributor

It might be better to split the POWERED flag to be separate. And it already exists, it was merged with #24634

It is not listed in the JSON_FLAGS.md, which is my fault, I did not realize there is extra doc for existing flags...

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 11, 2018

Contributor

I made a PR which adds POWERED flag into JSON_FLAGS.md #24789

This comment has been minimized.

Copy link
@Rail-Runner

Rail-Runner Aug 12, 2018

Author Contributor

Not sure, but this is most likely a different thing.

This comment has been minimized.

Copy link
@Robik81

Robik81 Aug 12, 2018

Contributor

I use it to differentiate between tools that have AXE quality but are not powered, like a wood axe, and tools that have AXE quality and are powered, like a chainsaw.

The only difference is that chainsaw has 2 states (or rather 2 items), one for chainsaw turned off, and one for chainsaw turned on. Since jackhammer has only one state - it could be considered as permanently on.

Instead of having "DIG_TOOL_POWERED" flag, you can give the jackhammers 2 flags, that is "DIG_TOOL", like pickaxe has, and "POWERED" on top of it.

In your condition, instead of testing
u.weapon.has_flag( "DIG_TOOL_POWERED" )
you would use
u.weapon.has_flag( "DIG_TOOL" ) && u.weapon.has_flag( "POWERED" )

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Aug 25, 2018

Contributor

I agree that it would be a more clean approach to use single POWERED flag.

Rail-Runner added some commits Aug 12, 2018

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2018

Done. Bringing back pit-digging constructions is unnecessary now.

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Aug 12, 2018

I see you copy-pasted the dig pit, tamp ground, till soil code to the burrower mutation code? I'd say this is the wrong approach, as duplicated pieces of code are prone to be forgotten when one piece of code is updated. Have you tried directly invoking iexamine::dig and all other corresponding iexamines in the burrower mutation code?

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2018

I tried, but couldn't get this to work.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Aug 14, 2018

Please remove the walk-to-till feature, this needs to be a big batch job, not slightly less micromanagement by auto-tilling while walking. The mining is ok because we're almost certainly never going to have df-style mining designation so that's probably as good as it gets.

In general, if your PR title says "something about mining", please resist the urge to add unrelated features like tilling.

@cainiaowu

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2018

While the PR could change its name to fit its contents more, I would like to point out that balancing though player tedium is not a really good way to do game balance.
Instead of using more keypress we can consider other factor like stamina cost/ingame time cost.

@Mecares

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2018

That is a shame tilling takes since the change to the hoe far too much keypresses. Tilling a decently sized field is now a chore for the player and not the ingame character.

Rail-Runner added some commits Aug 14, 2018

Revert "Resolve conflicts"
This reverts commit a6b2ce3.
@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

Can you resolve conflicts?

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2018

I tried to do that, but messed up the branch because the conflict is too large. How do I do that properly in this case?

Rail-Runner added some commits Aug 24, 2018

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2018

Seems like it's fixed now.

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Aug 24, 2018

@Mecares you can ask @Robik81 to add his fantastic zone-tilling feature from his Hybrid Inventory mod.

Rail-Runner added some commits Aug 25, 2018

@Rail-Runner

This comment has been minimized.

Copy link
Contributor Author

commented Aug 25, 2018

Done. Jackhammers use POWERED flag now.

@ZhilkinSerg ZhilkinSerg self-assigned this Aug 26, 2018

@ZhilkinSerg ZhilkinSerg merged commit a8e08d9 into CleverRaven:master Aug 26, 2018

0 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gorgon-ghprb Build triggered for merge commit.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Aug 26, 2018

@Rail-Runner Rail-Runner deleted the Rail-Runner:mining2 branch Aug 26, 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.