Skip to content

Conversation

caymansimpson
Copy link
Contributor

Related to Issue #530 added implementation to accept/reject open team sheets for formats that request it. By default, we reject open team sheets to save the privacy of the bots.

Changes:

  1. Fixed a small lint error that popped up from my last PR
  2. Added param on Player instantiation for open_team_sheet
  3. Added property for open_team_sheet
  4. Added a function to send the chat command to accept/reject team sheet request, upon request, with ps_client
  5. Added unit tests

Also, learned my lesson! It passes:

  1. isort
  2. black
  3. flake8
  4. pytest unit_tests
  5. pyright

Hopefully this one is more straightforward!

Copy link

codecov bot commented Apr 11, 2024

Codecov Report

Merging #533 (cd71673) into master (f458350) will decrease coverage by 0.15%.
Report is 29 commits behind head on master.
The diff coverage is 80.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #533      +/-   ##
==========================================
- Coverage   83.38%   83.24%   -0.15%     
==========================================
  Files          39       40       +1     
  Lines        3918     3998      +80     
==========================================
+ Hits         3267     3328      +61     
- Misses        651      670      +19     

Copy link
Owner

@hsahovic hsahovic left a comment

Choose a reason for hiding this comment

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

This looks good! I left a couple of minor comments, but this is almost ready to merge. Thanks a lot!

battle_format: str = "gen9randombattle",
log_level: Optional[int] = None,
max_concurrent_battles: int = 1,
open_team_sheet: bool = False,
Copy link
Owner

Choose a reason for hiding this comment

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

Can you rename this parameter to accept_open_team_sheet?

elif split_message[1] == "bigerror":
self.logger.warning("Received 'bigerror' message: %s", split_message)
elif split_message[1] == "uhtml":
if split_message[2] == "otsrequest":
Copy link
Owner

Choose a reason for hiding this comment

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

uhtml messages are currently handled in battle.parse_message - can you combine the two conditions (eg. elif split_message[1] == "uhtml" and split_message[2] == "otsrequest") so that other messages are still parsed by it?



def test_open_team_sheet():
assert SimplePlayer(open_team_sheet=True).open_team_sheet
Copy link
Owner

Choose a reason for hiding this comment

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

i would add a negative and a default test as well

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