From df531d37246646a14fc0e2b8d943c2af79fc3073 Mon Sep 17 00:00:00 2001
From: WinDanesz <31292708+windanesz@users.noreply.github.com>
Date: Thu, 28 Mar 2024 00:46:25 +0100
Subject: [PATCH] fix: Fixed all synchronized WizardData stored variables on
 dedicated servers. This issue was causing e.g. the Ancient Compass to not
 display its targets on servers. Fixes #857

---
 .../electroblob/wizardry/data/IStoredVariable.java    |  5 +++++
 .../java/electroblob/wizardry/data/IVariable.java     |  8 ++++++++
 .../java/electroblob/wizardry/data/WizardData.java    | 11 ++++++++---
 .../electroblob/wizardry/packet/PacketPlayerSync.java | 11 +++++++----
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/src/main/java/electroblob/wizardry/data/IStoredVariable.java b/src/main/java/electroblob/wizardry/data/IStoredVariable.java
index ea7d1a0d..1923d55f 100644
--- a/src/main/java/electroblob/wizardry/data/IStoredVariable.java
+++ b/src/main/java/electroblob/wizardry/data/IStoredVariable.java
@@ -64,6 +64,11 @@ public interface IStoredVariable<T> extends IVariable<T> {
 			this.ticker = (p, t) -> t; // Initialise this with a do-nothing function, can be overwritten later
 		}
 
+		@Override
+		public String getKey() {
+			return key;
+		}
+
 		/**
 		 * Replaces this variable's update method with the given update function. <i>Beware of auto-unboxing of
 		 * primitive types! For lambda expressions, check the second parameter isn't null before operating on it.
diff --git a/src/main/java/electroblob/wizardry/data/IVariable.java b/src/main/java/electroblob/wizardry/data/IVariable.java
index 3f123159..677deda2 100644
--- a/src/main/java/electroblob/wizardry/data/IVariable.java
+++ b/src/main/java/electroblob/wizardry/data/IVariable.java
@@ -47,7 +47,10 @@ public interface IVariable<T> {
 	 */
 	void write(ByteBuf buf, T value);
 
+	String getKey();
+
 	/**
+	 *
 	 * Reads this variable's value from the given {@link ByteBuf}.
 	 */
 	T read(ByteBuf buf);
@@ -110,6 +113,11 @@ public interface IVariable<T> {
 			// NYI
 		}
 
+		@Override
+		public String getKey() {
+			return "none"; // we don't mind as these are never synced (electroblob.wizardry.data.IVariable.Variable.isSynced)
+		}
+
 		@Override
 		public T read(ByteBuf buf){
 			return null; // NYI
diff --git a/src/main/java/electroblob/wizardry/data/WizardData.java b/src/main/java/electroblob/wizardry/data/WizardData.java
index d9e01589..6cd36067 100644
--- a/src/main/java/electroblob/wizardry/data/WizardData.java
+++ b/src/main/java/electroblob/wizardry/data/WizardData.java
@@ -199,9 +199,14 @@ public class WizardData implements INBTSerializable<NBTTagCompound> {
 	}
 
 	/** Returns a set containing the registered {@link IStoredVariable} objects for which {@link IVariable#isSynced()}
-	 * returns true. Used internally for packet reading. */
-	public static Set<IVariable> getSyncedVariables(){
-		return storedVariables.stream().filter(IVariable::isSynced).collect(Collectors.toSet());
+	 * returns true, ordered by their keys obtained from {@link IVariable#getKey()}. Used internally for packets. */
+	public static Set<IVariable> getSyncedVariablesOrderedByKey(){
+		Comparator<IVariable> keyComparator = Comparator.comparing(IVariable::getKey);
+
+		return storedVariables.stream()
+				.filter(IVariable::isSynced)
+				.sorted(keyComparator)
+				.collect(Collectors.toCollection(LinkedHashSet::new));
 	}
 
 	/**
diff --git a/src/main/java/electroblob/wizardry/packet/PacketPlayerSync.java b/src/main/java/electroblob/wizardry/packet/PacketPlayerSync.java
index 6fca18bc..c39b2d70 100644
--- a/src/main/java/electroblob/wizardry/packet/PacketPlayerSync.java
+++ b/src/main/java/electroblob/wizardry/packet/PacketPlayerSync.java
@@ -10,7 +10,11 @@ import net.minecraftforge.fml.common.network.simpleimpl.IMessage;
 import net.minecraftforge.fml.common.network.simpleimpl.IMessageHandler;
 import net.minecraftforge.fml.common.network.simpleimpl.MessageContext;
 
-import java.util.*;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 
 /**
  * <b>[Server -> Client]</b> This packet is sent to synchronise any fields that need synchronising in
@@ -54,9 +58,8 @@ public class PacketPlayerSync implements IMessageHandler<Message, IMessage> {
 
 			this.seed = buf.readLong();
 			this.selectedMinionID = buf.readInt();
-
 			this.spellData = new HashMap<>();
-			WizardData.getSyncedVariables().forEach(v -> spellData.put(v, v.read(buf)));
+			WizardData.getSyncedVariablesOrderedByKey().forEach(v -> spellData.put(v, v.read(buf)));
 			// Have to send empty tags to guarantee correct ByteBuf size/order, but no point keeping the resulting nulls
 			spellData.values().removeIf(Objects::isNull);
 
@@ -73,7 +76,7 @@ public class PacketPlayerSync implements IMessageHandler<Message, IMessage> {
 			buf.writeLong(seed);
 			buf.writeInt(selectedMinionID);
 
-			WizardData.getSyncedVariables().forEach(v -> v.write(buf, spellData.get(v)));
+			WizardData.getSyncedVariablesOrderedByKey().forEach(v -> v.write(buf, spellData.get(v)));
 
 			if(this.spellsDiscovered == null) return;
 			for(Spell spell : this.spellsDiscovered){