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

WIP: Better options for creating challonge tournaments #724

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

n8kim1
Copy link
Contributor

@n8kim1 n8kim1 commented Jan 15, 2024

Blocked by #723

Attachments change: To actually let us add attachments (in our case, replay links). Attaching via API doesn't work but at least you can do manually
Game name: This is required not by the API v2 but by the tournament settings website. (Either they do frontend validation or use v1 of API.) So if you ever change the settings manually via website, you have to go back and add the game name, which gets really painful.... Also all our other tournaments have game name anyways, so might as well keep that around.

Also includes some dev notes

acrantel
acrantel previously approved these changes Jan 16, 2024
Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

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

lgtm, great documentation!

@@ -59,6 +59,8 @@ def create_tournament(tournament: Tournament, *, is_private: bool):
"tournament_type": challonge_type,
"private": is_private,
"url": challonge_id,
"game_name": "Battlecode",
Copy link
Member

Choose a reason for hiding this comment

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

These changes just set up our tournaments to support replay url attachments in the future, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Also, will we need to merge #723 before merging this PR? b/c I see changes from camelcase to snakecase in that PR, but i'm not sure if that's a challonge v2 -> v2.1 change or challonge v1 -> v2 change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup (and also they do more things)
updated PR description for more context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good catch, idk if "game_name" is v2 or v2.1
I'll play it safe

@n8kim1 n8kim1 changed the title Better options for creating challonge tournaments WIP: Better options for creating challonge tournaments Jan 16, 2024
@n8kim1 n8kim1 changed the base branch from main to nkim-tour-api-v2.1 May 27, 2024 14:54
Base automatically changed from nkim-tour-api-v2.1 to main September 28, 2024 23:42
@n8kim1 n8kim1 dismissed acrantel’s stale review September 28, 2024 23:42

The base branch was changed.

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