[BTS] Simple(?) problem with a bit of onCityBuilt code

[Herostratus]

So my intent is that every time anyone founds a city using a Modern Settler, that city begins its existence at size 2. Seems simple enough, and I'm positive there are mods out there that do something similar.

Here is the CvEventManager code I borrowed from someplace to make it happen:

    def onCityBuilt(self, argsList):
        'City Built'
        city = argsList[0]
        pUnit = CyInterface().getHeadSelectedUnit()
        if pUnit:
            if pUnit.getUnitClassType() == gc.getInfoTypeForString("UNITCLASS_SETTLER"):
                ## Do Nothing
                pass

            elif pUnit.getUnitClassType() == gc.getInfoTypeForString("UNITCLASS_MODERN_SETTLER"):
                city.setPopulation(2)

The problem I'm having is that this only works for the human player. I've tested it by giving the AI a Modern Settler unit, and cities that the AI founds with them start at size 1.

As you might be able to infer, I don't exACTly know what I'm doing when it comes to Python! But my hope is it's a simple thing to resolve. Is anybody able to fix this for me?

 

[Nightinggale]

I checked the DLL code. It looks like onCityBuilt is called for both human and AI players, meaning the problem must be in python.

       pUnit = CyInterface().getHeadSelectedUnit()
        if pUnit:

That must be the problem. It reads the selected unit from the user interface and then it continues to work on that unit if it finds one. I suspect the problem is that the AI will not have a selected unit in the user interface, which makes that assign None to pUnit, which in turn prevents the if statement to be true.

Let's try to add AI only code to fix this problem:

pUnit = CyInterface().getHeadSelectedUnit()
if pUnit == None and city.isHuman() == False: # AI player failed to find founding unit
  iModernUnitClass = gc.getInfoTypeForString("UNITCLASS_MODERN_SETTLER") # cache output instead of calling in the loop for performance reasons
  plot = city.plot()
  for iUnit in range (plot.getNumUnits()): # loop through all units on the plot
    pUnit = plot.getUnit(iUnit)
    if pUnit.getUnitClassType() == iModernUnitClass and city.getOwner() == pUnit.getOwner():
      break # found a modern settler. Let's assume it's the founding one
if pUnit:

The idea here is that if an AI player fails to find the founding unit, it will loop the plot. If it finds a modern settler owned by the player, then it will assume that settler to be the founding unit. Sure if the AI has both a regular and a modern one and the regular is founding, then it will cheat, but the AI is unaware of this and will not exploit this bug. I can't think of a way to fix this bug without modding the DLL itself.

Also this piece of code is completely untested.

EDIT: now that I think about it, the selected unit is only the correct one in single player. This approach will cause desyncs in network games.

 

[Nightinggale]

If you go for the DLL approach, it should always work as intended, for all players and in multiplayer-

bool CvUnit::found()
{
...
GET_PLAYER(getOwnerINLINE()).found(getX_INLINE(), getY_INLINE());
// new code
if (getUnitClassType() == GC.getInfoTypeForString("UNITCLASS_MODERN_SETTLER"))
{
    plot()->getPlotCity()->setPopulation(2);
}
// end new code

This will solve all the issues and should always work. However it requires compiling a new DLL file. It's also interesting to see how much less code is required to do the same thing.

 

[Herostratus]

Yikes. DLL is not an option, and desyncing networked games is ALSO not an option!
Any other ideas?

Might it be simpler to tie this new-cities-size-2 thing to the discovery of a particular technology?

 

[Nightinggale]

def onCityBuilt(self, argsList):
  'City Built'
  # this will set population to 2 if founding player has a modern settler on the plot
  # however with the available data it's impossible to tell if that unit is actually the founding unit
  city = argsList[0]
  iModernUnitClass = gc.getInfoTypeForString("UNITCLASS_MODERN_SETTLER") # cache output instead of calling in the loop for performance reasons
  plot = city.plot()
  for iUnit in range (plot.getNumUnits()): # loop through all units on the plot
    pUnit = plot.getUnit(iUnit)
    if pUnit.getUnitClassType() == iModernUnitClass and city.getOwner() == pUnit.getOwner():
      city.setPopulation(2)
      return

That will bring the population to 2 if there is a modern settler on the plot. It should work in multiplayer and with AI, but it opens for the exploit that you can use settlers to found cities with 2 population by just placing a modern settler on the plot. You may or may not want that.

You can also go for checking if city owner has a certain tech and then make all cities start with 2 population if that is true. That should also work in networks and for AI.

Alternatively you can check when killing a unit if it is a modern settler not killed by enemies and on a plot with a city of population 1.

    def onUnitKilled(self, argsList):
       'Unit Killed'
       unit, iAttacker = argsList
       player = PyPlayer(unit.getOwner())
       # new code
       if iAttacker == -1 and unit.getUnitClassType() == gc.getInfoTypeForString("UNITCLASS_MODERN_SETTLER"):
        city = unit.plot().getPlotCity()
        if city != None and city.getPopulation() == 1 and city.getOwner() == unit.getOwner():
           # killed unit is not killed by enemies and is a modern settler. Let's assume it's founding
           city.setPopulation(2)
       # end new code
       attacker = PyPlayer(iAttacker)
       if (not self.__LOG_UNITKILLED):
           return
       CvUtil.pyPrint('Player %d Civilization %s Unit %s was killed by Player %d'
           %(player.getID(), player.getCivilizationName(), PyInfo.UnitInfo(unit.getUnitType()).getDescription(), attacker.getID()))

This too should work in multiplayer and for AI.

 

[Herostratus]

That will bring the population to 2 if there is a modern settler on the plot. It should work in multiplayer and with AI, but it opens for the exploit that you can use settlers to found cities with 2 population by just placing a modern settler on the plot. You may or may not want that.

That shouldn't be a big problem, since in this mod, once you can build modern settlers, you cease to be able to build other kinds.
I'm curious, though: using this code, if the user of the "exploit" (that is, the owner of the pre-modern settler) is NOT the same player as the modern settler's owner, would that exploit still work?

You can also go for checking if city owner has a certain tech and then make all cities start with 2 population if that is true. That should also work in networks and for AI.

If it's not too much bother, can you share what that code would look like? It certainly SOUNDS simple, but like I say, I'm ignorant!

Alternatively you can check when killing a unit if it is a modern settler not killed by enemies and on a plot with a city of population 1.

That ought to work too, I suppose. I can't think of a situation where this unit could be killed NOT by enemies WHILE on a city tile. The only thing that comes to mind is the Bermuda Triangle event, which (A) for all I know doesn't count as "killing" a unit but rather as deleting it, and (B) is sea-only, and I have no similar events for on land AFAIK.
...I mean, deleting a unit (hitting the Skull icon) doesn't count as a non-enemy kill, right?

Thanks so much Nightinggale!!

 

[Nightinggale]

city.getOwner() == pUnit.getOwner(): <-- that line should ensure it's the same player.

When disbanding a unit by clicking the skull icon, you do kill the unit. However how often do you do that on a city with just 1 population and don't want the city to go to 2 population? It checks both that city and unit shares owner and that the city has a population of 1.

def onCityBuilt(self, argsList):
  'City Built'
  city = argsList[0]
  iPlayer = city.getOwner()
  player = gc.getPlayer(iPlayer)
  iTeam = player.getTeam()
  team = gc.getTeam(iTeam)
  if iTeam.isHasTech(gc.getInfoTypeForString("TECH_WHATEVER")):
    city.setPopulation(2)

I think that should solve the search for a tech. Obviously the TECH_ should be replaced with the actual name.

 

[Herostratus]

When disbanding a unit by clicking the skull icon, you do kill the unit. However how often do you do that on a city with just 1 population and don't want the city to go to 2 population? It checks both that city and unit shares owner and that the city has a population of 1.

You're right, that's more of a feature than a bug. I think I'll give that one a try, and fall back on the tech option if it does something unexpected or if I decide I don't like it.
Awesome work, thanks!