magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.86k stars 761 forks source link

Support OpenJDK 11+ (java 9, java 11, java 17, java 21) #6197

Open allentiak opened 4 years ago

allentiak commented 4 years ago

Allowing people running XMage on OpenJDK 11+ would involve many steps: (UPDATE: expect this list to be updated dynamically)

After migrate:

JayDi85 commented 4 years ago

There are must be another problems too. New watcher code was introduced month ago, but people can't run xmage under java 9+ many years.

allentiak commented 4 years ago

I'm sure this is not the only problem. I just suggested a starting point...

JayDi85 commented 4 years ago

I came up with the idea that the old compatibility problem with java 9+ in different client/server java, not in code compatibility itself (well, except that watcher warning). Xmage uses object's binary serialization for client-server data transfer -- so it can fail on wrong java objects (e.g. server under java 8, but client under java 9). As example: if serialization fail then client can't receive data from server and user will see empty hand on game start.

allentiak commented 4 years ago

I am referring to a different thing: being able to run both the client and server under OpenJDK 11. (Let's remember that XMage can currently run only on an old, unsupported Java 8 Oracle build... - see #5723)

allentiak commented 4 years ago

6380 has been merged! (It closes #6256.)

allentiak commented 4 years ago

@JayDi85, @Craigacp: After your comments, I came up with a migration roadmap towards achieving OpenJDK11+ compatibility. Could you please tell me your thoughts?


I have been checking the dependencies from the project: they are... arcane. E.g. JBoss 4.2.2 GA is from... 2010! So, in order to update everything, I think we will have to divide this process into (at least) three stages:

  1. Update all XMage dependencies to their latest Oracle JDK8-supporting versions. As these updates would be non-breaking, they could be easily merged and released with future XMage versions.

  2. Update all dependencies to their latest OpenJDK8-supporting versions + Allow the Launcher to use an already-insalled OpenJDK8 + OpenJFX8 installation (#6462). Let's remember that, whereas JavaFX is bundled with Oracle JDK8, it is not with official OpenJDK builds. As an Oracle JDK8 drop-in replacement, maybe we could try using Zulu Community[1]? (An OpenJDK8 + OpenJFX8 build.)

  3. (Only once all prior steps have been achieved!) Update the deps and refactor the code to be compatible with OpenJDK 11+.


[1] Zulu Community claims to provide fully free OpenJDK8+OpenJFX8 binaries for all major XMage platforms:

https://www.azul.com/downloads/zulu-community/?version=java-8-lts&package=jdk-fx

allentiak commented 4 years ago

BTW: AFAIK, the only functionality that depends on JavaFX is the What's New dialog... Maybe could it be modified so it does not use it anymore? This may allow to run the game under OpenJDK8...

SpeedProg commented 4 years ago

Please don't, it would make all my work on getting openjfx11 with webkit built for my system naught ;). Is it really the only thing using javafx ? Like none of the GUI uses it? Probably would be a good idea to get rid of it then.

Craigacp commented 4 years ago

One issue with OpenJFX8 is that no-one is maintaining it. I think OpenJFX 11 is the earliest version that is being actively maintained.

allentiak commented 4 years ago

Thanks for your comment, Adam!

On Mon, 4 May 2020 at 16:05, Adam Pocock wrote:

One issue with OpenJFX8 is that no-one is maintaining it.

In the same way, Oracle JDK8 has been declared "unsupported" for non-Oracle subscribers around two years ago, and not publicly available more than a year ago -yet XMage still depends on it...

I think OpenJFX 11 is the earliest version that is being actively maintained.

I agree. As I said, this is just a proposal for a roadmap towards full OpenJFX + OpenJDK compatibility :-)

Craigacp commented 4 years ago

Upgrading the jboss dependencies is the core issue, and I don't think it will be possible without extensive code changes as the jboss remoting package is very different between v2 and v3. I couldn't find any documentation about the changes either, and the latest version is v5 which is basically considered as for internal use by Wildfly.

I'm fairly sure that the rest of the code will be fine on Java 11 once the jboss stuff is sorted, as it's not doing anything particularly fancy with class loading or serialization (which is what causes the jboss issues).

allentiak commented 4 years ago

@JayDi85 Should we pin this issue, so it's visible? Perhaps that could be a way of getting more hands to help here...

SpeedProg commented 4 years ago

I am not sure if it is even needed to update jboss for jdk11. I have the server running under jdk11 and client with java 8 and everything works fine (like I can play against bot etc). If I run the client on openjdk11 + openjfx11 (with some slight modifications like fixing the theme class name) it starts up fine but when I try to join a game the game view does not show up.

Craigacp commented 4 years ago

I get an exception thrown when I try to create a game, which comes out of the serialization infrastructure.


java.lang.reflect.UndeclaredThrowableException
    at com.sun.proxy.$Proxy11.getRoomChatId(Unknown Source)
    at mage.remote.SessionImpl.getRoomChatId(SessionImpl.java:650)
    at mage.client.SessionHandler.getRoomChatId(SessionHandler.java:278)
    at mage.client.table.TablesPanel.showTables(TablesPanel.java:699)
    at mage.client.table.TablesPane.showTables(TablesPane.java:51)
    at mage.client.MageFrame.prepareAndShowTablesPane(MageFrame.java:1140)
    at mage.client.dialog.ConnectDialog$ConnectTask.done(ConnectDialog.java:702)
<snip>
Caused by: org.jboss.remoting.InvocationFailureException: Unable to perform invocation; nested exception is: 
    org.jboss.serial.exception.SerializationException: Could not create instance of java.util.Optional - java.util.Optional
    at org.jboss.remoting.transport.socket.SocketClientInvoker.handleException(SocketClientInvoker.java:146)
    at org.jboss.remoting.transport.socket.MicroSocketClientInvoker.handleOtherException(MicroSocketClientInvoker.java:1104)
    at org.jboss.remoting.transport.socket.MicroSocketClientInvoker.transport(MicroSocketClientInvoker.java:966)
    at org.jboss.remoting.transport.bisocket.BisocketClientInvoker.transport(BisocketClientInvoker.java:470)
    at org.jboss.remoting.MicroRemoteClientInvoker.invoke(MicroRemoteClientInvoker.java:169)
    at org.jboss.remoting.Client.invoke(Client.java:2084)
    at org.jboss.remoting.Client.invoke(Client.java:879)
    at org.jboss.remoting.transporter.TransporterClient.invoke(TransporterClient.java:475)
    ... 35 more
Caused by: org.jboss.serial.exception.SerializationException: Could not create instance of java.util.Optional - java.util.Optional
    at org.jboss.serial.classmetamodel.ClassMetaData.newInstance(ClassMetaData.java:342)
    at org.jboss.serial.persister.RegularObjectPersister.readData(RegularObjectPersister.java:239)
    at org.jboss.serial.objectmetamodel.ObjectDescriptorFactory.readObjectDescriptionFromStreaming(ObjectDescriptorFactory.java:412)
    at org.jboss.serial.objectmetamodel.ObjectDescriptorFactory.objectFromDescription(ObjectDescriptorFactory.java:82)
    at org.jboss.serial.objectmetamodel.DataContainer$DataContainerDirectInput.readObject(DataContainer.java:643)
    at org.jboss.serial.persister.RegularObjectPersister.readSlotWithFields(RegularObjectPersister.java:353)
    at org.jboss.serial.persister.RegularObjectPersister.defaultRead(RegularObjectPersister.java:273)
    at org.jboss.serial.persister.RegularObjectPersister.readData(RegularObjectPersister.java:241)
    at org.jboss.serial.objectmetamodel.ObjectDescriptorFactory.readObjectDescriptionFromStreaming(ObjectDescriptorFactory.java:412)
    at org.jboss.serial.objectmetamodel.ObjectDescriptorFactory.objectFromDescription(ObjectDescriptorFactory.java:82)
    at org.jboss.serial.objectmetamodel.DataContainer$DataContainerDirectInput.readObject(DataContainer.java:643)
    at org.jboss.serial.io.JBossObjectInputStream.readObjectOverride(JBossObjectInputStream.java:163)
    at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:490)
    at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:457)
    at org.jboss.remoting.serialization.impl.jboss.JBossSerializationManager.receiveObject(JBossSerializationManager.java:189)
    at org.jboss.remoting.marshal.serializable.SerializableUnMarshaller.read(SerializableUnMarshaller.java:123)
    at org.jboss.remoting.transport.socket.MicroSocketClientInvoker.versionedRead(MicroSocketClientInvoker.java:1323)
    at org.jboss.remoting.transport.socket.MicroSocketClientInvoker.transport(MicroSocketClientInvoker.java:932)
    ... 40 more
Caused by: java.lang.InstantiationException: java.util.Optional
    at java.base/java.lang.Class.newInstance(Class.java:598)
    at org.jboss.serial.classmetamodel.ClassMetaData.newInstance(ClassMetaData.java:334)
    ... 57 more
Caused by: java.lang.NoSuchMethodException: java.util.Optional.<init>()
    at java.base/java.lang.Class.getConstructor0(Class.java:3427)
    at java.base/java.lang.Class.newInstance(Class.java:585)
    ... 58 more```
JayDi85 commented 4 years ago

EU, US and beta servers works on OpenJDK8 all the time -- it works fine with Oracle's and OpenJDK's clients. Also I tested jdk+jfx pack from zulu source -- it works fine too. But it needs some repack to support backward compatibility with oracle's jdk in xmage's launcher (as example: macos archive have different structure).

For next release I'll try to change default java from oracle to openjdk in launcher settings (all users will get new java update in launcher).

allentiak commented 4 years ago

On Thu, 7 May 2020 at 11:50, Oleg Agafonov notifications@github.com wrote:

For next release I'll try to change default java from oracle to openjdk in launcher settings.

Cool!

Craigacp commented 4 years ago

One thing to note is that OpenJDK 8 and Oracle JDK 8 use different rendering pipelines. They were unified in Java 9 (https://openjdk.java.net/jeps/265).

allentiak commented 4 years ago

Thanks for that data, @Craigacp ! From what you say, I get that we either support both Oracle's JDK 8 and OpenJDK 8, or we settle with OpenJDK 11+?

SpeedProg commented 4 years ago

Optionals should never make it into serializations anyway, so if that is happening that's an other thing that would be good to get rid of :/

Craigacp commented 4 years ago

I assume it's a remote method call that returns an optional, hence it's serialised over the wire.

allentiak commented 4 years ago

Closes #6652

JayDi85 commented 4 years ago

Original problem of xml packages compile in missing module-info.java files -- new Java 9+ modules system. Project must be migrated to it. Related issue #6820.

allentiak commented 4 years ago

Project must be migrated to [the Java module system]

Eventually, yes.

For now, #6703's https://github.com/magefree/mage/commit/1764f13b96e64a8db8a553a037d65a3eabd72129 makes the code work as-is. You will find a link to the relevant Java bug in the commit message.

ldeluigi commented 3 years ago

Done this:

Update the Launcher so it does allow to use a local JDK/JRE, instead of an old, unsopported Oracle JDK8

Here: https://github.com/magefree/Launcher/pull/14