Update SnakeYAML to 2.0 and shade it

[Phoenix616]

Recently Spigot as well as BungeeCord updated their included SnakeYAML version to 2.0 in the 1.20 update making SLP no longer load.

(It fails with this error on latest BungeeCord:

12:50:33 [WARNUNG] Exception encountered when loading plugin: ServerListPlus
java.lang.NoSuchMethodError: 'void org.yaml.snakeyaml.constructor.CustomClassLoaderConstructor.<init>(java.lang.ClassLoader)'
    at net.minecrell.serverlistplus.core.config.yaml.UnknownConfigurationConstructor.<init>(UnknownConfigurationConstructor.java:34)
    at net.minecrell.serverlistplus.core.config.yaml.ServerListPlusYAML.createWriter(ServerListPlusYAML.java:42)
    at net.minecrell.serverlistplus.core.ConfigurationManager.<init>(ConfigurationManager.java:57)
    at net.minecrell.serverlistplus.core.ServerListPlusCore.<init>(ServerListPlusCore.java:130)
    at net.minecrell.serverlistplus.core.ServerListPlusCore.<init>(ServerListPlusCore.java:92)
    at net.minecrell.serverlistplus.bungee.BungeePlugin.onEnable(BungeePlugin.java:93)
    at net.md_5.bungee.api.plugin.PluginManager.enablePlugins(PluginManager.java:266)
    at net.md_5.bungee.BungeeCord.start(BungeeCord.java:295)
    at net.md_5.bungee.BungeeCordLauncher.main(BungeeCordLauncher.java:67)
    at net.md_5.bungee.Bootstrap.main(Bootstrap.java:15)

)

This change both updates to SnakeYAML 2.0 as well as shades SnakeYAML directly into the resulting jars to avoid such issues in the future.

Technically the shading would only need to be done on platforms that don't ship SnakeYAML 2.0 yet (e.g. Velocity) but imo this is bad design: SnakeYAML is an implementation-detail of the different configuration APIs of those platforms and shouldn't be considered to be ever present in a specific version. As SLP directly uses it it should also provide it itself.

[stephan-gh]

Thanks a lot for reporting this and submitting a fix!

Technically the shading would only need to be done on platforms that don't ship SnakeYAML 2.0 yet (e.g. Velocity)

This would also limit SLP to being used with those more recent platform versions though. For example, right now SLP still supports everything back to MC 1.8.3 on Bukkit. Those have a much older snakeyaml version so requiring 2.0 would limit SLP to MC 1.20+. I know it's probably not worth to actively support such old versions anymore, but I also saw no need to break them so far given that SLP hasn't changed much lately. :)

Shading should also work for those old versions.

but imo this is bad design: SnakeYAML is an implementation-detail of the different configuration APIs of those platforms and shouldn't be considered to be ever present in a specific version. As SLP directly uses it it should also provide it itself.

Yeah, you're probably right. The hard question is where to make the distinction: should SLP also shade Guava and GSON or are those okay? One could argue that those are part of Minecraft, but that doesn't make them required for an independent application like BungeeCord or Velocity. For Bukkit, SnakeYAML is listed in the pom.xml above the not part of the API proper comment, so I'd consider it to be part of the API.

Shading all the dependencies would massively increase SLP's JAR size. SnakeYAML is probably the smallest one of them, but with ~327 KiB still almost as large as the current SLP JAR. Guava would be 3 MiB without minifying it.

Given that having Guava, GSON and SnakeYAML available was kind of a common property of most server implementations so far I just set that as assumption. But perhaps it's indeed easiest to shade SnakeYAML now that there are incompatible versions around.

The alternative would be introducing some semi-ugly runtime hacks to try both variants of the constructor calls. This shouldn't be too complex given that SnakeYAML has only made minor breaking changes in 2.0. One could assume that this will continue to work for quite some time if they don't release another breaking update anytime soon...

Not entirely sure right now, I'll have to think about this some more.

[stephan-gh]

Thanks a lot, I merged this in for now so people have a fixed version available for use. Will think about replacing the shading with some runtime version handling instead (still not sure if that would be better), but that's nothing I'd want to bother you with. :)

[stephan-gh]

SLP uses SnakeYAML quite heavily, extending or hooking into various classes. I believe that's the way it is supposed to be used. Still, I'm not sure how much of that can be really considered "stable API" rather than internal implementation classes. While it seems to have been stable for a long time there were actually problems with old SnakeYAML versions before, back in the CraftBukkit (vs Spigot) MC 1.7 days. CraftBukkit was shipping with a really old SnakeYAML version that was causing trouble with the SLP implementation. I carried the workaround for that for many years: https://github.com/Minecrell/ServerListPlus/commit/6d27fb5e76d3d0b6d6cb84c4c588267b1e936ef7

All in all, I've come to the conclusion that you are right and it's more reliable to use the shaded version. So let's keep it that way. I used the chance to also clean up the Canary port, which was downloading SnakeYAML at runtime so far because it's not available there: https://github.com/Minecrell/ServerListPlus/commit/aee1240a3b2f151c2f1426d2fc0c8124af8240f9 Although I have to say I would be massively surprised if anyone has actually used CanaryMod in the last few years... :D

Thanks again for the quick fix!