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

Rescue ability awarts unit's reward #86

Merged
merged 1 commit into from
May 13, 2018
Merged

Rescue ability awarts unit's reward #86

merged 1 commit into from
May 13, 2018

Conversation

RascalTwo
Copy link
Contributor

@RascalTwo RascalTwo commented May 13, 2018

Allow the rescue ability to award the number of points assigned to the unit being rescued.

Closes #64

@codecov-io
Copy link

codecov-io commented May 13, 2018

Codecov Report

Merging #86 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #86      +/-   ##
=========================================
- Coverage   90.41%   90.4%   -0.01%     
=========================================
  Files          80      80              
  Lines        1012    1011       -1     
  Branches      154     154              
=========================================
- Hits          915     914       -1     
  Misses         82      82              
  Partials       15      15
Impacted Files Coverage Δ
packages/warriorjs-abilities/src/rescue.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58629ad...4f0500e. Read the comment docs.

Copy link
Owner

@olistic olistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @RascalTwo! The changes look good. I just have one suggestion regarding the ability's description.

@@ -25,8 +25,8 @@ Rotate in the given direction (backward by default).

### `unit.rescue([direction])`:

Rescue a captive from his chains (earning `[rescuingReward]` points) in the
given direction (forward by default).
Rescue a captive from his chains (earning points) in the given direction
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that "earning a reward" is slightly better than "earning points".


function rescue() {
return unit => ({
action: true,
description: `Rescue a captive from his chains (earning ${rescuingReward} points) in the given direction (${defaultDirection} by default).`,
description: `Rescue a captive from his chains (earning points) in the given direction (${defaultDirection} by default).`,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem s/points/a reward/.

Allow the rescue ability to award the number
of points assigned to the unit being rescued.
@RascalTwo
Copy link
Contributor Author

Gotcha, description changed from earning points to earning a reward.

Copy link
Owner

@olistic olistic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks!

@olistic olistic merged commit 594712f into olistic:master May 13, 2018
@RascalTwo RascalTwo deleted the feature-rescue-variable-reward branch May 13, 2018 01:38
@olistic olistic changed the title Rescue ability awarts unit's reward (#64) Rescue ability awarts unit's reward May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants