Rfb support and new JUL logging

[gravit0]

This PR fixes bugs related to the old version of ASM in mods such as #148
Instead of launchwrapper, https://github.com/GTNewHorizons/RetroFuturaBootstrap is used which provides transformers that fix this and other problems.
About JUL
RFB breaks hacks used by Crucible to implement logging of plugins and mods. These hacks were removed, and a simplified version of the log4j-jul library was written instead. The log4j-jul library itself has a minimum version of 2.1 and is not compatible with log4j 2.0-beta9. Updating to the latest log4j is complicated by the project structure
About SystemClassLoader
RFB supports two launch options - with the argument -Djava.system.class.loader=com.gtnewhorizons.retrofuturabootstrap.RfbSystemClassLoader and with the argument -Drfb.skipClassLoaderCheck=true. I don't know which option will be better for Crucible, I didn't notice any difference in behavior during testing.
This change may be break something. Requires thorough testing!

[gravit0]

It would be nice to get rfb transformers working for plugins. I might need some help with that.

[juanmuscaria]

Plugin transforming is quite a special case, since you would have to delegate plugin classes to launchwrapper (or rfb in this case) but not define them in the lw/rfb classloader. My proposal would be enumerating transformers/plugins manually and having a config of "safe" transformers that can interact with plugins, then call it directly

[gravit0]

Plugin transforming is quite a special case, since you would have to delegate plugin classes to launchwrapper (or rfb in this case) but not define them in the lw/rfb classloader. My proposal would be enumerating transformers/plugins manually and having a config of "safe" transformers that can interact with plugins, then call it directly

I found method https://github.com/GTNewHorizons/RetroFuturaBootstrap/blob/c948dae9fd90c1db88385915c328dfd51971a294/src/main/java/com/gtnewhorizons/retrofuturabootstrap/URLClassLoaderWithUtilities.java#L37 that can be helpful. This method not define class itself, public and can be invoked from PluginClassLoader. Example use from LaunchClassLoader here: https://github.com/GTNewHorizons/RetroFuturaBootstrap/blob/c948dae9fd90c1db88385915c328dfd51971a294/src/main/java/net/minecraft/launchwrapper/LaunchClassLoader.java#L313
I dont know what context need for plugins. It looks easy

[gravit0]

An alternative option is to use rfb to load plugins directly without PluginClassLoader (the plugin must be ready for this). Unfortunately, such plugins will not be able to interact with others from PluginClassLoader

[juanmuscaria]

I dont know what context need for plugins.
We would need to grab https://github.com/GTNewHorizons/RetroFuturaBootstrap/blob/c948dae9fd90c1db88385915c328dfd51971a294/src/main/java/net/minecraft/launchwrapper/LaunchClassLoader.java#L68 somehow to decide the context, or keep our own transformer exclusion, or both.

About that method, seems like it only invokes rfb transformers while skipping normal lw transformers, which can be good and bad? Rfb transformers would be less common and would not cause as many issues as normal transformers, so less manual labor. But how should mixins be handled?

I'm open to more opinions on how plugin transformation should be handled at all.

[gravit0]

rfb transformers seem safe and are required to work on new versions of Java. We can make options for using rfb and regular transformers the same way remapping is configured now. We can use the runTransformers method via reflection, or if we really need it, create an issue to the authors of RFB.
I don't know what exactly is required for mixins to work (maybe some kind of registration of the mod in the system that is not performed in the case of plugins).
We are unlikely to encounter a situation where we need to load a class from the exclusion list

[gravit0]

Enabling rfb and lw transformers did not break the plugins I tested. However, plugins like NBT API and PowerNBT do not work correctly in both staging and rfb-support (this PR) branches. The problem is that Class.forName returns ClassNotFound if the requested name differs from the name of the received class. This problem persists on both Java 17+ and Java 8.

[juanmuscaria]

That issue in specific was caused by including unimixin, not sure why, but it breaks reflection on vanilla classes due to reflection trying to find unmapped classes.

[gravit0]

That issue in specific was caused by including unimixin, not sure why, but it breaks reflection on vanilla classes due to reflection trying to find unmapped classes.

I not using unimixins while testing.

[gravit0]

Replace Class.forName to classloader.loadClass works for my test plugin. May be need apply this fix to all plugins by asm transformer?

[gravit0]

I tested this PR with GTNH 5.6.1 and popular plugins

1. Small problem with console: small visual bugs with display command
2. WorldGuard/WorldEdit not compatibility with CoFH (for mods who fix this) lw transformer (but lw transformers for plugins are disabled by default)
3. Rfb transformers seems fully safe

Plugins (16): CoreProtect, Chatty, LuckPerms, MultiWorld, WorldEdit, Essentials, EssentialsProtect, EssentialsChat, EssentialsAntiBuild, ClearLag, WorldBorder, TestPlugin, EssentialsSpawn, WorldGuard, Multiverse-Core, Crucible_Server

Can you review this PR?

[juanmuscaria]

Small problem with console: small visual bugs with display command

The logger hack was supposed to fix that by handing over logger control to bukkit console handler

[juanmuscaria]

I wonder if it would be possible to be able to switch between rfb and lw with a simple option toggle, it would require changing crucible bootstrapping a bit though, so something for me to do later.

[gravit0]

The logger hack was supposed to fix that by handing over logger control to bukkit console handler

I try fix this without return logger hack

I wonder if it would be possible to be able to switch between rfb and lw with a simple option toggle, it would require changing crucible bootstrapping a bit though, so something for me to do later.

Probably a bad idea. Rfb contains modified lw classes

[juanmuscaria]

Probably a bad idea. Rfb contains modified lw classes

With a modified bootstrapping, it either loads crucible lw or rfb, so it would not have any conflict.

[juanmuscaria]

All in all looks good, I'll add the cruible start/end patch commends when I merge it. Can't test the changes in a live server at the moment, but if someone else can test it and give feedback, it would be helpful.