From 328cf56a22a46fa17cdf5ec302abfb47c1e346cf Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Mon, 6 Feb 2023 00:23:05 +0200 Subject: [PATCH] Resolved #8563 - ultra-rare 'destroy civilian unit before attack' crash --- .../logic/automation/unit/AttackableTile.kt | 4 +++- .../logic/automation/unit/BattleHelper.kt | 19 ++++++++++++------- core/src/com/unciv/logic/battle/Battle.kt | 12 +++++++----- core/src/com/unciv/logic/files/UncivFiles.kt | 4 ++-- .../ui/worldscreen/bottombar/BattleTable.kt | 4 ++-- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/core/src/com/unciv/logic/automation/unit/AttackableTile.kt b/core/src/com/unciv/logic/automation/unit/AttackableTile.kt index 21ac658443..3500bfd903 100644 --- a/core/src/com/unciv/logic/automation/unit/AttackableTile.kt +++ b/core/src/com/unciv/logic/automation/unit/AttackableTile.kt @@ -1,6 +1,8 @@ package com.unciv.logic.automation.unit +import com.unciv.logic.battle.ICombatant import com.unciv.logic.map.tile.Tile class AttackableTile(val tileToAttackFrom: Tile, val tileToAttack: Tile, - val movementLeftAfterMovingToAttackTile:Float) + val movementLeftAfterMovingToAttackTile:Float, + /** This is only for debug purposes */ val combatant:ICombatant) diff --git a/core/src/com/unciv/logic/automation/unit/BattleHelper.kt b/core/src/com/unciv/logic/automation/unit/BattleHelper.kt index 3c9eb76845..632b98a093 100644 --- a/core/src/com/unciv/logic/automation/unit/BattleHelper.kt +++ b/core/src/com/unciv/logic/automation/unit/BattleHelper.kt @@ -73,18 +73,23 @@ object BattleHelper { if (tile in tilesWithEnemies) attackableTiles += AttackableTile( reachableTile, tile, - movementLeft + movementLeft, + Battle.getMapCombatantOfTile(tile)!! ) else if (tile in tilesWithoutEnemies) continue // avoid checking the same empty tile multiple times else if (checkTile(unit, tile, tilesToCheck)) { tilesWithEnemies += tile - attackableTiles += AttackableTile(reachableTile, tile, movementLeft) - } else if (unit.isPreparingAirSweep()){ + attackableTiles += AttackableTile( + reachableTile, tile, movementLeft, + Battle.getMapCombatantOfTile(tile)!! + ) + } else if (unit.isPreparingAirSweep()) { tilesWithEnemies += tile - attackableTiles += AttackableTile(reachableTile, tile, movementLeft) - } else { - tilesWithoutEnemies += tile - } + attackableTiles += AttackableTile( + reachableTile, tile, movementLeft, + Battle.getMapCombatantOfTile(tile)!! + ) + } else tilesWithoutEnemies += tile } } return attackableTiles diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index 39c3520c2e..23a7fc3ea3 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/core/src/com/unciv/logic/battle/Battle.kt @@ -62,12 +62,14 @@ object Battle { */ if (attacker.getTile() != attackableTile.tileToAttackFrom) return false /** Rarely, a melee unit will target a civilian then move through the civilian to get - * to attackableTile.tileToAttackFrom, meaning that they take the civilian. This check stops - * the melee unit from trying to capture their own unit if this happens */ - if (getMapCombatantOfTile(attackableTile.tileToAttack)!!.getCivInfo() == attacker.getCivInfo()) return false + * to attackableTile.tileToAttackFrom, meaning that they take the civilian. + * This can lead to: + * A. the melee unit from trying to capture their own unit (see #7282) + * B. The civilian unit disappearing entirely (e.g. Great Person) and trying to capture a non-existent unit (see #8563) */ + val combatant = getMapCombatantOfTile(attackableTile.tileToAttack) + if (combatant == null || combatant.getCivInfo() == attacker.getCivInfo()) return false /** Alternatively, maybe we DID reach that tile, but it turned out to be a hill or something, - * so we expended all of our movement points! - */ + * so we expended all of our movement points! */ if (attacker.hasUnique(UniqueType.MustSetUp) && !attacker.unit.isSetUpForSiege() && attacker.unit.currentMovement > 0f diff --git a/core/src/com/unciv/logic/files/UncivFiles.kt b/core/src/com/unciv/logic/files/UncivFiles.kt index 45867f9a3a..4a3aeb0256 100644 --- a/core/src/com/unciv/logic/files/UncivFiles.kt +++ b/core/src/com/unciv/logic/files/UncivFiles.kt @@ -341,9 +341,9 @@ class UncivFiles( /** @throws IncompatibleGameInfoVersionException if the [gameData] was created by a version of this game that is incompatible with the current one. */ fun gameInfoFromString(gameData: String): GameInfo { val unzippedJson = try { - Gzip.unzip(gameData) + Gzip.unzip(gameData.trim()) } catch (ex: Exception) { - gameData + gameData.trim() } val gameInfo = try { json().fromJson(GameInfo::class.java, unzippedJson) diff --git a/core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt b/core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt index 7217614729..c6f6cb52e1 100644 --- a/core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt +++ b/core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt @@ -5,6 +5,7 @@ import com.badlogic.gdx.scenes.scene2d.Touchable import com.badlogic.gdx.scenes.scene2d.ui.Label import com.badlogic.gdx.scenes.scene2d.ui.Table import com.unciv.UncivGame +import com.unciv.logic.automation.unit.AttackableTile import com.unciv.logic.automation.unit.BattleHelper import com.unciv.logic.automation.unit.UnitAutomation import com.unciv.logic.battle.Battle @@ -13,7 +14,6 @@ import com.unciv.logic.battle.CityCombatant import com.unciv.logic.battle.ICombatant import com.unciv.logic.battle.MapUnitCombatant import com.unciv.logic.map.tile.Tile -import com.unciv.logic.automation.unit.AttackableTile import com.unciv.models.UncivSound import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.translations.tr @@ -267,7 +267,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() { } else if (attacker is CityCombatant) { val canBombard = UnitAutomation.getBombardableTiles(attacker.city).contains(defender.getTile()) if (canBombard) { - attackableTile = AttackableTile(attacker.getTile(), defender.getTile(), 0f) + attackableTile = AttackableTile(attacker.getTile(), defender.getTile(), 0f, defender) } } }