From b12c5df28ea41baa3a79498039a7da1997cb90c7 Mon Sep 17 00:00:00 2001 From: GGGuenni Date: Thu, 18 Mar 2021 12:54:52 +0100 Subject: [PATCH] last refactoring of terrain feature (#3702) * last terrainFeature refactorings * cut down terrainFeature as much as possible * aerialDistanceTo fixed wrapped distance should only be considered on wrapped worlds * Adding spaces * fixed tests not compiling * prevent floodplains from spawning on oasis * Check if terrain feature can be placed on tile in editor * Adding reviewed changes --- .../logic/automation/WorkerAutomation.kt | 20 +++---- core/src/com/unciv/logic/map/MapUnit.kt | 17 +++--- core/src/com/unciv/logic/map/TileInfo.kt | 52 +++++++------------ .../logic/map/mapgenerator/RiverGenerator.kt | 2 +- core/src/com/unciv/ui/CivilopediaScreen.kt | 2 +- .../ui/mapeditor/TileEditorOptionsTable.kt | 11 ++-- tests/src/com/unciv/logic/map/TileMapTests.kt | 18 ++++--- .../logic/map/UnitMovementAlgorithmsTests.kt | 3 +- 8 files changed, 60 insertions(+), 65 deletions(-) diff --git a/core/src/com/unciv/logic/automation/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/WorkerAutomation.kt index 10f9d1e5bd..a42754d4e2 100644 --- a/core/src/com/unciv/logic/automation/WorkerAutomation.kt +++ b/core/src/com/unciv/logic/automation/WorkerAutomation.kt @@ -182,10 +182,10 @@ class WorkerAutomation(val unit: MapUnit) { private fun chooseImprovement(tile: TileInfo, civInfo: CivilizationInfo): TileImprovement? { val improvementStringForResource: String? = when { tile.resource == null || !tile.hasViewableResource(civInfo) -> null - tile.terrainFeature == Constants.marsh && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Marsh" - tile.terrainFeature == "Fallout" && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Fallout" // for really mad modders - tile.terrainFeature == Constants.jungle && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Jungle" - tile.terrainFeature == Constants.forest && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Forest" + tile.terrainFeatures.contains(Constants.marsh) && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Marsh" + tile.terrainFeatures.contains("Fallout") && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Fallout" // for really mad modders + tile.terrainFeatures.contains(Constants.jungle) && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Jungle" + tile.terrainFeatures.contains(Constants.forest) && !isImprovementOnFeatureAllowed(tile, civInfo) -> "Remove Forest" else -> tile.getTileResource().improvement } @@ -207,11 +207,11 @@ class WorkerAutomation(val unit: MapUnit) { // I think we can assume that the unique improvement is better uniqueImprovement != null && tile.canBuildImprovement(uniqueImprovement, civInfo) -> uniqueImprovement.name - tile.terrainFeature == "Fallout" -> "Remove Fallout" - tile.terrainFeature == Constants.marsh -> "Remove Marsh" - tile.terrainFeature == Constants.jungle -> Constants.tradingPost - tile.terrainFeature == "Oasis" -> null - tile.terrainFeature == Constants.forest -> "Lumber mill" + tile.terrainFeatures.contains("Fallout") -> "Remove Fallout" + tile.terrainFeatures.contains(Constants.marsh) -> "Remove Marsh" + tile.terrainFeatures.contains(Constants.jungle) -> Constants.tradingPost + tile.terrainFeatures.contains("Oasis") -> null + tile.terrainFeatures.contains(Constants.forest) -> "Lumber mill" tile.isHill() -> "Mine" tile.baseTerrain in listOf(Constants.grassland, Constants.desert, Constants.plains) -> "Farm" tile.isAdjacentToFreshwater -> "Farm" @@ -228,7 +228,7 @@ class WorkerAutomation(val unit: MapUnit) { ?: return false val resourceImprovement = civInfo.gameInfo.ruleSet.tileImprovements[resourceImprovementName] ?: return false - return resourceImprovement.resourceTerrainAllow.contains(tile.terrainFeature!!) + return tile.terrainFeatures.any { resourceImprovement.resourceTerrainAllow.contains(it) } } private fun isAcceptableTileForFort(tile: TileInfo, civInfo: CivilizationInfo): Boolean { diff --git a/core/src/com/unciv/logic/map/MapUnit.kt b/core/src/com/unciv/logic/map/MapUnit.kt index 1cea0caf27..09c8255848 100644 --- a/core/src/com/unciv/logic/map/MapUnit.kt +++ b/core/src/com/unciv/logic/map/MapUnit.kt @@ -399,7 +399,7 @@ class MapUnit { tile.improvementInProgress!!.startsWith("Remove") -> { val tileImprovement = tile.getTileImprovement() if (tileImprovement != null - && tileImprovement.terrainsCanBeBuiltOn.contains(tile.terrainFeature) + && tile.terrainFeatures.any { tileImprovement.terrainsCanBeBuiltOn.contains(it) } && !tileImprovement.terrainsCanBeBuiltOn.contains(tile.baseTerrain)) { tile.improvement = null // We removed a terrain (e.g. Forest) and the improvement (e.g. Lumber mill) requires it! if (tile.resource != null) civInfo.updateDetailedCivResources() // unlikely, but maybe a mod makes a resource improvement dependent on a terrain feature @@ -407,11 +407,12 @@ class MapUnit { if (tile.improvementInProgress == "Remove Road" || tile.improvementInProgress == "Remove Railroad") tile.roadStatus = RoadStatus.None else { - // We put "tile.terrainFeature!=null" because of a strange edge case that SHOULD be solved from 3.11.11+, so we should remove it then and see - if (tile.terrainFeature != null && tile.tileMap.gameInfo.ruleSet.terrains[tile.terrainFeature!!]!!.uniques - .contains("Provides a one-time Production bonus to the closest city when cut down")) - tryProvideProductionToClosestCity() - tile.terrainFeature = null + val removedFeature = tile.improvementInProgress!!.removePrefix("Remove ") + if (tile.ruleset.terrains[removedFeature]!!.uniques + .contains("Provides a one-time Production bonus to the closest city when cut down")) { + tryProvideProductionToClosestCity(removedFeature) + } + tile.terrainFeatures.remove(removedFeature) } } tile.improvementInProgress == "Road" -> tile.roadStatus = RoadStatus.Road @@ -424,7 +425,7 @@ class MapUnit { tile.improvementInProgress = null } - private fun tryProvideProductionToClosestCity() { + private fun tryProvideProductionToClosestCity(removedTerrainFeature: String) { val tile = getTile() val closestCity = civInfo.cities.minByOrNull { it.getCenterTile().aerialDistanceTo(tile) } if (closestCity == null) return @@ -433,7 +434,7 @@ class MapUnit { if (tile.owningCity == null || tile.owningCity!!.civInfo != civInfo) productionPointsToAdd = productionPointsToAdd * 2 / 3 if (productionPointsToAdd > 0) { closestCity.cityConstructions.addProductionPoints(productionPointsToAdd) - civInfo.addNotification("Clearing a [${tile.terrainFeature}] has created [$productionPointsToAdd] Production for [${closestCity.name}]", closestCity.location, Color.BROWN) + civInfo.addNotification("Clearing a [$removedTerrainFeature] has created [$productionPointsToAdd] Production for [${closestCity.name}]", closestCity.location, Color.BROWN) } } diff --git a/core/src/com/unciv/logic/map/TileInfo.kt b/core/src/com/unciv/logic/map/TileInfo.kt index 4848783454..1f75dc6772 100644 --- a/core/src/com/unciv/logic/map/TileInfo.kt +++ b/core/src/com/unciv/logic/map/TileInfo.kt @@ -53,36 +53,17 @@ open class TileInfo { lateinit var baseTerrain: String val terrainFeatures: ArrayList = ArrayList() - //@Transient // So it won't be serialized from now on - @Deprecated(message = "Since 3.13.7 - gets replaced by terrainFeatures") - var terrainFeature: String? = null - get() { - //if terrainFeatures contains no terrainFeature maybe one got deserialized to field - if (terrainFeatures.firstOrNull() == null && field != null) { - terrainFeatures.add(field!!) - field = null - } - return terrainFeatures.firstOrNull() - } - set(value) { - if (terrainFeatures.isNotEmpty()) { - if (value == null) terrainFeatures.removeAt(0) - else terrainFeatures[0] = value - } else { - if (value != null) terrainFeatures.add(value) - } - field = null - } - + // Deprecation level can't be increased because of convertTerrainFeatureToArray + // Can't be flagged transient because it won't deserialize then + // but it should not serialize because it always has the default value on serialization and is flagged optional + // Can be removed together with convertTerrainFeatureToArray to drop support for save files from version 3.13.7 and below + @Deprecated(message = "Since 3.13.7 - replaced by terrainFeatures") + private var terrainFeature: String? = null private fun convertTerrainFeatureToArray() { - if (terrainFeatures.firstOrNull() == null && terrainFeature != null)// -> terranFeature getter returns terrainFeature field - terrainFeature = terrainFeature // getter returns field, setter calls terrainFeatures.add() - //Note to Future GGGuenni - //TODO Use the following when terrainFeature got changed everywhere to support old saves in the future - //if (terrainFeature != null){ - //terrainFeatures.add(terrainFeature) - //terrainFeature = null - //} + if (terrainFeature != null) { + terrainFeatures.add(terrainFeature!!) + terrainFeature = null + } } var naturalWonder: String? = null @@ -437,12 +418,15 @@ open class TileInfo { fun aerialDistanceTo(otherTile: TileInfo): Int { val xDelta = position.x - otherTile.position.x val yDelta = position.y - otherTile.position.y - val distance = listOf(abs(xDelta), abs(yDelta), abs(xDelta - yDelta)).maxOrNull()!! + val distance = maxOf(abs(xDelta), abs(yDelta), abs(xDelta - yDelta)) - val otherTileUnwrappedPos = tileMap.getUnWrappedPosition(otherTile.position) - val xDeltaWrapped = position.x - otherTileUnwrappedPos.x - val yDeltaWrapped = position.y - otherTileUnwrappedPos.y - val wrappedDistance = listOf(abs(xDeltaWrapped), abs(yDeltaWrapped), abs(xDeltaWrapped - yDeltaWrapped)).maxOrNull()!! + var wrappedDistance = Float.MAX_VALUE + if (tileMap.mapParameters.worldWrap) { + val otherTileUnwrappedPos = tileMap.getUnWrappedPosition(otherTile.position) + val xDeltaWrapped = position.x - otherTileUnwrappedPos.x + val yDeltaWrapped = position.y - otherTileUnwrappedPos.y + wrappedDistance = maxOf(abs(xDeltaWrapped), abs(yDeltaWrapped), abs(xDeltaWrapped - yDeltaWrapped)) + } return min(distance, wrappedDistance).toInt() } diff --git a/core/src/com/unciv/logic/map/mapgenerator/RiverGenerator.kt b/core/src/com/unciv/logic/map/mapgenerator/RiverGenerator.kt index 9e03c925b5..f01aaf5923 100644 --- a/core/src/com/unciv/logic/map/mapgenerator/RiverGenerator.kt +++ b/core/src/com/unciv/logic/map/mapgenerator/RiverGenerator.kt @@ -22,7 +22,7 @@ class RiverGenerator(val randomness: MapGenerationRandomness){ for(tile in map.values){ if(tile.isAdjacentToRiver()){ - if(tile.baseTerrain== Constants.desert && !tile.isHill()) tile.terrainFeatures.add(Constants.floodPlains) + if(tile.baseTerrain == Constants.desert && tile.terrainFeatures.isEmpty()) tile.terrainFeatures.add(Constants.floodPlains) else if(tile.baseTerrain== Constants.snow) tile.baseTerrain = Constants.tundra else if(tile.baseTerrain== Constants.tundra) tile.baseTerrain = Constants.plains tile.setTerrainTransients() diff --git a/core/src/com/unciv/ui/CivilopediaScreen.kt b/core/src/com/unciv/ui/CivilopediaScreen.kt index 9ac69c7fa2..b476ce2743 100644 --- a/core/src/com/unciv/ui/CivilopediaScreen.kt +++ b/core/src/com/unciv/ui/CivilopediaScreen.kt @@ -166,7 +166,7 @@ class CivilopediaScreen(ruleset: Ruleset) : CameraStageBaseScreen() { tileInfo.baseTerrain = terrain.turnsInto ?: Constants.grassland } TerrainType.TerrainFeature -> { - tileInfo.terrainFeature = terrain.name + tileInfo.terrainFeatures.add(terrain.name) tileInfo.baseTerrain = terrain.occursOn.lastOrNull() ?: Constants.grassland } else -> diff --git a/core/src/com/unciv/ui/mapeditor/TileEditorOptionsTable.kt b/core/src/com/unciv/ui/mapeditor/TileEditorOptionsTable.kt index 33adf1d0ee..be59a1a05a 100644 --- a/core/src/com/unciv/ui/mapeditor/TileEditorOptionsTable.kt +++ b/core/src/com/unciv/ui/mapeditor/TileEditorOptionsTable.kt @@ -85,7 +85,7 @@ class TileEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(Camera terrainFeaturesTable.add(getHex(getRedCross(50f, 0.6f)).apply { onClick { tileAction = { - it.terrainFeature = null + it.terrainFeatures.clear() it.naturalWonder = null it.hasBottomRiver = false it.hasBottomLeftRiver = false @@ -325,7 +325,7 @@ class TileEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(Camera tileInfo.baseTerrain = if (terrainObject.occursOn.isNotEmpty()) terrainObject.occursOn.first() else "Grassland" - tileInfo.terrainFeature = terrain + tileInfo.terrainFeatures.add(terrain) } else tileInfo.baseTerrain = terrain tileInfo.resource = resource.name @@ -346,7 +346,7 @@ class TileEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(Camera terrain.occursOn.isNotEmpty() -> terrain.occursOn.first() else -> "Grassland" } - tileInfo.terrainFeature = terrain.name + tileInfo.terrainFeatures.add(terrain.name) } else tileInfo.baseTerrain = terrain.name val group = makeTileGroup(tileInfo) @@ -354,7 +354,10 @@ class TileEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(Camera tileAction = { it.naturalWonder = null // If we're setting a base terrain it should remove the nat wonder when (terrain.type) { - TerrainType.TerrainFeature -> it.terrainFeature = terrain.name + TerrainType.TerrainFeature -> { + if (terrain.occursOn.contains(it.getLastTerrain().name)) + it.terrainFeatures.add(terrain.name) + } TerrainType.NaturalWonder -> it.naturalWonder = terrain.name else -> it.baseTerrain = terrain.name } diff --git a/tests/src/com/unciv/logic/map/TileMapTests.kt b/tests/src/com/unciv/logic/map/TileMapTests.kt index 50cd9ac753..ef736df94d 100644 --- a/tests/src/com/unciv/logic/map/TileMapTests.kt +++ b/tests/src/com/unciv/logic/map/TileMapTests.kt @@ -87,7 +87,8 @@ class TileMapTests { tile1.baseTerrain = Constants.hill tile1.setTerrainTransients() tile2.baseTerrain = Constants.grassland - tile2.terrainFeature = Constants.forest + tile2.terrainFeatures.clear() + tile2.terrainFeatures.add(Constants.forest) tile2.setTerrainTransients() tile3.baseTerrain = Constants.coast tile3.setTerrainTransients() @@ -114,7 +115,8 @@ class TileMapTests { @Test fun canSeeMountainFromForestOverHills() { tile1.baseTerrain = Constants.grassland - tile1.terrainFeature = Constants.forest + tile1.terrainFeatures.clear() + tile1.terrainFeatures.add(Constants.forest) tile1.setTerrainTransients() tile2.baseTerrain = Constants.hill tile2.setTerrainTransients() @@ -131,7 +133,8 @@ class TileMapTests { tile1.baseTerrain = Constants.hill tile1.setTerrainTransients() tile2.baseTerrain = Constants.grassland - tile2.terrainFeature = Constants.forest + tile2.terrainFeatures.clear() + tile2.terrainFeatures.add(Constants.forest) tile2.setTerrainTransients() tile3.baseTerrain = Constants.hill tile3.setTerrainTransients() @@ -172,10 +175,12 @@ class TileMapTests { @Test fun canNOTSeeOutThroughForest() { tile1.baseTerrain = Constants.grassland - tile1.terrainFeature = Constants.forest + tile1.terrainFeatures.clear() + tile1.terrainFeatures.add(Constants.forest) tile1.setTerrainTransients() tile2.baseTerrain = Constants.grassland - tile2.terrainFeature = Constants.forest + tile2.terrainFeatures.clear() + tile2.terrainFeatures.add(Constants.forest) tile2.setTerrainTransients() tile3.baseTerrain = Constants.grassland tile3.setTerrainTransients() @@ -190,7 +195,8 @@ class TileMapTests { tile1.baseTerrain = Constants.coast tile1.setTerrainTransients() tile2.baseTerrain = Constants.grassland - tile2.terrainFeature = Constants.jungle + tile2.terrainFeatures.clear() + tile2.terrainFeatures.add(Constants.jungle) tile2.setTerrainTransients() tile3.baseTerrain = Constants.coast tile3.setTerrainTransients() diff --git a/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt b/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt index fd70f009f8..2135064af9 100644 --- a/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt +++ b/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt @@ -97,7 +97,8 @@ class UnitMovementAlgorithmsTests { @Test fun canNOTEnterIce() { tile.baseTerrain = Constants.ocean - tile.terrainFeature = Constants.ice + tile.terrainFeatures.clear() + tile.terrainFeatures.add(Constants.ice) tile.setTransients() for (type in UnitType.values()) {