From 7b21505a53066063c6a5109714e403439b7cb19a Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Sun, 4 Aug 2019 19:13:11 +0300 Subject: [PATCH] Fixed rare "cloning exploredTiles" concurrency bug Removed .replace in ImageGetter.getImage() thus lowering memory consumption --- .../com/unciv/logic/civilization/CivilizationInfo.kt | 6 +++++- core/src/com/unciv/ui/EmpireOverviewScreen.kt | 2 +- .../unciv/ui/cityscreen/CityScreenCityPickerTable.kt | 4 ++-- core/src/com/unciv/ui/cityscreen/CityTileGroup.kt | 2 +- core/src/com/unciv/ui/mapeditor/MapScreenLoadTable.kt | 6 +++--- .../unciv/ui/pickerscreens/ImprovementPickerScreen.kt | 2 +- core/src/com/unciv/ui/pickerscreens/TechButton.kt | 2 +- core/src/com/unciv/ui/tilegroups/CityButton.kt | 10 +++++----- core/src/com/unciv/ui/tilegroups/TileGroup.kt | 2 +- core/src/com/unciv/ui/utils/ImageGetter.kt | 6 +++--- core/src/com/unciv/ui/utils/Tutorials.kt | 2 +- core/src/com/unciv/ui/utils/UnitGroup.kt | 2 +- .../com/unciv/ui/worldscreen/NotificationsScroll.kt | 2 +- core/src/com/unciv/ui/worldscreen/WorldScreen.kt | 8 +++++--- core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt | 2 +- .../com/unciv/ui/worldscreen/unit/UnitActionsTable.kt | 2 +- 16 files changed, 33 insertions(+), 27 deletions(-) diff --git a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt index 5abf2b307a..27f95da281 100644 --- a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt +++ b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt @@ -83,7 +83,11 @@ class CivilizationInfo { for(diplomacyManager in diplomacy.values.map { it.clone() }) toReturn.diplomacy.put(diplomacyManager.otherCivName, diplomacyManager) toReturn.cities = cities.map { it.clone() } - toReturn.exploredTiles.addAll(exploredTiles) + + // This is the only thing that is NOT switched out, which makes it a source of ConcurrentModification errors. + // Cloning it by-pointer is a horrific move, since the serialization would go over it ANYWAY and still led to concurrency prolems. + // Cloning it by iiterating on the tilemap values may seem ridiculous, but it's a perfectly thread-safe way to go about it, unlike the other solutions. + toReturn.exploredTiles.addAll(gameInfo.tileMap.values.asSequence().map { it.position }.filter { it in exploredTiles }) toReturn.notifications.addAll(notifications) toReturn.citiesCreated = citiesCreated return toReturn diff --git a/core/src/com/unciv/ui/EmpireOverviewScreen.kt b/core/src/com/unciv/ui/EmpireOverviewScreen.kt index 80cb30c563..87e4bcdb98 100644 --- a/core/src/com/unciv/ui/EmpireOverviewScreen.kt +++ b/core/src/com/unciv/ui/EmpireOverviewScreen.kt @@ -404,7 +404,7 @@ class EmpireOverviewScreen : CameraStageBaseScreen(){ companion object { fun getCivGroup(civ: CivilizationInfo, afterCivNameText:String,currentPlayer:CivilizationInfo): Table { val civGroup = Table() - val civGroupBackground = ImageGetter.getDrawable("OtherIcons/civTableBackground.png") + val civGroupBackground = ImageGetter.getDrawable("OtherIcons/civTableBackground") val civNameText = civ.civName.tr()+afterCivNameText val label = civNameText.toLabel() diff --git a/core/src/com/unciv/ui/cityscreen/CityScreenCityPickerTable.kt b/core/src/com/unciv/ui/cityscreen/CityScreenCityPickerTable.kt index 14b9f15ac2..668e15f0cf 100644 --- a/core/src/com/unciv/ui/cityscreen/CityScreenCityPickerTable.kt +++ b/core/src/com/unciv/ui/cityscreen/CityScreenCityPickerTable.kt @@ -29,12 +29,12 @@ class CityScreenCityPickerTable(val cityScreen: CityScreen) : Table(){ val cityNameTable = Table() if(city.isBeingRazed){ - val fireImage = ImageGetter.getImage("OtherIcons/Fire.png") + val fireImage = ImageGetter.getImage("OtherIcons/Fire") cityNameTable.add(fireImage).size(20f).padRight(5f) } if(city.isCapital()){ - val starImage = Image(ImageGetter.getDrawable("OtherIcons/Star.png").tint(Color.LIGHT_GRAY)) + val starImage = Image(ImageGetter.getDrawable("OtherIcons/Star").tint(Color.LIGHT_GRAY)) cityNameTable.add(starImage).size(20f).padRight(5f) } diff --git a/core/src/com/unciv/ui/cityscreen/CityTileGroup.kt b/core/src/com/unciv/ui/cityscreen/CityTileGroup.kt index 840f32dd14..fcb4d6bf36 100644 --- a/core/src/com/unciv/ui/cityscreen/CityTileGroup.kt +++ b/core/src/com/unciv/ui/cityscreen/CityTileGroup.kt @@ -19,7 +19,7 @@ class CityTileGroup(private val city: CityInfo, tileInfo: TileInfo, tileSetStrin isTransform=false // performance helper - nothing here is rotated or scaled addActor(yieldGroup) if (city.location == tileInfo.position) { - populationImage = ImageGetter.getImage("StatIcons/City_Center_(Civ6).png") + populationImage = ImageGetter.getImage("StatIcons/City_Center_(Civ6)") addActor(populationImage) } diff --git a/core/src/com/unciv/ui/mapeditor/MapScreenLoadTable.kt b/core/src/com/unciv/ui/mapeditor/MapScreenLoadTable.kt index 6234b8c936..425ebf4f87 100644 --- a/core/src/com/unciv/ui/mapeditor/MapScreenLoadTable.kt +++ b/core/src/com/unciv/ui/mapeditor/MapScreenLoadTable.kt @@ -35,9 +35,9 @@ class MapScreenLoadTable(mapEditorScreen: MapEditorScreen): PopupTable(mapEditor } add(loadFromClipboardButton).row() - val closeOptionsButtton = TextButton("Close".tr(), CameraStageBaseScreen.skin) - closeOptionsButtton.onClick { remove() } - add(closeOptionsButtton).row() + val closeOptionsButton = TextButton("Close".tr(), CameraStageBaseScreen.skin) + closeOptionsButton.onClick { remove() } + add(closeOptionsButton).row() open() } diff --git a/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt b/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt index b1bca7175c..1998c21b68 100644 --- a/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt +++ b/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt @@ -45,7 +45,7 @@ class ImprovementPickerScreen(tileInfo: TileInfo, onAccept: ()->Unit) : PickerSc val group = Table() val image = if(improvement.name.startsWith("Remove")) - ImageGetter.getImage("OtherIcons/Stop.png") + ImageGetter.getImage("OtherIcons/Stop") else ImageGetter.getImprovementIcon(improvement.name,30f) diff --git a/core/src/com/unciv/ui/pickerscreens/TechButton.kt b/core/src/com/unciv/ui/pickerscreens/TechButton.kt index 9233329185..e3a26b672b 100644 --- a/core/src/com/unciv/ui/pickerscreens/TechButton.kt +++ b/core/src/com/unciv/ui/pickerscreens/TechButton.kt @@ -17,7 +17,7 @@ class TechButton(techName:String, val techManager: TechManager) : Table(CameraSt init { touchable = Touchable.enabled defaults().pad(10f) - background = ImageGetter.getDrawable("OtherIcons/civTableBackground.png") + background = ImageGetter.getDrawable("OtherIcons/civTableBackground") if(ImageGetter.techIconExists(techName)) add(ImageGetter.getTechIconGroup(techName)) // this is 60*60 diff --git a/core/src/com/unciv/ui/tilegroups/CityButton.kt b/core/src/com/unciv/ui/tilegroups/CityButton.kt index f764c89668..c9750bfa1e 100644 --- a/core/src/com/unciv/ui/tilegroups/CityButton.kt +++ b/core/src/com/unciv/ui/tilegroups/CityButton.kt @@ -47,7 +47,7 @@ class CityButton(val city: CityInfo, internal val tileGroup: WorldTileGroup, ski if (!tileGroup.tileInfo.airUnits.isNotEmpty()) return val secondarycolor = city.civInfo.getNation().getSecondaryColor() val airUnitTable = Table().apply { defaults().pad(5f) } - airUnitTable.background = ImageGetter.getDrawable("OtherIcons/civTableBackground.png") + airUnitTable.background = ImageGetter.getDrawable("OtherIcons/civTableBackground") .tint(city.civInfo.getNation().getColor()) val aircraftImage = ImageGetter.getImage("OtherIcons/Aircraft") aircraftImage.color = secondarycolor @@ -92,16 +92,16 @@ class CityButton(val city: CityInfo, internal val tileGroup: WorldTileGroup, ski val secondaryColor = city.civInfo.getNation().getSecondaryColor() val iconTable = Table() iconTable.touchable=Touchable.enabled - iconTable.background = ImageGetter.getDrawable("OtherIcons/civTableBackground.png") + iconTable.background = ImageGetter.getDrawable("OtherIcons/civTableBackground") .tint(city.civInfo.getNation().getColor()) if (city.resistanceCounter > 0) { - val resistanceImage = ImageGetter.getImage("StatIcons/Resistance.png") + val resistanceImage = ImageGetter.getImage("StatIcons/Resistance") iconTable.add(resistanceImage).size(20f).pad(2f).padLeft(5f) } if (city.isBeingRazed) { - val fireImage = ImageGetter.getImage("OtherIcons/Fire.png") + val fireImage = ImageGetter.getImage("OtherIcons/Fire") iconTable.add(fireImage).size(20f).pad(2f).padLeft(5f) } if (city.isCapital()) { @@ -110,7 +110,7 @@ class CityButton(val city: CityInfo, internal val tileGroup: WorldTileGroup, ski .apply { color = secondaryColor } iconTable.add(cityStateImage).size(20f).pad(2f).padLeft(10f) } else { - val starImage = ImageGetter.getImage("OtherIcons/Star.png").apply { color = Color.LIGHT_GRAY } + val starImage = ImageGetter.getImage("OtherIcons/Star").apply { color = Color.LIGHT_GRAY } iconTable.add(starImage).size(20f).pad(2f).padLeft(10f) } } else if (city.civInfo.isCurrentPlayer() && city.isConnectedToCapital()) { diff --git a/core/src/com/unciv/ui/tilegroups/TileGroup.kt b/core/src/com/unciv/ui/tilegroups/TileGroup.kt index 47d7a55e83..ce071c6e14 100644 --- a/core/src/com/unciv/ui/tilegroups/TileGroup.kt +++ b/core/src/com/unciv/ui/tilegroups/TileGroup.kt @@ -57,7 +57,7 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings) val circleCrosshairFogLayerGroup = Group().apply { isTransform=false; setSize(groupSize,groupSize) } private val circleImage = ImageGetter.getCircle() // for blue and red circles on the tile - private val crosshairImage = ImageGetter.getImage("OtherIcons/Crosshair.png") // for when a unit is targete + private val crosshairImage = ImageGetter.getImage("OtherIcons/Crosshair") // for when a unit is targete protected val fogImage = ImageGetter.getImage(tileSetStrings.crosshatchHexagon) diff --git a/core/src/com/unciv/ui/utils/ImageGetter.kt b/core/src/com/unciv/ui/utils/ImageGetter.kt index d9caa283a2..807ba90baa 100644 --- a/core/src/com/unciv/ui/utils/ImageGetter.kt +++ b/core/src/com/unciv/ui/utils/ImageGetter.kt @@ -16,7 +16,7 @@ import com.unciv.models.gamebasics.Nation import com.unciv.models.gamebasics.tile.ResourceType object ImageGetter { - private const val whiteDotLocation = "OtherIcons/whiteDot.png" + private const val whiteDotLocation = "OtherIcons/whiteDot" // When we used to load images directly from different files, without using a texture atlas, // The draw() phase of the main screen would take a really long time because the BatchRenderer would @@ -28,7 +28,7 @@ object ImageGetter { fun getDot(dotColor: Color) = getWhiteDot().apply { color = dotColor} fun getExternalImage(fileName:String): Image { - return Image(TextureRegion(Texture("ExtraImages/$fileName.png"))) + return Image(TextureRegion(Texture("ExtraImages/$fileName"))) } fun getImage(fileName: String): Image { @@ -44,7 +44,7 @@ object ImageGetter { private fun getTextureRegion(fileName: String): TextureRegion { try { - val region = atlas.findRegion(fileName.replace(".png","")) + val region = atlas.findRegion(fileName) if(region==null) throw Exception("Could not find $fileName") diff --git a/core/src/com/unciv/ui/utils/Tutorials.kt b/core/src/com/unciv/ui/utils/Tutorials.kt index d4ed6bd7d4..d52499569a 100644 --- a/core/src/com/unciv/ui/utils/Tutorials.kt +++ b/core/src/com/unciv/ui/utils/Tutorials.kt @@ -63,7 +63,7 @@ class Tutorials{ val currentTutorial = tutorialTexts[0] val label = Label(currentTutorial.texts[0], CameraStageBaseScreen.skin) label.setAlignment(Align.center) - if(Gdx.files.internal("ExtraImages/"+currentTutorial.name+".png").exists()) + if(Gdx.files.internal("ExtraImages/"+currentTutorial.name).exists()) tutorialTable.add(Table().apply { add(ImageGetter.getExternalImage(currentTutorial.name)) }).row() tutorialTable.add(label).pad(10f).row() val button = TextButton("Close".tr(), CameraStageBaseScreen.skin) diff --git a/core/src/com/unciv/ui/utils/UnitGroup.kt b/core/src/com/unciv/ui/utils/UnitGroup.kt index df39b3de94..6771125eca 100644 --- a/core/src/com/unciv/ui/utils/UnitGroup.kt +++ b/core/src/com/unciv/ui/utils/UnitGroup.kt @@ -35,7 +35,7 @@ class UnitGroup(val unit: MapUnit, val size: Float): Group() { fun getBackgroundImageForUnit(unit: MapUnit): Image { return when { unit.isEmbarked() -> ImageGetter.getImage("OtherIcons/Banner") - unit.isFortified() -> ImageGetter.getImage("OtherIcons/Shield.png") + unit.isFortified() -> ImageGetter.getImage("OtherIcons/Shield") else -> ImageGetter.getCircle() } } diff --git a/core/src/com/unciv/ui/worldscreen/NotificationsScroll.kt b/core/src/com/unciv/ui/worldscreen/NotificationsScroll.kt index af29fa6b34..84815b1192 100644 --- a/core/src/com/unciv/ui/worldscreen/NotificationsScroll.kt +++ b/core/src/com/unciv/ui/worldscreen/NotificationsScroll.kt @@ -33,7 +33,7 @@ class NotificationsScroll(internal val worldScreen: WorldScreen) : ScrollPane(nu listItem.add(ImageGetter.getCircle() .apply { color=notification.color }).size(10f).pad(5f) - listItem.background(ImageGetter.getDrawable("OtherIcons/civTableBackground.png")) + listItem.background(ImageGetter.getDrawable("OtherIcons/civTableBackground")) listItem.add(label).pad(5f).padRight(10f) // using a large click area with no gap in between each message item. diff --git a/core/src/com/unciv/ui/worldscreen/WorldScreen.kt b/core/src/com/unciv/ui/worldscreen/WorldScreen.kt index 20ff4413e9..262f367bfa 100644 --- a/core/src/com/unciv/ui/worldscreen/WorldScreen.kt +++ b/core/src/com/unciv/ui/worldscreen/WorldScreen.kt @@ -204,7 +204,7 @@ class WorldScreen : CameraStageBaseScreen() { if (civInfo.tech.currentTechnology() == null) { val buttonPic = Table() - buttonPic.background = ImageGetter.getDrawable("OtherIcons/civTableBackground.png") + buttonPic.background = ImageGetter.getDrawable("OtherIcons/civTableBackground") .tint(colorFromRGB(7, 46, 43)) buttonPic.defaults().pad(10f) buttonPic.add("{Pick a tech}!".toLabel().setFontColor(Color.WHITE).setFontSize(22)) @@ -328,15 +328,17 @@ class WorldScreen : CameraStageBaseScreen() { override fun render(delta: Float) { - if (shouldUpdate) { // This is so that updates happen in the MAIN THREAD, where there is a GL Context, + // This is so that updates happen in the MAIN THREAD, where there is a GL Context, + // otherwise images will not load properly! + if (shouldUpdate) { shouldUpdate = false if (currentPlayerCiv != gameInfo.getCurrentPlayerCivilization()) { + UnCivGame.Current.worldScreen.dispose() // for memory saving UnCivGame.Current.screen = PlayerReadyScreen(gameInfo.getCurrentPlayerCivilization()) return } - // otherwise images will not load properly! update() showTutorialsOnNextTurn() } diff --git a/core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt b/core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt index 68ff55b2aa..a8de6be3f4 100644 --- a/core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt +++ b/core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt @@ -89,7 +89,7 @@ class WorldScreenTopBar(val screen: WorldScreen) : Table() { } internal fun getMenuButton(): Image { - val menuButton = ImageGetter.getImage("OtherIcons/MenuIcon.png") + val menuButton = ImageGetter.getImage("OtherIcons/MenuIcon") .apply { setSize(50f, 50f) } menuButton.color = Color.WHITE menuButton.onClick { diff --git a/core/src/com/unciv/ui/worldscreen/unit/UnitActionsTable.kt b/core/src/com/unciv/ui/worldscreen/unit/UnitActionsTable.kt index 0e9584f3a8..1c8ede9bf9 100644 --- a/core/src/com/unciv/ui/worldscreen/unit/UnitActionsTable.kt +++ b/core/src/com/unciv/ui/worldscreen/unit/UnitActionsTable.kt @@ -47,7 +47,7 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table(){ "Disband unit" -> return ImageGetter.getImage("OtherIcons/DisbandUnit") "Sleep" -> return ImageGetter.getImage("OtherIcons/Sleep") "Explore" -> return ImageGetter.getUnitIcon("Scout") - "Stop exploration" -> return ImageGetter.getImage("OtherIcons/Stop.png") + "Stop exploration" -> return ImageGetter.getImage("OtherIcons/Stop") "Create Fishing Boats" -> return ImageGetter.getImprovementIcon("Fishing Boats") "Create Oil well" -> return ImageGetter.getImprovementIcon("Oil well") "Pillage" -> return ImageGetter.getImage("OtherIcons/Pillage")