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

Fix getPromotionDate computation for youth players joining at 16+ y.o. #1713

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dominiquelee
Copy link
Contributor

A player has to have been in a team for a full season (i.e. 112 full days) before being promoted. For players that joined at 16 y.o., the hour at which they were hired must be taken into account.

A player has to have been in a team for a full season (i.e. 112 full days) before being promoted.
For players that joined at 16 y.o., the hour at which they were hired must be taken into account.
@minj
Copy link
Owner

minj commented Sep 2, 2023

If I recall correctly this has something to do with the hacky way we deal with dates, causing issues when daylight savings time (de)activates. Have you tested this scenario?

@dominiquelee
Copy link
Contributor Author

I didn't think about it and it sure sounds like a lot of unfun to handle.
I see addDaysToDate does ret.setDate(ret.getDate() + dayCt);, which, from some quick testing, should return the same hour when we pass a DST day, but with a different timezone.
I wonder if the right call isn't this to drop the timezone in this (specific?) computation (or at least, directly realign it with the initial value) - though, dropping the timezone is probably not the greatest idea and is likely to have many random side-effects.

@minj
Copy link
Owner

minj commented Sep 8, 2023

Yes, I hate having to deal with time zones in browsers.

I thought there was an issue to use moment.js but I cannot for the life of me find it any more. IIRC Hattrick includes the library on the site, it would be good to reuse their code as much as possible as I do not feel like shipping time zone definitions in Foxtrick.

Other than that I hear there are new browser localization libraries available...

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.

None yet

2 participants