From 35fef5176d81e917526a9c28718bb91a1aa1a291 Mon Sep 17 00:00:00 2001
From: Jonas Herzig <me@johni0702.de>
Date: Sat, 24 Jul 2021 14:41:41 +0200
Subject: [PATCH] Fix skins of server-only NPCs being unreliable (fixes #486)

---
 .../replay/mixin/Mixin_FixNPCSkinCaching.java | 31 +++++++++++++++++++
 .../resources/mixins.replay.replaymod.json    |  1 +
 2 files changed, 32 insertions(+)
 create mode 100644 src/main/java/com/replaymod/replay/mixin/Mixin_FixNPCSkinCaching.java

diff --git a/src/main/java/com/replaymod/replay/mixin/Mixin_FixNPCSkinCaching.java b/src/main/java/com/replaymod/replay/mixin/Mixin_FixNPCSkinCaching.java
new file mode 100644
index 00000000..925c2b0e
--- /dev/null
+++ b/src/main/java/com/replaymod/replay/mixin/Mixin_FixNPCSkinCaching.java
@@ -0,0 +1,31 @@
+package com.replaymod.replay.mixin;
+
+import net.minecraft.client.MinecraftClient;
+import net.minecraft.client.network.AbstractClientPlayerEntity;
+import net.minecraft.client.network.PlayerListEntry;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+import javax.annotation.Nullable;
+
+@Mixin(AbstractClientPlayerEntity.class)
+public abstract class Mixin_FixNPCSkinCaching {
+    @Shadow @Nullable protected abstract PlayerListEntry getPlayerListEntry();
+
+    @Inject(method = "<init>", at = @At("RETURN"))
+    private void forceCachePlayerListEntry(CallbackInfo ci) {
+        // Server-side NPCs (without a client mod) rely on the player list entry being cached in this class to function
+        // because the remove the entry shortly after spawning the player. However this method is only called when the
+        // player is rendered, and if we are currently jumping around in a replay then it most likely will not be
+        // rendered while the entry is still there. The result is that most NPCs get Steve/Alex skins instead of the
+        // intended ones. To fix that, we make this caching-glitch which servers have come to rely on an actual feature
+        // by just fetching the cache in the constructor which arguably is what MC should have done to begin with,
+        // especially because the spawn packet handling code already requires the entry to be present).
+        if (MinecraftClient.getInstance().getNetworkHandler() != null) { // will be null if this is the client player
+            this.getPlayerListEntry();
+        }
+    }
+}
diff --git a/src/main/resources/mixins.replay.replaymod.json b/src/main/resources/mixins.replay.replaymod.json
index ff2737f1..feb30fa7 100644
--- a/src/main/resources/mixins.replay.replaymod.json
+++ b/src/main/resources/mixins.replay.replaymod.json
@@ -5,6 +5,7 @@
   "mixins": [],
   "server": [],
   "client": [
+    "Mixin_FixNPCSkinCaching",
     //#if MC>=11600
     "Mixin_MoveRealmsButton",
     //#endif