feat: Added the Extra movement conditions #211
Conversation
Reviewer's Guide by SourceryThis pull request introduces new movement modes and condition-based movement logic. It allows the bot to use different movement strategies based on configurable conditions, enhancing its adaptability. The changes include the addition of 'RANDOM', 'GROUPVSSAFETY', and 'GROUPVS' movement modes, a system to prioritize movement modes based on a list of conditions, and integration of the new movement logic into existing modules. Sequence diagram for ExtraMovementLogic move methodsequenceDiagram
participant ExtraMovementLogic
participant SafetyFinder
participant MovementAPI
participant GroupAPI
participant HeroAPI
ExtraMovementLogic->ExtraMovementLogic: getMovementMode()
alt movementSelected == VSSAFETY or movementSelected == GROUPVSSAFETY
ExtraMovementLogic->SafetyFinder: tick()
alt tick() returns false
SafetyFinder-->>ExtraMovementLogic: false
ExtraMovementLogic->ExtraMovementLogic: break
else tick() returns true
SafetyFinder-->>ExtraMovementLogic: true
end
end
alt movementSelected == GROUPVSSAFETY or movementSelected == GROUPVS
ExtraMovementLogic->ExtraMovementLogic: getClosestMember()
alt groupMember != null
ExtraMovementLogic->GroupAPI: getMembers()
GroupAPI-->>ExtraMovementLogic: GroupMember
ExtraMovementLogic->GroupMember: getLocation()
GroupMember-->>ExtraMovementLogic: Location
ExtraMovementLogic->HeroAPI: getLocationInfo()
HeroAPI-->>ExtraMovementLogic: LocationInfo
ExtraMovementLogic->ExtraMovementLogic: groupMember.getLocation().distanceTo(hero)
alt distance < DISTANCE_TO_USE_VS_MODE
ExtraMovementLogic->ExtraMovementLogic: vsMove()
else distance >= DISTANCE_TO_USE_VS_MODE
ExtraMovementLogic->MovementAPI: moveTo(groupMember.getLocation())
end
else groupMember == null
ExtraMovementLogic->ExtraMovementLogic: vsMove()
end
else movementSelected == RANDOM
ExtraMovementLogic->MovementAPI: isMoving()
ExtraMovementLogic->MovementAPI: isOutOfMap()
alt !movement.isMoving() || movement.isOutOfMap()
ExtraMovementLogic->MovementAPI: moveRandom()
end
else movementSelected == VS
ExtraMovementLogic->ExtraMovementLogic: vsMove()
else default
ExtraMovementLogic->SafetyFinder: tick()
alt tick() returns true
SafetyFinder-->>ExtraMovementLogic: true
ExtraMovementLogic->ExtraMovementLogic: vsMove()
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
We encountered an error and are unable to review this PR. We have been notified and are working to fix it.
You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @dm94 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider extracting the condition checking logic into a separate utility class to improve readability and maintainability.
- The
DISTANCE_TO_USE_VS_MODEconstant should be configurable.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Review instructions: 3 issues found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| private Optional<Location> getDestination() { | ||
| Ship target = hero.getTargetAs(Ship.class); | ||
| if (target != null && target.isValid()) { |
There was a problem hiding this comment.
issue (bug_risk): Potential bug in getDestination condition check.
In the else branch, the condition is once again checking 'target' rather than the newly assigned 'other'. This appears to be a copy-paste mistake and may lead to incorrect behavior when hero.getTargetAs(Ship.class) is null but hero.getTarget() returns a valid entity.
| @Option("movement_config.condition") | ||
| public MovementWithConditions movementCondtion1 = new MovementWithConditions(); |
There was a problem hiding this comment.
nitpick (typo): Typo in property naming: movementCondtion1.
The field name 'movementCondtion1' appears to contain a typo. Standardizing the naming to 'movementCondition1' (and similarly for condition 2-5) would improve readability and reduce potential confusion in the future.
Suggested implementation:
public MovementWithConditions movementCondition1 = new MovementWithConditions(); public MovementWithConditions movementCondition2 = new MovementWithConditions(); public MovementWithConditions movementCondition3 = new MovementWithConditions(); public MovementWithConditions movementCondition4 = new MovementWithConditions();| MovementModeEnum movementSelected = getMovementMode(); | ||
|
|
||
| switch (movementSelected) { | ||
| case VSSAFETY: |
There was a problem hiding this comment.
suggestion (review_instructions): Consider clarifying the fall-through in the VSSAFETY case for better readability.
If the fall-through from VSSAFETY to VS is intentional, adding a comment (e.g., // fallthrough) would improve code clarity.
Review instructions:
Path patterns: *.java
Instructions:
It is a plugin for the darkbot bot in java, check that the code is clean and efficient.
| public MovementModeEnum defaultMovementMode = MovementModeEnum.VS; | ||
|
|
||
| @Option("movement_config.condition") | ||
| public MovementWithConditions movementCondtion1 = new MovementWithConditions(); |
There was a problem hiding this comment.
suggestion (review_instructions): Typo in variable name 'movementCondtion1' affecting code clarity.
Renaming 'movementCondtion1' to 'movementCondition1' (and similarly for the subsequent fields) would improve readability and maintainability.
Review instructions:
Path patterns: *.java
Instructions:
It is a plugin for the darkbot bot in java, check that the code is clean and efficient.
Summary by Sourcery
Adds new movement modes and condition-based movement logic. This allows the bot to use different movement strategies based on configurable conditions, enhancing its adaptability.
New Features: