Streamline and thereby reduce size of the save game json (#11728)

core/src/com/unciv/json/HashMapVector2.kt

@@ -1,25 +0,0 @@
-package com.unciv.json
-
-import com.badlogic.gdx.math.Vector2
-
-/**
- * @see NonStringKeyMapSerializer
- */
-class HashMapVector2<T> : HashMap<Vector2, T>() {
-    companion object {
-        fun createSerializer(): NonStringKeyMapSerializer<MutableMap<Vector2, Any>, Vector2> {
-            @Suppress("UNCHECKED_CAST") // kotlin can't tell that HashMapVector2 is also a MutableMap within generics
-            val mapClass = HashMapVector2::class.java as Class<MutableMap<Vector2, Any>>
-            return NonStringKeyMapSerializer(
-                mapClass,
-                Vector2::class.java
-            ) { HashMapVector2() }
-        }
-
-        fun getSerializerClass(): Class<MutableMap<Vector2, Any>> {
-            @Suppress("UNCHECKED_CAST") // kotlin can't tell that HashMapVector2 is also a MutableMap within generics
-            return HashMapVector2::class.java as Class<MutableMap<Vector2, Any>>
-        }
-    }
-}
-

core/src/com/unciv/json/LastSeenImprovement.kt

@@ -0,0 +1,63 @@
+package com.unciv.json
+
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.utils.Json
+import com.badlogic.gdx.utils.JsonValue
+import com.unciv.ui.components.extensions.toPrettyString
+
+/**
+ *  Dedicated HashMap with [Vector2] keys for [Civilization.lastSeenImprovement].
+ *
+ *  Deals with the problem that serialization uses map keys converted to strings as json object field names,
+ *   and generic deserialization can't convert them back,
+ *   by implementing Json.Serializable and parsing the key string explicitly.
+ *
+ *  Backward compatibility is implemented in [readOldFormat], but there can be no forward compatibility.
+ *  To remove compatibility, remove the `open` modifier, remove `class HashMapVector2`, remove `readOldFormat`, and the first two lines of `read`. Moving to another package is now allowed.
+ *  To understand the old solution with its nonstandard format that readOldFormat parses, use git history
+ *  to dig out the com.unciv.json.HashMapVector2 and com.unciv.json.NonStringKeyMapSerializer files.
+ */
+open class LastSeenImprovement(
+    private val map: HashMap<Vector2, String> = hashMapOf()
+) : MutableMap<Vector2, String> by map, Json.Serializable {
+    override fun write(json: Json) {
+        for ((key, value) in entries) {
+            val name = key.toPrettyString()
+            json.writeValue(name, value, String::class.java)
+        }
+    }
+
+    override fun read(json: Json, jsonData: JsonValue) {
+        if (jsonData.get("class")?.asString() == "com.unciv.json.HashMapVector2")
+            return readOldFormat(json, jsonData)
+        for (entry in jsonData) {
+            val key = entry.name.toVector2()
+            val value = if (entry.isValue) entry.asString() else entry.getString("value")
+            put(key, value)
+        }
+    }
+
+    private fun String.toVector2(): Vector2 {
+        val (x, y) = removeSurrounding("(", ")").split(',')
+        return Vector2(x.toFloat(), y.toFloat())
+    }
+
+    private fun readOldFormat(json: Json, jsonData: JsonValue) {
+        for (entry in jsonData.get("entries")) {
+            val key = json.readValue(Vector2::class.java, entry[0])
+            val value = json.readValue(String::class.java, entry[1])
+            put(key, value)
+        }
+    }
+
+    override fun equals(other: Any?) = when (other) {
+        is LastSeenImprovement -> map == other.map
+        is Map<*, *> -> map == other
+        else -> false
+    }
+
+    override fun hashCode() = map.hashCode()
+}
+
+/** Compatibility kludge required for backward compatibility. Without this, Gdx won't even run our overridden `read` above. */
+private class HashMapVector2 : LastSeenImprovement()

core/src/com/unciv/json/NonStringKeyMapSerializer.kt

@@ -1,54 +0,0 @@
-package com.unciv.json
-
-import com.badlogic.gdx.utils.Json
-import com.badlogic.gdx.utils.Json.Serializer
-import com.badlogic.gdx.utils.JsonValue
-import com.unciv.logic.automation.civilization.Encampment
-
-/**
- * A [Serializer] for gdx's [Json] that serializes a map that does not have [String] as its key class.
- *
- * Exists solely because [Json] always serializes any map by converting its key to [String], so when you load it again,
- * all your keys are [String], messing up value retrieval.
- *
- * To work around that, we have to use a custom serializer. A custom serializer in Json is only added for a specific class
- * and only checks for direct equality, and since we can't just do `HashMap<Any, *>::class.java`, only `HashMap::class.java`,
- * we have to create a completely new class and use that class as [mapClass] here.
- *
- * @param MT Must be a type that extends [MutableMap]
- * @param KT Must be the key type of [MT]
- */
-class NonStringKeyMapSerializer<MT: MutableMap<KT, Any>, KT>(
-    private val mapClass: Class<MT>,
-    private val keyClass: Class<KT>,
-    private val mutableMapFactory: () -> MT
-) : Serializer<MT> {
-
-    override fun write(json: Json, toWrite: MT, knownType: Class<*>) {
-        json.writeObjectStart()
-        json.writeType(mapClass)
-        json.writeArrayStart("entries")
-        for ((key, value) in toWrite) {
-            json.writeArrayStart()
-            json.writeValue(key)
-            json.writeValue(value, null)
-            json.writeArrayEnd()
-        }
-        json.writeArrayEnd()
-        json.writeObjectEnd()
-    }
-
-    override fun read(json: Json, jsonData: JsonValue, type: Class<*>?): MT {
-        val result = mutableMapFactory()
-        var entry = jsonData.get("entries").child
-        while (entry != null) {
-            val key = json.readValue(keyClass, entry.child)
-            val value = json.readValue<Any>(null, entry.child.next)
-
-            result[key!!] = value!!
-
-            entry = entry.next
-        }
-        return result
-    }
-}

core/src/com/unciv/json/UncivJson.kt

@@ -5,11 +5,7 @@ import com.badlogic.gdx.files.FileHandle
 import com.badlogic.gdx.utils.Json
 import com.badlogic.gdx.utils.JsonWriter
 import com.badlogic.gdx.utils.SerializationException
-import com.unciv.logic.civilization.CivRankingHistory
-import com.unciv.logic.civilization.Notification
-import com.unciv.logic.map.tile.TileHistory
 import com.unciv.ui.components.input.KeyCharAndCode
-import com.unciv.ui.components.input.KeyboardBindings
 import java.time.Duration
 
 
@@ -24,7 +20,6 @@ fun json() = Json(JsonWriter.OutputType.json).apply {
     setIgnoreDeprecated(true)
     ignoreUnknownFields = true
 
-    setSerializer(HashMapVector2.getSerializerClass(), HashMapVector2.createSerializer())
     setSerializer(Duration::class.java, DurationSerializer())
     setSerializer(KeyCharAndCode::class.java, KeyCharAndCode.Serializer())
 }

core/src/com/unciv/logic/GameInfo.kt

@@ -92,7 +92,7 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
     companion object {
         /** The current compatibility version of [GameInfo]. This number is incremented whenever changes are made to the save file structure that guarantee that
          * previous versions of the game will not be able to load or play a game normally. */
-        const val CURRENT_COMPATIBILITY_NUMBER = 3
+        const val CURRENT_COMPATIBILITY_NUMBER = 4
 
         val CURRENT_COMPATIBILITY_VERSION = CompatibilityVersion(CURRENT_COMPATIBILITY_NUMBER, UncivGame.VERSION)
 

core/src/com/unciv/logic/civilization/Civilization.kt

@@ -3,7 +3,7 @@ package com.unciv.logic.civilization
 import com.badlogic.gdx.math.Vector2
 import com.unciv.Constants
 import com.unciv.UncivGame
-import com.unciv.json.HashMapVector2
+import com.unciv.json.LastSeenImprovement
 import com.unciv.logic.GameInfo
 import com.unciv.logic.IsPartOfGameInfoSerialization
 import com.unciv.logic.MultiFilter
@@ -209,7 +209,7 @@ class Civilization : IsPartOfGameInfoSerialization {
 
     fun hasExplored(tile: Tile) = tile.isExplored(this)
 
-    private val lastSeenImprovement = HashMapVector2<String>()
+    private val lastSeenImprovement = LastSeenImprovement()
 
     // To correctly determine "game over" condition as clarified in #4707
     var hasEverOwnedOriginalCapital: Boolean = false

core/src/com/unciv/models/Counter.kt

@@ -1,14 +1,24 @@
 package com.unciv.models
 
+import com.badlogic.gdx.utils.Json
+import com.badlogic.gdx.utils.JsonValue
 import com.unciv.logic.IsPartOfGameInfoSerialization
 
+/**
+ *  Implements a specialized Map storing on-zero Integers.
+ *  - All mutating methods will remove keys when their value is zeroed
+ *  - [get] on a nonexistent key returns 0
+ *  - The Json.Serializable implementation ensures compact format, it does not solve the non-string-key map problem.
+ *  - Therefore, Deserialization works properly ***only*** with [K] === String.
+ *    (ignoring this will return a deserialized map, but the keys will violate the compile-time type and BE strings)
+ */
 open class Counter<K>(
     fromMap: Map<K, Int>? = null
-) : LinkedHashMap<K, Int>(fromMap?.size ?: 10), IsPartOfGameInfoSerialization {
+) : LinkedHashMap<K, Int>(fromMap?.size ?: 10), IsPartOfGameInfoSerialization, Json.Serializable {
     init {
         if (fromMap != null)
             for ((key, value) in fromMap)
-                put(key, value)
+                super.put(key, value)
     }
 
     override operator fun get(key: K): Int { // don't return null if empty
@@ -47,11 +57,7 @@ open class Counter<K>(
 
     fun sumValues() = values.sum()
 
-    override fun clone(): Counter<K> {
+    override fun clone() = Counter(this)
-        val newCounter = Counter<K>()
-        newCounter.add(this)
-        return newCounter
-    }
 
     companion object {
         val ZERO: Counter<String> = object : Counter<String>() {
@@ -59,5 +65,23 @@ open class Counter<K>(
                 throw UnsupportedOperationException("Do not modify Counter.ZERO")
             }
         }
+
+    }
+
+    override fun write(json: Json) {
+        for ((key, value) in entries) {
+            val name = if (key is String) key else key.toString()
+            json.writeValue(name, value, Int::class.java)
+        }
+    }
+
+    override fun read(json: Json, jsonData: JsonValue) {
+        for (entry in jsonData) {
+            @Suppress("UNCHECKED_CAST")
+            // Default Gdx does the same. If K is NOT String, then Gdx would still store String keys. And we can't reify K to check..
+            val key = entry.name as K
+            val value = if (entry.isValue) entry.asInt() else entry.getInt("value")
+            put(key, value)
+        }
     }
 }

tests/src/com/unciv/logic/SerializationTests.kt

@@ -1,14 +1,14 @@
 package com.unciv.logic
 
-import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.math.Vector2
-import com.unciv.json.HashMapVector2
 import com.unciv.logic.civilization.CivRankingHistory
 import com.unciv.logic.civilization.CivilopediaAction
 import com.unciv.logic.civilization.DiplomacyAction
+import com.unciv.json.LastSeenImprovement
 import com.unciv.logic.civilization.LocationAction
 import com.unciv.logic.civilization.Notification
 import com.unciv.logic.map.tile.TileHistory
+import com.unciv.models.Counter
 import com.unciv.testing.GdxTestRunner
 import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.KeyboardBinding
@@ -38,11 +38,12 @@ class SerializationTests {
     }
 
     @Test
-    fun `test HashMapVector2 serialization roundtrip`() {
+    //@RedirectOutput(RedirectPolicy.Show)
-        val data = HashMapVector2<Color>()
+    fun `test LastSeenImprovement serialization roundtrip`() {
-        data[Vector2.Zero] = Color.GRAY
+        val data = LastSeenImprovement()
-        data[Vector2.X] = Color.CORAL
+        data[Vector2.Zero] = "Borehole"
-        data[Vector2.Y] = Color.CHARTREUSE
+        data[Vector2.X] = "Smokestack"
+        data[Vector2.Y] = "Waffle stand"
         testRoundtrip(data)
     }
 
@@ -111,6 +112,13 @@ class SerializationTests {
         }
     }
 
+    /** Note that no other Counter<X> will pass this test */
+    @Test
+    fun `test Counter(String) serialization roundtrip`() {
+        val data = Counter(mapOf("Foo" to 1, "Bar" to 3, "Towel" to 42))
+        testRoundtrip(data)
+    }
+
     ///////////////////////////////// Helper
     private inline fun <reified T> testRoundtrip(
         data: T,