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

[Windows] Use GetFileTime for FileAccess #74830

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Mar 12, 2023

Prevents DST from rearranging file times.

_wstat does weird things with DST, which from I can find does not apply to Unix

Internally the same function is used but _wstat converts it to local time, without indicating DST, the approach here is based on that in Windows CRT, using creation time as fallback, and here ensuring the epoch is valid.

For all except DST overlap times this gives the same result, in UTC

Edit: Also resolves an inconsistency between MinGW and MSVC builds, see #92911

Fixes #74816
Fixes #92911
Fixes #96810
Fixes #96828
Fixes #96812

@AThousandShips AThousandShips requested a review from a team as a code owner March 12, 2023 16:26
@AThousandShips
Copy link
Member Author

AThousandShips commented Mar 12, 2023

For easy testing, use PowerShell and (Get-Item file).LastWriteTimeUtc=("date") and see that time during the hour of overlap with DST now works

@AThousandShips AThousandShips force-pushed the win_time_fix branch 2 times, most recently from 30c84f2 to bd34eed Compare March 12, 2023 20:43
@YeldhamDev YeldhamDev added this to the 4.x milestone Mar 13, 2023
@AThousandShips
Copy link
Member Author

AThousandShips commented Apr 27, 2024

Need to investigate an issue that has arisen with this, unsure what exactly but it no longer works on my setup, will try fix it when I find the time to dig through the Windows stuff

Edit: Works now, had to add a flag to be able to process directories, which broke detection, now it works correctly

@AThousandShips AThousandShips marked this pull request as ready for review May 1, 2024 11:05
@AThousandShips AThousandShips modified the milestones: 4.3, 4.4 Jun 28, 2024
@AThousandShips AThousandShips added cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Jun 28, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Jul 14, 2024

I marked this as fix for #92911, but after testing again now, the results are wrong. Did something change?

@AThousandShips
Copy link
Member Author

AThousandShips commented Jul 14, 2024

I will take a look, nothing should have changed though

In what way are the results wrong?

@KoBeWi
Copy link
Member

KoBeWi commented Jul 14, 2024

Expected results is that I get the same modified time in a dev build and an official build. This is no longer true.

@AThousandShips
Copy link
Member Author

This code hasn't changed so I suspect something with the official builds have changed, or windows libraries

@akien-mga
Copy link
Member

CC @bruvzg

Prevents DST from rearranging file times.
@akien-mga akien-mga changed the title [Windows] Use GetFileTime for FileAccess [Windows] Use GetFileTime for FileAccess Sep 6, 2024
@akien-mga akien-mga merged commit 88e9af6 into godotengine:master Sep 11, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the win_time_fix branch September 11, 2024 11:11
@AThousandShips
Copy link
Member Author

Thank you!

@The-Cyber-Captain
Copy link

LOL. You won't believe how insane this manifested itself when targeting VR; ie, iterating Win-based exports / remote debugs of Android APKs for standalone headsets. I thought I was going full-on LAWNMOWER MAN mad inside the VR there. (With changed resources apparently 'randomly' being updated or included in the next run... all based on what the Godot Editor under Windows thought had changed on file, obviously, in hindsight)

Thanks for the quick fix! Looks like you managed to get it rectified and merged in less time than it took me to even pinpoint and reproduce for a Bug Report. Bravo!!! [applause]

@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

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