From a70953dfb0bbdbc237cf95c66c948f6c22b9d1a2 Mon Sep 17 00:00:00 2001
From: Nassim Jahnke <nassim@njahnke.dev>
Date: Sat, 9 Nov 2024 21:10:45 +0100
Subject: [PATCH] Improve CraftEntity and CraftPlayer equals and hashCode

Make sure the hash code does not change and also remove outdated
equals logic from CraftPlayer. Long-term, the override there should
be entirely removed, but this is good enough for now.

Replacing some getHandle method calls with direct field access will
also reduce overhead from casts that the overridden methods come with,
at least until those are changed later on as well.
---
 .../craftbukkit/entity/CraftEntity.java       | 18 ++++++---------
 .../craftbukkit/entity/CraftPlayer.java       | 22 +++++--------------
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
index 0480fbeffd..d6241e2db3 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
@@ -437,7 +437,7 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
 
     @Override
     public UUID getUniqueId() {
-        return this.getHandle().getUUID();
+        return this.entity.getUUID();
     }
 
     @Override
@@ -496,21 +496,17 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
 
     @Override
     public boolean equals(Object obj) {
-        if (obj == null) {
-            return false;
-        }
-        if (this.getClass() != obj.getClass()) {
-            return false;
-        }
+        if (this == obj) return true;
+        if (obj == null || getClass() != obj.getClass()) return false;
+
         final CraftEntity other = (CraftEntity) obj;
-        return (this.getEntityId() == other.getEntityId());
+        return this.entity == other.entity; // There should never be duplicate entities with differing references
     }
 
     @Override
     public int hashCode() {
-        int hash = 7;
-        hash = 29 * hash + this.getEntityId();
-        return hash;
+        // The UUID and thus hash code should never change (unlike the entity id)
+        return this.getUniqueId().hashCode();
     }
 
     @Override
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
index ce22bf772a..6538a3ee38 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
@@ -626,22 +626,10 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
 
     @Override
     public boolean equals(Object obj) {
-        if (!(obj instanceof OfflinePlayer)) {
-            return false;
-        }
-        OfflinePlayer other = (OfflinePlayer) obj;
-        if ((this.getUniqueId() == null) || (other.getUniqueId() == null)) {
-            return false;
-        }
-
-        boolean uuidEquals = this.getUniqueId().equals(other.getUniqueId());
-        boolean idEquals = true;
-
-        if (other instanceof CraftPlayer) {
-            idEquals = this.getEntityId() == ((CraftPlayer) other).getEntityId();
-        }
-
-        return uuidEquals && idEquals;
+        // Long-term, this should just use the super equals... for now, check the UUID
+        if (obj == this) return true;
+        if (!(obj instanceof OfflinePlayer other)) return false;
+        return this.getUniqueId().equals(other.getUniqueId());
     }
 
     @Override
@@ -2052,7 +2040,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
     @Override
     public int hashCode() {
         if (this.hash == 0 || this.hash == 485) {
-            this.hash = 97 * 5 + (this.getUniqueId() != null ? this.getUniqueId().hashCode() : 0);
+            this.hash = 97 * 5 + this.getUniqueId().hashCode();
         }
         return this.hash;
     }