Commenting UnitManager.unitList stuff (#11463)

core/src/com/unciv/logic/civilization/managers/UnitManager.kt

@@ -5,6 +5,7 @@ import com.unciv.UncivGame
 import com.unciv.logic.city.City
 import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.civilization.NotificationCategory
+import com.unciv.logic.civilization.transients.CivInfoTransientCache
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.Tile
 import com.unciv.models.ruleset.unique.StateForConditionals
@@ -15,24 +16,38 @@ import com.unciv.models.ruleset.unit.BaseUnit
 class UnitManager(val civInfo: Civilization) {
 
     /**
-     * We never add or remove from here directly, could cause comodification problems.
+     *  All units of [civInfo], _ordered_.
-     * Instead, we create a copy list with the change, and replace this list.
+     *  Collection order and [nextPotentiallyDueAt] determine activation order when using "Next unit".
-     * The other solution, casting toList() every "get", has a performance cost
+     *
+     *  When loading a save, this is entirely rebuilt from Tile.*Unit.
+     *  * GameInfo.setTransients -> TileMap.setTransients -> Tile.setUnitTransients -> MapUnit.assignOwner -> [addUnit] (the MapUnit overload)
+     *
+     *  We never add or remove from here directly, could cause comodification problems.
+     *  Instead, we create a copy list with the change, and replace this list.
+     *  The other solution, casting toList() every "get", has a performance cost
      */
-    @Transient
     private var unitList = listOf<MapUnit>()
 
     /**
-     * Index in the unit list above of the unit that is potentially due and is next up for button "Next unit".
+     * Index in [unitList] of the [unit][MapUnit] that is potentially [due][MapUnit.due] and is next up for button "Next unit".
      */
-    @Transient
     private var nextPotentiallyDueAt = 0
 
+    /** Creates a new [MapUnit] and places it on the map.
+     *  @param  unitName The [BaseUnit] name to create a MapUnit instance of - auto-mapped to a nation equivalent if one exists
+     *  @param  city     The City to place the new unit in or near
+     *  @return          The new unit or `null` if unsuccessful (invalid unitName, no tile found where it could be placed, or civ has no cities)
+     */
     fun addUnit(unitName: String, city: City? = null): MapUnit? {
         val unit = civInfo.gameInfo.ruleset.units[unitName] ?: return null
         return addUnit(unit, city)
     }
 
+    /** Creates a new [MapUnit] and places it on the map.
+     *  @param  baseUnit The [BaseUnit] to create a MapUnit instance of - auto-mapped to a nation equivalent if one exists
+     *  @param  city     The City to place the new unit in or near
+     *  @return          The new unit or `null` if unsuccessful (no tile found where it could be placed, or civ has no cities)
+     */
     fun addUnit(baseUnit: BaseUnit, city: City? = null): MapUnit? {
         if (civInfo.cities.isEmpty()) return null
 
@@ -111,6 +126,11 @@ class UnitManager(val civInfo: Civilization) {
     // 'nextPotentiallyDueAt' unit is first here.
     private fun getCivUnitsStartingAtNextDue(): Sequence<MapUnit> = sequenceOf(unitList.subList(nextPotentiallyDueAt, unitList.size) + unitList.subList(0, nextPotentiallyDueAt)).flatten()
 
+    /** Assigns an existing [mapUnit] to this manager.
+     *
+     *  Used during load game via setTransients to regenerate a Civilization's list from the serialized Tile fields.
+     *  @param updateCivInfo When `true`, calls [updateStatsForNextTurn][Civilization.updateStatsForNextTurn] and possibly [updateCivResources][CivInfoTransientCache]
+     */
     fun addUnit(mapUnit: MapUnit, updateCivInfo: Boolean = true) {
         // Since we create a new list anyway (otherwise some concurrent modification
         // exception will happen), also rearrange existing units so that

core/src/com/unciv/logic/map/mapunit/MapUnit.kt

@@ -956,12 +956,19 @@ class MapUnit : IsPartOfGameInfoSerialization {
         civ.ruinsManager.selectNextRuinsReward(this)
     }
 
+    /** Assigns ownership to [civInfo], updating its [unit manager][Civilization.units] and our [cache].
+     *
+     *  Used during game load to rebuild the transient `UnitManager.unitList` from Tile data.
+     *  Cannot be used to reassign from one civ to another - doesn't remove from old owner.
+     */
     fun assignOwner(civInfo: Civilization, updateCivInfo: Boolean = true) {
         owner = civInfo.civName
         this.civ = civInfo
         civInfo.units.addUnit(this, updateCivInfo)
+        // commit named "Fixed game load": GameInfo.setTransients code flow and dependency requirements
+        // may lead to this being called before our own setTransients
         if (::baseUnit.isInitialized)
-        cache.updateUniques()
+            cache.updateUniques()
     }
 
     fun capturedBy(captor: Civilization) {