Resolved #3239 - simplified unit actions, which were unnecessarily complicated

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

@@ -7,8 +7,6 @@ import com.unciv.UncivGame
 import com.unciv.logic.automation.UnitAutomation
 import com.unciv.logic.automation.WorkerAutomation
 import com.unciv.logic.civilization.CivilizationInfo
-import com.unciv.logic.map.action.MapUnitAction
-import com.unciv.logic.map.action.StringAction
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.Unique
 import com.unciv.models.ruleset.unit.BaseUnit
@@ -46,23 +44,7 @@ class MapUnit {
     var currentMovement: Float = 0f
     var health:Int = 100
 
-    // todo: I see this is being serialized, should it be Transient?
+    var action: String?=null // work, automation, fortifying, I dunno what.
-    var mapUnitAction : MapUnitAction? = null
-
-    var action: String? // work, automation, fortifying, I dunno what.
-        // getter and setter for compatibility: make sure string-based actions still work
-        get() {
-            val mapUnitActionVal = mapUnitAction
-            if (mapUnitActionVal is StringAction)
-                return mapUnitActionVal.action
-            // any other unit action does count as a unit action, thus is not null. The actual logic is not based on an action string, but realized by extending MapUnitAction
-            if (mapUnitActionVal != null)
-                return ""
-
-            return null // unit has no action
-        }
-        set(value) { mapUnitAction = if (value == null) null else StringAction(this, value) } // wrap traditional string-encoded actions into StringAction
-
 
     var attacksThisTurn = 0
     var promotions = UnitPromotions()
@@ -302,7 +284,6 @@ class MapUnit {
     //region state-changing functions
     fun setTransients(ruleset: Ruleset) {
         promotions.unit = this
-        mapUnitAction?.unit = this
         baseUnit = ruleset.units[name]
                 ?: throw java.lang.Exception("Unit $name is not found!")
         updateUniques()
@@ -320,13 +301,11 @@ class MapUnit {
         val enemyUnitsInWalkingDistance = movement.getDistanceToTiles().keys
                 .filter { it.militaryUnit != null && civInfo.isAtWarWith(it.militaryUnit!!.civInfo) }
         if (enemyUnitsInWalkingDistance.isNotEmpty()) {
-            if (mapUnitAction?.shouldStopOnEnemyInSight() == true)
+            if (action?.startsWith("moveTo") == true) // stop on enemy in sight
-                mapUnitAction = null
+                action = null
             return  // Don't you dare move.
         }
 
-        mapUnitAction?.doPreTurnAction()
-
         val currentTile = getTile()
         if (isMoving()) {
             val destination = action!!.replace("moveTo ", "").split(",").dropLastWhile { it.isEmpty() }.toTypedArray()

core/src/com/unciv/logic/map/action/BuildLongRoadAction.kt

@@ -1,103 +0,0 @@
-package com.unciv.logic.map.action
-
-import com.badlogic.gdx.graphics.Color
-import com.unciv.logic.map.BFS
-import com.unciv.logic.map.MapUnit
-import com.unciv.logic.map.RoadStatus
-import com.unciv.logic.map.TileInfo
-
-class BuildLongRoadAction(
-        mapUnit: MapUnit = MapUnit(),
-        val target: TileInfo = TileInfo()
-) : MapUnitAction(mapUnit) {
-
-    override fun name(): String = "Build Long Road"
-
-    override fun shouldStopOnEnemyInSight(): Boolean = true
-
-    override fun isAvailable(): Boolean
-            = unit.hasUnique("Can build improvements on tiles")
-            && getPath(target).isNotEmpty()
-            && unit.civInfo.tech.getBestRoadAvailable() != RoadStatus.None
-
-    override fun doPreTurnAction() {
-
-        // we're working!
-        if (unit.currentTile.improvementInProgress != null)
-            return
-
-        if (startWorkingOnRoad())
-            return
-
-
-        // we reached our target? And road is finished?
-        if (unit.currentTile.position == target.position
-                && isRoadFinished(unit.currentTile)) {
-            unit.action = null
-            return
-        }
-
-        // move one step forward - and start building
-        if (stepForward(target)) {
-            startWorkingOnRoad()
-        } else if (unit.currentMovement > 1f) {
-            unit.civInfo.addNotification("[${unit.name}] canceled building road: can't move forward.", unit.currentTile.position, Color.GRAY)
-            unit.action = null
-            return
-        }
-
-    }
-
-    // because the unit is building a road, we need to use a shortest path that is
-    // independent of movement costs, but should respect impassable terrain like water and enemy territory
-    private fun stepForward(destination: TileInfo): Boolean {
-        var success = false
-        val tilesUnitCanCurrentlyReach = unit.movement.getDistanceToTiles().keys
-        for (step in getPath(destination).drop(1)) {
-            if(step !in tilesUnitCanCurrentlyReach) return false // we're out of tiles in reachable distance, no need to check any further
-
-            if (unit.currentMovement > 0f) {
-                if(unit.movement.canMoveTo(step)) {
-                    unit.movement.moveToTile(step)
-                    success = true
-                    // if there is a road already, take multiple steps, otherwise this is where we're going to build a road
-                    if (!isRoadFinished(step)) return true
-                }
-                else if(!isRoadFinished(step)){
-                    unit.civInfo.addNotification("[${unit.name}] skipped building road. It can't move here.", step.position, Color.GRAY)
-                }
-                // worker moves on even if the current step is blocked
-            } else break
-        }
-        return success
-    }
-
-    private fun isRoadFinished(tile: TileInfo): Boolean {
-        return tile.roadStatus >= unit.civInfo.tech.getBestRoadAvailable()
-    }
-
-    private fun getPath(destination: TileInfo): List<TileInfo> {
-        // BFS is not very efficient
-        return BFS(unit.currentTile) { isRoadableTile(it) }
-                .stepUntilDestination(destination)
-                .getPathTo(destination).reversed()
-    }
-
-    private fun isRoadableTile(it: TileInfo) = it.isLand && unit.movement.canPassThrough(it)
-
-    private fun startWorkingOnRoad(): Boolean {
-        val tile = unit.currentTile
-        if (unit.currentMovement > 0 && isRoadableTile(tile)) {
-            val roadToBuild = unit.civInfo.tech.getBestRoadAvailable()
-            roadToBuild.improvement(unit.civInfo.gameInfo.ruleSet)?.let { improvement ->
-                if (tile.roadStatus < roadToBuild && tile.improvementInProgress != improvement.name) {
-                    tile.startWorkingOnImprovement(improvement, unit.civInfo)
-                    return true
-                }
-            }
-        }
-        return false
-    }
-
-
-}

core/src/com/unciv/logic/map/action/MapUnitAction.kt

@@ -1,15 +0,0 @@
-package com.unciv.logic.map.action
-
-import com.unciv.logic.map.MapUnit
-
-open class MapUnitAction(
-        @Transient var unit: MapUnit = MapUnit()
-) {
-    open fun name(): String = ""
-    /** return true if this action is possible in the given conditions */
-    open fun isAvailable(): Boolean = true
-    open fun doPreTurnAction() {}
-    open fun shouldStopOnEnemyInSight(): Boolean = false
-}
-
-

core/src/com/unciv/logic/map/action/StringAction.kt

@@ -1,27 +0,0 @@
-package com.unciv.logic.map.action
-
-import com.unciv.logic.map.MapUnit
-
-/**
- * this class represents all actions that are identified by string only.
- * this is the traditional way of handling actions in UnCiv: by coding relevant information
- * into a string. This class is here to maintain compatibility to that method, preventing from a huge
- * refactoring going on here.
- */
-class StringAction(
-        unit: MapUnit = MapUnit(),
-        val action: String = "" // traditional string-encoded action like "moveTo x,y"
-) : MapUnitAction(unit) {
-
-    override fun shouldStopOnEnemyInSight(): Boolean = action.startsWith("moveTo")
-
-    override fun name(): String {
-        return when {
-            // translate string-encoded actions to user-readable names
-            action.startsWith("moveTo") -> "Moving"
-            else -> action
-        }
-    }
-
-
-}