Skip to content
This repository has been archived by the owner on Apr 20, 2024. It is now read-only.

Add configurable timeouts #137

Merged
merged 4 commits into from
Apr 15, 2023
Merged

Add configurable timeouts #137

merged 4 commits into from
Apr 15, 2023

Conversation

akshualy
Copy link
Collaborator

@akshualy akshualy commented Apr 14, 2023

Miss Minutes GIF

Description

Addresses one of the tasks in #124. Adds a map to the config with pre-defined timeouts. Leverages enum, so it's as little effort as possible on our side to add more timeout options in the future.

Notes

Fixes an issue where the bot got stuck if the session expires while being in-game.
Fixes an issue where we got an error if the match history returns None.
Now prints the timer if CTRL+C was pressed. Additionally limits the timer from being printed too many times.

I've also tried adding a fitting gif to follow your fancy PRs :D

Copy link
Owner

@Kyrluckechuck Kyrluckechuck left a comment

Choose a reason for hiding this comment

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

Generally looks good to me! Readability definitely takes a bit of a hit, but I think the benefit likely outweighs that.

Haha thank you, we really need to get PR templates up of the format:

<!--- Add a GIF describing your PR --->
![]()

## Description
<!--- What this PR is introducing or fixing --->

<!--- If there are any issues related, please mention them --->

<!--- An additional section (or more) for additional notes that don't make sense in the main description --->
<!--- ## Notes --->

@akshualy
Copy link
Collaborator Author

akshualy commented Apr 14, 2023

Yeah, a big hit in readability comes from "having" to put almost all of the timeouts into a variable, so we don't have to get it twice to log and use it. I'm not sure of a better way to do this, honestly, other than to wrap each timeout into it's own method, which feels a bit ridiculous lol
So I just went with - IMO - readable-ish variable names

@Kyrluckechuck Kyrluckechuck merged commit f1a8653 into main Apr 15, 2023
@Kyrluckechuck Kyrluckechuck deleted the Add-Configurable-Timeouts branch April 15, 2023 04:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants