elBukkit / MagicPlugin

A Bukkit plugin for spells, wands and other magic
http://mine.elmakers.com
MIT License
247 stars 147 forks source link

Incompatibility with Purpur / Yatopia (Paper Spigot) forks. #816

Closed mibby closed 3 years ago

mibby commented 3 years ago

RE: https://github.com/YatopiaMC/Yatopia/issues/313#issuecomment-741814539

[05:40:17] [Server thread/INFO]: [Magic] Enabling Magic v8.2-SNAPSHOT-aa1db08
[05:40:17] [Server thread/WARN]: [Magic] �[33m An error occurred, setting arrow lifespan will not work�[0m
java.lang.NoSuchFieldException: ar
    at java.lang.Class.getDeclaredField(Class.java:2489) ~[?:?]
    at com.elmakers.mine.bukkit.utility.NMSUtils.initialize(NMSUtils.java:1513) ~[?:?]
    at com.elmakers.mine.bukkit.magic.MagicPlugin.onEnable(MagicPlugin.java:171) ~[?:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:263) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:380) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:483) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at org.bukkit.craftbukkit.v1_16_R3.CraftServer.enablePlugin(CraftServer.java:501) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at org.bukkit.craftbukkit.v1_16_R3.CraftServer.enablePlugins(CraftServer.java:415) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at net.minecraft.server.v1_16_R3.MinecraftServer.loadWorld(MinecraftServer.java:471) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at net.minecraft.server.v1_16_R3.DedicatedServer.init(DedicatedServer.java:259) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at net.minecraft.server.v1_16_R3.MinecraftServer.w(MinecraftServer.java:941) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at net.minecraft.server.v1_16_R3.MinecraftServer.lambda$a$0(MinecraftServer.java:180) ~[patched_1.16.4.jar:git-Yatopia-"098c015"]
    at java.lang.Thread.run(Thread.java:832) [?:?]

Caused by this breaking patch? https://github.com/pl3xgaming/Purpur/blob/ver/1.16.4/patches/server/0121-Despawn-rate-config-options-per-projectile-type.patch

NathanWolf commented 3 years ago

It looks like it would've had to have been some prior patch since the lifespan field had already been renamed.

In general I do not support forks of Spigot, and the volatility of this change makes me think that's the right decision. If they're messing with NMS they're going to break NMS plugins, unfortunately.

This won't really have a big impact, it's just to make spells that shoot a lot of arrows (arrow rain) not leave a big mess around for long. They'll still despawn as normal.

mibby commented 3 years ago

Would it be possible to add an option to silence nms incompatibility verbose perhaps?

In any case, I'll forward to purpur for investigation. Thanks.

NathanWolf commented 3 years ago

I will change it to a one-line warning like the rest of the incompatibility messages.

I don't think Purpur is going to care much, I mean there's nothing they can really do. If they have to change NMS for one of their patches, there's no real way around this. It's why you're (plugins are) supposed to use the API only, but sadly there are some things I can only do with NMS.

jpenilla commented 3 years ago

Actually, this field has been called despawnCounter since 1.13+... (renamed in Spigot mappings, not by Paper or any fork) and this can be seen in your own source code here. https://github.com/elBukkit/MagicPlugin/blob/master/CompatibilityLib/src/main/java/com/elmakers/mine/bukkit/utility/NMSUtils.java#L1504

The issue is your plugin is for some reason trying to use your "1.8.3" ar mapping.

jpenilla commented 3 years ago

Actually, this field has been called despawnCounter since 1.13+... (renamed in Spigot mappings, not by Paper or any fork) and this can be seen in your own source code here. https://github.com/elBukkit/MagicPlugin/blob/master/CompatibilityLib/src/main/java/com/elmakers/mine/bukkit/utility/NMSUtils.java#L1504

The issue is your plugin is for some reason trying to use your "1.8.3" ar mapping.

Now I see the reason your plugin is doing this. You use getDeclaredMethod which will no longer work as the field has been moved to the parent class. The patch in question is a Paper PR which will be merged somewhat soon, so fixing this on your end is desirable

NathanWolf commented 3 years ago

Thanks for the info!

I think the issue is that it was moved to IProjectile - ha yeah what you just said.

NathanWolf commented 3 years ago

Well since we've talked so much about it now, I guess I should just fix it.

NathanWolf commented 3 years ago

Were you able to test out a dev build, @mibby ?

mibby commented 3 years ago

The latest dev build does seem to fix the problem. Thanks for quickly getting it resolved, @NathanWolf!