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.

Sign up for GitHub

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

Jump to bottom

Fix inconsistent last_modified_time handling in GDExtension #82603

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Fix inconsistent last_modified_time handling in GDExtension #82603

merged 1 commit into from
Oct 2, 2023

Conversation

kkolyan
Copy link
Contributor

@kkolyan kkolyan commented Oct 1, 2023 •
edited by dsnopek
Loading

Fixes #82601 fix inconsistent last_modified_time handling. GDExtension::library_path variable stores original DLL path now.

@kkolyan kkolyan requested a review from a team as a code owner October 1, 2023 01:23
@kkolyan kkolyan changed the title #82601 fix inconsistent last_modified_time handling. GDExtension::lib… #82601 fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Oct 1, 2023
@kkolyan
Copy link
Contributor Author

kkolyan commented Oct 1, 2023

Tested locally on Windows 10 - now reload is activated only if original DLL is really changed.

@kkolyan
Copy link
Contributor Author

kkolyan commented Oct 1, 2023

@dsnopek FYI

@fire fire changed the title #82601 fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Oct 1, 2023
dsnopek
dsnopek approved these changes Oct 1, 2023
View reviewed changes
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch! I definitely need to (re-)test on Windows more often in general.

I tried your PR on Windows 10 and it works great! I also tested on Linux just to make sure there aren't any non-Windows regressions, and it's also all good there.

@dsnopek dsnopek added this to the 4.2 milestone Oct 1, 2023
@dsnopek
Copy link
Contributor

dsnopek commented Oct 1, 2023

Ah, and we don't generally put issue numbers in commit messages. It'd probably be good to amend the commit message to match the PR title, which is more like the standard used for commit messages in Godot - the production team will like that better :-)

@akien-mga akien-mga changed the title Fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Oct 1, 2023
@akien-mga akien-mga changed the title Fix inconsistent last_modified_time handling (prevent GDExtension reload spam on Windows 10) Fix inconsistent last_modified_time handling in GDExtension Oct 1, 2023
@kkolyan kkolyan mentioned this pull request Oct 1, 2023
Merged
@akien-mga akien-mga merged commit 98d9119 into godotengine:master Oct 2, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@kkolyan
Copy link
Contributor Author

kkolyan commented Oct 2, 2023

yeah, that's fun to be part of :)

@dsnopek dsnopek mentioned this pull request Nov 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsnopek dsnopek dsnopek approved these changes

Assignees
No one assigned
Labels
bug platform:windows topic:gdextension
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

GDExtension reload spam on Windows 10 due to inconsistent last_modified_time handling
3 participants
@kkolyan @dsnopek @akien-mga