-
Notifications
You must be signed in to change notification settings - Fork 0
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
Rifle #2
base: master
Are you sure you want to change the base?
Rifle #2
Conversation
First, make sure all unit tests pass. PrecisionShotCooldown test is failing. |
} | ||
if (a.hasUsedAP()) { | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this unnecessary block
Please also setup checkstyle to catch the issues found on the line comments. I know that some of my code has checkstyle issues too (will be fixed in a future commit), but let's not add more when we can keep it maintainable fairly easily. |
Fixed the unit test issue. Forgot to modify after adding the weapon requirement check. |
Let's get rid of those 3 commits by squashing them. |
import com.pipai.wf.exception.IllegalActionException; | ||
import com.pipai.wf.unit.ability.SnapShotAbility; | ||
|
||
public class RifleAttackAction extends RangedWeaponAttackAction{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curly brace
Please set your eclipse formatter to wf_formatter.xml |
Because of the recent Configuration, Dependency Injection, and Test Suite Reorganization refactor, there may be some serious merge conflicts. During your next rebase to master, please make sure everything is correct (especially your Ability test needs to be moved to a different file, probably SnapShotAbilityTest.java in the com.pipai.wf.unit.ability package) |
Implemented Rifle class. Please review.