-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add basics for buy command #2795
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
Changes from all commits
4b51f61
7494900
3d15488
bd8a6f5
0409afe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| package com.earth2me.essentials.commands; | ||
|
|
||
| import com.earth2me.essentials.Trade; | ||
| import com.earth2me.essentials.User; | ||
| import com.earth2me.essentials.craftbukkit.InventoryWorkaround; | ||
| import com.earth2me.essentials.utils.NumberUtil; | ||
| import com.google.common.collect.Lists; | ||
|
|
||
| import org.apache.commons.lang.math.NumberUtils; | ||
| import org.bukkit.Server; | ||
| import org.bukkit.World; | ||
| import org.bukkit.inventory.ItemStack; | ||
|
|
||
| import static com.earth2me.essentials.I18n.tl; | ||
|
|
||
| import java.math.BigDecimal; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.logging.Level; | ||
|
|
||
| public class Commandbuy extends EssentialsCommand { | ||
| public Commandbuy() { | ||
| super("buy"); | ||
| } | ||
|
|
||
| @Override | ||
| public void run(final Server server, final User user, final String commandLabel, final String[] args) throws Exception { | ||
| if (args.length < 1) { | ||
| throw new NotEnoughArgumentsException(); | ||
| } | ||
|
|
||
| int amountToGive; | ||
|
|
||
| if (args[1] != null && NumberUtil.isInt(args[1])) { | ||
| amountToGive = NumberUtils.toInt(args[1]); | ||
| } else if (args[1] == null) { | ||
| amountToGive = 1; | ||
| } else { | ||
| throw new Exception("The second argument must be an integer."); | ||
| } | ||
|
|
||
| if (amountToGive <= 0) { | ||
| throw new Exception("You cannot buy 0 items."); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a localization lookup. |
||
| } | ||
|
|
||
| ItemStack is = ess.getItemDb().get(args[0]); | ||
| is.setAmount(amountToGive); | ||
| BigDecimal worthSingleItem = ess.getWorth().getBuyPrice(ess, is); | ||
| BigDecimal worth = worthSingleItem.multiply(BigDecimal.valueOf(amountToGive)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will cause a NPE if getBuyPrice doesn't find a worth - worthSingleItem will be null. The check on 45-47 should happen after 42. |
||
|
|
||
| if (worth == null) { | ||
| throw new Exception(tl("itemCannotBeSold")); | ||
| } | ||
|
|
||
| if (worth.compareTo(user.getMoney()) == -1) { | ||
| boolean isDropItemsIfFull = ess.getSettings().isDropItemsIfFull(); | ||
| BigDecimal leftoverValue = BigDecimal.ZERO; | ||
| Map<Integer, ItemStack> leftovers; | ||
|
|
||
| if (user.isAuthorized("essentials.oversizedstacks")) { | ||
| leftovers = InventoryWorkaround.addOversizedItems(user.getBase().getInventory(), ess.getSettings().getOversizedStackSize(), is); | ||
| } else { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like some indentation / formatting issues in this file - it makes it harder to read. |
||
| leftovers = InventoryWorkaround.addItems(user.getBase().getInventory(), is); | ||
| } | ||
|
|
||
| if (isDropItemsIfFull) { | ||
| for (ItemStack item : leftovers.values()) { | ||
| World w = user.getWorld(); | ||
| w.dropItemNaturally(user.getLocation(), item); | ||
| } | ||
| } else if(!leftovers.values().isEmpty()) { | ||
| for (ItemStack item : leftovers.values()) { | ||
| leftoverValue = leftoverValue.add(worthSingleItem.multiply(BigDecimal.valueOf(item.getAmount()))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is more expensive (cpu time) / more verbose than counting how many total leftovers there are then doing a single BigDecimal multiply operation (no need for subsequent BigDecimal add operations)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix this is a separate commit. |
||
| } | ||
|
|
||
| user.sendMessage("Not enough inventory space. Refunding $<amount>."); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a format string to it can be localized to different languages.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also did we really refund them if we never took it to begin with?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I couldn't think of a better way to phrase it. Maybe just a message of something like "You didn't have enough inventory space. You purchased X items instead of Y." Any input here is appreciated. |
||
| } | ||
|
|
||
| user.takeMoney(worth.subtract(leftoverValue)); | ||
| user.getBase().updateInventory(); | ||
| } else { | ||
| throw new Exception("You do not have enough money."); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also needs a localization lookup
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First time contributing to this repo, so please forgive my ignorance here. I understand how i18n works from the other files. Am I required to provide a translation for each language as a part of this PR? Do you have a recommended translator or translation tool I should use to do the translations?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't remember who told me this, but I add my messages to just |
||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected List<String> getTabCompleteOptions(Server server, User user, String commandLabel, String[] args) { | ||
| if (args.length == 1) { | ||
| return getItems(); | ||
| } else if (args.length == 2) { | ||
| return Lists.newArrayList("1", "64"); | ||
Ichbinjoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } else { | ||
| return Collections.emptyList(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,7 +84,7 @@ private BigDecimal itemWorth(CommandSource sender, User user, ItemStack is, Stri | |
| amount = ess.getWorth().getAmount(ess, user, is, args, true); | ||
| } | ||
|
|
||
| BigDecimal worth = ess.getWorth().getPrice(ess, is); | ||
| BigDecimal worth = ess.getWorth().getSellPrice(ess, is); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should probably be a way to configure whether or not it shows up, in the case that servers don't want to use both buy and sell at the same time, for example. |
||
|
|
||
| if (worth == null) { | ||
| throw new Exception(tl("itemCannotBeSold")); | ||
|
|
||
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 feel like this whole function (as well as getSellPrice) can be folded into getPrice() where getPrice() has an additional argument for which 'keyspace' it is looking up the price in i.e. the 'buy' keyspace vs the 'worth' keyspace.
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.
Totally agree. I just refactored this.