-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Adding EssentialsWarpEvent #1921
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Some Plugins could really use this for an api!
Edit: I have compiled it for use. If you're too lazy, look here: https://github.com/loper12/BanbeucmasEssPatch
mdcfe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly looks good, but I haven't been able to test it so I can't approve this yet.
| public void warp(IUser teleportee, String warp, Trade chargeFor, TeleportCause cause) throws Exception { | ||
| EssentialsWarpEvent event = new EssentialsWarpEvent(teleportee, warp); | ||
| Bukkit.getServer().getPluginManager().callEvent(event); | ||
| if(event.isCancelled()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be spaced out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing with Remote Debugging and also Checkpoints, the event was called. I haven't test the listeners tho
| private IUser user; | ||
| private String warp; | ||
| private boolean cancelled = false; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I add Trade also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do, I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok wait for me a moment
|
We could also add a method Also, prefixing the event with |
Like that? |
| return trade; | ||
| } | ||
|
|
||
| public void setWarp(String warp){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacing here too: public void setWarp(String warp) {
| UserWarpEvent event = new UserWarpEvent(teleportee, warp, chargeFor); | ||
| Bukkit.getServer().getPluginManager().callEvent(event); | ||
|
|
||
| if(event.isCancelled()){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should still be spaced out: if (event.isCancelled()) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I will do it tonight
| return warp; | ||
| } | ||
|
|
||
| public Trade getTrade() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps some javadocs would help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should help. I am not that familiar with Javadocs tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if I were to left documentation there, providing that all of other Events doesn't have any documentation except the heading of the class.
|
This has been merged into the 1.13 branch and is now part of experimental builds. |
|
Thanks for the merge. |
* Adding EssentialsWarpEvent for checking if player is wrapping * Spacing * Adding Trade parameter * Refactoring * Adding #setWarp() to the Event * Spacing * Documenting the purpose of the Event * Javadoc?
Tho you can still use PlayerTeleportEvent for this, I think it is better to have an event especially for EssentialsWarp for checking warps tho.
This should help a lot on dependency which are trying to hook into the plugin.