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

OS.get_time_from_unix_time(OS.get_unix_time()) returns wrong month at 0 o clock / sunday #4025

Closed
blurymind opened this issue Mar 13, 2016 · 8 comments

Comments

@blurymind
Copy link

Operating system or device:
Linux, github build from today

Issue description (what happened, and what was expected):
it is 0:45 - march 13, sunday, 2016

When I do: print (OS.get_time_from_unix_time(OS.get_unix_time()))
I get: (day:13), (hour:0), (minute:45), (month:2), (second:6), (weekday:0), (year:2016)

When I do: print (OS.get_date())
I get: (day:13), (dst:False), (month:3), (weekday:0), (year:2016)

When I print unix time a few minutes later, I get: 1457830499

Also is weekday 0 sunday? :)

@blurymind blurymind changed the title OS.get_time_from_unix_time(OS.get_unix_time()) returns wrong month at 0 o clock/ sunday OS.get_time_from_unix_time(OS.get_unix_time()) returns wrong month at 0 o clock / sunday Mar 13, 2016
@Razzlegames
Copy link
Contributor

Yeah but this seems wrong. Check out this code:

enum Month {
MONTH_JANUARY,
MONTH_FEBRUARY,
MONTH_MARCH,
MONTH_APRIL,
MONTH_MAY,
MONTH_JUNE,
MONTH_JULY,
MONTH_AUGUST,
MONTH_SEPTEMBER,
MONTH_OCTOBER,
MONTH_NOVEMBER,
MONTH_DECEMBER
};

Let me explain the meaning. Enums start at zero implicitly, unless you specify a number for any entry, then it increments by 1 contiguously. Anyhow, this means that MONTH_JANUARY == 0.

This means MONTH_JANUARY != 1 in the code. That's not good IMO.

Maybe I'll fix get_date?

@Razzlegames
Copy link
Contributor

screenshot - 03122016 - 06 45 47 pm

@Razzlegames
Copy link
Contributor

Check out this code. It's totally wrong:

    var dateDict = OS.get_date()
    if(OS.MONTH_MARCH != dateDict['month']):
        print("month: "+ str(dateDict['month']) + \
           " but OS.MONTH_MARCH = "+ str(OS.MONTH_MARCH))

It means it's essentially not march. This is broken everywhere actually.

@Razzlegames
Copy link
Contributor

I guess the easiest thing is to change the enum to start at 1 (and I'll update the docs). Also hope there's nowhere else it's inconsistent. So far I see duplicate structures in:

OS::Month and OS_::Month

@Razzlegames
Copy link
Contributor

LOL, I don't know what the convention is in Godot, but I am going to go with indexed at 1 since it is in all the get_date implementations and resolve the enums to follow this pattern.

Looks like it was followed from Windows SYSTEMTIME structure: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724950(v=vs.85).aspx

Razzlegames added a commit to Razzlegames/godot that referenced this issue Mar 13, 2016
- Also updated the docs to reflect this.
- Added some vim temp files to gitignore
- Changed NaCL to be consistent with the other OS_Unix::get_date implementation
   (added 1 to month to map to 1-12)

Ticket:
godotengine#4025
@blurymind
Copy link
Author

blurymind commented Jun 16, 2017

@Razzlegames I thanked you on facebook for the help on this a while ago, but now I see that I have overlooked thanking you here personally as well for submitting a patch!

Thank you 👍
:D
and also for the awesome support. You have helped me progress with gdscript and python quite a lot

Do you think we can close this issue now?

@vnen
Copy link
Member

vnen commented Jun 17, 2017

Yes, this can be closed since it was fixed by #4027.

@vnen vnen closed this as completed Jun 17, 2017
@Razzlegames
Copy link
Contributor

Razzlegames commented Jun 21, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants