pUnit set to unexpected value?

[davidlallen]

I have used a similar loop many times. I want to find all units which are within cultural borders to give them a promotion, and take away the promotion from all units which are not within cultural borders. Here is the code:

	def HomeGround(self):
		for iPlay in range(gc.getMAX_CIV_PLAYERS()):
			pPlay = gc.getPlayer(iPlay)
			if not pPlay.isAlive(): continue
			for iUnit in range(pPlay.getNumUnits()):
				pUnit = pPlay.getUnit(iUnit)
				if pUnit.isNone(): continue # defensive
				iType = pUnit.getUnitCombatType()
				self.DebugPrint("%d,%d cbt %d" % \
					(pUnit.getX(), pUnit.getY(), iType))

I have put in "if pUnit.isNone()" already as a defensive measure. But I am getting the following alert, and then a CTD:

Traceback (most recent call last):
  File "DuneWars", line 150, in onEndGameTurn
  File "DuneWars", line 783, in HomeGround
RuntimeError: unidentifiable C++ exception
ERR: Python function onEvent failed, module CvEventInterface

The offending line is "iType = pUnit.getUnitCombatType()".

What can be going wrong? This happens very early in the game, not on turn 1, but always within the first 100 turns. Of course I cannot print any statistics about the unit which is failing, since the object seems to be an invalid one. With earlier versions of the same routine, some units were coming up at location -1,-1 or were matching isNone(), both of which indicate invalid data somehow.

Is there some other defensive programming I should do? Or have I overlooked something silly?

 

[Sto]

The list of unit indexes for a player isn't range(pPlay.getNumUnits()). You should use the pyhelper function or rewrite it. That should fix your bug. If not, the problem should be not easy to find.

Tcho !

 

[davidlallen]

What ?? You are saying that it is incorrect to turn a iPlayer, iUnit pair into a pUnit by calling pPlayer.getUnit(iUnit)? Then I completely don't understand how a lot of the callbacks work in CvGameUtils. For example, unitCannotMoveInto takes an iPlayer, iUnit and I have always converted that into a pUnit by pPlayer.getUnit(iUnit). Is this wrong? Or have I misunderstood your comment?

I want to make sure my iterators are efficient; so I want to make sure firstUnit / nextUnit is not iterating over all units and just skipping the ones which do not belong to this player. Do you know how the objects are actually stored?

I use the same method of getNumXXX and getXXX for lots of things, like "iCity in range (pPlayer.getNumCities())" and then "pPlayer.getCity(iCity)". Are all of these range calls incorrect? In many cases they seem to be working fine.

EDIT: browsing my code, the other main integer iterators I use are

* for iU in range (pPlot.getNumUnits()): pUnit = pPlot.getUnit(iU)
* for iP in range(map.numPlots()): pPlot = map.plotByIndex(iP)

Are all four of these wrong, or just some of them?

EDIT2: converting the above loop to use firstUnit/nextUnit did, in fact solve my problem. But now I am worried that all of my range iterators are incorrect!

 

[Sto]

While it works with plots, that doesn't works with units and cities. The system refresh the list of units and cities every turns and not every time a change is done (unit or city created killed). That mean the list of unit/city indexes may not be [0, 1, 2, ...] but anything. At the same time (especially for cities) the instance is still valid but refer to a dead object where you can call about anything (like you have noticed you get -1,-1 for a unit coords but you can get some good attribute also). And even if you don't have an error while looping cities, you may skip some existing ones and apply your change to dead ones.

I don't have my PC so not sure about the names but check in PyHelper.py : getUnitList and getCityList. You will see your error with the implementation of the functions.

Tcho !

Edit :

* for iU in range (pPlot.getNumUnits()): pUnit = pPlot.getUnit(iU) -> OK
* for iP in range(map.numPlots()): pPlot = map.plotByIndex(iP) -> OK
The range(player.getNumUnits()) or range(player.getNumCities()) doesn't work

 

[God-Emperor]

Some of this is guesswork, but the important thing is that your code doesn't do what you expect so you need to change it. This may explain why and how, or they why part might be wrong but the how to fix it part should be OK.

It would seem that just the player/unit related one is wrong.

Someone would have to check the SDK to make sure, but it looks like every time you build a unit it gets a new ID and the CyPlayer object doesn't keep a list to map units 0 to NumUnits-1 to the actual ID values (or units). So CyPlayer.getUnit(unitID) wants the actual unit ID, not some count in the range of how many units the player currently has. Assuming it works something like that, if a player has built 1000 units over the course of the game, but the first 800 of them are dead and the rest still alive then there would probably be a unit with an id of 900, but getNumUnits would return 200. Or something like that. They might be unique across all players, but my first guess would be that they are not (so the player number and unit ID combined would be unique, but not just the unit id) - but, again, you'd have to check the SDK to know for sure.

Note that CyPlayer has a specific .firstUnit and .nextUnit functions but CyPlot doesn't. The CyPlots aparently keep a nice list of all the units on the plot with a local index into the list that is not the actual unit ID (which may not be unique without the player ID too anyway) - this is not surprising, since it keeps a list you can use an index into the list from 0 to list length -1. It would seem that CyPlayer doesn't keep such a list, thus the specific first/next iterators.

The PyHelper.py file has the PyPlayer class which includes a getUnitList function which returns, not too surprisingly, a list of units owned by the player. You shoud either use that, or base your loop code on how that function does it.

 

[davidlallen]

Thanks for the explanations. The iteration is easy to pull out of pyplayer. I will change my code to stop iterating on player range numCities and player range numUnits, and use first / next instead. I will continue to convert iPlay/iUnit and use plotByIndex same as today. Live and learn.

 

[EmperorFool]

The list of cities and units for each player can have holes in it as units are killed (includes upgrading) and cities are acquired/razed/gifted/whatever. These holes are not filled in immediately.

I didn't know that they are compacted each turn; this makes city and unit IDs volatile across turns making me a sad panda! Civ4lerts depends on city IDs being consistent throughout the game as long as they don't change hands. Perhaps that is the source of the strange "London will become pacified next turn" for a city that you just founded two turns ago.

@Sto - Are you sure about this part? Does it shift all units/cities up (A, maintains order), or does it pull up just enough from the bottom of the list (B, minimizes ID changes) to fill the holes?

 ID    A     B
000   000   000
---   002   006
002   003   002
003   005   003
---   006   007
005   007   005
006   ---   ---
007   ---   ---

Note: The IDs above show where the old city IDs are located in the list after compaction, but the IDs themselves would change to be sequential (0, 1, 2, 3, ...). Thus method A results in many cities changing their ID whereas method B only changes the IDs of cities that actually move to fill a hole.

 

[Sto]

I've made the test a long time ago with units. But i don't remember how things are done. I just remember every turn the list of indexes are updated. And also that you can get some info in a partial invalid remaining objet even if it trigger isNone(). The test was done before BtS so ...

I'll have a look when I'll come back. That peek my curiosity again.

 

[primordial stew]

I stumbled on a similar issue last night while testing some new events. Basically, the Vandals starting off in Pannonia trekked over to Gaul, and then began ripping through the Roman Empire continuing through Hispania, and later Africa, and more. In game terms I have them as 3 separate (though possibly connected) events: vandals1 vandals2 and vandals3. The functions are very similar with each checking if the Vandals are still alive since the previous group might have been wiped out:

          if (not gc.getPlayer(self.iVandalID).isAlive()):
	       newLeaderIdx = gc.getInfoTypeForString('LEADER_VANDAL')
	       newCivIdx = gc.getInfoTypeForString('CIVILIZATION_VANDALS')
               success = game.createNewPlayer( self.iVandalID, newCivIdx, newLeaderIdx, -1, False, 1, 1 )

The below code works CTDs. Putting them on separate turns works just fine.

        elif (self.iGameTurn == 338):    ### 1st turn of the 200AD scenario (202AD)   
	     self.createVandals1()   
	     self.createVandals2()

gc.getPlayer(self.iVandalID).isAlive() apparently doesn't work as expected for checking the same civ on the same turn. It is not the civ slot number though. iVandalID is a constant set during initialization.

 

[davidlallen]

I am not sure how game.createNewPlayer() works. I guess this must be a function added by Revolutions? I have not written any code which creates new players, but I have never had any problems with keeping a iPlayer constant from the beginning of the game. In general when objects are deleted or added, the indices could become invalid. But in the normal game, players are never deleted or added. Perhaps createNewPlayer also triggers some kind of renumbering.

 

[primordial stew]

Yes, it's from the Revolutions code.

It's just the abnormal case of trying to use the same civID to create a civ 2 times on the current turn. I'd guess Revolutions would run into this. Consider the situation where there is only 1 civ slot left, so rebels spawn using it, and are immedately eliminated. And then another rev happens on the same turn during a different civ's turn. The difference is it can't be during the same civ's turn, so between civ turns must be where the bookkeeping happens.