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

Remove Uncertain kind from Team type #1427

Merged
merged 2 commits into from
Oct 28, 2024
Merged

Conversation

knoellle
Copy link
Contributor

@knoellle knoellle commented Aug 7, 2024

Why? What?

Team::Uncertain is removed and wrapped with in Option where applicable (only in FilteredGameState::Ready)
Update: there is now only one kicking team in the GameControllerState and the Playing state contains a flag whether the kicking_team sent by the game controller is trustworthy.

This resolves the necessity to match the uncertain Team kind in places where it is always either Hulks or Opponent anyways.
This was especially annoying with the new behavior simulator but I wanted to make this a separate PR.

Background

The FilteredGameControllerState contains a kicking_team field as well as two fields of type FilteredGameState, one for each team.
The FilteredGameState::Ready kind is the only place in our code where the Team could be Uncertain and if it wasn't uncertain, then the kicking team inside this kind was equal to the kicking team in the FilteredGameState.

Fixes #

ToDo / Known Issues

Ideas for Next Iterations (Not This PR)

How to Test

Everything should work as before, there are no functional changes.

@knoellle knoellle added the is:Refactoring No changes in functionality, only in coding style. label Aug 7, 2024
@knoellle knoellle enabled auto-merge August 7, 2024 11:53
@oleflb oleflb self-assigned this Aug 7, 2024
@knoellle knoellle mentioned this pull request Aug 7, 2024
26 tasks
@knoellle
Copy link
Contributor Author

I basically changed everything again, see comment in PR description.
Please have another look.

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.

Looks good, but have not tested yet

@knoellle
Copy link
Contributor Author

Rebased due to conflict with #1429.
@oleflb what is the state here?

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.

Abfahrt 🚂

@knoellle knoellle added this pull request to the merge queue Oct 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 27, 2024
@knoellle knoellle added this pull request to the merge queue Oct 28, 2024
Merged via the queue into HULKs:main with commit 40a65a4 Oct 28, 2024
25 checks passed
@knoellle knoellle deleted the certainly branch October 28, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:Refactoring No changes in functionality, only in coding style.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants