From 18c46481f4b7915e18ee005c9e57f0c8073f3ba6 Mon Sep 17 00:00:00 2001
From: Koding <kodingdev@pm.me>
Date: Wed, 14 Sep 2022 21:44:43 +1000
Subject: [PATCH] Fix incorrect functionality of ClientSpectatePacket (#1402)

---
 .../event/player/PlayerSpectateEvent.java     | 29 +++++++++++++++++
 .../server/listener/SpectateListener.java     | 31 +++++++++++++++++--
 .../client/play/ClientSpectatePacket.java     |  5 +++
 3 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 src/main/java/net/minestom/server/event/player/PlayerSpectateEvent.java

diff --git a/src/main/java/net/minestom/server/event/player/PlayerSpectateEvent.java b/src/main/java/net/minestom/server/event/player/PlayerSpectateEvent.java
new file mode 100644
index 000000000..78a612ebf
--- /dev/null
+++ b/src/main/java/net/minestom/server/event/player/PlayerSpectateEvent.java
@@ -0,0 +1,29 @@
+package net.minestom.server.event.player;
+
+import net.minestom.server.entity.Entity;
+import net.minestom.server.entity.Player;
+import net.minestom.server.event.trait.PlayerEvent;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Called by the SpectateListener when a player starts spectating an entity.
+ */
+@SuppressWarnings("ClassCanBeRecord")
+public class PlayerSpectateEvent implements PlayerEvent {
+    private final Player player;
+    private final Entity target;
+
+    public PlayerSpectateEvent(Player player, Entity target) {
+        this.player = player;
+        this.target = target;
+    }
+
+    public Entity getTarget() {
+        return target;
+    }
+
+    @Override
+    public @NotNull Player getPlayer() {
+        return player;
+    }
+}
diff --git a/src/main/java/net/minestom/server/listener/SpectateListener.java b/src/main/java/net/minestom/server/listener/SpectateListener.java
index d058e2027..cab7033f3 100644
--- a/src/main/java/net/minestom/server/listener/SpectateListener.java
+++ b/src/main/java/net/minestom/server/listener/SpectateListener.java
@@ -1,7 +1,11 @@
 package net.minestom.server.listener;
 
 import net.minestom.server.entity.Entity;
+import net.minestom.server.entity.GameMode;
 import net.minestom.server.entity.Player;
+import net.minestom.server.event.EventDispatcher;
+import net.minestom.server.event.player.PlayerSpectateEvent;
+import net.minestom.server.instance.Instance;
 import net.minestom.server.network.packet.client.play.ClientSpectatePacket;
 
 import java.util.UUID;
@@ -9,15 +13,36 @@ import java.util.UUID;
 public class SpectateListener {
 
     public static void listener(ClientSpectatePacket packet, Player player) {
+        // Ignore if the player is not in spectator mode
+        if (player.getGameMode() != GameMode.SPECTATOR) {
+            return;
+        }
+
         final UUID targetUuid = packet.target();
         final Entity target = Entity.getEntity(targetUuid);
 
         // Check if the target is valid
-        if (target == null || target == player)
+        if (target == null || target == player) {
             return;
+        }
 
-        // TODO check if 'target' is in a different instance
-        player.spectate(target);
+        // Ignore if they're not attached to any instances
+        Instance targetInstance = target.getInstance();
+        Instance playerInstance = player.getInstance();
+        if (targetInstance == null || playerInstance == null) {
+            return;
+        }
+
+        // Ignore if they're not in the same instance. Vanilla actually allows for
+        // cross-instance spectating, but it's not really a good idea for Minestom.
+        if (targetInstance.getUniqueId() != playerInstance.getUniqueId()) {
+            return;
+        }
+
+        // Despite the name of this packet being spectate, it is sent when the player
+        // uses their hotbar to switch between entities, which actually performs a teleport
+        // instead of a spectate.
+        EventDispatcher.call(new PlayerSpectateEvent(player, target));
     }
 
 }
diff --git a/src/main/java/net/minestom/server/network/packet/client/play/ClientSpectatePacket.java b/src/main/java/net/minestom/server/network/packet/client/play/ClientSpectatePacket.java
index e2fc5e08e..33e689633 100644
--- a/src/main/java/net/minestom/server/network/packet/client/play/ClientSpectatePacket.java
+++ b/src/main/java/net/minestom/server/network/packet/client/play/ClientSpectatePacket.java
@@ -7,6 +7,11 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.UUID;
 
+/**
+ * The ClientSpectatePacket is sent when the client interacts with their hotbar to switch between entities.
+ * Contrary to its name, it is actually used to teleport the player to the entity they are switching to,
+ * rather than spectating them.
+ */
 public record ClientSpectatePacket(@NotNull UUID target) implements ClientPacket {
     public ClientSpectatePacket(BinaryReader reader) {
         this(reader.readUuid());