Tutorial Cheaty way to check if player is drawing bow!

Hey,
People all over have wondered how to get if a player is drawing their bow, and here is my way!
Puesdo:

Show Spoiler Hide Spoiler

On interact {
if(Material.BOW) {
add to hash map
}
}
onShoot {
if(In hash map) {
remove from hash map
}
}

Actual:

Show Spoiler Hide Spoiler

Code:

Set<String> drawing = new Set<String>();
@EventHandler
public void onDraw(PlayerInteractEvent e) {
      //On interact
      if(e.getItem() != null && e.getItem().getType() == Material.BOW) {
             if(e.getAction() == Action.RIGHT_CLICK_AIR || e.getAction() == Action.RIGHT_CLICK_BLOCK) {
                  //If item == Material.BOW
                 drawing.add(e.getPlayer().getName());
             }
      }
}
@EventHandler
public void onShoot(EntityShootBowEvent e) {
     if(e.getEntity() instanceof Player) {
         //Making sure the shooter is not a Skeleton, etc.
         Player shooter = (Player) shooter;
         //Variable to access that is a Player constructor.
         if(drawing.contains(shooter.getName()) {
              drawing.remove(shooter.getName());
         }
     }
}
@EventHandler
public void onChangeSlot(PlayerItemHeldEvent e)  {
    if(drawing.contains(e.getPlayer().getName()) {
         drawing.remove(e.getPlayer().getName();
    }
}
            

Now we have that, we can make some void methods in our class (OPTIONAL)

Code:

public boolean isDrawing(Player player) {
    return drawing.contains(player.getName());
}

And yes, You can also implement removing the player when leaving etc. but I'm lazy
Thats it! Thanks for reading.
*Also, tell me if there is a thing that you can do this easier with
-Blake (ChipDev)

 

@ChipDev You know you can cancel the shooting (Select another slot) ?

 

@MisterErwin
Isn't there an event for changing slots that you are holding?

 

@ChipDev Nice tutorial! Although I would use a Set instead of a Map.

 

Code:java

 
Map<String, Boolean> drawing = new HashMap<String, Boolean>
@EventHandler
public void onDraw(PlayerInteractEvent e) {
      //On interact
      if(e.getItem() != null && e.getItem().getType() == Material.BOW) {
             //If item == Material.BOW
            drawing.add(e.getPlayer().getName());
      }
}
 

This will add a bunch of people to drawing even if they aren't drawing a bow. For example, if a player left clicks the air, you've added them even though they did not draw their bow at all. You need to check that it is right click.

Also, this event can be cancelled, and so you need to ignoreCancelled = true on this event

Also, you've created a MAP, not a LIST which is what you've decided to switch to half way through your code. I would probably change Map<String, Boolean> to just a List<String>, and even better List<UUID> so that you don't need to do any extra casting in the method below.

Code:java

 
@EventHandler
public void onShoot(EntityShootBowEvent e) {
     if(e.getEntity() instanceof Player) {
         //Making sure the shooter is not a Skeleton, etc.
         Player shooter = (Player) shooter;
         //Variable to access that is a Player constructor.
         if(drawing.contains(shooter.getName()) {
              drawing.remove(shooter.getName());
 
         }
     }
}
 

As I said above, you can change to a List<UUID> and simply do:

Code:java

 
if(drawing.contains(e.getEntity().getUniqueId()))
    drawing.remove(e.getEntity().getUniqueId());
 

Did you even put your code into an IDE, at this point I've lost faith because returning a boolean would definitely not be a void method... You should really confirm your code works, or even compiles at all before posting it in the resource section.

 

Silly me.
No, Not in an IDE. < Lazy much. Edited
ALSO: For uuid's good idea, but also no one is going to change their username while drawing a bow!
^ Sarcastic.

 

http://jd.bukkit.org/rb/apidocs/org/bukkit/event/player/PlayerItemHeldEvent.html


@ChipDev A Set would be much better than a Map or a List. If a player is not in the Set, they are not drawing their bow. Simple as that.

Code:

if(drawing.contains(shooter.getName()) {
              drawing.remove(shooter.getName());
         }

I laugh at these code snippets. Read the documentation on .remove() (it's the same for Maps as well).

 

I said to use uuids to simplify the onShoot method, not to prevent loosing a player's name/

 

Your last method can me simplified into this :

Code:

public boolean isDrawing(Player player) {
    return drawing.contains(player.getName());
}

 

@ChipDev Or just this :

Code:

public boolean isUsingItem(Player player)
{
    return ((CraftPlayer)player).getHandle().bx();
}

However, this class changes often. It's bx() in 1.7.9 and by() in 1.7.10. There are other methods there which can tell you for how long you have been using the item.

 

@ChipDev
1. You're still not using a Set as advised, and are using a List. Considering you've taken on feedback from other suggestions, might I ask if there's a particular reason for using a List over a Set?
2. There's not a whole lot of point in "if contains, remove" rather than just remove
3. You're still adding people to the drawing when you ought not to be. You're assuming that they actually have an arrow in their inventory, and can therefore draw a bow. This is not always the case.

 

As I said, it is a very cheaty. You can add alot more ways to check, and yes, that is one
I like to ensure if contains, remove, you may have reloaded when they have been drawing.
As for the list/set, is there an extreme difference? (Not sure!)

 

Fairly certain the remove method already checks if the object is present before it actually does the removing.

 

Please, don't fight about almost null checks, it doesn't matter much.

 

Excuse me? I wasn't arguing over anything. I was just pointing out something that would allow your code to be more efficient. *My sincerest apologies*.

 

Nah, Wasn't pointing to you wrong words!

 

Not to be a bummer, and not to be rude at all - you've gotten much more skilled since I last remember, but it actually does matter when you're dealing with an ArrayList, with a large number of entries.

Let's take, for example, this ridiculous hypothetical situation I just came up with to overly exaggerate the case.

You have an ArrayList storing T, who's equals() method does some really complicated math with the other object, possibly involving a large amount of square rooting.

This ArrayList you have contains 100 T objects, and you want to check if this arbitary 'T' is contained.

remove() goes through the ArrayList in O(n) speed, meaning worst case scenario you do that really expensive math 100 times before removing an entry. That's pretty unfortunate.

However, now you've gone and called 'contains', which has to re-do that expensive math, to no avail because it's redundant information.

Now your complexity is O(2n), which... O(n) is sounding a lot better now, right?

Again, not trying to be a bummer like I was before, but this is pretty important to keep in mind as you start getting into the stuff where every wasted ms is crucial (eg ticks in Minecraft)

 

@zombiekiller753 @ChipDev A simple read from the documentation would be sufficient.

 

@zombiekiller753 @teej107 K, you guys won the battle
And, checking the javadocs because I didn't write in the IDE,
'remove(Object o)
Removes the specified element from this set if it is present (optional operation).'
It removes it if it is IN the set!
EDIT: Heh,
it if it is in

 

Apologies for bringing up an old thread, but you should also check for player death events and player leave events to remove them from the set.

 

I wonder what happens if you just right click with a bow without firing it

 

@RainoBoy97 You're right D: This should fix it

Code:

if (e.getItem() != null && e.getItem().getType() == Material.BOW)
            if (e.getPlayer().getInventory().containsAtLeast(new ItemStack(Material.ARROW), 1) || e.getPlayer().getGameMode() == GameMode.CREATIVE)
                bow_Drawers.add(e.getPlayer().getUniqueId());

Will only add the player if the player is in condition to actually draw the bow