WiIIiam278 / HuskHomes

The powerful & intuitive homes, warps, and teleportation suite
https://william278.net/project/huskhomes
Apache License 2.0
127 stars 82 forks source link

java.lang.NullPointerException when command is run from a command block #668

Closed SzczurekYT closed 3 months ago

SzczurekYT commented 3 months ago

To reproduce

No harm really, but spams in the console.

Bug location

I needed a quick solution to silence this error, so I dug a little bit into the code, and found that the error happens a this line https://github.com/WiIIiam278/HuskHomes/blob/0eeab972e45bef2f077e513ea8e9b8a6a8216da9/common/src/main/java/net/william278/huskhomes/teleport/TeleportBuilder.java#L78 because, executor is null. So I just wrapped this displayMessage call in a != null if statement, however that is probably not the fix.

I also found another bug with command blocks right now while testing this. With this command in command block tp MyName ~ ~1 ~ on vanilla I would get teleported to the top of the command block, while with HuskHomes I always end up at -1, 2, -1

Log

[09:29:36 WARN]: [HuskHomes] Plugin HuskHomes v4.7 generated an exception while executing task 9
java.lang.NullPointerException: null
        at java.base/java.util.Objects.requireNonNull(Objects.java:233) ~[?:?]
        at HuskHomes-Paper-4.7.jar/net.william278.huskhomes.teleport.TeleportationException.displayMessage(TeleportationException.java:43) ~[HuskHomes-Paper-4.7.jar:?]
        at HuskHomes-Paper-4.7.jar/net.william278.huskhomes.teleport.TeleportBuilder.buildAndComplete(TeleportBuilder.java:78) ~[HuskHomes-Paper-4.7.jar:?]
        at HuskHomes-Paper-4.7.jar/net.william278.huskhomes.command.TpCommand.execute(TpCommand.java:109) ~[HuskHomes-Paper-4.7.jar:?]
        at HuskHomes-Paper-4.7.jar/net.william278.huskhomes.command.TpCommand.execute(TpCommand.java:72) ~[HuskHomes-Paper-4.7.jar:?]
        at HuskHomes-Paper-4.7.jar/net.william278.huskhomes.command.Command.lambda$onExecuted$0(Command.java:49) ~[HuskHomes-Paper-4.7.jar:?]
        at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:86) ~[paper-1.21.jar:1.21-123-0a1be9a]
        at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:57) ~[paper-1.21.jar:1.21-123-0a1be9a]
        at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) ~[paper-1.21.jar:?]
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]
WiIIiam278 commented 3 months ago

While it shouldn't throw an error, the expected behaviour is that HuskHomes commands don't work in command blocks. It's a bit of a fundamental limitation of the bukkit command system - and honestly largely mixing data packs and plugins is liable to lead to issues. So in essence this is a wontfix unfortunately. Honestly I'm surprised the executing logic ran as far as it did.

You can still use vanilla commands in command blocks with the Minecraft prefix (just replace the command with minecraft:tp

SzczurekYT commented 3 months ago

Thanks makes sense, and yeah I changed it to minecraft:tp, however as you said it shouldn't throw the error in the console, I didn't on older version, and that's what the issue is about.

While it shouldn't throw an error, the expected behaviour is that HuskHomes commands don't work in command blocks. It's a bit of a fundamental limitation of the bukkit command system

No problem here, I don't expect that, however if you ever were interested in this, it is actually possible with the new paper brigadier api combined with the bootstrap mechanism, and also the CommandAPI api plugin can do this too by just hooking directly into the nms, at least that's my guess.

WiIIiam278 commented 3 months ago

Closing as wontfix for now regardless as this is out of scope

SzczurekYT commented 3 months ago

But shouldn't be the error silenced like it was in previous versions? Or even better it would be to replace it with a message that someone put a huskhomes command in a command block and it's not supported, so that people know what's going on instead of getting random null pointer exceptions spammed in their logs.

SzczurekYT commented 3 months ago

@WiIIiam278

WiIIiam278 commented 3 months ago

Hi, please don't ping me -- I don't plan on fixing this as command blocks are out of scope.

SzczurekYT commented 3 months ago

Hi, please don't ping me -- I don't plan on fixing this as command blocks are out of scope.

Hi, sorry for that, but I thought you didn't see my last message, and I still think you didn't (the one after the issue was closed). I know they are out of scope, no problem with that, but the entire issues is not about adding the support, but improving the errors, so the situation will be clear to other users of huskhomes.