Use arrow as ball_is_off_ground.py validation test visual #3512
Conversation
rectangle cannot be rendered at an angle
…ests" This reverts commit d8ef22f.
nycrat
requested review from
Andrewyx,
sauravbanna and
williamckha
as code owners
|
NOTE: a lot of changes come from the pre-commit auto-formatting, I'm not sure which one is the correct formatting. It was mentioned here #3499 (comment) |
|
nvm the bot updated the formatting its all good |
| if direction.x() == 0.0 and direction.y() == 0.0: | ||
| # NOTE if ball is not moving, displays arrow pointing to the right | ||
| direction = tbots_cpp.Vector(1.0, 0.0) |
There was a problem hiding this comment.
Why do we do this? If it is an arbitrary design decision, I think we should think of some other more intuitive way of representing non-movement. Consider the following example: suppose we have a bug where the ball is moving at 0.0000000001 m/s to the right. In this case, we will not be able to visually differentiate the fact that the ball is drifting which might cause us to fail to catch this bug.
There was a problem hiding this comment.
Yea I agree, maybe I'll try implementing a different shape like an octagon to represent that the ball is stopped ?
| line_width = 0.08 | ||
| line_length = 0.2 | ||
| triangle_height = 0.15 | ||
| triangle_width = 0.2 |
There was a problem hiding this comment.
Ideally, we should use either existing constants for these values, and in the worst case, we can define then as static values either in this class, or if appropriate, in some constants class.
There was a problem hiding this comment.
Sure, I'll try to look for some existing constants, and if nothing makes sense in this context I'll define them as static values in the class
| line_bottom_left = start_point - perpendicular * (line_width / 2) | ||
| line_bottom_right = start_point + perpendicular * (line_width / 2) | ||
| line_top_right = end_point + perpendicular * (line_width / 2) | ||
| line_top_left = end_point - perpendicular * (line_width / 2) | ||
|
|
||
| triangle_top = end_point + direction * triangle_height | ||
| triangle_bottom_left = end_point + perpendicular * (triangle_width / 2) | ||
| triangle_bottom_right = end_point - perpendicular * (triangle_width / 2) |
There was a problem hiding this comment.
Looks like there is a lot of repeated calculations here. Maybe encapsulating some of this in to a helper will clean things up and make the calculations easier to interpret.
There was a problem hiding this comment.
I'll make a helper function named something like "get_arrow(start, end)" that will generate the geometry. I also now realize that I could make the entire arrow a polygon geom instead of two polygons combined.
Do you think I should keep this helper local to this validation class or add it to another helper file in case we would want to reuse arrows?
Andrewyx
left a comment
Andrewyx
left a comment
There was a problem hiding this comment.
Would it be possible to attach a clip + pics of this new visual test in action? We usually try to do this for features which make visual verifiable changes 👍
|
I'll attach some videos and screenshots of the visual test in this PR after I make some changes to resolve the comments you made about the code |
nycrat
force-pushed
the
avah/thunderscope_ball_off_ground_visual
branch
from
f4e4f85 to
1057d84
Compare
2 participants
Description
Create a 2D arrow under the ball directed to its velocity, shows where and the direction the ball is chipped in tests. Shows octagon (stop sign) when ball is not moving.
ball_is_off_ground_validation.mp4
Testing Done
Ran
crease_defender_tactics_test, which is the only test that uses the ball_is_off_ground validationResolved Issues
resolves #3244
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