search

JorelAli / CommandAPI

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

Support changing sender entity via /execute #2

Closed Combustible closed 5 years ago

Combustible commented 5 years ago

This project is freaking awesome! I've been hunting for something that did exactly this - and got lucky on a google search that led me here.

I have been hunting for a way to run Spigot commands within functions and /execute. This project lets me do exactly that - because commands registered in this way are accessible to those vanilla mechanics.

However - though I can run my commands with /execute, the context of the command returned via the CommandExecutor is still the original sender of the command - not the "as" entity.

For example, i'd like to be able to do: /execute as @e[type=chicken,limit=1] run myplugincommand

And I should be able to somehow get the chicken entity in my handler for the command. Spigot prior to 1.12.2 used to do this via the ProxiedCommandSender - perhaps it would be possible to make use of this too?

Would like your thoughts on this - I'm inclined to hook this up myself and submit a pull request but it would help to have your input.

JorelAli commented 5 years ago

I never even knew that it was possible for chickens to execute commands! I am not very versed with the /execute command and this has been quite an eye opener. As you can probably guess, I only ever assumed that players would execute such commands.

Feel free to open a pull request and play around with it!

Combustible commented 5 years ago

/execute has become my favorite tool in 1.12.

Most minecraft commands are structured like: /kill @e[type=armor_stand,distance=..12,scores={test=1..5}]

Which is great... except that parsing those entity selector arguments is awful in spigot if you want to implement something similar like: /mycustomcommand @e[type=armor_stand,distance=..12,scores={test=1..5}]

However, in 1.12 you used to be able to do this, and get all that target selecting for free: /execute @e[type=armor_stand,r=12,score_test_min=1,score_test=5] ~ ~ ~ mycustomcommand All the entities targeted would show up in your onCommand() handler as ProxiedCommandSender's, with the callee as the target. It was great. I'm basically trying to get that same thing working again in 1.13.

Will dig in a bit and maybe ask for your thoughts when I have a plan in mind.

JorelAli commented 5 years ago

Most minecraft commands are structured like: /kill @e[type=armor_stand,distance=..12,scores={test=1..5}]

You mentioned that most Minecraft commands are structured in the sense that they can take any specific type of entity in this format using the @ symbol (in short, it's a selector if I believe correctly?)

Minecraft handles this with a specific type of argument known as the Entity Argument. I believe it is possible to port this over as a wrapper class (for example, an ArgumentEntity class) to have this exact functionality if you wish.

I should state that I've never really used selectors other than @r, @p and @a, (since they have had little use in other plugin commands), but now you've showed me the opportunity for @e and a decent example of how it could be used, I will try and implement an ArgumentEntity class to allow such a selector.

Don't forget that other developers may use this API as well. I don't want to confuse those that are versed with the new funky mechanics of 1.12 and entity selectors compared to regular entities: For example, /kill cow 20 is currently more intuitive to regular players than /kill @e[type=cow,distance=20], and I don't want developers to accidentally choose Entity selectors over EntityTypes.

Despite the functionality of the /execute command, I don't believe that going via the /execute approach is justified. Sure, it creates the correct output, but I don't believe that /execute should be used to perform the commands of another plugin (i.e. I don't think /execute should be used over the plugin command itself)

JorelAli commented 5 years ago

I have made great progress with an entity selector argument. The Minecraft implementation (which my wrapper is build from), allows for two constraints:

I plan to implement these as well, giving the developer control over these constraints 😄

JorelAli commented 5 years ago

EntitySelectorArgument has been added, with the parameter EntitySelector allowing the developer to add the constraint over what type of entity can be selected. View the commit here.

Combustible commented 5 years ago

Awesome! I will try this out. I still think there's value in getting the execute to work as well - but this is definitely the thing most plugins will want to use and is most in-line with other Minecraft commands.

Thanks! I'll post back once I've tried this out.

Combustible commented 5 years ago

Ok, tested this out and it works! This is awesome.

I imagine you didn't intentionally try to make this syntax work, but it totally does: /execute as @e[type=chicken,distance=..5,limit=1] at @s run bossfight @s boss_charger ~ ~2 ~

This is (IMO) game changing for plugins to be able to natively use targeting selectors. I'm going to point every thread I come across back to this plugin... seriously, this is amazing work.

A couple minor things I discovered:

1 - If your targeting selector doesn't pick up anything, you get this stacktrace in the log: 

[16:03:43] [Server thread/WARN]:    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[16:03:43] [Server thread/WARN]:    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[16:03:43] [Server thread/WARN]:    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[16:03:43] [Server thread/WARN]:    at java.lang.reflect.Method.invoke(Method.java:498)
[16:03:43] [Server thread/WARN]:    at io.github.jorelali.commandapi.api.SemiReflector.lambda$generateCommand$0(SemiReflector.java:215)
[16:03:43] [Server thread/WARN]:    at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:132)
[16:03:43] [Server thread/WARN]:    at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:72)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.CommandDispatcher.a(CommandDispatcher.java:188)
[16:03:43] [Server thread/WARN]:    at org.bukkit.craftbukkit.v1_13_R2.command.VanillaCommandWrapper.execute(VanillaCommandWrapper.java:45)
[16:03:43] [Server thread/WARN]:    at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:141)
[16:03:43] [Server thread/WARN]:    at org.bukkit.craftbukkit.v1_13_R2.CraftServer.dispatchCommand(CraftServer.java:699)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.PlayerConnection.handleCommand(PlayerConnection.java:1646)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.PlayerConnection.a(PlayerConnection.java:1481)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.PacketPlayInChat.a(PacketPlayInChat.java:45)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.PacketPlayInChat.a(PacketPlayInChat.java:1)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.PlayerConnectionUtils.a(SourceFile:10)
[16:03:43] [Server thread/WARN]:    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[16:03:43] [Server thread/WARN]:    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.SystemUtils.a(SourceFile:199)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.MinecraftServer.b(MinecraftServer.java:900)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.DedicatedServer.b(DedicatedServer.java:417)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.MinecraftServer.a(MinecraftServer.java:835)
[16:03:43] [Server thread/WARN]:    at net.minecraft.server.v1_13_R2.MinecraftServer.run(MinecraftServer.java:733)
[16:03:43] [Server thread/WARN]:    at java.lang.Thread.run(Thread.java:748)
[16:03:43] [Server thread/WARN]: Caused by: com.mojang.brigadier.exceptions.CommandSyntaxException: No entity was found

This however still calls my handler - which I'm not sure is expected behavior for EntitySelector.ONE_ENTITY. Not sure though.

2 - This doesn't work from a command block. Haven't tried functions. With the same command I had above, you get this stack trace when run from a command block: 

java.lang.reflect.InvocationTargetException
    at sun.reflect.GeneratedMethodAccessor33.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at io.github.jorelali.commandapi.api.SemiReflector.lambda$generatePermissions$1(SemiReflector.java:244)
    at com.mojang.brigadier.tree.CommandNode.canUse(CommandNode.java:78)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:183)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:214)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:214)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:218)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:214)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:218)
    at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:218)
    at com.mojang.brigadier.CommandDispatcher.parse(CommandDispatcher.java:163)
    at com.mojang.brigadier.CommandDispatcher.execute(CommandDispatcher.java:71)
    at net.minecraft.server.v1_13_R2.CommandDispatcher.a(CommandDispatcher.java:188)
    at net.minecraft.server.v1_13_R2.CommandDispatcher.a(CommandDispatcher.java:169)
    at net.minecraft.server.v1_13_R2.CommandDispatcher.dispatchServerCommand(CommandDispatcher.java:165)
    at net.minecraft.server.v1_13_R2.CommandBlockListenerAbstract.a(CommandBlockListenerAbstract.java:111)
    at net.minecraft.server.v1_13_R2.BlockCommand.a(BlockCommand.java:95)
    at net.minecraft.server.v1_13_R2.BlockCommand.a(BlockCommand.java:81)
    at net.minecraft.server.v1_13_R2.IBlockData.a(SourceFile:261)
    at net.minecraft.server.v1_13_R2.WorldServer.b(WorldServer.java:677)
    at net.minecraft.server.v1_13_R2.TickListServer.a(TickListServer.java:77)
    at net.minecraft.server.v1_13_R2.WorldServer.q(WorldServer.java:659)
    at net.minecraft.server.v1_13_R2.WorldServer.doTick(WorldServer.java:301)
    at net.minecraft.server.v1_13_R2.MinecraftServer.b(MinecraftServer.java:956)
    at net.minecraft.server.v1_13_R2.DedicatedServer.b(DedicatedServer.java:417)
    at net.minecraft.server.v1_13_R2.MinecraftServer.a(MinecraftServer.java:835)
    at net.minecraft.server.v1_13_R2.MinecraftServer.run(MinecraftServer.java:733)
    at java.lang.Thread.run(Thread.java:748)
Caused by: com.mojang.brigadier.exceptions.CommandSyntaxException: An entity is required to run this command here
Combustible commented 5 years ago

Oh - and to point out why I'm using execute in my command instead of just this: bossfight @e[type=chicken,distance=..5,limit=1] boss_charger ~ ~2 ~

This command of mine (/bossfight) needs to change the reference coordinates to that of the target. If I run that command as-is, I end up with ~ ~2 ~ being a block just above my head - instead of just above the chicken's head.

JorelAli commented 5 years ago

I've fixed the two bugs of derpy stacktraces and implementing the wrong command sender

Combustible commented 5 years ago

Awesome. I think that's it for this issue! I have one more new thing, and I'll open another thread...