PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.67k stars 2.25k forks source link

Plugin Server Software Commands(ex. /restart) do not work in functions. #6441

Closed Bowswa closed 3 months ago

Bowswa commented 3 years ago

Expected behavior

Expected behavior is that when i use the /restart command inside of a function the server restarts just fine.

Observed/Actual behavior

When using the /restart command in a function it produces this error: [ERROR] Failed to load function sched_restart:restart/paper

java.util.concurrent.CompletionException: java.lang.IllegalArgumentException: Whilst parsing command on line 1: Unknown or incomplete command, see below for error at position 0: <--[HERE] **Where the restart command is.

at java.util.concurrent.CompletableFuture.encodeThrowable(Unknown Source) ~[?:?]

at java.util.concurrent.CompletableFuture.completeThrowable(Unknown Source) [?:?]

at java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source) [?:?]

at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:?]

at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:?]

at java.lang.Thread.run(Unknown Source) [?:?]

Caused by: java.lang.IllegalArgumentException: Whilst parsing command on line 1: Unknown or incomplete command, see below for error at position 0: <--[HERE]

at net.minecraft.commands.CustomFunction.a(CustomFunction.java:68) ~[patched_1.17.1.jar:git-Paper-196]

at net.minecraft.server.CustomFunctionManager.lambda$reload$3(CustomFunctionManager.java:86) ~[patched_1.17.1.jar:git-Paper-196]

... 4 more

Steps/models to reproduce

https://github.com/Razboy20/version-proxy-mc-datapack Auto Restart Message v1.0.0.zip Use these datapacks on a paper server.

This will produce the error on startup.

Plugin list

None

Paper version

This server is running Paper version git-Paper-196 (MC: 1.17.1) (Implementing API version 1.17.1-R0.1-SNAPSHOT) (Git: 59d449d)

You are running the latest version

Agreements

Other

No response

Machine-Maker commented 3 years ago

Fairly sure this affects upstream as well. This is also kind of a related issue to plugin (and spigot/paper commands) not working when part of the /execute command.

electronicboy commented 3 years ago

is this an issue with functions loading well before plugins and such or a command system issue, i wonder

Machine-Maker commented 3 years ago

I don't think so, last I checked the commands aren't "parsed" until the function is actually ran, just saved as a list of strings that are sent to the brig command system.

electronicboy commented 3 years ago

nms.Main#178 That call is what loads the datapacks

The big issue here is that this stuff is loaded weeeeelll before plugins and such are loaded

Machine-Maker commented 3 years ago

Yeah I knew they were loaded well before, but I thought they were just stored as strings until they ran, but there is parsing done (CommandFunction line L42). I guess either it didnt use to do that, or I just had that wrong. My PR (https://github.com/PaperMC/Paper/pull/5527) would actually fix that, by product of adding custom advancement triggers.

Machine-Maker commented 3 years ago

The big issue here is that this stuff is loaded weeeeelll before plugins and such are loaded

Yeah, so even if you enable the datapack after the plugins are already loaded, like you do /datapack enable it still comes out with the same error.

Yeah, this issue is really because of bukkit's whole CommandMap thing taking over command dispatching. The server's CommandDispatcher doesn't know about any plugin, bukkit, spigot, or paper commands. If that was fixed, then you'd run into the Unknown command thing cause stuff wasn't loaded. Only that second part would be fixed by that linked PR.

Bowswa commented 3 years ago

so i guess do i need to report to bukkit? or will it still be fixed here

Machine-Maker commented 3 years ago

You can go ahead an open an issue on spigot's JIRA if you want, but this doesn't strike me as a "simple" fix. PRs are welcome for sure though.

MiniDigger commented 3 years ago

but if you plan to work on this, please come to our discord and discuss your idea for a solution first, before potentially spending time going on a path we don't see viable.

Machine-Maker commented 3 years ago

@MiniDigger whoops, see the linked PR for a solution that does fix this, but might break something else depending on how much I don’t know about how Bukkit took over commands.

Machine-Maker commented 3 months ago

This is fixed as of b98d20a8ac9c21789d532652df86638a202093c7. If you register your commands inside the PluginBootstrap, they will be available to datapack functions. Any commands registered outside of there will not be available as they are registered too late in the startup process.