Do not inline the version string

[Geolykt]

Few will be aware of the constant inlining done by javac so this may hurt some library authors (though outside of that few should be affected by the constant inlining). Plainly appending .toString() makes so javac does not inline the string, meaning that the version string can be used to obtain the runtime version of libGDX, not the version used to compile the library with.

Note: Obviously this won't retroactively affect older binaries, so older libraries that are compiled with older versions of libGDX may still display this anomaly

Given that the JIT compiler should inline the constant anyways (even if javac doesn't) and this field is to be outside of hot code anyways performance is not a concern. I do not expect any regressions even with non-desktop environments, but have not tested them.

[tommyettinger]

What situations does this affect? The one place I have seen Version used is in gdx-liftoff, on this line and on a few others in the repo. There it is used to match a generated project's version with the version of libGDX that Liftoff was compiled with, since there's no way Liftoff can know if a future release will be 1.12.2 or 1.13.0 (or 2.0.0!). From what I can tell, Version.java exists to allow code that depends on libGDX to know what version is being depended on. I don't know what the effects of constant inlining are in this case, nor if some versions of Java may inline .toString() on final immutable classes such as String.

[Geolykt]

I am not aware of any JDKs that are so quick as to inline even .toString() invocations. More specifically, javac should only inline fields that have a ConstantValue attribute (https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-4.html#jvms-4.7.2) - as I'm not seeing that ConstantValue in the class file I doubt that it does any sort of inlining.

The problems with inlining is that the value that is used at compile-time will be used, where as most people would expect the runtime version. This is okay for applications where runtime and compiletime libGDX versions are identical (liftoff is probably one of these applications), but in case of libraries this field shouldn't be relied upon as the API consumer could decide to go with a different version. Of course, people should be using MAJOR and MINOR for compatibility comparisons, but it is always someone that doesn't do it the intended way.

[tommyettinger]

So... I'm not quite clear here. When building with Gradle, there's one version of each dependency that gets used, right? I can run gradlew core:dependencies and see what version was chosen, and what versions may have been overridden. How could constant inlining from a version that isn't actually being used affect the version that is being used?

[Geolykt]

When doing ./gradlew compileJava, the produced *.class files have the VERSION constant inlined. That is if my library does something like

metrics.setLibraryVersion("libGDX", com.badlogic.gdx.Version.VERSION);

then after compilation with libGDX 1.12.1 on the compile classpath, the *.class file will have the following content:

metrics.setLibraryVersion("libGDX", "1.12.1");

the problem here is that at runtime (so quite a bit after compilation), this may no longer hold true.
So if the javac in libGDX can inline the assignment expression to VERSION to a ConstantValue, then so can the javac executable for other projects (including libraries).

---

edit: Also just to make triple sure: This is Java, not Rust nor C. Gradle consumes prebuilt binaries (the .class files within jars downloaded from the maven repositories) and will not try to compile from source again. Hence, it very well matters what versions of libGDX dependencies were compiled with (though only in the context of constant inlining, the only optimization done by javac), even if your end-user application uses a different version of libGDX.

[EtK2000]

Some Java compilers will remove the .toString() and inline, because String::toString returns itself which is still a constant value.

I could in theory see a use for not inlining so a library could call different methods for backwards compatibility or something, but it's super niche and not common as far as I'm aware.

Also, the value is still accessible at runtime even if inlined (for example via reflection), but that's a can of worms not worth the effort for this.

[Geolykt]

Which java compilers remove the .toString()? 99% of optimization occurs during JIT, not during class compilation. And most importantly, are those compilers used to compile libGDX? I don't think we are using GCJ, Eclipse compiler, janino or any other obscure compiler here that might as well do these optimizations.

I could of course deprecate the field for removal and mandate access be done via a method instead (in fact that was what I first suggested over at discord), but that comes with it's usual problems of changing nothing to people that ignore deprecation warnings. We could of course do more "expensive" calculations that cannot be inlined but that is nonsense too.

[EtK2000]

Whatever Android Studio is using inlines it, it actually complicated a few tests for me in the past.

The question is how big is the benefit from this?
As of now, I don't see enough of a reason to change the status quo.

[Geolykt]

As long as just the compiler used by the IDE but not the compiler used by gradle or maven removes .toString(), all is good.

The main benefit of this change is that it removes a potential vector for bugs that is not well understood for beginners or people that have switched to java recently

[tommyettinger]

for beginners or people that have switched to java recently

So, not library authors, hopefully. I'll see what happens if I make a single commit of anim8-gdx (which uses a rather old libGDX version for compatibility) store and print Version.VERSION, then depend on that from a current project.

EDIT: Yep, the old version persists if a dependency was built using that old version.

That said, it might be useful to know if some bug in a library stems from it being compiled with an older libGDX.