Skip to content

Feature/game controller hsl cleanup#955

Merged
jgueldenstein merged 3 commits into
mainfrom
feature/game-controller-hsl-cleanup
Jul 1, 2026
Merged

Feature/game controller hsl cleanup#955
jgueldenstein merged 3 commits into
mainfrom
feature/game-controller-hsl-cleanup

Conversation

@texhnolyze

@texhnolyze texhnolyze commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Make architecture cleaner, by not having different components rely on game_controller_hsl.

Proposed changes

  • fix(scripts): automatically select pixi env

based on IS_ROBOT environment variable

  • refactor(game_controller_hsl): extract response to own component

after properly setting up a PlayerStateResponse in
bit-bots/game_controller_hsl#1

This makes the architecture much cleaner as we do not have dependencies
to game_controller_hsl in multiple components, which could also
function without any game controller.

Related issues

Checklist

  • Run pixi run build
  • Write documentation
  • Test on your machine
  • Te st on the robot
  • Create issues for future work
  • Triage this PR and label it

@texhnolyze texhnolyze requested review from Flova and cleWu03 July 1, 2026 05:46
@github-project-automation github-project-automation Bot moved this to 🆕 New in Software Jul 1, 2026
@texhnolyze texhnolyze self-assigned this Jul 1, 2026
@texhnolyze texhnolyze added the misc label Jul 1, 2026
Flova
Flova previously requested changes Jul 1, 2026

@Flova Flova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This changes a lot of things and we need to also adapt all downstream subscriptions of e.g. game state. Otherwise the behavior and localization will crash.

@github-project-automation github-project-automation Bot moved this from 🆕 New to 🏗 In progress in Software Jul 1, 2026
@MegaIng

MegaIng commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

This PR works, in simulation, as far as I can test it. (which means only a single robot because the game controller software doesn't work otherwise)

@Flova

Flova commented Jul 1, 2026

Copy link
Copy Markdown
Member

Then I am missing something. Because e.g. the penalty field has a new type and has no downstream adaptions. Same with secs remaining. I am pretty sure all of these are referenced in localization, behavior.

after properly setting up a PlayerStateResponse in
bit-bots/game_controller_hsl#1

This makes the architecture much cleaner as we do not have dependencies
to `game_controller_hsl` in multiple components, which could also
function without any game controller.
based on `IS_ROBOT` environment variable
@texhnolyze

Copy link
Copy Markdown
Contributor Author

The changes to the GameState are as follows:

  • The only place where secs_remaining were ever used was in src/bitbots_behavior/bitbots_blackboard/bitbots_blackboard/capsules/game_status_capsule.py, which I adapted.
    Maybe this was used in the past in other components, but is not currently.
  • The warnings field was removed, but was also never used in our code
  • The seconds_till_unpenalized field changed from uint16 to uint8 in accordance with the game controller protocol, but I did not find any place where it was actually used
  • The message_budget field changed from uint8 to uint16 in accordance with the game_controller protocol, but I also did not find any place where it was used yet
  • The penalty field was newly added to allow teams to track the penalty state. So far we've only ever checked if we are penalized/penalized_in_place in behavior or localization, but not for which reason.

@texhnolyze texhnolyze force-pushed the feature/game-controller-hsl-cleanup branch from cb89865 to 6a3c87d Compare July 1, 2026 09:51
…c/lib/game_controller_hsl

subrepo:
  subdir:   "src/lib/game_controller_hsl"
  merged:   "7c48f279e"
upstream:
  origin:   "git@github.com:bit-bots/game_controller_hsl.git"
  branch:   "feature/abstract-for-open-source"
  commit:   "7c48f279e"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "ea10886"
@texhnolyze texhnolyze force-pushed the feature/game-controller-hsl-cleanup branch from 6a3c87d to e2a72b8 Compare July 1, 2026 10:00

@jgueldenstein jgueldenstein left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tested in sim with multiple robots, and on a single real robot

@jgueldenstein jgueldenstein dismissed Flova’s stale review July 1, 2026 10:06

tested in sim and real world, works,

@jgueldenstein jgueldenstein merged commit b7cd28e into main Jul 1, 2026
3 checks passed
@jgueldenstein jgueldenstein deleted the feature/game-controller-hsl-cleanup branch July 1, 2026 10:06
@github-project-automation github-project-automation Bot moved this from 🏗 In progress to ✅ Done in Software Jul 1, 2026
@Flova

Flova commented Jul 1, 2026

Copy link
Copy Markdown
Member

The changes to the GameState are as follows:

  • The only place where secs_remaining were ever used was in src/bitbots_behavior/bitbots_blackboard/bitbots_blackboard/capsules/game_status_capsule.py, which I adapted.
    Maybe this was used in the past in other components, but is not currently.
  • The warnings field was removed, but was also never used in our code
  • The seconds_till_unpenalized field changed from uint16 to uint8 in accordance with the game controller protocol, but I did not find any place where it was actually used
  • The message_budget field changed from uint8 to uint16 in accordance with the game_controller protocol, but I also did not find any place where it was used yet
  • The penalty field was newly added to allow teams to track the penalty state. So far we've only ever checked if we are penalized/penalized_in_place in behavior or localization, but not for which reason.

My bad. I missread the diff and thought penalized was removed. Sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants