Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Changed ball touches ground detection #9

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fc769b4
Changed ball touches ground detection
skyborgff Apr 14, 2019
db8fc50
Fixed ball stop detection
skyborgff Apr 14, 2019
2935385
fixed ball stop detection typo
skyborgff Apr 14, 2019
196ff5a
Fixed actions overriding
skyborgff Apr 14, 2019
1407a1c
fixed dribbling and pushing
skyborgff Apr 14, 2019
66827a8
Adds prop_bot
skyborgff Apr 16, 2019
b362e79
Adds unittests for rl_graders
skyborgff Apr 16, 2019
6cc8798
Refactor and Improves rl_grader, Adds More Tests
skyborgff Apr 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions rlbottraining/common_graders/rl_graders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
This module contains graders which mimic the the behaviour of Rocket League custom training.
"""

import math

from dataclasses import dataclass
from typing import Optional, Mapping, Union
Expand All @@ -29,13 +30,77 @@ def __init__(self, timeout_seconds=4.0, ally_team=0):
])

class FailOnBallOnGroundAfterTimeout(FailOnTimeout):
def __init__(self, max_duration_seconds):
super().__init__(max_duration_seconds)
self.previous_ang_x = None
self.previous_ang_y = None
self.previous_ang_z = None
self.previous_total_goals = None

def set_previous_angular_velocity(self, ball, reset = False):
Copy link
Collaborator

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.

Copy link
Member Author

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

Copy link
Collaborator

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.

Copy link
Member Author

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

if not reset:
self.previous_ang_x = ball.angular_velocity.x
self.previous_ang_y = ball.angular_velocity.y
self.previous_ang_z = ball.angular_velocity.z
self.previous_vel_z = ball.velocity.z
else:
self.previous_ang_x = None
self.previous_ang_y = None
self.previous_ang_z = None
self.previous_vel_z = None

def set_previous_total_goals(self, total_goals, reset=False):
if not reset:
self.previous_total_goals = total_goals
else:
self.previous_total_goals = None

def current_total_goals(self, packet):
total_goals = 0
for car_id in range(packet.num_cars):
goal = packet.game_cars[car_id].score_info.goals
own_goal = packet.game_cars[car_id].score_info.own_goals
total_goals += goal + own_goal
return total_goals

def on_tick(self, tick: TrainingTickPacket) -> Optional[Grade]:
grade = super().on_tick(tick)
if grade is None:
return None
assert isinstance(grade, FailOnTimeout.FailDueToTimeout)
ball = tick.game_tick_packet.game_ball.physics
if ball.location.z < 100 and ball.velocity.z >= 0:
return grade
hit_ground = False

if self.previous_ang_z is None:
self.set_previous_angular_velocity(ball)
else:
max_ang_vel = 5.9999601985025075 #Max angular velocity possible
previous_ang_norm = math.sqrt(self.previous_ang_x**2 + self.previous_ang_y**2 + self.previous_ang_z**2)
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) :
Copy link
Collaborator

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?

Copy link
Member Author

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

Copy link
Collaborator

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.

Copy link
Member Author

@skyborgff skyborgff Apr 15, 2019 •
edited
Loading

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

Copy link
Collaborator

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:
Copy link
Collaborator

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?

Copy link
Member Author

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.

Copy link
Member Author

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

Copy link
Collaborator

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.

Copy link
Member Author

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

# if it was at maximum angular velocity, it may have changed z axis not to exceed max
# fallback on old behaviour
if ball.location.z < 100 and ball.velocity.z >= 0:
hit_ground = True
if ball.location.z <= 93.5 and ball.velocity.z >= 0:
# if the car is pushing the ball on the ground
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):
Copy link
Collaborator

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.

Copy link
Member Author

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.

# There was a goal, let the goal handler handle it
hit_ground = False
else:
hit_ground = True

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)
Copy link
Collaborator

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?

Copy link
Member Author

@skyborgff skyborgff Apr 15, 2019 •
edited
Loading

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

Copy link
Collaborator

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.

return grade