update crease defender to perimeter step instead of using shotcones #3521
Conversation
annieisawesome2
requested review from
Andrewyx,
sauravbanna and
williamckha
as code owners
|
Resolved issues with previous tests with shot cones. Will work on more advanced tests for edge case angles. |
…2/Software into improve-find-block-threat
| Point primary_threat_position = enemy_threats.empty() | ||
| ? event.common.world_ptr->ball().position() | ||
| : enemy_threats.front().robot.position(); |
There was a problem hiding this comment.
How are enemy_threats generated? Is this an ordered list based on some threat level determined by some cost function? If so, does that imply that all enemy robots in play will be contained in this list?
There was a problem hiding this comment.
getAllEnemyThreats function in enemy_threat.cpp will create an EnemyThreat for each enemy robot. The list is sorted by decreasing threat order from sortThreatsInDecreasingOrder(). There are some super helpful comments of how we calculate the threat within the comments in the method. A quick summary is: robots with the ball are considered the most threatening and robots with better shot angles rank higher. For robots without the ball, those that can receive the ball in fewer passes are ranked higher. Finally, if the passing options are equal, the robot with the larger goal angle is considered more threatening.
To answer if all enemy robots in the play will be contained in the list, the answer is basically yes with one condition: if include_goalie is false, then we simply exclude the goalie.
Thanks for taking the time to review! Let me know if there are any other questions or things I should fix ;)
There was a problem hiding this comment.
It might be worth investigating a different robot tactic assignment when there is a spare crease defender:
Instead of reassigning the extra robot as a pass defender, make it a crease defender for the second largest threat, so that in the event of a pass we can cover the main issue opening really quickly.
There was a problem hiding this comment.
That is a great idea! I will look into it this weekend at our meeting :)
| Point primary_threat_position = enemy_threats.empty() | ||
| ? event.common.world_ptr->ball().position() | ||
| : enemy_threats.front().robot.position(); |
There was a problem hiding this comment.
It might be worth investigating a different robot tactic assignment when there is a spare crease defender:
Instead of reassigning the extra robot as a pass defender, make it a crease defender for the second largest threat, so that in the event of a pass we can cover the main issue opening really quickly.
| if (contains(field.friendlyGoal(), position.value())) | ||
| { | ||
| continue; // inside the goal - invalid | ||
| } | ||
|
|
||
| if (distance(position.value(), goal_line) < robot_diameter) | ||
| { | ||
| continue; // invalid | ||
| } |
There was a problem hiding this comment.
Whats the difference between these two cases?
There was a problem hiding this comment.
The first one checks if robot is inside goal rectangle. Second checks when we are close too the goalie but too close (just simply enforces we need at least a min distance of one robot diameter from the goal line)... I also just added the extra checks to pass some of the tests.
|
|
||
| double step_distance = 2.0 * ROBOT_MAX_RADIUS_METERS; | ||
|
|
||
| Point stepped_position = stepAlongPerimeter( | ||
| defense_perimeter, center_position.value(), | ||
| (crease_defender_alignment == TbotsProto::CreaseDefenderAlignment::LEFT) | ||
| ? -step_distance | ||
| : step_distance); |
There was a problem hiding this comment.
Maybe I'm confused, but given we step along the perimeter of the goal line, and we check for any robots that lie inside the goal line, I think that condition will never be fulfilled. So effectively, we will always have three robots defending the crease regardless of if we need to or not.
sauravbanna
left a comment
sauravbanna
left a comment
There was a problem hiding this comment.
i think this PR is more complicated than it needs to be. your changes just concern where the crease defenders are positioned, which is only done in CreaseDefenderFSM::findBlockThreatPoint.
i think you should handle dynamically changing the number of crease defenders assigned depending on the enemy position in a different PR
| defender_assignment_config.max_num_crease_defenders(); | ||
|
|
||
| // Get primary threat position for calculating pass defender positions | ||
| Point primary_threat_position = enemy_threats.empty() |
There was a problem hiding this comment.
Our code style guide suggests avoiding ternary operators for clarity:
Software/docs/code-style-guide.md
Line 276 in 91e3d04
5 participants
Description
The existing crease defense FSM positioned up to three defenders along the goal crease by dividing the enemy’s shot cone into fixed 1/6, 3/6, and 5/6 intervals of the goal width. When the shot cone was narrow, these fixed positions were too close together relative to robot size, causing defenders to cluster and sometimes collide.
The solution was for the crease defenders to step along the goal area perimeter rather than relying on fixed divisions of the shot cone. Once the box is represented as a polygon, stepAlongPerimeter() can be called to move a defender along the perimeter according to their alignment relative to the opposing threat.
Edge case:
Defenders occasionally stepped onto the goal line when the cone was positioned near the boundary. The solution for this was to reassign the robot that would step over the goal line as a pass defender instead when the enemy was close to the net and the shot angle is too small for 3 robots to all be crease defenders. This reassignment was done in defense_play_fsm.cpp where we also validate the crease defender’s position.
Testing Done
Updated google tests for crease_defender_fsm_test.cpp to accommodate for perimeter stepping.
Resolved Issues
#2995
#3374 (the robot reassignment was similar to this ticket. We should also remove this issue since it is no longer valid. )
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue