-
Notifications
You must be signed in to change notification settings - Fork 1.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
chore(JNA): update JNA to 5.6.0, remove com.sun.jna from jopenvr.jar #4169
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.
At this point, we'd just as soon remove jopenvr.jar
entirely, but there are a number of places where there are references to it and I guess that's a bit more work than this PR.
JNA 4.x to 5.x is a major version change. Terasology, outside of jopenvr, uses so little of the API than I don't expect that will be a problem for us.
I'm giving 👍 to this code review but do note I have not tested it under Windows as the test plan requires, so I won't be merging it straight away.
Jopenvr have jna 3.5 :p (i see it in sourcecodes) |
For Windows testing, we want a confirmation that this fixes #4151. So you need a Windows system that reproduces the "can't find dependent libraries" error seen there. I'm not sure what the best way to get that environment is; any Windows installation that's been around for a while may have had MSVC redistributables installed by any number of things. Maybe it would be easiest to use a VM for that? Performance in a VM may not be great but we don't need to actually play the game to validate this. Getting to the main menu is probably enough. Maybe make a default Simple game and make sure you can see the world for a second if you're feeling thorough. Would these VMs work for that? https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/ |
Yeah, virtual machine is great. Another way. Check dependencies for extracted dlls. |
we can use one of these windows VM. their setup for dev environment and are valid for like a month: https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/ you can just download a new one if it goes bad. |
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.
I grabbed this locally for testing to at least make sure it would work normally on Windows, but this is a seasoned instance so I doubt that validates the actual issue - but as long as this works regularly I think we can merge and see if anybody resurfaces with the original issue :-)
Contains
Updating JNA
Removing embeded jna from jopenvr.jar
Fixes #4151
Fixes #4146
How to test
Check what VR works:D (Seems, It is not works many time...)Outstanding before merging