JorelAli / CommandAPI

A Bukkit/Spigot API for the command UI introduced in Minecraft 1.13
https://commandapi.jorel.dev
MIT License
504 stars 60 forks source link

Cannot load on 1.20.4 mojang-mapped Paper server #551

Closed frengor closed 2 months ago

frengor commented 2 months ago

CommandAPI version

9.4.0

Minecraft version

1.20.4

Are you shading the CommandAPI?

No

What I did

I put the mojang-mapped CommandAPI plugin inside the plugins folder and started a 1.20.4 mojang-mapped Paper server.

What actually happened

A bunch of errors appeared in console.

What should have happened

CommandAPI should have loaded without errors.

Server logs and CommandAPI config

Logs: https://paste.gg/p/anonymous/181a4dcd5405451e919576b4857dee81

Other

I originally discovered this while upgrading UltimateAdvancementAPI from using CommandAPI 9.3.0 to 9.4.0. UltimateAdvancementAPI uses the shaded version, but the issue also occurs with the plugin version.

Also, the issue doesn't occur in 1.20.5 and 1.20.6, but it's present in older versions (tested with mojang-mapped Paper 1.19.4 and 1.18.2).

JorelAli commented 2 months ago

It may have been missed from the documentation, but when shading there's an extra step required in the CommandAPI.onLoad() method. In your CommandAPIBukkitConfig, there's an extra method useMojangMappings(boolean) which needs to be set to true.

frengor commented 2 months ago

Definitely missed that, thank you! However, the errors are appearing on both the plugin and shaded versions, not just the latter (the attached logs are from a server with only the CommandAPI plugin). Also, setting useMojangMappings(true) while using the shaded version doesn't fix the errors.


Off-topic question: why is calling useMojangMappings(boolean) required? Isn't using commandapi-bukkit-shade-mojang-mapped instead of commandapi-bukkit-shade enough?

JorelAli commented 2 months ago

I would have expected useMojangMappings(true) to have worked, because that sets an internal flag in the CommandAPI to use different field names for certain reflection calls.

I'm currently away for the weekend (and Monday), so won't have an opportunity to investigate this further until early next week.

With regards to why using useMojangMappings is even required, you're right - it shouldn't be. This was a minor implementation detail that was overlooked (we implemented that method first, before we decided to go with separate modules and forgot to remove this extra method and "bake in" the logic directly into the new modules)

JorelAli commented 2 months ago

Fixed for 9.4.1, due to release later today. Also fixed the need for useMojangMappings(true) - this will no longer be necessary when using commandapi-bukkit-shade-mojang-mapped

JorelAli commented 2 months ago

Fixed in 9.4.1.