Fix Great Person Create Improvement requiring Resource (#8979)

* Fix GP create with Improvement requiring Resource

* Unit Test: GP create with Improvement requiring Resource

core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActions.kt

@@ -479,16 +479,18 @@ object UnitActions {
         val civResources = unit.civ.getCivResourcesByName()
 
         for (unique in uniquesToCheck) {
+            // Skip actions with a "[amount] extra times" conditional - these are treated in addTriggerUniqueActions instead
             if (unique.conditionals.any { it.type == UniqueType.UnitActionExtraLimitedTimes }) continue
 
             val improvementName = unique.params[0]
             val improvement = tile.ruleset.tileImprovements[improvementName]
                 ?: continue
-            if (usagesLeft(unit, unique)==0) continue
+            if (usagesLeft(unit, unique) == 0) continue
 
             val resourcesAvailable = improvement.uniqueObjects.none {
-                it.isOfType(UniqueType.ConsumesResources) &&
+                improvementUnique ->
-                        (civResources[unique.params[1]] ?: 0) < unique.params[0].toInt()
+                improvementUnique.isOfType(UniqueType.ConsumesResources) &&
+                        (civResources[improvementUnique.params[1]] ?: 0) < improvementUnique.params[0].toInt()
             }
 
             finalActions += UnitAction(UnitActionType.Create,

tests/src/com/unciv/uniques/TestGame.kt

@@ -51,7 +51,7 @@ class TestGame {
         gameInfo.ruleset = ruleset
         gameInfo.difficultyObject = ruleset.difficulties["Prince"]!!
         gameInfo.speed = ruleset.speeds[Speed.DEFAULTFORSIMULATION]!!
-        gameInfo.currentPlayerCiv = Civilization()
+        gameInfo.currentPlayerCiv = Civilization()  // Will be uninitialized, do not build on for tests
 
         // Create a tilemap, needed for city centers
         gameInfo.tileMap = TileMap(1, ruleset, false)

tests/src/com/unciv/uniques/UnitUniquesTests.kt

@@ -1,9 +1,12 @@
 package com.unciv.uniques
 
 import com.badlogic.gdx.math.Vector2
+import com.unciv.models.ruleset.unique.UniqueType
+import com.unciv.models.translations.fillPlaceholders
 import com.unciv.testing.GdxTestRunner
 import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions
-import org.junit.Assert.assertNotNull
+import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getImprovementConstructionActions
+import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -37,6 +40,51 @@ class UnitUniquesTests {
         // then
         val giftAction = UnitActions.getGiftAction(greatPerson, unitTile)
 
-        assertNotNull("Great Person should have a gift action", giftAction)
+        Assert.assertNotNull("Great Person should have a gift action", giftAction)
+    }
+    @Test
+    fun CanConstructResourceRequiringImprovement() {
+        // Do this early so the uniqueObjects lazy is still un-triggered
+        val improvement = game.ruleset.tileImprovements["Manufactory"]!!
+        val requireUnique = UniqueType.ConsumesResources.text.fillPlaceholders("3", "Iron")
+        improvement.uniques.add(requireUnique)
+        Assert.assertFalse("Test preparation failed to add ConsumesResources to Manufactory",
+            improvement.uniqueObjects.none { it.type == UniqueType.ConsumesResources })
+
+        val civ = game.addCiv(isPlayer = true)
+        val centerTile = game.getTile(Vector2.Zero)
+        val capital = game.addCity(civ, centerTile)
+
+        // Place an Engineer and see if he could create a Manufactory
+        val unitTile = game.getTile(Vector2(1f,0f))
+        val unit = game.addUnit("Great Engineer", civ, unitTile)
+        unit.currentMovement = unit.baseUnit.movement.toFloat()  // Required!
+        val actionsWithoutIron = try {
+            getImprovementConstructionActions(unit, unitTile)
+        } catch (ex: Throwable) {
+            // Give that IndexOutOfBoundsException a nicer name
+            Assert.fail("getImprovementConstructionActions throws Exception ${ex.javaClass.simpleName}")
+            return
+        }.filter { it.action != null }
+        Assert.assertTrue("Great Engineer should NOT be able to create a Manufactory modded to require Iron with 0 Iron",
+            actionsWithoutIron.isEmpty())
+
+        // Supply Iron
+        val ironTile = game.getTile(Vector2(0f,1f))
+        ironTile.resource = "Iron"
+        ironTile.resourceAmount = 3
+        ironTile.improvement = "Mine"
+        civ.tech.addTechnology("Mining")
+        civ.tech.addTechnology("Iron Working")
+        // capital already owns tile, but this relinquishes first - shouldn't require manual setTerrainTransients, updateCivResources called automatically
+        capital.expansion.takeOwnership(ironTile)
+        val ironAvailable = civ.getCivResourcesByName()["Iron"] ?: 0
+        Assert.assertTrue("Test preparation failed to add Iron to Civ resources", ironAvailable >= 3)
+
+        // See if that same Engineer could create a Manufactory NOW
+        val actionsWithIron = getImprovementConstructionActions(unit, unitTile)
+            .filter { it.action != null }
+        Assert.assertFalse("Great Engineer SHOULD be able to create a Manufactory modded to require Iron once Iron is available",
+            actionsWithIron.isEmpty())
     }
 }