Fixing infinite bug

I have a gun made for my plugin, but I don't want to use a bow, so I set it as a stick.. ( I know, pretty dumb, but making my own custom one maybe. ) But there is one HUGE problem, that bugs me, and everyone else that's used my plugin.. INFINITE AMMO! The code :

Code:

    @EventHandler(priority = EventPriority.NORMAL)
    public void onPlayerInteract(PlayerInteractEvent e) {
        Player p = e.getPlayer();
        World w = p.getWorld();
        ItemStack arrow = new ItemStack(Material.ARROW);
        if (on) {
        }
        else if (e.getAction() == Action.LEFT_CLICK_AIR) {
        }
        else if (p.getItemInHand().getType() == Material.STICK) {
            for (int i = 1; i<=5; i++) {
                p.launchProjectile(Arrow.class);
                p.getInventory().remove(arrow);
                w.playSound(p.getLocation(), Sound.EXPLODE, 1, 1);
                w.playEffect(p.getEyeLocation().add(0, 0.50, 0), Effect.SMOKE, 0);
                w.playEffect(p.getEyeLocation().add(0, 0, 0), Effect.CLICK1, 1);
            }
        }
    }

 

My friend xmarinusx has a plugin thats uses snowballs as ammo and it runs out too, he could probablt help you out, i'll let him know about this thread.

 

Maybe you should check if the player has an arrow before you shoot one.
You are shooting arrows no matter how much arrows the player has. And then you remove 1 from his inventory - no matter whether he has arrows or not.

 

Yeah do if(player.getInventory().contains(arrow)
then run your code, then }else{
event.setCancelled(true);
player.sendMessage("You have no ammo");

 

Code:

@EventHandler(priority = EventPriority.NORMAL)
        public void onPlayerInteract(PlayerInteractEvent e) {
            Player p = e.getPlayer();
            World w = p.getWorld();
            ItemStack arrow = new ItemStack(Material.ARROW);
            if(p.getInventory().containsAtLeast(arrow, 1)){
                if (on) {
                }
                else if (e.getAction() == Action.LEFT_CLICK_AIR) {
                }
                else if (p.getItemInHand().getType() == Material.STICK) {
                    for (int i = 1; i<=5; i++) {
                        p.launchProjectile(Arrow.class);
                        p.getInventory().remove(arrow);
                        w.playSound(p.getLocation(), Sound.EXPLODE, 1, 1);
                        w.playEffect(p.getEyeLocation().add(0, 0.50, 0), Effect.SMOKE, 0);
                        w.playEffect(p.getEyeLocation().add(0, 0, 0), Effect.CLICK1, 1);
                    }   
            }
            }
        }

 

StyL_TwisT I've already made such a plugin, here's my code for removing ammo:

Code:

public void removeAmmo(int amount) {
        ItemStack[] contents = PLAYER.getInventory().getContents();
        int remaining = amount;
        for (ItemStack i : contents) {
            if(remaining == 0){
                break;
            }
            if (i != null) {
                int id = i.getTypeId();
                byte data = i.getData().getData();
                if (id == YOURITEM.getTypeId() && data == YOURITEM.getData().getData()) {
                    if (remaining > 0) {
                        if (i.getAmount() == amount) {
                            i.setTypeId(0);
                            break;
                        }
                        if (i.getAmount() > amount) {
                            i.setAmount(i.getAmount() - amount);
                            break;
                        }
                        if (i.getAmount() < amount) {
                            remaining = remaining - i.getAmount();
                            i.setTypeId(0);
                        }
                    }
                }
            }
        }
        PLAYER.getInventory().setContents(contents);
        PLAYER.updateInventory();
    }

 

Here:

Code:

if(player.getInventory().contains(Material.ARROW)){
                // TODO here.
            }else{
                event.setCancelled(true);
                player.sendMessage(ChatColor.DARK_RED + "You have no ammunition!");
            }

 

Uhh, none of this is working..

 

It will work for sure if you're doing it right

 

Code:

private boolean hasArrow(Player p){
    return p.getInventory().contains(Material.ARROW);
}

?

 

Please explain, how I am to do it 'right' ? If this is easy, would you not help, instead of criticizing?

ZeusAllMighty11 Nope, no work. It has an error that its not used.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

There are so many people who either explained how to do it (that's what I did) or even gave you some code.
What did you try? Did you debug your code?

 

Yes I debugged my code, and I incorporated the code that DarkBladee12 gave me, into my code. Its completely disabled my shooting now..

 

You could show us your current code...

 

Code:

    @SuppressWarnings("deprecation")
    @EventHandler(priority = EventPriority.NORMAL)
    public void onPlayerInteract(PlayerInteractEvent e, int amount) {
        Player p = e.getPlayer();
        World w = p.getWorld();
        ItemStack arrow = new ItemStack(Material.ARROW);
        if(p.getInventory().containsAtLeast(arrow, 1)){
            if (on) {
            }
            else if (e.getAction() == Action.LEFT_CLICK_AIR) {
            }
            else if (p.getItemInHand().getType() == Material.STICK) {
                for (int x = 1; x<=5; x++) {
                    p.launchProjectile(Arrow.class);
                    p.getInventory().remove(arrow);
                    w.playSound(p.getLocation(), Sound.EXPLODE, 1, 1);
                    w.playEffect(p.getEyeLocation().add(0, 0.50, 0), Effect.SMOKE, 0);
                    w.playEffect(p.getEyeLocation().add(0, 0, 0), Effect.CLICK1, 1);
                    ItemStack[] contents = p.getInventory().getContents();
                    int remaining = amount;
                    for (ItemStack i : contents) {
                        if(remaining == 0){
                            break;
                        }
                        if (i != null) {
                            int id = i.getTypeId();
                            byte data = i.getData().getData();
                            if (id == arrow.getTypeId() && data == arrow.getData().getData()) {
                                if (remaining > 0) {
                                    if (i.getAmount() == amount) {
                                        i.setTypeId(0);
                                        break;
                                    }
                                    if (i.getAmount() > amount) {
                                        i.setAmount(i.getAmount() - amount);
                                        break;
                                    }
                                    if (i.getAmount() < amount) {
                                        remaining = remaining - i.getAmount();
                                        i.setTypeId(0);
                                    }
                                }
                            }
                        }
                    }
                    p.getInventory().setContents(contents);
                    p.updateInventory();
                }
            }
        } 
    }

( p.updateInventory(); has a line through it, thats why the "deprecation" is there. )

 

Why is there this piece of code:

Code:

if (on) {
}
else if (e.getAction() == Action.LEFT_CLICK_AIR) {
}

?

Maybe "on" is true?
I'd send a message to the player (player.sendMessage) after every if-statement to see where the error is.

 

Oh, no thats for a private boolean on something else I have in the code.. I can remove.
public static boolean on = true;

 

Well...... Look at your code:

Code:

if (on) 
{
 
}
else if (e.getAction() == Action.LEFT_CLICK_AIR) 
{
 
}
else if (p.getItemInHand().getType() == Material.STICK) 
{
 
}

 

I know, its a linking thing for something that I previously removed.. Its still definitely not the reason, because it wouldn't have an effect on the code any more.

 

Well if on is declared like this:

public static boolean on = true;

than this has a huge effect on your code... Maybe you should look at some java tutorials first...
I guess what you want to do is this:

Code:

if ((on) && (e.getAction() == Action.LEFT_CLICK_AIR) && (p.getItemInHand().getType() == Material.STICK)) 
{
   // code for shooting arrows
}

The only problem is that you don't know how if-statements/java works....

 

Yeah , I'm not 100% sure on
try {
catch and the other thing.. I don't understand them.

 

To translate your code into english:

if "on" is true then do nothing
ELSE
if the player clicked with his left mousebutton in the air then do nothing
ELSE
if the item in the player's hand is a stick than shoot the arrow

I'm pointing the ELSEs out because the code will only do one of those thing - the first which is true.
And because "on" is always true it does nothing (as written in the if-statement for "on") and then leave.

What you wanted to do:

if "on" is true AND the player clicked with his left mousebutton in the air AND he is holding a stick THEN
shoot an arrow.

This translated into code is:

if ((on) && (e.getAction() == Action.LEFT_CLICK_AIR) && (p.getItemInHand().getType() == Material.STICK))
{
// code for shooting arrows
}

 

Yes, but the infinite arrows.. Its still doing it! I need a way to disable this stupid infinite arrows before changing the rest of this..

 

I think you can't set the type of an item in your inventory to air to remove it. You have to set the itemstack of this slot to null.

 

Hmm.. I will test this tonight, and leave it for tomorrow morning..

 

You don't say ...

You have to call the method, do you know java?

 

As other people have said, you're not checking for arrows correctly. DarkBlade's method is pretty close or correct, I don't have time to read it all since I have to leave in a few. Basically when you remove something from someone's inventory, you can't just remove it by forwarding an ItemStack with an amount of 1 to the remove() function. Because that will only remove an arrow from the inventory if there is a stack in the inventory of size 1. You basically have to write a function to search the person's inventory for an item, to determine how many they have, and if it's enough to fire. Then you have to write a function to remove that many from the person's inventory regardless of how their stacks are set up. Fortunately, I have done this in the past since I needed it for many of my plugins. Here's the code I wrote, take it or leave it or use it how you like:

Code:

    public int getAmountInInventory(PlayerInventory p,Material m,short d)
    {
        int total = 0;
        for (int i = 0;i<p.getSize();i++)
        {
            ItemStack pm = p.getItem(i);
            if (pm != null && pm.getType() == m && pm.getDurability() == d)total += pm.getAmount();
        }
        return total;
    }
 
    public void takeItemFromPlayer(PlayerInventory p, Material m, short data, int amt)
    {
        int amounttotake = amt;
       
        do
        {
            ItemStack tstack = null;
            for (ItemStack i:p.getContents()) if (i != null) if (i.getType() == m && i.getDurability() == data) tstack = i;
            if (tstack != null)
            {
                int samt = tstack.getAmount();
                int pos = p.first(tstack);
                if (samt <= amounttotake)
                {
                    p.clear(pos);
                    amounttotake -= samt;
                }
                else if (samt > amounttotake)
                {
                    p.setItem(pos, new ItemStack(m,samt - amounttotake,data));
                    amounttotake = 0;
                }
            }
        }
        while (amounttotake > 0);       
        return;
    }

These functions take a data value as well so you can search for/remove things like colored wool and different wood types, etc.

 

Taien , I got no idea where the hell this is supposed to go in it, I've tried after the Interact, I've tried before it, and I've tried in it.. This shits really confusing for me.. And you said
'These functions take a data value as well so you can search for/remove things like colored wool and different wood types, etc. ' Which I don't know where these data values are. And what DarkBladee12 posted, I didn't know where to put this. I might just remove the guns if its like this, and add them later when I'm better at this..

 

These are just methods (a coding term for a bit of code that can be called to perform a certain function). You can put them anywhere in the same class as the EventHandler stuff (but outside any other methods, so not inside the EventHandler, but inside the class). Then you just call them. I'm not trying to demean you here, but this is pretty basic coding stuff, so I'll try to explain it a bit more since you don't seem to understand.

Look at the line:

Code:

public int getAmountInInventory(PlayerInventory p,Material m,short d)

This is called the signature for the method. The method's name is getAmountInInventory, and it requires a PlayerInventory object, a Material, and a short (a kind of integer), and it returns an int. So like, say you're trying to check if someone has Arrows. You'd call it in your code like this:

Code:

if (getAmountInInventory(player.getInventory(), Material.ARROW, (short)0) > 0)
{
  //they have more than 0 arrows, do stuff
}

The (short) before the third argument just casts (or changes the type of) that variable from int to short. That variable represents the data value for the item, which for arrows (and most items) is 0. Different colors of wool have other ones, though, like purple wool I believe is 9, etc.

Now if you wanted to use the second method I posted (takeItemFromPlayer), you'd add it to the first bit of code here like this:

Code:

if (getAmountInInventory(player.getInventory(), Material.ARROW, (short)0) > 0)
{
    //they have more than 0 arrows, do stuff
    takeItemFromPlayer(player.getInventory(), Material.ARROW, (short)0, 1);
    //voila, you just took their arrow
}

The method takeItemFromPlayer will continually cycle through their inventory until it takes out all the items you requested, so you should never use it unless you checked for those items with getAmountInInventory first, or you could have an infinite loop that hangs your server.

 

Ok, so what you're trying to tell me, is I do not need the EventHandler, just to set that method anywhere, and call it? Or should I keep the EventHandler, and put the method on the bottom, and move it around ? Because right now, the whole gun code just fucked up, and is now dead.. :/ I need to fix that as well.