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

RF: Use classic game_state for communication with the bots #851

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Debilski
Copy link
Member

@Debilski Debilski commented Mar 6, 2025

This undos the split in team/enemy in the state that a team receives.

The reasoning is that this format makes it easier to reuse functions from game.py in one’s own code.

Works and checks all tests but will need some cleanup and full implementation of #844 to be really useful.

@Debilski Debilski force-pushed the feature/bot-api branch 2 times, most recently from 9900f1c to 6d5989d Compare March 12, 2025 22:36
Debilski and others added 8 commits March 21, 2025 20:56
This undos the split in team/enemy in the state that a team receives.

The reasoning is that this format makes it easier to reuse functions
from game.py in one’s own code.
wip
@otizonaizit
Copy link
Member

Hey, I don't want to give a premature review, but I am a bit worried by the direction this is taking.

I appreciated a lot the idea of cleaning up a bit the use of the game state and rationalizing it's usage thorough the code base. But now I see types creeping in everywhere and I think this is a bigger endeavor and a contentious one at that.
Except for helping the IDEs to give better completions, I am not sure what all the new type definitions are doing for the code base. It's not like we have some form of static type checking going on in the CI or so.
But more importantly, this makes the code harder to read and harder to work with, because now you have another thing to worry about when you look at the code and it becomes harder and harder to edit the code without a properly configured IDE. Especially if we want to keep pelita approachable by relative inexperience programmers and increase the bus factor to a number much higher than 1, I think it is of paramount importance to have easy to understand code at the cost of less efficiency, and also at the cost of less consistency.

This is a big discussion, so maybe this is not the right forum to have it.

@Debilski
Copy link
Member Author

Except for helping the IDEs to give better completions

Exactly. I am using these to catch errors during development with the help of my (pretty much unconfigured) IDE. Apart from that, these are WIP commits in a draft PR. ;)

Main motivation though: Why put things in a comment that no-one reads like in https://github.com/ASPP/pelita/blob/main/pelita/network.py#L17-L32 :

## Pelita network data structures

# ControlRequest
# {__action__}

# ViewerUpdate
# {__action__, __data__}

# Request
# {__uuid__, __action__, __data__}

# Reply
# {__uuid__, __return__}

# Error
# {__uuid__, __error__, __error_msg__}

when we can use modern Python features for the same thing? In that sense, yes, I am planning to rewrite these comments to TypedDicts. Whether we use them to actually annotate functions is another story.

@otizonaizit
Copy link
Member

ok, ok, then I'll retreat and wait for the WIP to be finalized.

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.

None yet

2 participants