Forge Support

[bergerkiller]

BkCommonLib version: 1.16.2-v1-SNAPSHOT

Spigot version: 1.12.2 / 1.15.2 / 1.16

Problem or bug:

BKCommonLib has limited to no support for Bukkit + Forge server implementations.

Progress

Recent development builds have integrated remapping handlers for supporting forge servers, such as Magma, Mohist and CatServer.

TODO

Other Forge server implementations that need work supporting:

• ArcLight: https://github.com/IzzelAliz/Arclight
• Kettle: https://github.com/KettleFoundation/Kettle
• Lava: https://github.com/LavaPowered/Lava

Related

Issue with Magma: bergerhealer/TrainCarts#390

[vico93]

I dont think Kettle is active anymore... i thought in that case the "torch" was passed to Mohist/Magma..

ArcLight would be interesting to have support when 1.16.x builds start appearing, that would be a amazing headstart to have your plugins support in 1.16 so early.

[bergerkiller]

Also note that forge support discussions have been ongoing on the Discord for a while now, just added this issue ticket so that this progress is visible on the outside. There's already 99% completed Forge support for 1.12.2 / Magma/Mohist/CatServer, with a new Remapper API in Mountiplex used for example here: https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/server/MohistServer.java#L108

To support ArcLight would involve adding a Server detection for it, plus registering a remapper for it. If it's similar to the remappers used by the other servers, this shouldn't be hard. Do note however that so far I've only tested/worked on Forge 1.12.2 support. 1.16 probably has a lot more problems.

[bergerkiller]

536a29d

Arclight 1.14 is having too many problems in the remapper code itself. Theres a bug with an internal cache that I can't control, getName() fails randomly on field objects and theres TileEntityRenderer client code mixed in with server code causing a ClassNotFound error at runtime. I don't know if 1.15/1.16 is any better, since this is 'legacy'. But currently it's not looking rosy.

Will try 1.15/1.16 tomorrow, to see if it fares any better.

https://paste.traincarts.net/cuxenagiba.rb

[vico93]

Just a question: where is your discord server? I tried searching on spigot home from each of your plugins but cant find any invites...

[bbayu123]

It's on the traincarts page, right below the video showcases

[IzzelAliz]

Can you take a look at this report? My users are experiencing errors when running latest BKCommonLib.

[bergerkiller]

@IzzelAliz what server is that on? It looks like the remapper isnt working at all

[IzzelAliz]

@IzzelAliz what server is that on? It looks like the remapper isnt working at all

It is Arclight 1.15

[bergerkiller]

I am working on this

[bergerkiller]

Should be all good now with both 1.15 and 1.16 arclight. 5d9c27d

https://ci.mg-dev.eu/job/BKCommonLib/998/

[IzzelAliz]

Some news for you. Since this commit IzzelAliz/Arclight@65ed291 Arclight can handle remapped sources so you don't need to manually call the remapper(by simply "remove" Arclight support). However there are some error I'm not sure how to deal with.
https://pastebin.com/MphhLzXR
If anything I can do on my side I would happy to help.

[bergerkiller]

What exactly was changed? I doubt I can stop calling the remapper directly, because the JVM/javassist must see the 'real' symbols or it fails to compile. So some translation is inevitable here.

Maybe about how the remapper would remap generated classes? Nothing really needs to be changed for that, all it does right now is call the defineClass method using reflection, which doesn't add enough overhead to remove. Works either way :p

The error usually is preceded by a warning or error about a missing requirement, but its absent from the log. Maybe further up? I can look at it later today too

[IzzelAliz]

Some modifications made to remapper and plugins now can read a net/minecraft/server/v1_16_R1/Xxxxx class and get remapped byte source, so your compilers will find all symbols, and defineClass is handled and generated classes will be remapped.

[bergerkiller]

Ah I see, that's pretty nifty. If this gives javaassist class symbols that are remapped to be bukkit-like then that should work, yeah. Only difficulty is that right now, in part because of other forge server projects, it has to make sure a class remapper doesn't remap generated classes. But I can maybe add a global toggle somewhere so for Arclight it allows it.

[bergerkiller]

So the problem was a little bit hidden.

Thing of note: The mpl$ prefix is just a little flag indicator to tell the javaassist resolver to treat what follows as the final name. This prevents double-remapping. I use this so that I can use the compiled names together with to-be-resolved names in the source code.

The reason it says ??worldserver?? is because it failed to find the class worldserver. Which makes sense, because it is a field name.

It actually didnt manage to find a local field matching 'spigotConfig' in class 'WorldServer'. This was further confirmed when I added some extra logging in lookupField, which reveals the first try fails:
FAILED TO LOOKUP LOCAL FIELD net.minecraft.server.v1_16_R3.WorldServer -> spigotConfig

So for some reason, it is unable to find that field. It did manager to find the class names just fine though, so the remapper is doing what it is intended to do.

Any idea what this might be? The spigotConfig field can be found through reflection, since it passed the #exists check in the template.

[bergerkiller]

This version, besides adding support, also prints a better error revealing this local field lookup failed as well.
e583241

What I think is happening: I think spigotConfig is defined in nms.World rather than nms.WorldServer, and for some reason the bytecode generating by Arclight does something wrong to cause this to fail. Might be a remapping class inheritance failure.

Error: https://paste.traincarts.net/enabohoqag.java

https://ci.mg-dev.eu/job/BKCommonLib/1008/

Any input is appreciated, or @mention me on discord and we can discuss there when you have time :)