Solved request system

Really? I think I read somewhere that using the player instead of the players name is bad? Maybe it was for configs

 

You read correct. The person that said it was bad was giving misleading advice. Instead of telling the real solution for the real problem they simply took the lazy way out and just said dont do it ever.

 

MrHatz2 "I need an example script" just translates to "write my code". If you want people to write your code, go and make a plugin request.

 

-

 

xTigerRebornx
yah but I just want to make it myself because I want to make plugins like worldedit and essentials

 

-

 

MrHatz2 If you want to make your own plugins, then why are you asking for us to give you code? We would be making it, not you.

 

xTigerRebornx
im not asking for code I just want an example to see what it looks like so I can do the same.

 

MrHatz2
Which means that you're asking for code.

 

MrHatz2 The Javadocs are that example, learn how to use them if you don't know.

 

MrHatz2 I gave you an example

 

Copy pasting will never get you anywhere. You need to comprehend the code enough to write it yourself. Everyone has told you in extreme detail on how to accomplish what you want to do. If you don't understand plain English explanations, then you need to do more research on the programming language. And saying things like "I learn by seeing other people's code" is a b.s. If you cant be bothered to research the English explanations why should we believe you are going to research the code explanations?

 

xTigerRebornx zombiekiller753 Skionz
k I came up with this script. Is there anything wrong?

Code:java

 
Map<Player, Player> tpa = new HashMap<Player, Player>();
 
// request logic
if(Bukkit.getPlayer(a[0]) == null){
    //no exist player
    return true;
}
 
Player target = Bukkit.getPlayer(a[0]);
 
tpa.put(p, target);
 
return true;
 
//accepting request logic
if(Bukkit.getPlayer(a[0]) == null){
    //no exist player
    return true;
}
 
Player requester = Bukkit.getPlayer(a[0]);
 
if(tpa.get(requester) != p){
     //no request from him
     return true;
}
 
//now I put code here
 
tpa.remove(requester);
 
return true;

 

Only a quick glance I see you used Bukkit.getPlayer() four times. I guess you missed my post earlier where I said this was a Bad Thing(TM).

 

BillyGalbreath
But the requester inputs a name for a player so I want to make sure the player is online is that how you check if they are online?

I also want player object of name so i search and it say to use that

 

Post all of your code. With it being out of context we can't tell what you're doing here.

 

BillyGalbreath
k my onCommand:

Code:java

private Map<Player, Player> tpa = new HashMap<Player,  Player>();
public boolean onCommand(CommandSender s, Command c, String l, String[] a){
    if(!(s instanceof Player)){
        //is console
        return true;
    }
 
    Player p = (Player) s;
 
    if(c.getName().equalsIgnoreCase("tparequest")){
 
        if(a.length == 0){
            //no arguments
            return true;
        }
 
        if(Bukkit.getPlayer(a[0])){
             //no exist player
             return true;
         }
 
         Player target = Bukkit.getPlayer(a[0]);
 
         //Now request here right?
         tpa.put(p, target);
 
         return true;
    }
 
    if(c.getName().equalsIgnoreCase("tpaccept")){
 
         if(a.length == 0){
             //no arguements
             return true;
         }
 
         if(Bukkit.getPlayer(a[0]) == null){
             //no exist player
             return true;
         }
 
         Player requester = Bukkit.getPlayer(a[0]);
 
         if(tpa.get(requester) != p){
             //no request from him
             return true;
          }
 
          //now rest of code here
 
          tpa.remove(requester);
          return true;
 
    }
 
    return true;
 
}

 

If you know Java, you know how to use HashMaps.

 

1) Why is tpaccept accepting arguments? I think its easier for the user to just /tpaccept to the last request sent instead of specifying a requester. But, its your plugin and if thats what you want, thats what you want.

2) You still use Bukkit.getPlayer() too much. I see it four times, where it could easily be only two. And if you dont need arguments in tpaccept then you need only one.

3) I suggest using more descriptive variable names. tpa, a, p, s, c, l... In this use-case its relatively simple to understand them, but practicing descriptive variables will make it easier on you in the future when you're writing bigger plugins.

4) You are never actually teleporting the player.

 

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Test what you have. If there is an error, come back with a pastebin of the it.

 

BillyGalbreath
K you were very helpful and I will make the changes

BillyGalbreath
Wait but for tpaccept how can I accept a player who request for him because more than one people can request for him so that's why I made tpaccept have argument

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

And thats your choice to do so, if you wish. ^_^

Here's what I mean about having too many Bukkit.getPlayer() lines, though:

Change this:

Code:java

 
if(Bukkit.getPlayer(a[0])) {
    //no exist player
    return true;
}
Player target = Bukkit.getPlayer(a[0]);
 

Into this:

Code:java

 
Player target = Bukkit.getPlayer(a[0]);
if(target == null) {
    //no exist player
    return true;
}
 

Instead of calling that heavy method twice, its now only once.

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Oh.

Kay.

Obviously it's a bit—shall we say—early in your experiences to start worrying about fine details such as these, but don't make a habit out of storing players. Would be best to store the UUID. Memory leaks, and such sometimes happen from storing Player, isn't that right?

 

No.

 

Wait, really? But the instance of the player is saved and prevents the system from releasing RAM, doesn't it?

Whoa.

EDIT: Actually did look it up, many, many people think it is in fact a bad idea to store players. I think it has to do with the fact that Java GC can't remove the player if he logs off (so I've heard... but PlayerQuitEvent, anybody?) and it's redundant to store the ENTIRE Player to a hashmap when all you can access him through UUID.

More lightweight, I guess. Could you elaborate? I'm very interested in this topic.

 

ColonelHedgehog Memory leak won't occur if you properly cleanup references (such as removing onLogout/disconnect), Quit event is fired before the Player is removed from the server, allowing for cleanup.

 

​

That's what I thought. However, it seems to be a heck of a lot less painful just to use UUIDs, and as I said before, no need to save the entire instance of a Player.

 

ColonelHedgehog Eh, may be of use if you plan on using the "Player" that the UUID belongs to (like, if you are commonly calling Bukkit#getPlayer(UUID) it may be better to do that or use some sort of WeakRef Map/List/Set).

 

I use UUIDs all the time. In fact, they're much better than player names because they're accessed quicker (I forget why, though. ;-. Bukkit#getPlayer(UUID) is a pretty useful function. I just don't see why you'd ever want to store a player and not his UUID.

 

Imagine a server with 100+ people running around and you have Bukkit.getPlayer(UUID) to get the Player object in the PlayerMoveEvent, all so you can check what they are holding in their hands as they run around (so you know wht actions to take)... You just lagged out your server. But if you had a Player object already, it would not lag out. In fact, it wouldnt even drop any TPS most likely...

Edit: This is just one of infinite number of examples.