-
Notifications
You must be signed in to change notification settings - Fork 71
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
add item_pre_grindstone, item_pre_smithing; partial fix modify_event for item_pre_craft #1386
Conversation
@@ -938,6 +943,7 @@ public Map<String, Mixed> evaluate(BindableEvent event) throws EventException { | |||
for(int i = 0; i < mi.length; i++) { | |||
matrix.set(i, ObjectGenerator.GetGenerator().item(mi[i], t), t); | |||
} | |||
ret.put("inventorytype", new CString(e.getInventory().getType().toString(), t)); |
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 isn't documented. However, is this even needed for something? It only returns CRAFTING or WORKBENCH right now, which can already be inferred by the matrix size, which is the only meaningful difference as far as this event is concerned. Maybe if this fired for CRAFTER too, but it doesn't.
In any case, one of the other needs to change: document this or remove it.
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.
Removed.
ret.put("third_item", ObjectGenerator.GetGenerator().item(smithing.getInputMaterial(), Target.UNKNOWN)); | ||
try { | ||
ret.put("recipe", ObjectGenerator.GetGenerator().recipe(smithing.getRecipe(), Target.UNKNOWN)); | ||
} catch(NullPointerException ex) { |
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 assume this occurs because SmithingInventory.getRecipe() can return null, and then we're wrapping the null in BukkitMCSmithingRecipe(). Instead, we should check if recipe is null in BukkitMCSmithingInventory.getRecipe() and return null there if it is. Then ObjectGenerator will correctly return CNull for a null recipe.
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.
Yup, your assumption is correct. I've moved the null check to BukkitMCSmithingInventory
as suggested.
modify_event('result', ...);
foritem_pre_craft
.item_pre_grindstone
(PrepareGrindstoneEvent
) event.item_pre_smithing
(PrepareSmithingEvent
) event.