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

Fix android method call #91

Merged
merged 1 commit into from
Aug 30, 2018

Conversation

martin308
Copy link
Contributor

Discovered by @willispinaud in #88, the way we were calling this method fails on android v6. Dropping back down to the lower level JNI calls resolves this.

Have tested this on android 4, 5 and 6 on a device.

@martin308 martin308 requested a review from a team August 22, 2018 21:57
For some reason the way we are calling this method does not work on
at least android v6. I have tested this on android 4, 5 and 6
@martin308 martin308 force-pushed the martin308/android-compatibility branch from 0b88785 to e93c134 Compare August 22, 2018 22:15
@fractalwrench
Copy link
Contributor

Would it be possible to get an explanation of what the issue was on Android? Is this an issue on later devices e.g. Android 7, 8?

@martin308
Copy link
Contributor Author

It wasn't entirely obvious what the issue was. My biggest hunch is that it couldn't infer which method to call based on the arguments provided anymore. Although I'm not sure how this relates to just the android version changing. I'm currently working on getting unity android maze-runner tests running so that I can test on other versions

@fractalwrench
Copy link
Contributor

Adding new tests that cover this functionality, and running them on all API levels sounds like a good start. I think it'd also be worth spending a bit of time trying to understand why the original code was causing a crash, as it may reveal that the fix doesn't work for all inputs, or that similar issues exist elsewhere in our use of the JNI.

@martin308 martin308 merged commit fa9c0f4 into martin308/unification Aug 30, 2018
@martin308 martin308 deleted the martin308/android-compatibility branch August 30, 2018 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants