Solved How to save player last messages ?

Discussion in 'Spigot Plugin Development' started by Mr_dsa1, Jan 7, 2020.

  1. Hello. I know that there are already similar topics, but this is not what I need. I need to save the last 2 player messages for work with them in the code (the message that the player has just sent and the previous one). I tried to do it myself, but I got really confused, maybe someone will provide a simple example.

    P.s Sorry for bad English
     
  2. What exactly are you looking into achieving with saving the last 2 player messages?
     
  3. Before a player sends a message to chat compare the current message with the previous one. (prohibit repetition of messages).
     
  4. Sounds like you need to save only one, like this:

    Code (Java):
    Map<Player,String> lastMessages = new HashMap<>();
     
    public void onMessageReceived(AsyncPlayerChatEvent e) {
        final Player player = e.getPlayer();
        final String lastMessage = lastMessages.get(player);
        if (lastMessage != null) {
            final String message = e.getMessage();
            final String lastMessage = lastMessages.get(player);
            // do some smart compare
            if (message.equalsIgnoreCase(lastMessage)) player.sendMessage("YOU SHALL NOT PASS");
            e.setCancelled(true);
        }
    }
     
    #4 Schottky, Jan 7, 2020
    Last edited: Jan 8, 2020
    • Like Like x 1
  5. You should check for the player's uuid instead of the whole player object tho (-> For more information, take a look here)
     
    • Agree Agree x 2
    • Like Like x 1
    • Optimistic Optimistic x 1

  6. Code (Java):
    public class CraftPlayer {

    ...

    public boolean equals(Object obj) {
            if (!(obj instanceof OfflinePlayer)) {
                return false;
            } else {
                OfflinePlayer other = (OfflinePlayer)obj;
                if (this.getUniqueId() != null && other.getUniqueId() != null) {
                    boolean uuidEquals = this.getUniqueId().equals(other.getUniqueId());
                    boolean idEquals = true;
                    if (other instanceof CraftPlayer) {
                        idEquals = this.getEntityId() == ((CraftPlayer)other).getEntityId();
                    }

                    return uuidEquals && idEquals;
                } else {
                    return false;
                }
            }
        }

    ...
    }
    Edit 2: whoever wrote that does not know you can override Object#equals() and that CraftPlayer does indeed do that
     
    • Like Like x 1
  7. That was not the point of my message. Read the 5th point of the thread I sent and you'll understand what I mean
     
  8. look. Tell me what an Object is (from the viewpoint of the processor, garbage collector, and memory) and I will tell you why (even thou weak references are generally to be preferred) this is absolutely irrelevant (from an optimization standpoint).

    I only accept the usage of UUID's compared to Player's when it comes to easier custom serialization/ deserialization.
     
  9. The only reason I can see to why you should be using a UUID as a key for the map, is the fact that you needn't filter the key out of the map once the player leaves the server. This wouldn't end up causing a memory leak since if the player were to rejoin, the same uuid would be used in the map and another one wouldn't be added. In contrary to using the player object.

    However, in the case above, the player object is generally used for sending messages and whatso, in other words. Though that's the only case of the player object being used, thus, if I were to code this, I'd go with UUID. Not because of the memory leak possibility, but because I'm not generally using the player object too much, thus not needed, and I'd like to not have a player quit event being listened to so as to remove the entry from the map each time.

    If you were to make heavy use of the player object, I'd suggest you sticked to having the player object as a key for the map and listened to playerquitevent and playerkickevent while removing the entry from the map at that point. (not sure if PQE is fired when the user is kicked).
     
  10. The player will also be overridden since it is the same object. Maps compare using equals, which is overridden using the UUID.
    For the player as well as for the UUID you need to listen in order to remove the object, however, I'm gonna admit that the player consumes more heap space than a UUID and, if not removed properly, will cause memory leaks. Thou a lot of players would have to join continuously (probably over 1000 thou that is just some magic number) to actually notice that leak. Looking at the actual code, a wrong world can cause way worse leaks.
     
  11. What’s a “wrong world”?
     
    • Agree Agree x 1
  12. No, that's not the case. The map entry won't be overriden as the map doesn't use #equals but rather compares hashes. You can try it yourself by having a map with a player object as a key and anything as a value, and simply put an entry with the same player key in the map. Though before adding the second entry, make your player relog. Let me know what the results are.
     
    • Like Like x 1
  13. Okay. Both of us are right.
    A HashMap compares Key and Hash (Key using equals). From the implementation:
    Code (Java):
    if (e.hash == hash &&
                      ((k = e.key) == key || (key != null && key.equals(k))))
    Also, the player implementation overrides the hash, so the check for a player is always a check for the UUID. From CraftPlayer:

    Code (Java):
     public int hashCode() {
            if (this.hash == 0 || this.hash == 485) {
                this.hash = 485 + (this.getUniqueId() != null ? this.getUniqueId().hashCode() : 0);
            }

            return this.hash;
        }
    Now onto the player. You are right, that the player does indeed get incremented when I log in and out:

    Code (Java):
    @EventHandler
    public void onPlayerLogin(PlayerLoginEvent event) {
            playerMap.put(event.getPlayer()), count);
            count += 1;
            System.out.println(playerMap);
    }
    will result in multiple entries in the map.
    However, the reason for this is not that a player is considered a different Object because of different hashes, I guess it's that when he logs out he somehow gets invalidated (maybe the UUID gets set to null, maybe something else). I can say this because I have made a second test using an OfflinePlayer:

    Code (Java):
    @EventHandler
    public void onPlayerLogin(PlayerLoginEvent event) {
            playerMap.put(Bukkit.getOfflinePlayer(event.getPlayer().getUniqueId()), count);
            count += 1;
            System.out.println(playerMap);
    }
    Here, I used a HashMap<OfflinePlayer,Integer> and the result was that a second (or third login for that matter) does not matter and the count of players stays the same.

    Do you know remember that guy that wanted to load 1k x 1k chunks?
    Probably a corrupt world is a better phrase...


    Edit1: actually a check of hashes only could not work since hashes do not have to be different

    Edit2: To elaborate on why there is a new player stored on every new login on the first method, here's the reason. It's totally not obvious since the hash for a new player is exactly the same after each login (and the UUIDs are the same too, so equals() should return the same player, right?). Again, the Hash derived from its UUID (at least that's the case for CraftPlayer).

    The actual reason is this little snippet of code (from CraftPlayer#equals(Object)):

    Code (Java):
    boolean idEquals = true;
    if (other instanceof CraftPlayer) {
        idEquals = this.getEntityId() == ((CraftPlayer)other).getEntityId();
    }
    In other words, if the player is "only" an OfflinePlayer, no id-check will be performed. However, each entity has an id (that is distinct from its "unique id"... which sounds weird but I suppose it's to check that that the same player cannot be on a server twice).

    So to sum up, yes, I was wrong. If a player logs in twice and a Player-object is stored in a HashMap, the HashMap will contain two entries. On the other hand; if the same player is stored as an OfflinePlayer, the object will not be stored twice.
    This means, @MeliodasX is right but for the wrong reason :p
     
    #13 Schottky, Jan 8, 2020
    Last edited: Jan 8, 2020
  14. FrostedSnowman

    Resource Staff

    Don’t visit the Map twice if you don’t need to(or any collection for that matter), considering there’s a result returned in the insertion call.

    Instead of using both Map::containsKey and then Map::get, use Map::get and then check if it’s null, if it is, then the entry isn’t present in the map.
     
    • Agree Agree x 2
    • Useful Useful x 1