feat: Added time until next match, when there are no live matches. #99
Conversation
src/Browser.py
Outdated
| days, remainder = divmod(total_seconds, 86400) | ||
| hours, remainder = divmod(remainder, 3600) | ||
| minutes, seconds = divmod(remainder, 60) | ||
| return f'{days}d {hours}h {minutes}m' |
There was a problem hiding this comment.
Please, change this to something like f'None - next in {days}d {hours}h {minutes}m'.
Another light improvement could be excluding the days if it is 0 - f'None - next in {str(days) + "d" if days else ""} {hours}h {minutes}m'.
There was a problem hiding this comment.
Otherwise we get a lot of questions from users who are used to the old text
There was a problem hiding this comment.
Maybe its an idea to place this outside the table, since this has no relation with Live matches
Maybe using rich.layout? https://rich.readthedocs.io/en/stable/layout.html
There was a problem hiding this comment.
In the new PR it says "None - Next one in x hours and x minutes.
I think placing it outside, would just look off, and will add a bit of clutter.
If Poro thinks your suggestion is better i can change it, but tbh, i think this looks very good.
…isplay hours and minutes. Also added "None - " in front of time until next match, as requested.
src/Browser.py
Outdated
| minutes, seconds = divmod(remainder, 60) | ||
| return f"None - next in {str(days)}d" if days else f'None - next in {hours}h {minutes}m' | ||
| except: | ||
| return "" |
There was a problem hiding this comment.
Change it to "None" to keep it consistent.
|
Other than the two minor things, it's ready to merge :) Nice PR |
|
Unsure if this was the correct way, will it ever reach if res.status_code? :I |
| liveMatchesMsg = f"{', '.join(liveMatchesStatus)}" | ||
| newDrops = self.browser.checkNewDrops(self.stats.getLastDropCheck(self.account)) | ||
| self.stats.updateLastDropCheck(self.account, int(datetime.now().timestamp()*1e3)) | ||
| else: | ||
| liveMatchesMsg = "None" | ||
| liveMatchesMsg = self.browser.getTimeUntilNextMatch() |
There was a problem hiding this comment.
Would ideally like to see the addition of a timeout between each check of getTimeUntilNextMatch but it is not necessary. It would mostly be for efficiency purposes, so you aren't making multiple unnecessary requests per thread.
There was a problem hiding this comment.
I think we should actually refactor the whole program and create a new thread that would handle common tasks. Like get live, schedule etc.
Those actions do not require auth.
I fixed it. It wouldn't reach the status code check. |
Example:
