Cast to Buffer

[SimonIT]

If you compile you android app with a jdk greater than 8 and try to run it on an android device with API level lower than 29 (from my experience) the app crashes. The methods position, limit, flip and clear from buffer subtypes are the problem.

Detailed description is here:
https://stackoverflow.com/a/58435689/7912520

I just applied the suggested fix.

@PokeMMO found an issue on the google issue tracker, but they seem no to care. https://issuetracker.google.com/issues/123015428

[tommyettinger]

Thank you! This doesn't only affect Android; if libGDX were compiled on any machine using Java 9 or higher, that set of JARs would have been incompatible with Java 8, without this PR. There's ways to compile with JDK 9 or higher and target JDK 8, but they can only be used if you're compiling with JDK 9 or higher, not with JDK 8, so the build files get more complex. I like this solution, and I hope it works on GWT -- buffers already were a problem there in places like Pixmap.getPixels().

[SimonIT]

I executed some of the gwt tests with buffer in the name and didn't find any problems.
I can add some more casts. There should only be a few in the individual backends

[noblemaster]

I think that's great to have in regards to compatibility. +1 to have this merged.

[crykn]

+1. I think the alternative of adding the --release 8 compiler option would be more clean, however that would also complicate our build files, as this flag mustn't be used for Android projects and can only be used if the local JDK is 9+.

[PokeMMO]

Another option I'm liking more, is to use Gradle 6.7's toolchains support. It doesn't require any workarounds to know when to apply the release 8 option, and simply just always uses java8 to compile.

https://docs.gradle.org/current/userguide/toolchains.html

[SimonIT]

@PokeMMO Does it work in combination with android? Because from the link it looks like a java plugin feature which can't be applied to android projects

[PokeMMO]

99.9% sure it does, I tested by setting it to an unavailable JDK and it complained.

Seems like the following format works

android {
    java {
        toolchain {
            languageVersion = JavaLanguageVersion.of(8)
        }
    }
}

[SimonIT]

Just tested it and it seem to work.
One thing to notice is:
The new Java toolchain feature cannot be used at the project level in combination with source and/or target compatibility
Also a separate java 8 installation is needed.
So with this way android developers who want to use agp version 7 or greater will need jdk 8 and minimum 11

[PokeMMO]

It also includes auto-provisioning (via adoptopenjdk) a JDK if necessary, which makes setup easier if we do end up utilizing multiple JDKs.

Auto download can be turned off via gradle property, for paranoid folks like myself.

[tommyettinger]

I'm in favor of changing the source to use (Buffer) casts, because I've copied lots of code from libGDX into my own projects and while I can't always reproduce the build settings, I would always be able to copy the Java code. There's no reason we can't do both; I think the Buffer cast is a no-op on Java 8, and it should work even if, for some reason, the Gradle build gets broken and we need to use Maven or Ant.

[noblemaster]

+1 to what @tommyettinger said. There is no reason we can't have both. For users of libGDX, there are no negatives as the casts are hidden for them. Although I am using gradle for Android & iOS, I am still using Ant for my desktop builds (binaries for Steam/etc.) and I don't see a reason to change to gradle given how things are evolving, i.e. breaking changes with each new release.

This still says it's a "Draft". Looks sort of finished? I'd say, we merge if this is complete? Then someone would probably have to add the toolchain stuff to gradle later.

[PokeMMO]

I'm in favor of both, we can merge this now and still use toolchains via gradle later if we decide to go that route.

[SimonIT]

It's only a draft because @mgsx-dev said that there is stuff to discuss

[obigu]

I'm not against merging it in the meantime (while we add release 8 logic) but this patch is extremely fragile. It's just a matter of time until somebody forgets to add a cast on new code or gets rid of one of the added ones deeming it unnecessary.

Just for reference, this is the full list of affected methods signatures:

• position​(int newPosition)
• limit​(int newLimit)
• flip​()
• clear​()
• mark​()
• reset​()
• rewind​()

And a nice blog post that covers the issue https://www.morling.dev/blog/bytebuffer-and-the-dreaded-nosuchmethoderror/

[mgsx-dev]

Indeed, this change (weird cast) is very fragile, as obigu said we could miss some cast or remove some accidentally in the future. Also, user code can use Buffer (they are not hidden) and should not forget to cast them in their own code as well for some obscure reasons.

In the worse case scenario (if it can't be fixed by some build options), maybe we should totally abstract Buffer.

If i'm not wrong, for now the workaround is to build with Java 8. I think it's fine to keep it like this until we find a better solution.

[noblemaster]

Yes, let's keep it until we find a better solution.