From 50699be5757a5291bfdc42770b08b952fed313f8 Mon Sep 17 00:00:00 2001
From: Harley Gilpin <hg@harleygilpin.com>
Date: Tue, 1 Sep 2026 16:24:51 -0700
Subject: [PATCH] Stop player save snapshots sharing mutable state with the
 player

Player.copy() defensively copies every field except friends, which was
passed by reference, and offers, where copyOf() copies the array but not
the ExchangeOffer elements behind it.

The snapshot is serialized on Dispatchers.IO while the game thread can
still write to both. FriendsList and ClanChat assign into player.friends,
and an offer's state, completed and coins are var, so a save can observe
a half-updated snapshot or throw ConcurrentModificationException - after
Config.fileWriter has already truncated the target file.

friends is copied with toMap() and offers elementwise. ExchangeHistory
is entirely val, so history.toList() is already safe.
---
 .../gregs/voidps/engine/data/PlayerSave.kt    |  6 ++--
 .../voidps/engine/data/PlayerSaveTest.kt      | 32 +++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)
 create mode 100644 engine/src/test/kotlin/world/gregs/voidps/engine/data/PlayerSaveTest.kt

diff --git a/engine/src/main/kotlin/world/gregs/voidps/engine/data/PlayerSave.kt b/engine/src/main/kotlin/world/gregs/voidps/engine/data/PlayerSave.kt
index 72b97ee8ed..92fec8c681 100644
--- a/engine/src/main/kotlin/world/gregs/voidps/engine/data/PlayerSave.kt
+++ b/engine/src/main/kotlin/world/gregs/voidps/engine/data/PlayerSave.kt
@@ -389,7 +389,7 @@ data class PlayerSave(
                 colours = colours,
                 variables = variables,
                 inventories = inventories,
-                friends = friends,
+                friends = friends.toMap(),
                 ignores = ignores,
                 offers = offers,
                 history = history,
@@ -410,8 +410,8 @@ internal fun Player.copy() = PlayerSave(
     colours = body.colours.copyOf(),
     variables = variables.data.toMap(),
     inventories = inventories.instances.mapValues { it.value.items.map { itm -> itm.copy() }.toTypedArray() },
-    friends = friends,
+    friends = friends.toMap(),
     ignores = ignores.toList(),
-    offers = offers.copyOf(),
+    offers = Array(offers.size) { offers[it].copy() },
     history = history.toList(),
 )
diff --git a/engine/src/test/kotlin/world/gregs/voidps/engine/data/PlayerSaveTest.kt b/engine/src/test/kotlin/world/gregs/voidps/engine/data/PlayerSaveTest.kt
new file mode 100644
index 0000000000..e4f560a8a4
--- /dev/null
+++ b/engine/src/test/kotlin/world/gregs/voidps/engine/data/PlayerSaveTest.kt
@@ -0,0 +1,32 @@
+package world.gregs.voidps.engine.data
+
+import org.junit.jupiter.api.Test
+import world.gregs.voidps.engine.data.exchange.ExchangeOffer
+import world.gregs.voidps.engine.entity.character.player.Player
+import world.gregs.voidps.engine.entity.character.player.chat.clan.ClanRank
+import kotlin.test.assertEquals
+
+internal class PlayerSaveTest {
+
+    @Test
+    fun `Snapshot doesn't share the player's friends`() {
+        val player = Player(accountName = "name")
+        player.friends["first"] = ClanRank.Friend
+
+        val save = player.copy()
+        player.friends["second"] = ClanRank.Friend
+
+        assertEquals(mapOf("first" to ClanRank.Friend), save.friends, "Snapshot changed after the player edited their friends list")
+    }
+
+    @Test
+    fun `Snapshot doesn't share the player's exchange offers`() {
+        val player = Player(accountName = "name")
+        player.offers[0] = ExchangeOffer(id = 1, item = "coins", amount = 10, price = 5)
+
+        val save = player.copy()
+        player.offers[0].completed = 7
+
+        assertEquals(0, save.offers[0].completed, "Snapshot changed after the player's offer progressed")
+    }
+}