Solved another 2 rare concurrency problems - CityInfo.workedTiles and CityInfo.tiles are not edited directly but switched out

This commit is contained in:
Yair Morgenstern 2018-12-30 20:40:48 +02:00
parent 11066b84e6
commit 8ad37530b2
6 changed files with 51 additions and 27 deletions

View File

@ -319,7 +319,7 @@ BuildingIcons/Palace
orig: 100, 100
offset: 0, 0
index: -1
BuildingIcons/Paper maker
BuildingIcons/Paper Maker
rotate: false
xy: 1864, 1232
size: 100, 100
@ -697,6 +697,20 @@ ImprovementIcons/Quarry
orig: 100, 100
offset: 0, 0
index: -1
ImprovementIcons/Railroad
rotate: false
xy: 1734, 722
size: 100, 100
orig: 100, 100
offset: 0, 0
index: -1
OtherIcons/Railroad
rotate: false
xy: 1734, 722
size: 100, 100
orig: 100, 100
offset: 0, 0
index: -1
ImprovementIcons/Road
rotate: false
xy: 347, 407
@ -774,20 +788,6 @@ OtherIcons/Pentagon
orig: 100, 100
offset: 0, 0
index: -1
OtherIcons/Railroad
rotate: false
xy: 1734, 722
size: 100, 100
orig: 100, 100
offset: 0, 0
index: -1
ImprovementIcons/Railroad
rotate: false
xy: 1734, 722
size: 100, 100
orig: 100, 100
offset: 0, 0
index: -1
OtherIcons/Shield
rotate: false
xy: 692, 622

View File

@ -118,7 +118,7 @@ class NextTurnAutomation{
private fun reassignWorkedTiles(civInfo: CivilizationInfo) {
for (city in civInfo.cities) {
city.workedTiles.clear()
city.workedTiles = hashSetOf()
city.population.specialists.clear()
for (i in 0..city.population.population)
city.population.autoAssignPopulation()

View File

@ -3,6 +3,8 @@ package com.unciv.logic.city
import com.badlogic.gdx.graphics.Color
import com.unciv.logic.automation.Automation
import com.unciv.logic.map.TileInfo
import com.unciv.ui.utils.withItem
import com.unciv.ui.utils.withoutItem
class CityExpansionManager {
@Transient
@ -87,9 +89,9 @@ class CityExpansionManager {
}
fun relinquishOwnership(tileInfo: TileInfo){
cityInfo.tiles.remove(tileInfo.position)
cityInfo.tiles = cityInfo.tiles.withoutItem(tileInfo.position)
if(cityInfo.workedTiles.contains(tileInfo.position))
cityInfo.workedTiles.remove(tileInfo.position)
cityInfo.workedTiles = cityInfo.workedTiles.withoutItem(tileInfo.position)
tileInfo.owningCity=null
}
@ -97,7 +99,7 @@ class CityExpansionManager {
if(tileInfo.isCityCenter()) throw Exception("What?")
if(tileInfo.getCity()!=null) tileInfo.getCity()!!.expansion.relinquishOwnership(tileInfo)
cityInfo.tiles.add(tileInfo.position)
cityInfo.tiles = cityInfo.tiles.withItem(tileInfo.position)
tileInfo.owningCity = cityInfo
cityInfo.population.autoAssignPopulation()
cityInfo.cityStats.update()

View File

@ -11,6 +11,7 @@ import com.unciv.models.gamebasics.GameBasics
import com.unciv.models.gamebasics.tile.ResourceType
import com.unciv.models.gamebasics.tile.TileResource
import com.unciv.models.stats.Stats
import com.unciv.ui.utils.withoutItem
import kotlin.math.min
class CityInfo {
@ -79,8 +80,8 @@ class CityInfo {
toReturn.population = population.clone()
toReturn.cityConstructions=cityConstructions.clone()
toReturn.expansion = expansion.clone()
toReturn.tiles.addAll(tiles)
toReturn.workedTiles.addAll(workedTiles)
toReturn.tiles = tiles
toReturn.workedTiles = workedTiles
toReturn.isBeingRazed=isBeingRazed
toReturn.isConnectedToCapital = isConnectedToCapital
return toReturn
@ -210,7 +211,7 @@ class CityInfo {
// now that the tiles have changed, we need to reassign population
workedTiles.filterNot { tiles.contains(it) }
.forEach { workedTiles.remove(it); population.autoAssignPopulation() }
.forEach { workedTiles = workedTiles.withoutItem(it); population.autoAssignPopulation() }
// Remove all national wonders
for(building in cityConstructions.getBuiltBuildings().filter { it.requiredBuildingInAllCities!=null })

View File

@ -5,6 +5,8 @@ import com.unciv.logic.automation.Automation
import com.unciv.logic.map.TileInfo
import com.unciv.models.stats.Stat
import com.unciv.models.stats.Stats
import com.unciv.ui.utils.withItem
import com.unciv.ui.utils.withoutItem
import kotlin.math.roundToInt
class PopulationManager {
@ -94,7 +96,7 @@ class PopulationManager {
//assign population
if (valueBestTile > valueBestSpecialist) {
if (bestTile != null)
cityInfo.workedTiles.add(bestTile.position)
cityInfo.workedTiles = cityInfo.workedTiles.withItem(bestTile.position)
} else {
if (bestJob != null) {
specialists.add(bestJob, 1f)
@ -105,9 +107,9 @@ class PopulationManager {
fun unassignExtraPopulation() {
for(tile in cityInfo.workedTiles.map { cityInfo.tileMap[it] }) {
if (tile.getCity() != cityInfo)
cityInfo.workedTiles.remove(tile.position)
cityInfo.workedTiles = cityInfo.workedTiles.withoutItem(tile.position)
if(tile.arialDistanceTo(cityInfo.getCenterTile()) > 3) // AutoAssignPopulation used to assign pop outside of allowed range, fixed as of 2.10.4
cityInfo.workedTiles.remove(tile.position)
cityInfo.workedTiles = cityInfo.workedTiles.withoutItem(tile.position)
}
while (getFreePopulation()<0) {
@ -131,7 +133,7 @@ class PopulationManager {
//un-assign population
if ((valueWorstTile < valueWorstSpecialist && worstWorkedTile != null)
|| worstJob == null) {
cityInfo.workedTiles.remove(worstWorkedTile!!.position)
cityInfo.workedTiles = cityInfo.workedTiles.withoutItem(worstWorkedTile!!.position)
} else {
specialists.add(worstJob, -1f)
}

View File

@ -160,6 +160,15 @@ fun <T> ArrayList<T>.withItem(item:T): ArrayList<T> {
return newArrayList
}
/**
* Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
*/
fun <T> HashSet<T>.withItem(item:T): HashSet<T> {
val newHashSet = HashSet(this)
newHashSet.add(item)
return newHashSet
}
/**
* Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
*/
@ -168,3 +177,13 @@ fun <T> ArrayList<T>.withoutItem(item:T): ArrayList<T> {
newArrayList.remove(item)
return newArrayList
}
/**
* Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
*/
fun <T> HashSet<T>.withoutItem(item:T): HashSet<T> {
val newHashSet = HashSet(this)
newHashSet.remove(item)
return newHashSet
}