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.

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

ignore goalie in has_open_shot #2206

Merged
merged 1 commit into from
Mar 4, 2024
Merged

ignore goalie in has_open_shot #2206

merged 1 commit into from
Mar 4, 2024

Conversation

sid-parikh
Copy link
Contributor

@sid-parikh sid-parikh commented Mar 1, 2024

Description

Taking the goalie into account makes us deny every shot. Let's assume corner shots can evade the goalie, and only take into account defenders. Important to shoot more.

Associated / Resolved Issue

Resolves clickup card: https://app.clickup.com/t/86azgzx41

Steps to Test

In sim:

  1. Place goalie in opponents defense area
    Result: Shot still taken
  2. Place other defender in the way
    Result: Pass instead

Key Files to Review

offense.*pp

@sid-parikh sid-parikh requested a review from jacksherling March 1, 2024 23:06
Copy link
Contributor

@jacksherling jacksherling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

rj_geometry::Point enemy_vec = enemy.pose.position() - ball_position;

// I think this means enemy is behind shot (sid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep (I think)!

@rishiso rishiso self-requested a review March 4, 2024 17:38
Copy link
Contributor

@rishiso rishiso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Works as intended during testing

@sid-parikh sid-parikh merged commit 738ab3e into ros2 Mar 4, 2024
2 checks passed
@sid-parikh sid-parikh deleted the has_open_shot branch March 4, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants