PlayerDeathEvent being strange?

Discussion in 'Plugin Development' started by DatCookiez, Jan 17, 2015.

Thread Status:
Not open for further replies.
  1. Offline

    DatCookiez

    Hey guys, I have this code and it works perfectly if I put the Main.pyro.contains(killer);
    but doesn't work if I put the pyro as the getEntity();

    Code:
        @EventHandler
        public void onPyroDeath(PlayerDeathEvent e){
            Player p = e.getEntity();
            Player killer = p.getKiller();
           
            Location loc = p.getLocation();
           
            if (Main.pyro.contains(p)){
                       
                        loc.getWorld().playEffect(loc, Effect.MOBSPAWNER_FLAMES, 20);
                        loc.getWorld().playEffect(loc, Effect.MOBSPAWNER_FLAMES, 50);
                        loc.getWorld().playEffect(loc, Effect.MOBSPAWNER_FLAMES, 90);
                        ParticleEffect.EXPLOSION_HUGE.display(1, 1, 1, 5, 5, loc, 50);
                        loc.getWorld().playSound(loc, Sound.DONKEY_DEATH, 1, 10);
                        killer.setHealth(killer.getHealth() - 30);
                        killer.sendMessage(ChatColor.GREEN + "Pyros last message was handed to you!");
                        if(killer.getHealth() <=30){
                            killer.setHealth(0);
                            killer.sendMessage(ChatColor.RED + "Killed by Pyros final message!");
                            p.sendMessage(ChatColor.AQUA + "" + ChatColor.ITALIC + "Ultimate used!");
                    }
                }    
    Off-topic to this thread, but I'm also trying to detect if a player is hit in the back. I have tried every thread and all seem to not work. Even tried Math.abs, still not working. I have the code for after the detection, I just need help on the detection part..

    Thanks in advance, DatCookiez

    Bump

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 13, 2016
  2. Offline

    mine-care

    first of all, i assume main is your... main class, Main.pyro.contains(p), i dont think this arraylist has to be static there :/ dont abuse static. Secondly, can we see the list used as well?
    About that you need to play with target player's direction, if it is about equal to the killers means that its a backstab :/
    Please provide the list pyro or your main class (better) Also remove static, especially when it is abour a list with players! player object list tends to cause ram leakage
     
  3. Offline

    DatCookiez

    @mine-care
    Pyro list
    public static HashSet<Player> pyro = new HashSet<Player>();
     
  4. Offline

    1Rogue

    Code:java
    1. Main.pyro = Collections.unmodifiableSet(new HashSet<>());


    I just called this from an external plugin. How would you handle that?

    (Also, pro-tip: If the collection is only relevant to the listener class, why not keep the collection in the listener class?)
     
  5. Offline

    mine-care

    @1Rogue has a point... Also static is the cause of some ram leaks so please avoid it :3
     
  6. Offline

    1Rogue

    Static doesn't cause "ram leaks"...
     
  7. Offline

    DatCookiez

    So can anyone help me in me quest to fixing the death event? because at the moment I'm not understanding how changing the static is helping.
    @mine-care @1Rogue

    @1Rogue
    Because in my Main class I have events that call upon the pyro HashSet

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 13, 2016
  8. Offline

    mine-care

    @1Rogue ask I read in a tutorial by teej ( I think it was his) the way bukkit reloads does not handle static variables and they are kept in ram without use. But still a static array wit player objects in it is not a good idea if players are not handled correctly.
    Thanks.
     
  9. Offline

    teej107

    mine-care likes this.
  10. Offline

    DatCookiez

    Still seeking help as to why my event isn't working
     
  11. Offline

    1Rogue

    That quote is inaccurate by the way, it's not that the old plugin class will have references - static is not owned by any one instance. The collections argument makes sense, but not in the way that they are explaining. What you will get is a stale reference to an object that either a) bukkit no longer references (e.g. a Player object) or b) should not be known of (e.g. a timer or scheduled task) or c) should have been recycled before.

    Typically with fields that are a Collection or Map you should set them to final:

    Code:java
    1. private final Map<String, ?> example = new HashMap<>();


    And clear the references via:

    Code:java
    1. example.clear();


    This allows you to ensure that someone doesn't "hotswap" your elements and as an overall OOP principle: The less modifiable, the better.

    The other point that isn't quite accurate is the "Plugin class loader" argument, but I won't go into that.

    The main thing to note is that static is a tool, it's just a tool that a lot of people happen to misuse and misunderstand. If using static would simplify your code (read: Not making accessing elements easier, I mean something such as using an enum singleton versus hand-writing all your config values), and is proper to represent through a global state, then by all means.

    (Edit: As an additional note, when bukkit no longer references your plugin's main class, usually 95-100% of the created plugin objects will be collected. Static members may also be garbage collected, however there is no guarantee of this happening, hence the potential for them to be a problem if your plugin is unloaded but the server is not stopped)

    Try using a collection with the player's UUID instead of the player object, however using getEntity() versus a player variable wouldn't make a difference, they both reference the same thing from what you've shown so far.
     
    teej107 likes this.
  12. Offline

    DatCookiez

    @1Rogue
    But I just want to get the player that's died..
     
  13. Offline

    1Rogue

    You don't need a collection of players to determine who died. You can compare the UUIDs every time. You never need to retrieve a value from the collection.
     
  14. Offline

    DatCookiez

    @1Rogue
    Thing is, it's a class system, if the player that died was in the pyro HashSet, then the event would fire. But it's not working
     
  15. Offline

    1Rogue

    Add debug statements. See what the contents of the pyro set are, and the outcome of .contains().
     
  16. Offline

    Captain Dory

    Did you register events? c:
     
  17. Offline

    mine-care

    @teej107 hmm miss understood this sori :(
     
  18. Offline

    DatCookiez

    @Captain Dory
    Yes, all events are registered because everything else in this class works.
     
  19. Offline

    1Rogue

     
Thread Status:
Not open for further replies.

Share This Page