Solved Wondering how efficient this is.

Hi, I'm making a plugin that "detects" if a player is using MineChat. I made it so that if a player runs the "/spawn" command between 40 and 50 ticks, then give them the suffix "[Mobile]" and put them on a hashmap, and then I have this event set up to check if they move:

Code:

    @EventHandler
    public void onPlayerMove(PlayerMoveEvent e) {
        final Player p = e.getPlayer();
        final String name = e.getPlayer().getName();
        if (map.containsKey(name)) {
            map.remove(name);
            Bukkit.getServer().dispatchCommand(Bukkit.getConsoleSender(), "manudelv " + name + " suffix");
        }
    }

This is because technically any player could run /spawn between 40 and 50 ticks of joining, but when a player moves if they're on the hashmap, then it removes them and removes the suffix. It's not perfect, however I think the idea is fine for my server.

SO my question is: Is this an efficient way to check? Or is there a better way?

Thanks in advance to anyone that may be able to shed some light!



All of my code:

Code:

package me.themineshack;

import java.util.HashMap;

import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerCommandPreprocessEvent;
import org.bukkit.event.player.PlayerJoinEvent;
import org.bukkit.event.player.PlayerMoveEvent;
import org.bukkit.event.player.PlayerQuitEvent;
import org.bukkit.plugin.java.JavaPlugin;

public class MobileTracker extends JavaPlugin implements Listener {
  
    HashMap<String, String> map = new HashMap<String, String>();
  
  
    @Override
    public void onEnable() {
        Bukkit.getServer().getPluginManager().registerEvents(this, this);
    }
  
    @Override
    public void onDisable() {
      
    }
  
    @EventHandler
    public void onPlayerJoin(PlayerJoinEvent e) {
        final Player p = e.getPlayer();
        final String name = e.getPlayer().getName();
        this.getServer().getScheduler().scheduleSyncDelayedTask(this,  new Runnable() {
            public void run() {
                map.put(name, "PC " + p.getLocation());
            }
        }, 40L);
        this.getServer().getScheduler().scheduleSyncDelayedTask(this,  new Runnable() {
            public void run() {
                if (map.containsKey(name)) {
                    String[] val = map.get(name).split(" ");
                    String pl = val[0];
                    String loc = val[1];
                    if (pl.equalsIgnoreCase("PC"))
                        return;
                    if (pl.equalsIgnoreCase("Mobile")) {
                        Bukkit.getServer().dispatchCommand(Bukkit.getConsoleSender(), "manuaddv " + name + " suffix &8 [&9Mobile&8]");
                    }
                }
            }
        }, 50L);
    }
  
    @EventHandler
    public void onPlayerQuit(PlayerQuitEvent e) {
        final Player p = e.getPlayer();
        final String name = e.getPlayer().getName();
        if (map.containsKey(name)) {
            map.remove(name);
            Bukkit.getServer().dispatchCommand(Bukkit.getConsoleSender(), "manudelv " + name + " suffix");
        }
    }
  
    @EventHandler
    public void onCommand(PlayerCommandPreprocessEvent e) {
        final Player p = e.getPlayer();
        final String name = e.getPlayer().getName();
        if (e.getMessage().equalsIgnoreCase("/spawn")) {
            if (map.containsKey(name)) {
                map.remove(name);
                map.put(name, "Mobile " + p.getLocation());
            }
        }
    }
  
    @EventHandler
    public void onPlayerMove(PlayerMoveEvent e) {
        final Player p = e.getPlayer();
        final String name = e.getPlayer().getName();
        if (map.containsKey(name)) {
            map.remove(name);
            Bukkit.getServer().dispatchCommand(Bukkit.getConsoleSender(), "manudelv " + name + " suffix");
        }
    }
  
} 

 

I would watch out. However if you'd notice, Minechat makes a player entity where it should be, and it's not moveable at all, and If I'm correct, you can't teleport it either.

 

@Agentleader1 You can teleport it, and it can warp, however it won't throw a PlayerMoveEvent. My main concern is that technically a normal player could run the /spawn command and get the [Mobile] tag, but it would be removed when they moved(and provided that the person is actually on MineChat, they'd never move). I wanted to know if it was an efficient way to check or not.

 

You don't need to check to see if the Map contains an Object to remove it. Just remove it.

 

@teej107 If I don't check, then it would cause the console to run manudelv <name> suffix every time someone moved. First gotta make sure they're even on the map. Am I correct?

 

@oceantheskatr Read the documentation for #remove() in Map. Let's see if you see what I see.

 

@teej107 Err, am I missing something? If I take out the check, then every time someone moves it would run manudelv <name> suffix.

See?

With the check:

 

@oceantheskatr You need to check if "map.remove()" returned true. If the value isn't in the map, it will return false ==> User didn't have the prefix. If it returns true, the user HAD the prefix, as the removal was successful.
So,

Code:

if(remove != null) // remove returned not null ==> player had a mobile suffix
   delete suffix
else // player didn't have a mobile suffix
  do nothing

EDIT: Edited, due to stupidness.... I hope it's better now @teej107

 

I think you need to read the documentation as well. The OP isn't storing booleans

 

@teej107 Oh. My bad. I totally messed up map and set... embarrassing. I will do better next time, i hope xD

 

@I Al Istannen @teej107 Thanks both of you!

 

@teej107 @oceantheskatr In terms of efficiently. What about a sheduler every 20 ticks checking if the x, y or z position changed. Wouldn't that be more efficient?

EDIT: I made some (very, very likly) unbelievable bad testing sheduler:

Code:

public class RunnableUsed extends BukkitRunnable {

   HashSet<Long> timeSet;
   HashMap<UUID, Location> playerLoc;

   public RunnableUsed() {
     this.playerLoc = new HashMap<>();
     
     for(Player p : Bukkit.getOnlinePlayers()) {
       playerLoc.put(p.getUniqueId(), p.getLocation());
       
       timeSet = new HashSet<>();
     }
   }
   
   @Override
   public void run() {
         
     long startTime = System.nanoTime();
     
     for(Player p : Bukkit.getOnlinePlayers()) {
       if(playerLoc.containsKey(p.getUniqueId())) {
         if(playerLoc.get(p.getUniqueId()).distanceSquared(p.getLocation()) != 0) {
           playerLoc.remove(p.getUniqueId());
           p.sendMessage("Removed");
         }
         if(timeSet.size() > 0) {
           System.out.println("Average: " + ((timeSet.stream().mapToLong(Long::longValue).sum())/timeSet.size()));
         }
       }
     }

     timeSet.add(System.nanoTime() - startTime);
     System.out.println("This iteration: " + (System.nanoTime() - startTime));
   }
   
}

One player in list:

Show Spoiler Hide Spoiler

Average: 160000 nano Seconds ==> 0.16 mili seconds

Two player in list:

Show Spoiler Hide Spoiler

Average: 180000 nano Seconds ==> o.18 mili seconds

Zero player in list:

Show Spoiler Hide Spoiler

Average: 24000 nano Seconds ==> 0.024 mili seconds

The other one:

Code:

  @EventHandler
   public void onPlayerMoveEvent(PlayerMoveEvent e) {
     long startTime = System.nanoTime();
     
     if(playerLoc.containsKey(e.getPlayer().getUniqueId())) {
       
     }
     
     if(time.size() > 0) {
       System.out.println("Average: " + ((time.stream().mapToLong(Long::longValue).sum())/time.size()));
     }
     
     time.add(System.nanoTime() - startTime);
     
     System.out.println("This iteration: " + (System.nanoTime() - startTime));
   }

I have just one mouse, so just for one player:
Average: 190000 ns ==> 0.19 ms
PER CALL of the event! This gets even called when a player moves his head, the other one is once a second.


PLEASE, correct me. I have literally no idea, how many things I have done wrong with the above things, it would be nice if you could correct the mistakes. We want a correct result, don't we?

 

True! I totally forgot, but if you take a look at my code you'll see that I store the players location I think I'll do that, as it wouldn't be checking every player's movement all the time.

EDIT: Oh wait, but if the player got tp'd somewhere, then I guess that'd mess up that idea. :/

 

@oceantheskatr What about adding the player to a list in your CommandPreProcessEvent? Then, if the players Location changed you would remove the player from the Set/List/Map, whatever you are using. If the remove method returns the answer, that the player was on it before, update the location of the player, but don't remove the suffix. If the remove method returns, that there was no mapping, the player moved and wasn't teleported.

So you would need to add to the list, when the player teleports. And just check, if the sheduler noticed the player moved.

There is a teleport event, I haven't used it though.