From 4daa22ddea1857dc4528ddf2b05e403315c0e7e4 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sun, 15 Aug 2021 18:44:31 +0200 Subject: [PATCH] BFS cleanup (#4859) * BFS cleanup - Kotlin ArrayDeque is no longer experimental - Client code sidestepped class API even where API existed -> closed it down - nextStep when already ended now no-op instead of crash, suits usecases - predicate test very likely to be more expensive than tilesReached lookup - getPathTo only used asSequence() - why not a Sequence in the first place - Compleat documentation. This is very low level, so better help future coders to understand. - Lint: One annoying hardcoded conditional breakpoint, one unhelpful exception * BFS cleanup - patch1 --- core/src/com/unciv/logic/GameStarter.kt | 4 +- .../com/unciv/logic/automation/Automation.kt | 4 +- .../logic/automation/WorkerAutomation.kt | 4 +- core/src/com/unciv/logic/map/BFS.kt | 61 +++++++++++++------ core/src/com/unciv/logic/map/TileMap.kt | 4 -- .../unciv/logic/map/UnitMovementAlgorithms.kt | 6 +- 6 files changed, 50 insertions(+), 33 deletions(-) diff --git a/core/src/com/unciv/logic/GameStarter.kt b/core/src/com/unciv/logic/GameStarter.kt index 8854ffa3d3..414b46268a 100644 --- a/core/src/com/unciv/logic/GameStarter.kt +++ b/core/src/com/unciv/logic/GameStarter.kt @@ -347,7 +347,7 @@ object GameStarter { while (landTiles.any()) { val bfs = BFS(landTiles.random()) { it.isLand && !it.isImpassible() } bfs.stepToEnd() - val tilesInGroup = bfs.tilesReached.keys + val tilesInGroup = bfs.getReachedTiles() landTiles = landTiles.filter { it !in tilesInGroup } if (tilesInGroup.size > 20) // is this a good number? I dunno, but it's easy enough to change later on landTilesInBigEnoughGroup.addAll(tilesInGroup) @@ -463,4 +463,4 @@ object GameStarter { val distanceFromCenter = HexMath.getDistance(vector, Vector2.Zero) return hexagonalRadius - distanceFromCenter >= n } -} \ No newline at end of file +} diff --git a/core/src/com/unciv/logic/automation/Automation.kt b/core/src/com/unciv/logic/automation/Automation.kt index 0a2da97b14..511d187485 100644 --- a/core/src/com/unciv/logic/automation/Automation.kt +++ b/core/src/com/unciv/logic/automation/Automation.kt @@ -71,7 +71,7 @@ object Automation { val findWaterConnectedCitiesAndEnemies = BFS(city.getCenterTile()) { it.isWater || it.isCityCenter() } findWaterConnectedCitiesAndEnemies.stepToEnd() - if (findWaterConnectedCitiesAndEnemies.tilesReached.keys.none { + if (findWaterConnectedCitiesAndEnemies.getReachedTiles().none { (it.isCityCenter() && it.getOwner() != city.civInfo) || (it.militaryUnit != null && it.militaryUnit!!.civInfo != city.civInfo) }) // there is absolutely no reason for you to make water units on this body of water. @@ -218,4 +218,4 @@ enum class ThreatLevel{ Medium, High, VeryHigh -} \ No newline at end of file +} diff --git a/core/src/com/unciv/logic/automation/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/WorkerAutomation.kt index a93449d7b3..7b5f348e75 100644 --- a/core/src/com/unciv/logic/automation/WorkerAutomation.kt +++ b/core/src/com/unciv/logic/automation/WorkerAutomation.kt @@ -85,10 +85,10 @@ class WorkerAutomation(val unit: MapUnit) { // we order cities by their closeness to the worker first, and then check for each one whether there's a viable path // it can take to an existing connected city. for (bfs in citiesThatNeedConnectingBfs) { - while (bfs.tilesToCheck.isNotEmpty()) { + while (!bfs.hasEnded()) { bfs.nextStep() for (city in connectedCities) - if (bfs.tilesToCheck.contains(city)) { // we have a winner! + if (bfs.hasReachedTile(city)) { // we have a winner! val pathToCity = bfs.getPathTo(city).asSequence() val roadableTiles = pathToCity.filter { it.roadStatus < targetRoad } val tileToConstructRoadOn: TileInfo diff --git a/core/src/com/unciv/logic/map/BFS.kt b/core/src/com/unciv/logic/map/BFS.kt index d653f06066..161233fb87 100644 --- a/core/src/com/unciv/logic/map/BFS.kt +++ b/core/src/com/unciv/logic/map/BFS.kt @@ -1,59 +1,80 @@ package com.unciv.logic.map -// Kotlin's ArrayDeque is experimental -import java.util.ArrayDeque +import kotlin.collections.ArrayDeque /** - * Defines intermediate steps of a breadth-first search, for use in either get shortest path or get onnected tiles. + * Defines intermediate steps of a breadth-first search, for use in either get shortest path or get connected tiles. */ -class BFS(val startingPoint: TileInfo, val predicate : (TileInfo) -> Boolean) { - var tilesToCheck = ArrayDeque() +class BFS( + val startingPoint: TileInfo, + private val predicate : (TileInfo) -> Boolean +) { + /** remaining tiles to check */ + private val tilesToCheck = ArrayDeque(37) // needs resize at distance 4 /** each tile reached points to its parent tile, where we got to it from */ - val tilesReached = HashMap() + private val tilesReached = HashMap() init { tilesToCheck.add(startingPoint) tilesReached[startingPoint] = startingPoint } + /** Process fully until there's nowhere left to check */ fun stepToEnd() { while (!hasEnded()) nextStep() } - + /** + * Process until either [destination] is reached or there's nowhere left to check + * @return `this` instance for chaining + */ fun stepUntilDestination(destination: TileInfo): BFS { while (!tilesReached.containsKey(destination) && !hasEnded()) nextStep() return this } + /** + * Process one tile-to-search, fetching all neighbors not yet touched + * and adding those that fulfill the [predicate] to the reached set + * and to the yet-to-be-processed set. + * + * Will do nothing when [hasEnded] returns `true` + */ fun nextStep() { - val current = tilesToCheck.remove() + val current = tilesToCheck.removeFirstOrNull() ?: return for (neighbor in current.neighbors) { - if (predicate(neighbor) && !tilesReached.containsKey(neighbor)) { + if (neighbor !in tilesReached && predicate(neighbor)) { tilesReached[neighbor] = current tilesToCheck.add(neighbor) } } } - fun getPathTo(destination: TileInfo): ArrayList { - val path = ArrayList() - path.add(destination) + /** + * @return a Sequence from the [destination] back to the [startingPoint], including both, or empty if [destination] has not been reached + */ + fun getPathTo(destination: TileInfo): Sequence = sequence { var currentNode = destination - while (currentNode != startingPoint) { - val parent = tilesReached[currentNode] - if (parent == null) return ArrayList()// destination is not in our path + while (true) { + val parent = tilesReached[currentNode] ?: break // destination is not in our path + yield(currentNode) + if (currentNode == startingPoint) break currentNode = parent - path.add(currentNode) } - return path } + /** @return true if there are no more tiles to check */ fun hasEnded() = tilesToCheck.isEmpty() - fun hasReachedTile(tile: TileInfo) = - tilesReached.containsKey(tile) -} \ No newline at end of file + /** @return true if the [tile] has been reached */ + fun hasReachedTile(tile: TileInfo) = tilesReached.containsKey(tile) + + /** @return all tiles reached so far */ + fun getReachedTiles(): MutableSet = tilesReached.keys + + /** @return number of tiles reached so far */ + fun size() = tilesReached.size +} diff --git a/core/src/com/unciv/logic/map/TileMap.kt b/core/src/com/unciv/logic/map/TileMap.kt index b134b15500..b4a9eff6dd 100644 --- a/core/src/com/unciv/logic/map/TileMap.kt +++ b/core/src/com/unciv/logic/map/TileMap.kt @@ -254,10 +254,6 @@ class TileMap { val containsViewableNeighborThatCanSeeOver = cTile.neighbors.any { bNeighbor: TileInfo -> val bNeighborHeight = bNeighbor.getHeight() - if(cTile.resource=="Marble" - && bNeighbor.terrainFeatures.contains("Forest") - ) - println() viewableTiles.contains(bNeighbor) && ( currentTileHeight > bNeighborHeight // a>b || cTileHeight > bNeighborHeight // c>b diff --git a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt index 3d9c48ad83..dde76c7a00 100644 --- a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt +++ b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt @@ -183,12 +183,12 @@ class UnitMovementAlgorithms(val unit:MapUnit) { val distanceToTiles = getDistanceToTiles() - class UnreachableDestinationException : Exception() + class UnreachableDestinationException(msg: String) : Exception(msg) // If the tile is far away, we need to build a path how to get there, and then take the first step if (!distanceToTiles.containsKey(finalDestination)) return getShortestPath(finalDestination).firstOrNull() - ?: throw UnreachableDestinationException() + ?: throw UnreachableDestinationException("$unit ${unit.currentTile.position} cannot reach $finalDestination") // we should be able to get there this turn if (canMoveTo(finalDestination)) @@ -565,4 +565,4 @@ class PathsToTilesWithinTurn : LinkedHashMap