Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support changing sender entity via /execute #2

Closed
Combustible opened this issue Oct 5, 2018 · 10 comments
Closed

Support changing sender entity via /execute #2

Combustible opened this issue Oct 5, 2018 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@Combustible
Copy link
Contributor

Combustible commented Oct 5, 2018

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
Copy link
Owner

JorelAli commented Oct 5, 2018

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
Copy link
Contributor Author

/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
Copy link
Owner

JorelAli commented Oct 6, 2018

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
Copy link
Owner

JorelAli commented Oct 6, 2018

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

  • Whether the entity selector returns a player or an Entity
  • Whether the entity selector allows for one or more entities

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

@JorelAli
Copy link
Owner

JorelAli commented Oct 6, 2018

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.

@JorelAli JorelAli self-assigned this Oct 6, 2018
@JorelAli JorelAli added the enhancement New feature or request label Oct 6, 2018
@Combustible
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

JorelAli commented Oct 6, 2018

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

@Combustible
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants