1.12.2 Here's how to create spectator mode in survival

Discussion in 'Spigot Plugin Development' started by FlameFOxYT, Feb 15, 2020.

  1. Code (Java):
    import java.util.ArrayList;
    import java.util.List;
    import org.bukkit.Bukkit;
    import org.bukkit.entity.Entity;
    import org.bukkit.entity.Player;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.entity.EntityDamageByEntityEvent;
    import org.bukkit.event.entity.EntityTargetEvent;
    import org.bukkit.event.entity.PlayerDeathEvent;
    public class spectatorListener implements Listener{
        private main plugin;
        public static List<Player> dead = new ArrayList<>(); //dead list
        public spectatorListener(main plugin) {
         
            this.plugin = plugin;
            Bukkit.getPluginManager().registerEvents(this, plugin);
         
        }
     
     
     
        @EventHandler
        public void deathListener(EntityDamageByEntityEvent e) { //this method will disable damage to players when after they die
         
            if (e.getEntity() instanceof Player && e.getDamager() instanceof Player) {
             
                Player d = (Player) e.getDamager();
                Player p = (Player) e.getEntity();
                if (dead.contains(p)) {
                 
                    e.setCancelled(true);
     
                } else if (dead.contains(d)) {
                 
                    e.setCancelled(true);
                 
                }
            } else if (e.getEntity() instanceof Player && e.getDamager() instanceof Entity) {
             
                Player p = (Player) e.getEntity();
                if (dead.contains(p)) {
                 
                    e.setCancelled(true);
                 
                }
            } else if (e.getEntity() instanceof Entity && e.getDamager() instanceof Player) {
     
                Player d = (Player) e.getDamager();
                if (dead.contains(d)) {
                 
                    e.setCancelled(true);
                 
                }
            }
        }
     
        @EventHandler
        public void deathTargetListener(EntityTargetEvent e) { //this method will disable target from mobs when the player dies
            if (e.getTarget() instanceof Player) {
             
                Player p = (Player) e.getTarget();
                if (dead.contains(p)) {
                 
                    e.setCancelled(true);
                 
                }
             
            }
        }
     
        @EventHandler
        public void onDeath(PlayerDeathEvent e) {
         
            Player p = (Player) e.getEntity();
            spectatorListener.dead.add(p); //adds the player to the dead list
            p.setAllowFlight(true); //allows the player to fly
            p.setFlying(true);
         
            Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable() { public void run() {
             
                spectatorListener.dead.remove(p);
                p.setAllowFlight(false); //disallows the player to fly
                p.setFlying(false);
             
            }}, 200l); // after 10 seconds it will remove him from the dead list
        }
     
    }
     
    #1 FlameFOxYT, Feb 15, 2020
    Last edited by a moderator: Feb 21, 2020 at 1:59 AM
  2. Wtf is this?
     
    • Agree Agree x 7
  3. You might want to use code tags.
    Code (Text):
     
     
  4. Strahan

    Benefactor

    Just so you know, you can use the [PLAIN] tag to be able to write the tag as visible and not interpreted. I.E.
    Code (Text):
    You might want to use [plain][CODE][/plain] tags
     
  5. What is going on here
     
  6. Strahan

    Benefactor

    I don't think you understand what all spectator entails. All you are doing, and a bit awkwardly I might add, is giving people the ability to fly for a short time and cancelling targeting. Also you should follow Java naming conventions and use PascalCase for classes, not camelCase. Oh, and that list is definitely not static so remove that keyword.
     
    • Like Like x 1
  7. Dude using static will allow you to access this list in any class.
    There's no reason to use all stuff in one class
     
  8. "Dude", please read Beginner Programming Mistakes and Why You're Making Them. Static abuse and lack of encapsulation are what pop out right away, but I'm sure there's other issues as well.
     
    • Agree Agree x 3
    • Like Like x 1
  9. Static is not an access modifier..
     
    • Agree Agree x 4
  10. Strahan

    Benefactor

    (sigh)
     
  11. .... This thread was a mistake I don't think I'll ever share something I created again, you guys are so toxic.

    Static will allow more fast usage from all the other classes, by using the name of the class dot and the method, instead of using
    Classname name = new Classname();
    And then using name dot.
    I said using static not what all it's usages are.

    It can be used as a fast access modifier,
    And that's what inside the code.

    Also I'm not a beginner.
     
    #11 FlameFOxYT, Feb 20, 2020
    Last edited: Feb 20, 2020
  12. This thread was not a mistake. It's a learning experience. If not for you, then at least for someone else.

    You clearly don't know what the static keyword is for, though. It's not for "fast usage". A static field represents something that belongs to the class, not an instance of the class. In your example, the list belongs to the instance, not the class. Therefore, it should objectively not be static.
    If you want to enforce the singleton pattern, you're welcome to do so. But the list would still be a private member field rather than a public static one.
     
    • Agree Agree x 4
  13. In your case, you are creating a listener that handles the player death. It would be more conventional (and preferred) to not make the list static, as in terms of OOP, the list should be per-object (or death listener in your case).

    Static is not meant to be used to "circumvent object-oriented patterns". It is not a way to "not create a new object". In an object oriented language like Java, object patterns should be used instead of abusive singleton usage. Unless by "fast access" you are referring to how a static invocation is slightly faster than a virtual invocation in the JVM, however this varies between machines and the difference is so subtle that it is negligible.
     
  14. Choco

    Moderator

    Absolutely not. Please continue to share but take precautions to ensure that what you're sharing is valuable. I've edited your original post to enclose your code in [CODE=java][/CODE] tags as to make it more readable (maintaining indentation and syntax highlighting). Here is what I noticed in your resource that can be improved upon.

    The Thread:
    • Please prefix your thread instead with the Resource prefix. This denotes that it is a resource for developers (code, tutorial, etc.)
    • Explain what your code is doing. Just shoving a bunch of code down developer's throats is hardly teaching and encourages copy/paste without understanding what the code does. Go step by step and explain why things are done the way they are
    • This is more of a note for the future because I've edited the thread for you but please surround any code in CODE tags
    The Code:
    • Abide by Java's naming conventions. Classes should be PascalCase (UpperCamelCase) and fields/methods/variables lowerCamelCase.
      • Avoid names such as d or p unless you're in a functional implementation (i.e. a lambda, but even then - avoid if possible) as they are much more difficult to read and understand. Members and variables should have purpose
    • Finalize any and all fields that are not later modified (i.e. your plugin field should be final)
    • The dead field should not be static. @drives_a_ford linked a resource which explains why in more detail. It is not an access modifier
      • Additionally, it should be private and final. Why is this public? Your listeners seem to be the only one using it. If it's so you can check whether or not a player is a spectator, you should be properly encapsulating and have a member method that returns the result of dead.contains()
      • Under no circumstance should there be any reason for any code outside of this listener to be modifying that list
      • This list should also probably be a HashSet of UUIDs. You're using a lot of calls to contains() and unnecessarily holding references to Players.
    • I often recommend avoiding registration of events in constructors. It leads to registration calls being done in various different places and makes onEnable() methods feel dry. Blind construction of objects is never ideal (meaning new SpectatorListener() in your onEnable() method just looks strange without an assignment)
    • Your death listener only prevents damage from entities. Why? They can still be damaged from other sources despite being in a "pseudo-spectator" mode
      • There's also quite a number of instances where DRY (don't repeat yourself) principles have been broken - the name should be self explanatory
    • Your if and else if statements in various places can be replaced with just an ||. Both blocks execute the same code
    • Why are they being removed from the spectator list after 10 seconds? Perhaps make this more flexible or let developers supply a Consumer that gets called after x amount of seconds.
    Over all, there's room for improvement. That doesn't necessarily mean you're wrong or that you should give up. Take this opportunity to understand why there is as harsh of criticism in response to this thread and work on creating a more robust resource in the future. We encourage resources very much as they're not all too common :) Just make sure it's something people will use and that it's very heavily revised.
     
    #14 Choco, Feb 21, 2020 at 2:12 AM
    Last edited: Feb 21, 2020 at 2:58 AM
    • Winner Winner x 4
    • Agree Agree x 1
  15. TIL that "PascalCase" has a name. Always thought it was camel case in general.