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

Replace player number with jersey number #1449

Conversation

MaikRe
Copy link
Contributor

@MaikRe MaikRe commented Sep 19, 2024

Why? What?

This takes apart the double task that player number was performing and splits it into jersey number and behavior dependent on active field robots, regardless of their jersey number.

Created to replace #1021 in order to not do double work on refactoring the behavior simulator.

ToDo / Known Issues

Depends on #1428
Fixes #1417
Fixes #1298

  • Fix sacrificial lamp somehow as right now the chosen robot is hard coded based on jersey number
  • Fix robot initial pose order in localization
  • Fix robot initial pose order in behavior simulator
  • Handle role assignment correctly, what do we need to prioritize? Is it possible to set offensive and defensive strategies and decide based on that?
    - [ ] HULKs aufstellung in intercept ball?
  • Handle replacement keeper delay, probably via leading edge detection on replacement keeper priority
  • Look at referee, check which positions need to do this
  • Goal keeper should have widestance action in node.rs
  • cleanup

Ideas for Next Iterations (Not This PR)

  • Consider when and how goal keeper should perform widestance. Currently in intercept ball since he is the only player on the field he becomes striker and thus does not perform widestance. Is this the desired behavior?
  • Implement better aufstellung for special scenarios (intercept ball, oscillating obstacle, penalty shootouts etc)
  • Set number of defensive and offensive players based on game situation and opponent penalties

How to Test

The CI being green is the first sign that on the behavior side everything seems to work.
The rest needs to be tested in test games with various player uhh jersey number configurations.

@MaikRe MaikRe self-assigned this Sep 19, 2024
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch 2 times, most recently from 0a55ea8 to 6f203d9 Compare September 22, 2024 11:01
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch from 859349a to 3f9bcc8 Compare October 7, 2024 13:01
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch 3 times, most recently from 1d0948d to 2f51548 Compare October 16, 2024 09:50
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch from f18844e to 9d81cbf Compare October 26, 2024 13:29
@MaikRe MaikRe enabled auto-merge October 26, 2024 13:55
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch 2 times, most recently from 2f6ee0e to 19a8b91 Compare November 5, 2024 12:27
@MaikRe MaikRe removed their assignment Nov 6, 2024
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch from 2bfa5a4 to 37d75a7 Compare November 16, 2024 15:22
@MaikRe MaikRe force-pushed the 1_2_3_4_5_6_7_8_9_10_11_12_13_14_15_16_17_18_19_20 branch from 37d75a7 to e57a4f5 Compare November 24, 2024 12:50
@pejotejo pejotejo requested a review from oleflb November 27, 2024 14:49
@oleflb oleflb self-assigned this Nov 27, 2024
Copy link
Contributor

@oleflb oleflb left a comment

Choose a reason for hiding this comment

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

Oof, I expected this to be easier

crates/argument_parsers/src/lib.rs Outdated Show resolved Hide resolved
crates/bevyhavior_simulator/src/aufstellung.rs Outdated Show resolved Hide resolved
crates/bevyhavior_simulator/src/aufstellung.rs Outdated Show resolved Hide resolved
Comment on lines +212 to +231
GameControllerCommand::SetGameState(game_state) => {
match game_state {
GameState::Ready | GameState::Standby => {
for mut robot in &mut robots {
let parameters = &robot.parameters;
let initial_pose = parameters
.localization
.initial_poses
.get(robot.database.main_outputs.walk_in_position_index)
.cloned()
.unwrap_or_default();
robot.database.main_outputs.ground_to_field = Some(
generate_initial_pose(&initial_pose, &parameters.field_dimensions)
.as_transform(),
);
}
}
_ => {}
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a task of the autoref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and the time.ticks problem are connected. The robots only generate their proper poses if localization is running, which is not the case in the behavior simulator. This needs to be called at least once between standby and ready, which explicitly calling GameState::Ready combined with the code above does. The alternative is tracking the last game state in the robots resource to call it there. @knoellle Maybe you have a suggestion of whats best practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alternative is to include a sorting function in the hulks_setup which sorts all of the robots ahead of time. I had this implemented before but found it to be a little bit messier than needed

crates/control/src/role_assignment.rs Show resolved Hide resolved
});
if !send_spl_striker_message && is_lowest_number_without {
new_role = Role::ReplacementKeeper;
if let (Some(0), Some(x)) = (
Copy link
Contributor

Choose a reason for hiding this comment

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

what is x. Consider renaming your variable

@@ -461,10 +473,14 @@ fn process_role_state_machine(
_ => decide_if_claiming_striker_or_other_role(
Copy link
Contributor

Choose a reason for hiding this comment

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

this function now has 10 arguments, just saying

crates/control/src/role_assignment.rs Show resolved Hide resolved
crates/control/src/role_assignment.rs Show resolved Hide resolved
@MaikRe
Copy link
Contributor Author

MaikRe commented Jan 16, 2025

Closed as benefit of having more jersey numbers does not outweigh potential drawbacks this PR introduces. See #1587

@MaikRe MaikRe closed this Jan 16, 2025
auto-merge was automatically disabled January 16, 2025 14:08

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

impl Substitutes post-game task: 4 on 7 collisions during walk-in
3 participants