-
Notifications
You must be signed in to change notification settings - Fork 113
Penalty kick play pytest #3475
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
base: master
Are you sure you want to change the base?
Penalty kick play pytest #3475
Conversation
@@ -38,7 +39,7 @@ cc_test( | |||
) | |||
|
|||
cc_test( | |||
name = "penalty_kick_play_test", |
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.
You can completely delete this test
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.
The files as well?
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.
Yup yeah
src/software/ai/hl/stp/play/penalty_kick/penalty_kick_play_test.py
Outdated
Show resolved
Hide resolved
src/software/ai/hl/stp/play/penalty_kick/penalty_kick_play_test.py
Outdated
Show resolved
Hide resolved
…/Software into penalty_kick_play_pytest
Probably going to add the pytest for the tactic to this branch as well. Just gotta figure out how to write that... |
…/Software into penalty_kick_play_pytest
# Always Validation | ||
inv_always_validation_sequence_set = [[]] | ||
|
||
ag_always_validation_sequence_set = [[FriendlyAlwaysHasBallPossession(), BallAlwaysMovesForward(ball_initial_pos)]] |
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.
Why can't this be inv_always
?
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.
It immediately fails cause I think the ball doesn't start moving forward? Or the robot doesn't immediately start with the ball. I forget which.
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.
In fact it is both.
create_world_state( | ||
yellow_robot_locations=[], | ||
blue_robot_locations=[tbots_cpp.Point(-3, 0)], | ||
ball_location=tbots_cpp.Point(0, 0), |
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.
penalty mark is 6m from the opponent net (-1.5, 0)
https://robocup-ssl.github.io/ssl-rules/sslrules.html#_penalty_mark
from proto.import_all_protos import * | ||
|
||
|
||
def test_penalty_kick_no_goalie(simulated_test_runner): |
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.
you should break this test into 2 different tests (setup and taking):
https://robocup-ssl.github.io/ssl-rules/sslrules.html#_penalty_kick
setup: checks that all friendly robots are behind the ball and only 1 robot is near the ball
take: checks that we score and the ball always moves forward. we need to issue NORMAL_START
so that the penalty kick taker actually starts manipulating the ball. you would do this by adding these lines
simulated_test_runner.gamecontroller.send_gc_command(
gc_command=Command.Type.NORMAL_START, team=Team.BLUE
)
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.
yeah I realized after I wrote this test lol
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.
This test only has one robot though, so I think this should be in the play test, and not the tactic test?
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.
yeah it should be under src/software/ai/hl/stp/play/penalty_kick/
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.
yeah change was made to the right file
Description
Adds a pytest for penalty kick
Basically this test just assess whether the ball goes into the goal. I may update it later to pass if the ball enters the crease, as in theory the goalie can defend the ball and we would still consider the play to succeed, even if the ball didn't score.
Testing Done
It compiles and the broken penalty kick ai fails the test (gotta fix that)
Resolved Issues
working on #3389
Length Justification and Key Files to Review
there are a couple of comments on lines I wasn't sure about.
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.h
file) 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