Skip to content

Comments

Added a clash of code module#5

Open
RedsTom wants to merge 13 commits intoadraug:masterfrom
AsyncCommunityDiscord:feature/clash_of_code
Open

Added a clash of code module#5
RedsTom wants to merge 13 commits intoadraug:masterfrom
AsyncCommunityDiscord:feature/clash_of_code

Conversation

@RedsTom
Copy link

@RedsTom RedsTom commented May 19, 2023

Object: Adding a module to be able to share a clash of code game with part or all a discord server

Clash of code module

Main features:

  • /clash-of-code [link] to share a game of clash of code
  • /config coc-role [role] to configure the role to be pinged when a game is shared
  • /config coc-channel [channel] to configure the channel where the ping message will be sent
  • Live update of the game's information

Technical features:

  • Checks for the validity of the link
  • Updates the announcement message even on restart thanks to vesta/services/clash_of_code_helper#resume_update_loops

Breaking changes:

  • The structure of the Guild table has been updated to add some more configuration

Technical changes:

  • Rework of the /config command to be able to add a configuration command more easily. Still needs some work, but in an other PR.
  • Added the requests module to the list of dependencies to be able to poll the Codingame API
  • Renamed lang to lang_file to avoid confusion between the lang.py file and the lang variable managing the translations

@RedsTom RedsTom marked this pull request as ready for review May 19, 2023 23:57
@RedsTom
Copy link
Author

RedsTom commented May 19, 2023

Review requested:

Copy link
Collaborator

@Astremy Astremy left a comment

Choose a reason for hiding this comment

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

Don't put the lang changes in the same PR, don't do synchronous requests in an asynchronous bot, just use a foreign key to another table in the guild table, don't put _ in front of half of the functions... some other things

return

try:
(game, game_id) = _get_game(link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why some _ in the start of the function? That's not a library, and that's not some not-to-call methods of an object

Copy link
Author

Choose a reason for hiding this comment

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

Will fix

from yaml import load, Loader
with open("vesta/data/lang.yml") as file:
lang = Lang(load(file.read(), Loader), session_maker)
lang_file = Lang(load(file.read(), Loader), session_maker)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why letting the lang file named as "lang" and renaming the lang as "lang file"?

Copy link
Author

Choose a reason for hiding this comment

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

Ask @adraug

start_update_loop(message=announcement_message, guild=interaction.guild)


def _get_game(link: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't put some _ in front of the function name, as said before

Copy link
Author

Choose a reason for hiding this comment

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

Will fix

return game, game_id


def _get_guild_game(interaction: discord.Interaction) -> ClashOfCodeGuildGame:
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you have only one clash of code game per guild, maybe just use a foreign key in the guild table to a clash of code game

Copy link
Author

Choose a reason for hiding this comment

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

Will fix


:return: Nothing
"""
from .. import session_maker, vesta_client
Copy link
Collaborator

Choose a reason for hiding this comment

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

Weird imports place

Copy link
Author

Choose a reason for hiding this comment

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

If imported at the top of the file, creates a circular import, so it's a workaround that works to avoid it, needs more debugging to find where the circular import takes place

:param guild: The guild to update the data for
:return: Nothing
"""
from .. import session_maker
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, weird imports

Copy link
Author

Choose a reason for hiding this comment

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

Same reason as above


await asyncio.sleep(10)

asyncio.create_task(update_loop(fetched_game)) No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

NO. Don't start a while True task for each one. If you want to do something like that properly, start one task, and use an updating queue. It will prevent any "oh it never actually finish for some weird reasons" problem

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's an absolutely must not

Copy link
Author

Choose a reason for hiding this comment

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

Should never happen because the API always ends the game at a moment

Copy link
Author

Choose a reason for hiding this comment

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

Will fix anyway

__tablename__ = "clash_of_code_guild_game"

guild_id = db.Column(db.BigInteger, nullable=False, primary_key=True)
last_clash_id = db.Column(db.String(511), nullable=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a need for a so long ID? verify the actual size before setting random values

Copy link
Author

Choose a reason for hiding this comment

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

Will fix


session = session_maker()

class ClashOfCodeGuildGame(Base):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it not just "ClashOfCodeGame" if there is only one per guild?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As said maybe use a foreign key from guild to link it up

Copy link
Author

Choose a reason for hiding this comment

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

ClashOfCodeGame is already used to talk about the representation of an api object

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