-
Notifications
You must be signed in to change notification settings - Fork 4
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
Changed ball touches ground detection #9
base: master
Are you sure you want to change the base?
Conversation
This is a much improved ball on ground detection that brings it closer to the behaviour of rocket league this allows for touch detection on top of the goal in throwback stadium, and for the ball stop on ground.
if there was a goal, but at the same time the ball was teleported to the center, the detection for ball stopped would override the goal detection and the level would fail
fixed dribbling not changing angular velocity z, and pushing the ball on the ground changing it
hit_ground = True | ||
if math.sqrt(ball.velocity.x**2 + ball.velocity.y**2 + ball.velocity.z**2) == 0 and self.previous_vel_z != 0: | ||
# ball is stop on ground and not on its apex, which means it should fail anyway | ||
if self.previous_total_goals != self.current_total_goals(tick.game_tick_packet): |
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.
I don't think goal logic should be in FailOnBallOnGroundAfterTimeout
at all.
Probably better to put something in RocketLeagueStrikerGrader
such that a Pass
from the goal grader overrides a Fail
from the timeout grader.
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.
I will look into that, I agree with you.
self.previous_ang_z = None | ||
self.previous_total_goals = None | ||
|
||
def set_previous_angular_velocity(self, ball, reset = False): |
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.
Probably better to store / copy a ball object rather than separating out the variables.
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.
I did that at first, but it passed by reference, so it would have the current values and not the previous ones
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.
Yes, that's why we need to copy the object not just create another reference to it.
copy.deepcopy
may be what we're after if the ball is a normal object.
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.
Ah, thank you :)
I will do that, i don't like what i did either, but i didn't knew an alternative
self.set_previous_angular_velocity(ball) | ||
self.set_previous_total_goals(self.current_total_goals(tick.game_tick_packet)) | ||
if hit_ground: | ||
self.set_previous_angular_velocity(ball, reset = True) |
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 reset seems unnecessary to me. How come it's here?
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.
Reset is there so that in the next shot, after you set the state, it will not fail because the velocity changed. From what you said I'm guessing a new shot will initializate a new instance of a grader so that will never happen. I did not test for that. I will delete the reset and 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.
Yes, exercises are created and used once such that the graders are automatically and fully reset.
if ball.location.z <= 1900: #Making sure it doesnt count the ceiling | ||
if (ball.angular_velocity.x != self.previous_ang_x or ball.angular_velocity.y != self.previous_ang_y): | ||
# If the ball hit anything its angular velocity will change in the x or y axis | ||
if self.previous_ang_z == ball.angular_velocity.z and not ( 130 < ball.location.z < 150) : |
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 130 < ball.location.z < 150
seems rather arbitrary.
Could such a hit between the car/ball happen in other places? e.g. dribbling on top of the goal.
Could such a hit happen on that z
on other maps?
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.
That is true that it could happens at another z, I did not like that way as well. After sleeping on the case I have some new ideas to try out, like seeing if the last touch was on this tick, and if the ball is a certain couple of units above one of the cars
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.
I think that'll make it better but you should keep some of the tricky cases in mind:
potential false positives:
- the ball is hit directly upwards using the nose of the car
- the punching glove mutator hitting the ball upwards.
potential false negatives:
- the ball bounces off the flat top of the goal while the car is directly under the ceiling of the goal.
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.
potential false positives:
- the ball is hit directly upwards using the nose of the car
seeing the the last touch was this tick should avoid this problem
- the punching glove mutator hitting the ball upwards.
punching doesn't update the last touch. this may become a problem, but currently we cant test it
potential false negatives:
- the ball bounces off the flat top of the goal while the car is directly under the ceiling of the goal.
Once again, the last touch check should not allow this to happen
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.
👍 I didn't think of using the last touch info.
# if the z angular velocity did not change it means it was a flat plane. | ||
#ignore dribbling | ||
hit_ground = True | ||
elif previous_ang_norm == max_ang_vel: |
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.
I'd recommend not doing exact equality on floating point numbers.
Do we even have cases that get into this?
Could you please add unit tests?
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.
I tested various times and the max was always like that. I would think that it could have some error from time to time, but all the tested times had exactly that value.
Yes, I believe it is shot 7 on the linkuru pack, it will have max angular velocity when hitting the ground.
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.
I would like some help with how should I add unit testing, I'm not familiar with how you are doing that, neither how that is done on general
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.
https://github.com/RLBot/RLBotPythonExample/blob/master/training/unit_tests.py
Tests go here: https://github.com/RLBot/RLBotTraining/tree/master/tests
To run all tests, run test_all.bat
in the root of this repo. test_quick.bat
is for anything that completes in less than a second (i.e. does not launch rocket league)
To run tests only that file, do the usual python test_my_foo.py
To test these kinds of cases, create exercises within the test file itself which use this grader. e.g.
- an exercise with zero seconds remaining and the ball dropping onto the car from a height should fail after a specific time (which is longer than what it takes to bounce off the car once).
- an exercise with zero seconds remaining where the ball drops onto specific locations of the goal and it should quickly end in within a small period of seconds.
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.
Ah, thank you, this will also make my testing easier :)
I will try to implement this this afternoon
prop_bot does nothing. Can be used to test interactions between the ball and the bot
This will help improve and maintain the functionality of the rl_grader Following commits should take advantage of this to fix rl_grader
|
||
|
||
class RocketLeagueStrikerGrader(CompoundGrader): | ||
""" | ||
A Grader which aims to match the striker training. | ||
""" | ||
|
||
def __init__(self, timeout_seconds=4.0, ally_team=0): | ||
def __init__(self, timeout_seconds=4.0, ally_team=0, timeout_override=False, ground_override=False): |
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 add documentation about what the overrides do.
Will users of this class care about this configurability?
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.
Probably users will not care, I cant think of a good use case beyond testing the grader.
def test_rl_graders(self): | ||
from tests.test_exercises.rl_grader_exercises import make_default_playlist | ||
self.assertGrades( | ||
run_playlist(make_default_playlist()), |
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.
I think it's easier to understand the test case if the exercises are constructed in this function.
Main reason is to make it easy to see which grade corresponds with which exercise.
Note: the reason the other tests work differently is because the make_default_playlist() has other uses as well, whereas here it does not.
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.
Ok, i will keep working on a separate file until I have all the tests done, then I will move it here.
|
||
class PropBot(BaseAgent): | ||
""" | ||
A bot which just sits there like a prop. |
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.
BrickBot already does this
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.
Brick bot has handbrake on, this causes the ball to move it quite a bit when it hits it.
Now dribbling doesnt cause a ground_hit error. Consequence: pushing ball on the ground doesnt cause it as well. Tests needed to improve Adds some more tests for rl_grader
Work has been halted due to rocket league update, |
This is a much improved ball on ground detection that brings it closer to the behaviour of rocket league
this allows for touch detection on top of the goal in throwback stadium, and for the ball stop on ground.