Skip to content

Move get_messages() from ServerGame to Game#104

Merged
aphedges merged 1 commit intomainfrom
move-get_messages
Jul 21, 2025
Merged

Move get_messages() from ServerGame to Game#104
aphedges merged 1 commit intomainfrom
move-get_messages

Conversation

@aphedges
Copy link
Copy Markdown

This allows the method to be used by clients, preventing them from ever needing to interact with the unfiltered Game.messages field. Hopefully, this prevents any accidental message leakage.

@Obertura777
Copy link
Copy Markdown
Collaborator

I am not sure how to review this. get_messages() returns a native dict object but game.messages is of type sorted_dict. It does not create any game-breaking bugs now but I am not sure it will stay that way if we move to local Game class. What do you think?

@aphedges
Copy link
Copy Markdown
Author

I think it will be fine. I wasn't aware of that until now, but looking at SortedDict, I don't think we use any of its special methods.

Most of our code already uses Game.filter_messages() anyway. This PR is mostly to remove the need to explicitly call the method. I converted all the bots in chiron-utils without issues (ALLAN-DIP/chiron-utils#44), and we can always directly use Game.messages in a specific case if it's absolutely needed.

@aphedges aphedges force-pushed the move-get_messages branch from c328a55 to 0ea9c32 Compare July 18, 2025 20:11
Copy link
Copy Markdown
Collaborator

@Obertura777 Obertura777 left a comment

Choose a reason for hiding this comment

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

Seems good to me!

This allows the method to be used by clients, preventing them from ever
needing to interact with the unfiltered `Game.messages` field.
Hopefully, this prevents any accidental message leakage.
@aphedges aphedges force-pushed the move-get_messages branch from 0ea9c32 to 5160c52 Compare July 21, 2025 14:29
@aphedges aphedges merged commit b9b95e5 into main Jul 21, 2025
2 checks passed
@aphedges aphedges deleted the move-get_messages branch July 21, 2025 17:41
Obertura777 pushed a commit that referenced this pull request Dec 12, 2025
Move `get_messages()` from `ServerGame` to `Game`
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.

2 participants