Need help with cancelling task for a specific player

Discussion in 'Plugin Development' started by AdamTHM, Feb 21, 2021.

  1. Offline

    AdamTHM

    My current code works with no errors, but if another player is changing their mode while you are changing your mode to power 3 (there's a delay before you can hit an entity as it gives weakness) it cancels it and makes you stuck in that delay until you change your power.

    Code:
    package me.Adam.Hockey.Systems;
    
    import me.Adam.Hockey.Main;
    
    import org.bukkit.Bukkit;
    import org.bukkit.Material;
    import org.bukkit.entity.Item;
    import org.bukkit.entity.Player;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.player.PlayerDropItemEvent;
    import org.bukkit.inventory.ItemFlag;
    import org.bukkit.inventory.meta.ItemMeta;
    import org.bukkit.potion.PotionEffect;
    import org.bukkit.potion.PotionEffectType;
    import org.bukkit.scheduler.BukkitScheduler;
    import org.bukkit.scheduler.BukkitTask;
    // import org.bukkit.event.player.PlayerSwapHandItemsEvent; // COULD BE USED TO CALL THE PUCK
    import org.bukkit.enchantments.Enchantment;
    
    
    
    public class PlayerPower implements Listener{
       
        private Main plugin;
       
        public PlayerPower(Main main){
            this.plugin = main;
            Bukkit.getPluginManager().registerEvents(this, plugin);
        }
    
        @EventHandler
       
    
    public void onItemDrop(PlayerDropItemEvent e)
    {
       
            Player player = (Player)e.getPlayer();
            BukkitScheduler sched = player.getServer().getScheduler();
           
            Item item = (Item)e.getItemDrop();
            e.setCancelled(true);
            player.setExp(0);
            if(item.getItemStack().getType().equals(Material.STONE_AXE)){ //Checks if it's a stick.;
                sched.cancelTasks(plugin);
                    if (player.getLevel() == 3) { // Detects if the player has level 1
                        player.setLevel(1);
                        sched.runTaskLater(plugin, new Runnable() { // Delays it by 1 tick to stop it from bugging out
                            @SuppressWarnings("deprecation")
                            public void run(){
                                ItemMeta itemMeta = player.getInventory().getItemInMainHand().getItemMeta();
                                itemMeta.spigot().setUnbreakable(true);
                                itemMeta.addItemFlags(ItemFlag.HIDE_ENCHANTS);
                                itemMeta.addItemFlags(ItemFlag.HIDE_UNBREAKABLE);
                                player.removePotionEffect(PotionEffectType.WEAKNESS);
                                player.getInventory().getItemInMainHand().setItemMeta(itemMeta);
                                player.getInventory().getItemInMainHand().addUnsafeEnchantment(Enchantment.KNOCKBACK, 1);
                            }
                        }
                            ,1);
    
                       
                    }
                    else if (player.getLevel() == 1) { // Detects if the player has level 2
                        player.setLevel(2);
                        sched.runTaskLater(plugin, new Runnable(){ // Delays it by 1 tick to stop it from bugging out
                            @SuppressWarnings("deprecation")
                            public void run(){
                                ItemMeta itemMeta = player.getInventory().getItemInMainHand().getItemMeta();
                                itemMeta.spigot().setUnbreakable(true);
                                itemMeta.addItemFlags(ItemFlag.HIDE_ENCHANTS);
                                itemMeta.addItemFlags(ItemFlag.HIDE_UNBREAKABLE);
                                player.removePotionEffect(PotionEffectType.WEAKNESS);
                                player.getInventory().getItemInMainHand().setItemMeta(itemMeta);
                                player.getInventory().getItemInMainHand().addUnsafeEnchantment(Enchantment.KNOCKBACK, 2);
                            }
                        }
                            ,1);
    
                    }
                    else if (player.getLevel() == 2) {  // Detects if the player has level 3
                        player.setLevel(3);
                        player.addPotionEffect(new PotionEffect(PotionEffectType.WEAKNESS, Integer.MAX_VALUE, 5));
                        sched.runTaskLater(plugin, new Runnable(){ // Delays it by 1 tick to stop it from bugging out
                            @SuppressWarnings("deprecation")
                            public void run(){
                                ItemMeta itemMeta = player.getInventory().getItemInMainHand().getItemMeta();
                                itemMeta.spigot().setUnbreakable(true);
                                itemMeta.addItemFlags(ItemFlag.HIDE_ENCHANTS);
                                player.removePotionEffect(PotionEffectType.WEAKNESS);
                                itemMeta.addItemFlags(ItemFlag.HIDE_UNBREAKABLE);
                                player.getInventory().getItemInMainHand().setItemMeta(itemMeta);
                                player.getInventory().getItemInMainHand().addUnsafeEnchantment(Enchantment.KNOCKBACK, 4);
                                player.playSound(player.getLocation(), "block.anvil.place", 1.0f, 1.0f);
                            }
                        }
                            ,30);
                       
                    }
                    else if ((player.getLevel() <1 || player.getLevel() >3)) {  // Detects if a player doesn't have the levels 1,2 or 3.
                        player.setLevel(1);
                        sched.runTaskLater(plugin, new Runnable(){ // Delays it by 1 tick to stop it from bugging out
                            @SuppressWarnings("deprecation")
                            public void run(){
                                ItemMeta itemMeta = player.getInventory().getItemInMainHand().getItemMeta();
                                itemMeta.spigot().setUnbreakable(true);
                                itemMeta.addItemFlags(ItemFlag.HIDE_ENCHANTS);
                                itemMeta.addItemFlags(ItemFlag.HIDE_UNBREAKABLE);
                                player.removePotionEffect(PotionEffectType.WEAKNESS);
                                player.getInventory().getItemInMainHand().setItemMeta(itemMeta);
                                player.getInventory().getItemInMainHand().addUnsafeEnchantment(Enchantment.KNOCKBACK, 1);
                               
                            }
                        }
                            ,1);
                    }
               
                   
            }
           
           
           
           
        }
    
    }
    
     
  2. Offline

    Strahan

    Wow, there is a lot to unpack here.
    • Why are you using the player's XP as a way to decide the knockback level?? That's nuts.
    • You are applying this globally. You aren't performing any checks at all, so if someone tries to drop ANYTHING it will not work and will wipe out their current XP
    • You are cancelling all tasks if they are dropping a stone axe. If there are other runnables totally unrelated to this, they too will be terminated.
    • Why do you even care about canceling the task? Your tasks only have a 1 tick delay; they will not last long enough for you to have to worry about cancelling them in the first place.
    • PlayerDropItemEvent#getItemDrop() already returns an Item, so casting it is pointless
    • Abstraction!! Any time you find yourself copying and pasting blocks of code with minimal changes, you need to stop and reconsider your approach.
    I'd recommend you at least
    1. Apply filters before performing actions
    2. Change the level usage from XP to something else; PDC if it's going to be tied to the item for example
    3. Refactor this to get rid of the unnecessary and inefficient code duplication
     

Share This Page