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

[B + C] Adds a villager trading API. Adds BUKKIT-2138, BUKKIT-3935 #833

Closed
wants to merge 3 commits into from
Closed

Conversation

thefishlive
Copy link

The Issue:

Currently in bukkit there is no way to get or set merchant trades or get a merchant from its inventory

PR Breakdown:

This PR adds a way to get, set and add merchant trades and adds a way to get the merchant from the MerchantInventory

Testing Results and Materials:

https://github.com/thefishlive/Bukkit-test - source
https://www.dropbox.com/sh/9v7cy8c2acsuywn/mqdvl8kL3S/Bukkit-Test-0.0.1-SNAPSHOT.jar - binary

Relevant PR(s):

CB-1111 -Bukkit/CraftBukkit#1111 - implmentation

JIRA Ticket:

https://bukkit.atlassian.net/browse/BUKKIT-2138
https://bukkit.atlassian.net/browse/BUKKIT-3935

* @param trade a new trade for the villager to perform
*/
public void addTrade(MerchantTrade trade);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces are evil.

*
* @param price the price of the trade, a maximum of two items
*/
public void setPrice(ItemStack... price) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a throws IllegalArgumentException in the Javadoc and a throws on the end of the method

@ase34
Copy link

ase34 commented Apr 4, 2013

This will be useful! I hope this PR will be accepted!

@riking
Copy link
Contributor

riking commented Apr 10, 2013

The core team has stated that this does not feel like an appropriate way to approach villager trading; one problem is that the idea of a "Merchant" and its purpose is very ill-defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants