Skip to content

Cleanup strat runner#108

Merged
energy-in-joles merged 7 commits intomainfrom
cleanup_strat_runner
Feb 25, 2026
Merged

Cleanup strat runner#108
energy-in-joles merged 7 commits intomainfrom
cleanup_strat_runner

Conversation

@energy-in-joles
Copy link
Copy Markdown
Member

cleanup strat runner by abstracting side related info (my_game, my_motion_controller) into a dataclass SideRuntime.

…ate class. This will make it easier to maintain and extend the strategy runner in the future.
@energy-in-joles energy-in-joles marked this pull request as ready for review February 21, 2026 22:51
Copilot AI review requested due to automatic review settings February 21, 2026 22:51
@energy-in-joles energy-in-joles self-assigned this Feb 21, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the StrategyRunner class to improve code organization by introducing a SideRuntime dataclass that encapsulates all per-side (friendly/opponent) runtime state. This reduces code duplication and makes the codebase more maintainable by grouping related attributes together.

Changes:

  • Introduced SideRuntime dataclass to encapsulate strategy, refiners, motion controller, game state, and game history for each side
  • Refactored StrategyRunner to use self.my and self.opp (SideRuntime instances) instead of separate my_* and opp_* attributes
  • Updated all tests to use the new structure (runner.my.strategy, runner.my.game, etc.)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
utama_core/run/strategy_runner.py Added SideRuntime dataclass and refactored StrategyRunner to use it for managing per-side state
utama_core/tests/strategy_runner/test_runner_misconfig.py Updated tests to use new attribute structure (e.g., base_runner.my.strategy)
utama_core/tests/strategy_runner/test_error_handling.py Updated mock setup and assertions to use new SideRuntime structure
utama_core/tests/strategy_runner/integration_test.py Updated field assertions to use runner.my.game instead of runner.my_game
main.py Updated to use runner.my.strategy.render() instead of runner.my_strategy.render()

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 21, 2026 22:55
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

fred-huang122

This comment was marked as outdated.

@fred-huang122
Copy link
Copy Markdown
Collaborator

fred-huang122 commented Feb 24, 2026

alternatively, this could work to make it all the methods consistent by reordering the order of what is run so everything uses the same way of calling my and opp strategies:

  1. setup_behaviour_tree
  2. _init_refiners → construct SideRuntime ← moved up
  3. _assert_exp_robots() ← no more explicit params
  4. _load_sim(...) ← no more strategy params
  5. _setup_vision_and_referee
  6. _load_robot_controllers
  7. _load_game

Copilot AI review requested due to automatic review settings February 24, 2026 17:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings February 25, 2026 12:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@energy-in-joles energy-in-joles merged commit 93677b8 into main Feb 25, 2026
2 checks passed
@energy-in-joles energy-in-joles deleted the cleanup_strat_runner branch February 25, 2026 21:51
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