Skip to content

Conversation

@Zhennan-Xu
Copy link

let's say in a StartTime = 0, EndTime = 0, Duration = 604800, cron = “0 0 * * 1“ settings, it will not correctly making the startActiveUnix and endActiveUnix as 1 week apart. After some investigation I found the calculation logic in the core_tournament.go’s calculateTournamentDeadlines() function forgot to add .UTC() in two places.

Adding the two missing .UTC() to time.Time in the calculateTournamentDeadlines() fix this issue.

@CLAassistant
Copy link

CLAassistant commented Nov 25, 2025

CLA assistant check
All committers have signed the CLA.

@zyro
Copy link
Member

zyro commented Nov 25, 2025

Thanks for opening this PR!

I had a quick look and as far as I can see wherever the calculateTournamentDeadlines is invoked the input is always a time.Time with the timezone set to UTC, so this change shouldn't be needed. 🤔

Have we missed that somewhere, or is there some other issue?

@Zhennan-Xu
Copy link
Author

It is because cronexpr interprets the cron in the time zone of the time.Time you pass in (fromTime.Location()), not in some global/embedded timezone.

Even though you compare .Unix() values (which are timezone-agnostic), the schedule itself has now shifted from “local wall-clock schedule” to “UTC wall-clock schedule.”

@Zhennan-Xu
Copy link
Author

I just added an unit test for you to quickly check the difference in patched version vs master branch

@zyro
Copy link
Member

zyro commented Nov 25, 2025

Your unit test shows exactly what I mean, you set the timezone on the input time.Time given to the function. My point is I've checked everywhere the function is called and as far as I can tell all those times use UTC timezone - unless there are some where it's missed of course!

@Zhennan-Xu
Copy link
Author

You’re right — I was reusing calculateTournamentDeadlines in some new code I wrote and wasn’t doing the .UTC() conversion on the caller side.

I initially fixed it inside calculateTournamentDeadlines because I wasn’t sure if there might be other non-UTC call sites now or in the future, and normalizing to UTC there felt like a safer guard against accidental non-UTC inputs.

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.

3 participants