SpaiR / imgui-java

JNI based binding for Dear ImGui

MIT License

547 stars 90 forks source link

[API] Fix native loading by stopping creating tmpDir every time #197

Closed kusaanko closed 9 months ago

kusaanko commented 10 months ago

Description

imgui-java tries delete the native library when JVM shutdowns, but JVM locks the native library until JVM shutdowns, so deleteOnExit() will fail.

Instead of creating a temporary directory, imgui-java creates a directory that depends on its version on tmp dir of OS. This enables us to launch different versions of imgui-java at the same time. This way is used by LWJGL.

Fixes #182

Type of change

[x] Minor changes or tweaks (quality of life stuff)
[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

kusaanko commented 10 months ago

Oh I found this works in example code, but do not works in external project. I'll fix this so please wait.

kusaanko commented 10 months ago

I fixed it.

SpaiR commented 10 months ago

This one is really good!

Yet I think we can do it a little bit easier. With all imgui-java jars we provide manifest build.gradle#L21-L30 Which already containes field Implementation-Version with basically what you've added in the properties file. To avoid data duplication and additional logic around it you can try to strip up the logic around props file and modify your code to get the info from MANIFEST.MF file.

kusaanko commented 10 months ago

I think we should not use MANIFEST.MF file. If you make a fat jar, the MANIFEST.MF would not be included. And there are someone who delete META-INF/ in order to modify jar files and disable signature. imgui-java.It may be better if imgui-java.properties created in imgui/ directory.

SpaiR commented 10 months ago

I think we should not use MANIFEST.MF file. If you make a fat jar, the MANIFEST.MF would not be included. And there are someone who delete META-INF/ in order to modify jar files and disable signature. imgui-java.It may be better if imgui-java.properties created in imgui/ directory.

Good call. Yeah, in that case let's keep it in generated properties.

kusaanko commented 10 months ago

Thank you for your nice advice!

kusaanko commented 10 months ago

I think imgui-java should load the native library in this order, how about you?

From imgui.library.path
From java.library.path
From resources

SpaiR commented 10 months ago

I think imgui-java should load the native library in this order, how about you?

From imgui.library.path

From java.library.path

From resources

Sounds reasonable 👍

kusaanko commented 10 months ago

And I think it should be added checking hash of native library. If user set imgui.library.path and put modified native library in it, it might be a danger.

SpaiR commented 9 months ago

I'll do merge on the next week. With it I plan to make a release, so if you are willing to do any other changes to the library - I'll glad to review them. 👍