-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
GDExtension: Copy DLL to a temp file before opening #80188
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great!
I finally got a new Windows development environment going so I could test this. Without this PR, I got "access denied" when trying to re-compile my GDExtension with the editor open. But with this PR, I was able to re-compile just fine, and the temporary copy got cleaned up when the editor closed.
The code looks good to me too.
07bca3d
to
e9b573e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fairly straightforward.
Didn't test, CC @bruvzg if you're interested in checking it, but should be mergeable as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me.
I would probably make the temp file hidden (but I do not think we have an API for this).
Yeah, I couldn't find a way to make it hidden. |
e9b573e
to
0094482
Compare
Updated to remove the define. |
Needs rebase to fix a temporary extension compat error. |
Very very nice, fantastic job! 💖 I manually rebased this onto f2acfb1 and tried it with godot-rust. |
You could now update this to use the |
This is done only in the editor and only on Windows, to avoid a file lock that prevents the original library being updated (e.g. by a compiler). When the game runs it will load the original DLL and pick up any changes, only the editor will stay with the copy (until it is restarted and create a new copy). The copy is done in place by prepending a `~` to the original file name, so dependencies that are loaded with a relative file path still work. When the library is unloaded the copy file is deleted. The copy is also marked as hidden to not show up in explorer.
0094482
to
cff69b0
Compare
Updated to set the temp file as hidden. I didn't set the read attribute because I think it's not relevant (the file can't be written anyway because of the open lock). Given the name it's also set as an "protected OS file" so it does not show unless you have the option on to show these types of files, which is an extra bonus. |
Discussed at the GDExtension, and we think this looks awesome! |
I retested this with the latest changes, and I noticed a minor issue: if the .dll exists but fails to load for any reason, the copy won't get cleaned up when the editor closes. Probably this should delete the copy right away if there's an error opening the library. Other than, still seems to be working great! |
It probably just requires adding some cleanup code on each possible early return/fail condition in the loading function. |
Sounds good! I can make the follow up :-) |
Thanks! |
This is done only in the editor and only on Windows, to avoid a file lock that prevents the original library being updated (e.g. by a compiler).
When the game runs it will load the original DLL and pick up any changes, only the editor will stay with the copy (until it is restarted and create a new copy).
The copy is done in place by prepending a
~
to the original file name, so dependencies that are loaded with a relative file path still work. When the library is unloaded the copy file is deleted.Closes godotengine/godot-cpp#955
Note: This still does not enable actual reloading for GDExtension, but it is a step towards it.