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

GDExtension: Remove DLL copy if it fails to load #80720

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Aug 17, 2023

This is a follow-up to PR #80188

That PR causes the Godot editor (on Windows) to make a copy of the GDExtension's DLL in order to prevent Windows from locking the original DLL, so the developer can re-compile it without having to stop the editor first. When the editor is shutdown, any DLL copies for loaded GDExtensions will get cleaned up.

But, currently, if the DLL fails to load, the copy won't get cleaned up. I've been testing this by compiling a GDExtension with scons use_mingw=yes use_static_cpp=no which will fail to load because it can't find the C++ runtime libraries.

Anyway, with this PR, it will clean up the DLL if it fails to load!

@dsnopek dsnopek added this to the 4.x milestone Aug 17, 2023
@dsnopek dsnopek requested a review from akien-mga August 17, 2023 13:54
@dsnopek dsnopek requested a review from a team as a code owner August 17, 2023 13:54
@akien-mga akien-mga modified the milestones: 4.x, 4.2 Aug 17, 2023
@Bromeon
Copy link
Contributor

Bromeon commented Aug 17, 2023

It probably makes sense to add your description as a comment in the code, so that it's clear when reading later 🙂

@dsnopek
Copy link
Contributor Author

dsnopek commented Aug 17, 2023

What comment text would you suggest? I can't seem to come with something that doesn't seem redundant in context.

I feel like it's pretty clear what's going on when you can see the ~5-15 lines that come before this:

Selection_062

@Bromeon
Copy link
Contributor

Bromeon commented Aug 17, 2023

Maybe based on:

When the editor is shutdown, any DLL copies for loaded GDExtensions will get cleaned up.

But, currently, if the DLL fails to load, the copy won't get cleaned up.

We could simply have something like:

// If the DLL fails to load, make sure that temporary DLL copies are cleaned up.

@dsnopek dsnopek force-pushed the gdextension-dll-copy-error branch from 141a2fa to 908b8c0 Compare August 17, 2023 18:23
@dsnopek
Copy link
Contributor Author

dsnopek commented Aug 17, 2023

Ok, thanks, I've added the comment

@akien-mga akien-mga merged commit 5dc7e23 into godotengine:master Aug 17, 2023
@akien-mga
Copy link
Member

Thanks!

@dsnopek dsnopek deleted the gdextension-dll-copy-error branch July 22, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants