Improve CraftEntity and CraftPlayer equals

patches/server/Fix-CraftEntity-hashCode.patch

@@ -1,46 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Aikar <aikar@aikar.co>
-Date: Sun, 10 Jun 2018 20:20:15 -0400
-Subject: [PATCH] Fix CraftEntity hashCode
-
-hashCodes are not allowed to change, however bukkit used a value
-that does change, the entityId.
-
-When an entity is teleported dimensions, the entity reference is
-replaced with a new one with a new entity ID.
-
-For hashCode, we can simply use the UUID's hashCode to keep
-the hashCode from changing.
-
-equals() is ok to use getEntityId() because equals() should only
-be true if both the left and right are the same reference.
-
-Since entity ids can not duplicate during runtime, this
-check is essentially the same as this.getHandle() == other.getHandle()
-
-However, replaced it too to make it clearer of intent.
-
-diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
-index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
---- a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
-+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
-@@ -0,0 +0,0 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
-             return false;
-         }
-         final CraftEntity other = (CraftEntity) obj;
--        return (this.getEntityId() == other.getEntityId());
-+        return (this.getHandle() == other.getHandle()); // Paper - while logically the same, this is clearer
-     }
- 
-+    // Paper - Fix hashCode. entity ID's are not static.
-+    // A CraftEntity can change reference to a new entity with a new ID, and hash codes should never change
-     @Override
-     public int hashCode() {
--        int hash = 7;
--        hash = 29 * hash + this.getEntityId();
--        return hash;
-+        return getUniqueId().hashCode();
-+        // Paper end
-     }
- 
-     @Override

patches/server/Improve-CraftEntity-and-CraftPlayer-equals-and-hashC.patch

@@ -0,0 +1,103 @@
+From 0000000000000000000000000000000000000000 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.
+
+diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
+--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
++++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+@@ -0,0 +0,0 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
+ 
+     @Override
+     public UUID getUniqueId() {
+-        return this.getHandle().getUUID();
++        return this.entity.getUUID();
+     }
+ 
+     @Override
+@@ -0,0 +0,0 @@ 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/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
+--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
++++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+@@ -0,0 +0,0 @@ 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
+@@ -0,0 +0,0 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
+ 
+     @Override
+     public boolean canSee(org.bukkit.entity.Entity entity) {
+-        return this.equals(entity) || entity.isVisibleByDefault() ^ this.invertedVisibilityEntities.containsKey(entity.getUniqueId()); // SPIGOT-7312: Can always see self
++        return this.getUniqueId().equals(entity.getUniqueId()) || entity.isVisibleByDefault() ^ this.invertedVisibilityEntities.containsKey(entity.getUniqueId()); // SPIGOT-7312: Can always see self
+     }
+ 
+     public boolean canSeePlayer(UUID uuid) {
+@@ -0,0 +0,0 @@ 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;
+     }