Added PlayerBookEvent, PlayerBookEditEvent & PlayerBookSignEvent. #756
Conversation
|
I'm not sure about providing the slot index... actually, now that it's possible to switch the active slot (not yet in the API) maybe this is a good idea. |
|
Yeah, given that plugin A could make a player.setActiveSlot() (or whatever) call in response to a book edit/sign event, plugin B would then not be able to get the book via player.getItemInHand(). So having the slot in the event could be useful there. Although that reasoning could be applied to events other than just book events (e.g. PlayerInteractEvent)... |
|
PR updated: setBookMeta() is now setNewBookMeta() for consistency with getNewBookMeta(), and the getter and setter both now clone the meta data, as suggested. |
|
+1 |
ghost
assigned 
| * @param bookMeta New book meta data | ||
| */ | ||
| public void setNewBookMeta(BookMeta newBookMeta) { | ||
| this.newBookMeta = newBookMeta.clone(); |
There was a problem hiding this comment.
This will throw a NPE if you provide null for the newBookMeta.
|
@evilmidget38 I've updated the PR to account for all your comments, thanks. testbookevents plugin is now also up to date wrt bukkit 1.5.1-R0.1-SNAPSHOT |
|
Collapsed the three events into a single PlayerEditBookEvent as suggested by @Wolvereness |
| * | ||
| * @param bookMeta New book meta data | ||
| */ | ||
| public void setNewBookMeta(BookMeta newBookMeta) { |
There was a problem hiding this comment.
Add a throws declaration to this, as well as Javadoc
|
Added throws declaration to setNewBookMeta() method and its Javadoc as requested by @gmcferrin |
Events related to book & quill and written book items. https://bukkit.atlassian.net/browse/BUKKIT-1995 Changes as requested by evilmidget38 oldBookMeta -> previousBookMeta Collapsed new book edit events into single PlayerEditBookEvent As per discussion with Wolvereness and evilmidget38 Added throws declarations to setNewBookMeta() and associated Javadoc More updates as requested by gmcferrin/Wolvereness
|
Deleted that comment because you already did it, heh. |
| * Sets the book meta data that will actually be added to the book | ||
| * | ||
| * @param bookMeta new book meta data | ||
| * @throws IllegalArgumentException if the new book meta data is null |
There was a problem hiding this comment.
@throws IllegalArgumentException if the new bookmeta is null or of the incorrect type
There was a problem hiding this comment.
I'm thinking the wording should be "...if the new book meta data is null or not created by the {@link ItemFactory}"
There was a problem hiding this comment.
But the CraftItemFactory doesn't do any creation, just validation.
if (meta1 != null && !(meta1 instanceof CraftMetaItem)) {
throw new IllegalArgumentException("First meta of " + meta1.getClass().getName() + " does not belong to " + CraftItemFactory.class.getName());
The purpose of the check is to head off early people creating their own ItemMeta implementations, which would cause a class cast error elsewhere in the code. (I'm not quite sure where, but I trust Wolvereness to get that right.)
There was a problem hiding this comment.
Right, understand the purpose. "of the incorrect type" just sounds a little vague. Valid creators of ItemMeta (CraftMetaItem) objects today are CraftItemFactory and CraftItemStack. So what's a good way of expressing that succinctly in a one-line comment?
|
Pulled, thanks. |
Adds three new events: PlayerBookEvent (abstract), PlayerBookEditEvent and PlayerBookSign Event. These are fired when a player edits or signs a book & quill item.
Implements: https://bukkit.atlassian.net/browse/BUKKIT-1995
CraftBukkit PR: Bukkit/CraftBukkit#996
Test Plugin: https://github.com/desht/testbookevents