Skip to content

Call ServerCommandEvent on Bukkit#dispatchCommand#9661

Closed
PapiCapi wants to merge 2 commits into
PaperMC:masterfrom
PapiCapi:server-command-event-patch
Closed

Call ServerCommandEvent on Bukkit#dispatchCommand#9661
PapiCapi wants to merge 2 commits into
PaperMC:masterfrom
PapiCapi:server-command-event-patch

Conversation

@PapiCapi
Copy link
Copy Markdown

@PapiCapi PapiCapi commented Aug 27, 2023

Patches #9644
Supersedes #9646

I used the same plugin i used for my issue ( #9644 ) and now it works fine.
This is my first PR so i don't really know if i made these things right :)

@PapiCapi PapiCapi requested a review from a team as a code owner August 27, 2023 15:36
@olijeffers0n
Copy link
Copy Markdown
Member

Thanks for your contribution, however why are we superseding this 2 day old PR?

@PapiCapi
Copy link
Copy Markdown
Author

The author of the first PR created it really fast and a comment was added one hour after its push.
As this addition will help me a lot, after 2 days without a change on the first PR, i tried doing it myself supposing that the first author didn't see the comment.
I'm sorry if this is not the way you do usually

@A248
Copy link
Copy Markdown
Contributor

A248 commented Aug 30, 2023

An important note: this is a potentially breaking API change. Some plugins may rely on the behavior that ServerCommandEvent is not called by using dispatchCommand.

@A248
Copy link
Copy Markdown
Contributor

A248 commented Aug 30, 2023

Mhm, yes, I was considering pinging @4drian3d whose plugin SignedVelocity relies on command and chat events. However, this PR is only for the ServerCommandEvent, so that plugin will not be affected.

Other plugins might. Really, dispatching a console command is something of a universal interface between plugins, and lots of plugins implement features that use dispatchCommand to call into other plugins in certain places when timing is appropriate: shop plugins when granting purchases, skyblock plugins when completing rewards, rank plugins upon promotion or role granting, punishments with additional actions, proxy command bridging solutions, hub selector plugins when items are clicked, plus personal Skript scripts, anything. Whether any of these would rely on ServerCommandEvent not being called is unknown, but it is possible. Maybe some servers implement rate limiting or logging for console commands executed from the console, but not by other plugins.

@electronicboy
Copy link
Copy Markdown
Member

For years theres been a general contract that API calls, outside of niche cases, will not fire events; I fail to see the argument for breaking this assertion here

@PapiCapi
Copy link
Copy Markdown
Author

I created this PR because in my opinion, an event should be fired because, as the javadocs says : "This event is called when a command is run by a non-player.". As dispatching a command with the console is running it by "a non-player", i though that it would be legitimate.
The more, this patch keeps the purpose of the event : being able to log executed commands.

image

I understand that maybe some plugins may run differently because of this patch but for me, only some plugins that log these entries. I can't see a situation where it would break.

What about creating a new event (DispatchCommandEvent, APIDispatchCommandEvent ?) or creating a config option ?

@lynxplay lynxplay linked an issue Sep 14, 2023 that may be closed by this pull request
@lynxplay
Copy link
Copy Markdown
Contributor

Ended up closing the original issue as it right now isn't feasible to implement this way.
Thank you so much anyway for your contribution, it is greatly appreciated.

I hope we see more contributions from you in the future :)

@lynxplay lynxplay closed this Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

Bukkit.dispatchCommand does not launch a ServerCommandEvent

5 participants