Skip to content

Conversation

@franklin-yuan
Copy link

Description

Made goalie line up with the ball and the closest enemy robot to the ball

Associated / Resolved Issue

Resolves #86b6ubvaq or ClickUp card

Design Documents

Link

Steps to Test

Test Case 1

Setup penalty, set goalie state to penalty

Expected result:
Goalie should move automatically between ball and opposing kicker

Key Files to Review

Group 1

  • File 1
  • File 2

Group 2

  • File 3
  • File 4

Review Checklist

  • Docstrings: All methods and classes should have the file appropriate docstrings which follow the guidelines in the "Contributing" page of our docs.
  • Remove extra print statements: Any print statements used for debugging should be removed
  • Tag reviewers: Tag some people for review and ping them on Slack

(Optional) Sub-issues (for drafts)

Note: if you find yourself breaking this PR into many smaller features, it may make sense to break up the PR into logical units based on these features.

  • Step 1
  • Step 2

automated style fixes

Co-authored-by: franklin-yuan <franklin-yuan@users.noreply.github.com>
@rishiso rishiso self-requested a review January 19, 2026 01:07
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.

left a few comments

// return this->field_dimensions_.our_goal_loc();
// be smart
// find robot on their team closest to ball
std::vector<RobotState> const their_robots = last_world_state_->their_robots;
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to make a copy of their_robots. If you really want to keep this, make their_robots a reference.

Comment on lines +208 to +209
rj_geometry::Point enemy_location;
rj_geometry::Point enemy_shooter_location;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rj_geometry::Point enemy_location;
rj_geometry::Point enemy_shooter_location;

rj_geometry::Point enemy_shooter_location;

double closest_distance = std::numeric_limits<double>::infinity();
RobotState curr_shooter;
Copy link
Contributor

Choose a reason for hiding this comment

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

curr_shooter can just store the enemy robot's position, no need to store the entire RobotState

Comment on lines +228 to +232

// SPDLOG_INFO("botpos {} {}", enemy_shooter_location.x(), enemy_shooter_location.y());
// SPDLOG_INFO("ballpos {} {}", ball_pos.x(), ball_pos.y());
// SPDLOG_INFO("GOALX {}", field_dimensions_.goal_width());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// SPDLOG_INFO("botpos {} {}", enemy_shooter_location.x(), enemy_shooter_location.y());
// SPDLOG_INFO("ballpos {} {}", ball_pos.x(), ball_pos.y());
// SPDLOG_INFO("GOALX {}", field_dimensions_.goal_width());

Comment on lines +241 to +245

// SPDLOG_INFO("enemy {}", goalie_pos_x);

rj_geometry::Point goaliepose = {goalie_pos_x, goalie_pos_y};
return goaliepose;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// SPDLOG_INFO("enemy {}", goalie_pos_x);
rj_geometry::Point goaliepose = {goalie_pos_x, goalie_pos_y};
return goaliepose;
rj_geometry::Point goalie_pos{goalie_pos_x, goalie_pos_y};
return goalie_pos;

To keep consistent with other uses

(ball_pos.x() - enemy_shooter_location.x())) /
(ball_pos.y() - enemy_shooter_location.y());

goalie_pos_x = std::clamp(goalie_pos_x, -1 * field_dimensions_.goal_width() / 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the field_dimensions_.our_goal_segment() to make this easier for someone to understand

// SPDLOG_INFO("GOALX {}", field_dimensions_.goal_width());

double goalie_pos_y = field_dimensions_.our_goal_loc().y();
float goalie_pos_x =
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this a double

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: use the built-in intersect functions for this

State latest_state_ = IDLING;

rj_geometry::Point penalty_location();
rj_geometry::Point penalty_location(WorldState* world_state);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Consider passing in just the parts of the world_state you need instead of the entire thing

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