Bug Reports and Discussion

I did not realize until this evening that 2.61 had already been released. I am in the middle of merging it into MagisterModmod right now.

While fixing the unitAIs in CIV4UnitInfos.xml, I noticed that UNIT_GOBLIN_SCORPION_CLAN now has the unitclass set as <Class>UNITAI_ATTACK</Class> instead of <Class>UNITCLASS_SCOUT</Class>, which I am pretty sure is not right.
 
I had a bit of trouble trying to update WorldBuilder to handle Temporary Map Items last night.

I created a dummy spell to test the new functions, and got an error:
Spoiler :
Code:
Traceback (most recent call last):

  File "CvSpellInterface", line 30, in cast

  File "<string>", line 0, in ?

  File "CvSpellInterface", line 7875, in spellTestTerrain

AttributeError: 'CyPlot' object has no attribute 'getRealTerrainType'
ERR: Python function cast failed, module CvSpellInterface

It appears that pPlot.getRealTerrainType() was not exposed to Python as it was supposed to be.

Of all the temp functions that is the one which would find widest use. I'm not sure I want to bother updating worldbuilder before this is fixed.

After commenting out that part I can confirm that the other new pPlot.getTemp_ and pPlot.getReal_ functions seem to work fine. I haven't tested the new pPlot.setTemp_ functions yet.
 
While fixing the unitAIs in CIV4UnitInfos.xml, I noticed that UNIT_GOBLIN_SCORPION_CLAN now has the unitclass set as <Class>UNITAI_ATTACK</Class> instead of <Class>UNITCLASS_SCOUT</Class>, which I am pretty sure is not right.

Good catch. I'm kind of surprised that the game doesn't throw errors for that. Will be fixed in 2.62

It appears that pPlot.getRealTerrainType() was not exposed to Python as it was supposed to be.

Will fix for 2.62. Will shoot for an end of month release.
 
Good catch. I'm kind of surprised that the game doesn't throw errors for that. Will be fixed in 2.62

I believe that as far as the game is concerned, the string 'UNITAI_ATTACK' is basically equivalent to its index, i.e., the number "4." When it looks up the entry unitclass entry at index 4, it finds that 'UNITAI_ATTACK' means the same thing as UNITCLASS_SUPPLIES.

It is not really different from the common error of using a BULDINGCLASS instead of a BUILDING, which makes the wrong building appear but would not cause any errors as buildings outnumber buildingclasses.
 
I maybe found a wrong Assert in CvSelectionGroup::getPathEndTurnPlot():

Spoiler CvSelectionGroup::getPathEndTurnPlot() :

Code:
CvPlot* CvSelectionGroup::getPathEndTurnPlot() const
{
	FAStarNode* pNode;

	pNode = getPathLastNode();

	if (NULL != pNode)
	{
		if ((pNode->m_pParent == NULL) || (pNode->m_iData2 == 1))
		{
			return GC.getMapINLINE().plotSorenINLINE(pNode->m_iX, pNode->m_iY);
		}

		while (pNode->m_pParent != NULL)
		{
			if (pNode->m_pParent->m_iData2 == 1)
			{
				return GC.getMapINLINE().plotSorenINLINE(pNode->m_pParent->m_iX, pNode->m_pParent->m_iY);
			}

			pNode = pNode->m_pParent;
		}
	}

	[B]FAssert(false);[/B]

	return NULL;
}


The assert is obviously triggered only if getPathLastNode() is NULL, or no of the parents of pNode has m_iData2 == 1 (whatever that means). I think it shouldn't trigger in the first case:

The only way the function can return NULL is either in the last line (triggering the assert) or if plotSorenINLINE() returns NULL, which is only possible if the coordinates are out of the map boundary.

I got this assert when the method was called from CvUnitAI::AI_assaultSeaTransport():
Spoiler A part of CvUnitAI::AI_assaultSeaTransport()) :

Code:
if (getGroup()->AI_getMissionAIType() == MISSIONAI_ASSAULT)
{
	if (getPathEndTurnPlot() != NULL)
	{
		getGroup()->pushMission(MISSION_MOVE_TO, getPathEndTurnPlot()->getX(), getPathEndTurnPlot()->getY(), MOVE_AVOID_ENEMY_WEIGHT_3);
	}
	else
	{
		getGroup()->pushMission(MISSION_MOVE_TO, getGroup()->AI_getMissionAIPlot()->getX(), getGroup()->AI_getMissionAIPlot()->getY(), MOVE_AVOID_ENEMY_WEIGHT_3);
	}
	return true;
}


The possibility of getPathEndTurnPlot() == NULL is explicitly considered here, and I doubt this is related to any coordinates being out of bounds.
I have no idea what the code means, though, so I could be wrong. Maybe getPathEndTurnPlot() is allowed to have the coordinates (-1,-1), but not NULL?

My suggested code would be the following:
Spoiler CvSelectionGroup::getPathEndTurnPlot() :

Code:
CvPlot* CvSelectionGroup::getPathEndTurnPlot() const
{
	FAStarNode* pNode;

	pNode = getPathLastNode();

	if (NULL != pNode)
	{
		if ((pNode->m_pParent == NULL) || (pNode->m_iData2 == 1))
		{
			return GC.getMapINLINE().plotSorenINLINE(pNode->m_iX, pNode->m_iY);
		}

		while (pNode->m_pParent != NULL)
		{
			if (pNode->m_pParent->m_iData2 == 1)
			{
				return GC.getMapINLINE().plotSorenINLINE(pNode->m_pParent->m_iX, pNode->m_pParent->m_iY);
			}

			pNode = pNode->m_pParent;
		}
		[B]FAssert(false);[/B]
	}

	return NULL;
}


That way it only triggers if no of the node's parent's m_iData2 == 1.
 
So after hearing about the disciples of acheron bug coming back in the EMM thread I started looking to see where they were at in my current game I'm playing (1/3 of the way throu a marathon game) and acheron nor the disciples are anywhere to be found. Opened up the event log ingame and I have no mention of him even being spawned thou Orthus is mentioned. My settings are
Spoiler :
 
So after hearing about the disciples of acheron bug coming back in the EMM thread I started looking to see where they were at in my current game I'm playing (1/3 of the way throu a marathon game) and acheron nor the disciples are anywhere to be found. Opened up the event log ingame and I have no mention of him even being spawned thou Orthus is mentioned. My settings are
Spoiler :

Is that bug from ExtraModMod, or were you playing with More Naval AI? If it is from EMM, then please report it in the ExtraModMod thread instead of here.
 
MNAI 2.61, only thing I changed was fixing the death promotions thing for creating/upgrading adepts.

I was originally going to post over there what mine were doing in MNAI (to try and help determine if the bug was a EMM thing or in MNAI) but since they haven't even spawned I ended up reporting a bug in MNAI.
 
Me and a buddy are thinking about playing FFH2 (with MNAI) again, so I got into the mood to look through some code again.

Two small patches so far:
  1. Allow hawks to rebase in team & vassal territories.
  2. Allow hawks to rebase to carriers (hunters/frigates) outside own/team territory.

I have attached diffs for both changes, and also the final contents of the affected file (CvSelectionGroup.cpp). The changes were based on r1772.


I also noticed something peculiar in XML/Units/CIV4UnitInfos.xml. Why does the frigate unit have iAirUnitCap=1? Only air units should have anything other than zero there.
 

Attachments

  • hawk_rebase_patches.zip
    1.6 KB · Views: 49
  • hawk_rebase_files.zip
    21.5 KB · Views: 48
Two small patches so far:
  1. Allow hawks to rebase in team & vassal territories.
  2. Allow hawks to rebase to carriers (hunters/frigates) outside own/team territory.

I have attached diffs for both changes, and also the final contents of the affected file (CvSelectionGroup.cpp).

Awesome! Thanks!


I also noticed something peculiar in XML/Units/CIV4UnitInfos.xml. Why does the frigate unit have iAirUnitCap=1? Only air units should have anything other than zero there.

Good catch. I will fix it for 2.62
 
A bug that I came across a long time ago, but never got around to reporting:
When upgrading a unit that is standing in/near an allied city, the upgrade is blocked if the upgrade is to a national unit and the owner of the city already has the maximum number of that unit type.

My proposed fix is not very elegant, but the code that deals with unit upgrades is a convoluted mess, and I wanted to at least keep the patch fairly simple.

The attached files are based on r1772.
 

Attachments

  • national_unit_upgrade_patch.zip
    1.5 KB · Views: 38
  • national_unit_upgrade_files.zip
    310.8 KB · Views: 29
I have not yet confirmed this for base MNAI yet, but I've found that in my modmod the End of Winter game option often leads to riverside tiles which have Flood Plains set as their Real feature type but with a Temp Timer of 0, causing the Flood Plains never to appear after the Tundra reverts to Desert. When I use my latest WorldBuilder updates to test it and increment that timer by one, then the feature gets properly placed on the next turn. I suspect that the issue has to do with a blizzard being placed on the tile and then removed.
 
I suspect that the issue has to do with a blizzard being placed on the tile and then removed.

Thats why I never added code to make Blizzards replace features. Did you implement this in your mod? There were a couple of extra checks that had to happen to make sure that the features were correctly reset when the Blizzard moved.
 
I did not change Blizzards.py in that regard. (I did change it so that any player with the White Hand Religion is treated like the Illians are in MNAI, and so as to take into account the hell terrain equivalents of Tundra and Snow.)

I did change the Call Blizzard spell to handle Blizzards as temporary features. I frankly don't think I coded this very well before the release, and just changed it a bit. If someone was using Call Blizzard extremely often, it could certain explain the issue. However, most of the game where I have noticed the issue did not include the Illians or the White Hand religion at all. I suppose it is possible for a Priest of the Hand to have been rescued from a lair and used to move blizzards around without ever founding its religion, but I've noticed this issuing being so widespread that I very much doubt that it is the case.

Edit:

I just ran a test using the new functions in my most recently posted WorldBuilder update, and believe that I have discovered the real problem.

It seems that the associated Temporary Timer is always reduced to 0 (without changing the associated Real type) whenever the normal type of Feature, Improvement, Bonus, or Route is changed. (I presume that the same also goes for the terrain type, although I cannot test that so easily until you expose those functions for me.)

That would explain why your Blizzard code would be discarding the Temp Timer information needed to make Flood Plains appear properly.
 
Any unit that is immune to magic is also prevented from getting any beneficial spells from being applied to them when you are only relying upon the xml tags for the requirements. So for instance Cure Disease and Loyalty spells can't be cast on any magic immune unit (because the requirements for those are just the xml tags) while Heal can be because it's requirements are in the python and not depending on the xml tags only.

Another thing I've noticed is that even if the unit has the Winterborn promotion it's still taking damage from blizzards when it first gets created. There are also 2 messages being sent out each time a unit takes damage from a blizzard. There are quite a few units that upon creation don't get the Winterborn promotion either. For instance the Eidolon aren't getting it and any unit that is created via ritual aren't getting it either (except for the Priest of Winter but that's because they start with it anyhow). I do note that since siege and naval units don't get Winterborn they take damage which does tend to limit the Illian's ability to use them.
 
Any unit that is immune to magic is also prevented from getting any beneficial spells from being applied to them when you are only relying upon the xml tags for the requirements. So for instance Cure Disease and Loyalty spells can't be cast on any magic immune unit (because the requirements for those are just the xml tags) while Heal can be because it's requirements are in the python and not depending on the xml tags only.

Intended behavior (IIUC).

Another thing I've noticed is that even if the unit has the Winterborn promotion it's still taking damage from blizzards when it first gets created. There are also 2 messages being sent out each time a unit takes damage from a blizzard. There are quite a few units that upon creation don't get the Winterborn promotion either. For instance the Eidolon aren't getting it and any unit that is created via ritual aren't getting it either (except for the Priest of Winter but that's because they start with it anyhow). I do note that since siege and naval units don't get Winterborn they take damage which does tend to limit the Illian's ability to use them.

Also seems intended (apart from the first part). Non-living units don't get Winterborn, but can still be harmed by blizzards.
 
It's intended behavior that you can wither a magic immune unit and never be able to remove the wither because you can't cast cure disease on it but you can cast Heal on the magic immune unit to magically heal hitpoints? That does not sound very logical to me.
 
A bug that I came across a long time ago, but never got around to reporting:
When upgrading a unit that is standing in/near an allied city, the upgrade is blocked if the upgrade is to a national unit and the owner of the city already has the maximum number of that unit type.

Interesting bug. Thanks for the code!

It seems that the associated Temporary Timer is always reduced to 0 (without changing the associated Real type) whenever the normal type of Feature, Improvement, Bonus, or Route is changed. (I presume that the same also goes for the terrain type, although I cannot test that so easily until you expose those functions for me.)

Yes. That is done so that permanent changes to Map Items can override temporary changes. Blizzards are technically features so when they move they are replacing the current feature and invalidating the original feature. Pain in the butt...


Any unit that is immune to magic is also prevented from getting any beneficial spells from being applied to them when you are only relying upon the xml tags for the requirements. So for instance Cure Disease and Loyalty spells can't be cast on any magic immune unit (because the requirements for those are just the xml tags) while Heal can be because it's requirements are in the python and not depending on the xml tags only.

Added a fix so that Heal wont affect Magic Immune units. If there are other spells that are bypassing Magic Immunity, please let me know.

Another thing I've noticed is that even if the unit has the Winterborn promotion it's still taking damage from blizzards when it first gets created. There are also 2 messages being sent out each time a unit takes damage from a blizzard. There are quite a few units that upon creation don't get the Winterborn promotion either. For instance the Eidolon aren't getting it and any unit that is created via ritual aren't getting it either (except for the Priest of Winter but that's because they start with it anyhow). I do note that since siege and naval units don't get Winterborn they take damage which does tend to limit the Illian's ability to use them.

Unfortunately, not sure I can do anything about the initial damage. When a unit is created, it calls the setXY function, which is the same function called when a unit moves. This in turn triggers the python blizzard function which deals out damage. The free promotions arent applied until after creation. I guess I could possibly add a hack to the python code. I'll think about that.

Winterborn is a racial promotion. Eidolons are Demons. Units can only have one racial promotion and whatever is set in UnitInfos overrides everything else. And as Q^7 said, racial promotions are only applied to living units.
 
I too would prefer if you could change the code so that the onMove and atRange python effects were not triggered when a unit was first initialized but after their promotions were added.

That said, I believe you could get be sure to get around all possible onMove Blizzard problems by changing the code to this:
Code:
def onMoveBlizzard(pCaster, pPlot):
	bDamage = True
	iWinterborn = gc.getInfoTypeForString('PROMOTION_WINTERBORN')
	if pCaster.getRace() == iWinterborn:
		bDamage = False
	elif pCaster.getRace() == -1:
		if gc.getUnitInfo(pCaster.getUnitType()).getFreePromotions(iWinterborn):
			bDamage = False
		elif pCaster.isAlive():
			if not pCaster.isAnimal():
				if gc.getCivilizationInfo(pCaster.getCivilizationType()).getDefaultRace() == iWinterborn:
					if not isWorldUnitClass(pCaster.getUnitClassType()):
						if pCaster.getDomainType() == gc.getInfoTypeForString('DOMAIN_LAND'):
							bDamage = False

	if bDamage:
		pCaster.doDamageNoCaster(10, 50, gc.getInfoTypeForString('DAMAGE_COLD'), False)


On a related issue, atRangeNecrototem causes fear in newly initialized units which would be immune to fear if their promotions were already granted. This is particularly noteworthy in summons.

I created a workaround for this in my modmod quite a while back, which you are of course free to borrow.
I changed this:
Code:
def atRangeNecrototem(pCaster, pPlot):
	if cf.doFear(pCaster, pPlot, -1, False):
		CyInterface().addMessage(pCaster.getOwner(),True,25,CyTranslator().getText("TXT_KEY_MESSAGE_FEAR", (gc.getUnitInfo(pCaster.getUnitType()).getDescription(), )),'',1,'Art/Interface/Buttons/Improvements/Necrototem.dds',ColorTypes(7),pCaster.getX(),pCaster.getY(),True,True)
To this:
Code:
def atRangeNecrototem(pCaster, pPlot):
#Fear from Necrototems can apply to units as soon as they are created, before promotions that grant immunity to fear are applied.
	info = gc.getUnitInfo(pCaster.getUnitType())
	for sProm in [	'PROMOTION_COURAGE',
			'PROMOTION_ANGEL',
			'PROMOTION_AVATAR',
			'PROMOTION_DEMON',
			'PROMOTION_DRAGON',
			'PROMOTION_ELEMENTAL',
			'PROMOTION_ILLUSION',
			'PROMOTION_UNDEAD',
			'PROMOTION_GOLEM',
			'PROMOTION_TIMOR_MASK',
			'PROMOTION_DEMON_POSSESSED',
			'PROMOTION_HEARTSTONE',
			'PROMOTION_ADVENTURER',
			'PROMOTION_AERONS_CHOSEN',
			'PROMOTION_BURNING_BLOOD',
			'PROMOTION_HERALDS_BLESSING']:
		if info.getFreePromotions(gc.getInfoTypeForString(sProm)):
			break
	else:
		if cf.doFear(pCaster, pPlot, -1, False):
			CyInterface().addMessage(pCaster.getOwner(),True,25,CyTranslator().getText("TXT_KEY_MESSAGE_FEAR", (pCaster.getName(), )),'',1,'Art/Interface/Buttons/Improvements/Necrototem.dds',ColorTypes(7),pCaster.getX(),pCaster.getY(),True,True)

(The list includes all the promotions which grant immunity to fear in my modmod. You would of course remove those which do not exist in yours.)

I originally wrote it like this, without hardcoding a list of promotions, but found that there was not a function exposed to python to tell whether a promotion grants immunity to fear.
Code:
def atRangeNecrototem(pCaster, pPlot):
#Fear from Necrototems can apply to units as soon as they are created, before promotions that grant immunity to fear are applied.
	info = gc.getUnitInfo(pCaster.getUnitType())
	for iProm in range(gc.getNumPromotionInfos()):
		if info.getFreePromotions(iProm):
			gc.getPromotionInfo(iProm).isImmuneToFear():
				break
	else:
		if cf.doFear(pCaster, pPlot, -1, False):
			CyInterface().addMessage(pCaster.getOwner(),True,25,CyTranslator().getText("TXT_KEY_MESSAGE_FEAR", (pCaster.getName(), )),'',1,'Art/Interface/Buttons/Improvements/Necrototem.dds',ColorTypes(7),pCaster.getX(),pCaster.getY(),True,True)

If it isn't too much trouble, exposing that to python might be useful. Ideally I'd like it if all a promotions tags could be checked in python.
 
Top Bottom