search

SpongePowered / Sponge

The SpongeAPI implementation targeting vanilla Minecraft and 3rd party platforms.
MIT License
389 stars 211 forks source link

WorldEdit (Forge) command Registrar issue on server start #3540

Open reiyooki opened 3 years ago

reiyooki commented 3 years ago

I am currently running

Issue Description When loading SpongeForge alongside WorldEdit, the server will load as expected, but produces a stacktrace indicating there's an issue with the command registrar/handler.

  1. Place both SpongeForge 1.16.5 RC900 & WorldEdit 7.2.5 in the mods directory
  2. Start the server
  3. The error occurs after the server has fully started (Spawn loaded, and "Help" message output to console)

Issue does not occur without SpongeForge running.

Note: This issue was found after removing the old sponge dependency from WorldEdit's mods.toml file, which prevents it from loading on SpongeForge as of 7.2.5. That issue has been reported to the WorldEdit team Here.

Full Error Stacktrace

[17Sep2021 09:29:17.062] [Server thread/INFO] [com.sk89q.worldedit.extension.platform.PlatformCommandManager/]: Registering commands with com.sk89q.worldedit.forge.ForgePlatform
[17Sep2021 09:29:17.065] [Server thread/ERROR] [com.sk89q.worldedit.util.eventbus.EventBus/]: Could not dispatch event: com.sk89q.worldedit.event.platform.PlatformReadyEvent@35c046b6 to handler EventHandler{priority=NORMAL}
java.lang.reflect.InvocationTargetException: null
    at com.sk89q.worldedit.util.eventbus.EventHandler.handleEvent(EventHandler.java:75) ~[worldedit:?]
    at com.sk89q.worldedit.util.eventbus.EventBus.dispatch(EventBus.java:193) ~[worldedit:?]
    at com.sk89q.worldedit.util.eventbus.EventBus.post(EventBus.java:181) ~[worldedit:?]
    at com.sk89q.worldedit.forge.ForgeWorldEdit.serverStarted(ForgeWorldEdit.java:214) ~[worldedit:?]
    at net.minecraftforge.eventbus.ASMEventHandler_9_ForgeWorldEdit_serverStarted_FMLServerStartedEvent.invoke(.dynamic) ~[?:?]
    at net.minecraftforge.eventbus.ASMEventHandler.invoke(ASMEventHandler.java:85) ~[eventbus-4.0.0.jar:?]
    at org.spongepowered.forge.launch.event.SpongeEventBus.post(SpongeEventBus.java:120) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at org.spongepowered.forge.launch.event.ForgeEventManager.post(ForgeEventManager.java:125) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at org.spongepowered.forge.launch.event.ForgeEventManager.post(ForgeEventManager.java:111) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at net.minecraftforge.fml.server.ServerLifecycleHooks.handleServerStarted(ServerLifecycleHooks.java:106) ~[forge:?]
    at net.minecraft.server.MinecraftServer.func_240802_v_(MinecraftServer.java:622) ~[?:?]
    at net.minecraft.server.MinecraftServer.func_240783_a_(MinecraftServer.java:232) ~[?:?]
    at java.lang.Thread.run(Unknown Source) [?:1.8.0_301]
Caused by: java.lang.reflect.InvocationTargetException
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_301]
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_301]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_301]
    at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_301]
    at com.sk89q.worldedit.util.eventbus.MethodEventHandler.dispatch(MethodEventHandler.java:58) ~[worldedit:?]
    at com.sk89q.worldedit.util.eventbus.EventHandler.handleEvent(EventHandler.java:73) ~[worldedit:?]
    ... 12 more
Caused by: java.lang.IllegalStateException: Cannot register command without knowing its origin.
    at org.spongepowered.common.command.registrar.BrigadierCommandRegistrar.lambda$register$0(BrigadierCommandRegistrar.java:96) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at java.util.Optional.orElseThrow(Unknown Source) ~[?:1.8.0_301]
    at org.spongepowered.common.command.registrar.BrigadierCommandRegistrar.register(BrigadierCommandRegistrar.java:96) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at org.spongepowered.common.command.brigadier.dispatcher.DelegatingCommandDispatcher.register(DelegatingCommandDispatcher.java:55) ~[spongeforge:1.16.5-36.2.1-8.0.0-RC900]
    at com.sk89q.worldedit.forge.CommandWrapper.register(CommandWrapper.java:67) ~[worldedit:?]
    at com.sk89q.worldedit.forge.ForgePlatform.registerCommands(ForgePlatform.java:168) ~[worldedit:?]
    at com.sk89q.worldedit.extension.platform.PlatformCommandManager.registerCommandsWith(PlatformCommandManager.java:449) ~[worldedit:?]
    at com.sk89q.worldedit.extension.platform.Capability$3.initialize(Capability.java:64) ~[worldedit:?]
    at com.sk89q.worldedit.extension.platform.PlatformManager.choosePreferred(PlatformManager.java:187) ~[worldedit:?]
    at com.sk89q.worldedit.extension.platform.PlatformManager.handlePlatformReady(PlatformManager.java:300) ~[worldedit:?]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_301]
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_301]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_301]
    at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_301]
    at com.sk89q.worldedit.util.eventbus.MethodEventHandler.dispatch(MethodEventHandler.java:58) ~[worldedit:?]
    at com.sk89q.worldedit.util.eventbus.EventHandler.handleEvent(EventHandler.java:73) ~[worldedit:?]
    ... 12 more
dualspiral commented 3 years ago

This is going to be somewhat problematic and I may need to basically "drop" namespaced commands if a suitable workaround cannot be found. The current implementation of the system relies on knowing which mod or plugin registers a command and namespaces it appropriately. So, for example, we would expect WE's // to have an alias worldedit:/, so if someone else tried to register //, at least you can guarantee that we retain the namespaced version.

In the case of mods, however, I think we don't have the PluginContainer that is registering the command in the case during the event because of the way Forge's EventBus works - and without that we can't determine the namespace that we need to prepend to the command - which is the problem that is occurring here.

The problem, then, is what to do about it. A "simple" fix would be to attempt to namespace if we can, and forget about it if we can't. A consistent fix would be to drop all namespacing, but if you have many mods and plugins, namespacing can be very useful. So... not sure what the best thing to do is.

(Note that this only affects commands registered directly on Brigadier - we can namespace everything else because we only allow registration during Sponge events and we can attach the plugin container to the cause in those cases.)

mikeprimm commented 3 years ago

Seeing same issue with Dynmap - but not surprising, given the above explanation. Clearly, need a default solution - but it would be cool if there was a way for a Forge mod to offer a 'hint' of some sort to drive the namespace without overtly being Sponge dependent (e.g. a 'Sponge friendly' thing versus a 'Sponge dependent' thing). @dualspiral - anything in your PR along these lines?

dualspiral commented 3 years ago

Right now, no, nothing specific in the PR, but you do bring up an important point.

If you are able to have a soft dependency on Sponge, the solution is to put your mod's ModContainer onto the cause stack before you register the command - that's what's driving the namespace that's selected. However, I'll poke around and see if that information can be passed to the system in an entirely Forge way.

mikeprimm commented 3 years ago

I just pulled down the latest build, which includes this change (RC927): Dynmap 3.2.1 for Forge 1.16.5 is still hitting an exception during command registration when SF is present:

[17:36:23] [Server thread/ERROR] [ne.mi.ev.EventBus/EVENTBUS]: Exception caught during firing event: null Index: 1 Listeners: 0: NORMAL 1: ASM: org.dynmap.forge_1_16_5.DynmapMod@758721ba onServerStarting(Lnet/minecraftforge/fml/event/server/FMLServerStartingEvent;)V java.lang.NullPointerException at org.spongepowered.common.command.registrar.BrigadierCommandRegistrar.applyNamespace(BrigadierCommandRegistrar.java:250) at org.spongepowered.common.command.registrar.BrigadierCommandRegistrar.register(BrigadierCommandRegistrar.java:99) at org.spongepowered.common.command.brigadier.dispatcher.DelegatingCommandDispatcher.register(DelegatingCommandDispatcher.java:55) at org.dynmap.forge_1_16_5.DynmapCommandHandler.register(DynmapPlugin.java:1980) at org.dynmap.forge_1_16_5.DynmapPlugin.onStarting(DynmapPlugin.java:1519) at org.dynmap.forge_1_16_5.DynmapMod.onServerStarting(DynmapMod.java:118) at net.minecraftforge.eventbus.ASMEventHandler_1_DynmapMod_onServerStarting_FMLServerStartingEvent.invoke(.dynamic) at net.minecraftforge.eventbus.ASMEventHandler.invoke(ASMEventHandler.java:85) at org.spongepowered.forge.launch.event.SpongeEventBus.post(SpongeEventBus.java:120) at org.spongepowered.forge.launch.event.ForgeEventManager.post(ForgeEventManager.java:125) at org.spongepowered.forge.launch.event.ForgeEventManager.post(ForgeEventManager.java:111) at net.minecraftforge.fml.server.ServerLifecycleHooks.handleServerStarting(ServerLifecycleHooks.java:101) at net.minecraft.server.dedicated.DedicatedServer.func_71197_b(DedicatedServer.java:201) at net.minecraft.server.MinecraftServer.func_240802_v_(MinecraftServer.java:621) at net.minecraft.server.MinecraftServer.func_240783_a_(MinecraftServer.java:232) at java.lang.Thread.run(Thread.java:748)

If looks like the 'unknown' case needs to be handled here, as well, when the pluginContainer is null:

https://github.com/SpongePowered/Sponge/blob/api-8/src/main/java/org/spongepowered/common/command/registrar/BrigadierCommandRegistrar.java#L250

mikeprimm commented 3 years ago

Further check - seeing same NPE at the same line when I try using WorldEdit 7.2.5 (with the mod.toml change descriped in the OP).