Troubles with Arrays and Arrays of Arrays...

Afforess · Oct 16, 2009

Okay, some questions:

Code:

		FAssertMsg(m_paiTechExtraBuildingHappiness==NULL, "about to leak memory, CvTeam::m_aiVictoryCountdown"); //Afforess
		m_paiTechExtraBuildingHappiness = new int[GC.getNumBuildingInfos()];
		for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
		{
			m_paiTechExtraBuildingHappiness[iI] = -1;
		}
		
		FAssertMsg(m_paiTechExtraBuildingHealth==NULL, "about to leak memory, CvTeam::m_aiVictoryCountdown");
		m_paiTechExtraBuildingHealth = new int[GC.getNumBuildingInfos()];
		for (iI = 0; iI < GC.getNumBuildingInfos(); iI++)
		{
			m_paiTechExtraBuildingHealth[iI] = -1;
		}

This in CvTeam. I think it is correct, but I just want to double check, this code should loop over NumBuildingInfos, correct?

Also, could my problems be the result of BULL, as you said earlier:

Oh, if you have BULL merged into your mod you'll need to modify getBuildingAdditionalHappiness/Health(). This is the downside to keeping the arrays separate.

Other than that, I can't seem to find any other errors. I guess, if it's neither of those, I'll have to force some dummy values in and see what happens.

EmperorFool · Oct 16, 2009

Those initializations are correct if you want to start with every building getting a +1

and +1

. This would be great for testing purposes, but in the final code you need 0s instead of -1s in that code. BTW, you have "m_aiVictoryCountdown" in the assert messages. You should change those to the fields you are initializing, but this is a display-only issue.

Given these initializations, each city should be suffering increasing unhappiness and unhealthiness as you add buildings. Given that you don't see this, I would check the code in CvCity that calls CvPlayer::getTechBuildingHappiness/Health(). Can you post the latest code here?

As for BULL, the only thing that won't work is BULL's Building Actual Effects feature where it shows you how much

/

/

a building will add if you build it in the city. This is a CvGameTextMgr issue and thus display-only.

Afforess · Oct 16, 2009

EmperorFool said:
Those initializations are correct if you want to start with every building getting a +1 and +1. This would be great for testing purposes, but in the final code you need 0s instead of -1s in that code. BTW, you have "m_aiVictoryCountdown" in the assert messages. You should change those to the fields you are initializing, but this is a display-only issue.

Well, it's not doing anything, so the code must be borked up elsewhere. Oh, And I guess to can tell where I cloned the code from.

Given these initializations, each city should be suffering increasing unhappiness and unhealthiness as you add buildings. Given that you don't see this, I would check the code in CvCity that calls CvPlayer::getTechBuildingHappiness/Health(). Can you post the latest code here?

/* Returns the happiness added to a single building type by all acquired techs.
*
* eBuilding - the building type to look up
*/
int CvTeam::getTechExtraBuildingHappiness(BuildingTypes eIndex) const //Afforess
{
FAssertMsg(eIndex >= 0, "eIndex is expected to be non-negative (invalid Index)");
FAssertMsg(eIndex < GC.getNumBuildingInfos(), "eIndex is expected to be within maximum bounds (invalid Index)");
return m_paiTechExtraBuildingHappiness[eIndex];
}

Code:

/* Adds iChange to the current happiness added to a single building type by all acquired techs.
 * Called from processTech() when a tech is acquired or lost.
 *
 * eIndex - the building type to change
 * iChange - the additional happiness to add to the existing value
 */
void CvTeam::changeTechExtraBuildingHappiness(BuildingTypes eIndex, int iChange)
{
    FAssertMsg(eIndex >= 0, "eIndex is expected to be non-negative (invalid Index)");
    FAssertMsg(eIndex < GC.getNumBuildingInfos(), "eIndex is expected to be within maximum bounds (invalid Index)");

    if (iChange != 0)
    {
        setTechExtraBuildingHappiness(eIndex, getTechExtraBuildingHappiness(eIndex) + iChange);
    }
}

int CvTeam::getTechExtraBuildingHealth(BuildingTypes eIndex) const 
{
    FAssertMsg(eIndex >= 0, "eIndex is expected to be non-negative (invalid Index)");
    FAssertMsg(eIndex < GC.getNumBuildingInfos(), "eIndex is expected to be within maximum bounds (invalid Index)");
    return m_paiTechExtraBuildingHealth[eIndex];
}


void CvTeam::changeTechExtraBuildingHealth(BuildingTypes eIndex, int iChange)
{
    FAssertMsg(eIndex >= 0, "eIndex is expected to be non-negative (invalid Index)");
    FAssertMsg(eIndex < GC.getNumBuildingInfos(), "eIndex is expected to be within maximum bounds (invalid Index)");

    if (iChange != 0)
    {
        setTechExtraBuildingHealth(eIndex, getTechExtraBuildingHappiness(eIndex) + iChange);
    }
}

void CvTeam::AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(BuildingTypes eBuilding)
{
    int iI;
    int iLoopCity;
    CvCity* pLoopCity;
    
    for (iI = 0; iI < MAX_PLAYERS; iI++)
    {
        CvPlayer& kLoopPlayer = GET_PLAYER((PlayerTypes)iI);
        if (kLoopPlayer.isAlive() && kLoopPlayer.getTeam() == getID())
        {
            for (pLoopCity = kLoopPlayer.firstCity(&iLoopCity); pLoopCity != NULL; pLoopCity = kLoopPlayer.nextCity(&iLoopCity))
            {
                if (pLoopCity->getNumActiveBuilding(eBuilding) > 0)
                {
                    pLoopCity->AI_setAssignWorkDirty(true);
                }
            }
        }
    }
}
/* Sets the happiness added to a single building type by all acquired techs.
 *
 * eBuilding - the building type to change
 * iNewValue - the new happiness value for the building
 */
void CvTeam::setTechExtraBuildingHappiness(BuildingTypes eBuilding, int iValue)
{
    if (m_paiTechExtraBuildingHappiness[eBuilding] != iValue)
    {
        m_paiTechExtraBuildingHappiness[eBuilding] = iValue;
        AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(eBuilding);
    }
}

void CvTeam::setTechExtraBuildingHealth(BuildingTypes eBuilding, int iValue)
{
    if (m_paiTechExtraBuildingHealth[eBuilding] != iValue)
    {
        m_paiTechExtraBuildingHealth[eBuilding] = iValue;
        AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(eBuilding);
    }
}

In

Code:

void CvTeam::write(FDataStreamBase* pStream)
{

    pStream->Write(GC.getNumBuildingInfos(), m_paiTechExtraBuildingHappiness); //Afforess
    pStream->Write(GC.getNumBuildingInfos(), m_paiTechExtraBuildingHealth);

In

Code:

void CvTeam::read(FDataStreamBase* pStream)
{
    pStream->Read(GC.getNumBuildingInfos(), m_paiTechExtraBuildingHappiness); //Afforess
    pStream->Read(GC.getNumBuildingInfos(), m_paiTechExtraBuildingHealth);

In

Code:

void CvTeam::processTech(TechTypes eTech, int iChange)
{

for (iI = 0; iI < GC.getNumBuildingInfos(); ++iI)
    {
        changeTechExtraBuildingHappiness((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHappinessChanges(eTech) * iChange);
        changeTechExtraBuildingHealth((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHealthChanges(eTech) * iChange);
    }

It's in CvTeam, but same thing...

As for BULL, the only thing that won't work is BULL's Building Actual Effects feature where it shows you how much /// a building will add if you build it in the city. This is a CvGameTextMgr issue and thus display-only.

Okay.

EmperorFool · Oct 16, 2009

All that looks fine. What about the code in CvCity that calls those get() functions? CvTeam:

rocessTech() calls CvTeam::changeTechBuildingXXX(), but CvCity must call CvTeam::getTechBuildingXXX() or those values go nowhere. Didn't you add get() calls in [un]happyLevel() or getBuildingXXX()?

Afforess · Oct 16, 2009

EmperorFool said:
All that looks fine. What about the code in CvCity that calls those get() functions? CvTeam:rocessTech() calls CvTeam::changeTechBuildingXXX(), but CvCity must call CvTeam::getTechBuildingXXX() or those values go nowhere. Didn't you add get() calls in [un]happyLevel() or getBuildingXXX()?

Yes. I did.

Here's my CvCity code:

Code:

int CvCity::getBuildingGoodHealth(BuildingTypes eBuilding) const
{
    iHealth += std::max(0, GET_TEAM(getTeam()).getTechExtraBuildingHealth(eBuilding));
...
}

int CvCity::getBuildingBadHealth(BuildingTypes eBuilding) const
{
    iHealth += std::min(0, GET_TEAM(getTeam()).getTechExtraBuildingHealth(eBuilding));
...
}

int CvCity::getBuildingHappiness(BuildingTypes eBuilding) const
{
    iHappiness += GET_TEAM(getTeam()).getTechExtraBuildingHappiness(eBuilding);
...
}

That's it.

EmperorFool · Oct 17, 2009

There are two fields that hold the total building good/bad health (and ones for happiness) in CvCity. These must be changed any time you change the values in CvTeam as well. Since these functions call AI_setAssignWorkDirty() themselves, you need to change CvTeam::setTechBuildingXXX().

1. Don't call that new AI_setAssignWorkDirtyInEveryPlayerCity...() function. This will be handled by the code below.

2. Copy and modify the code in that function above to loop over every players' city and call changeBuildingGood/BadHealth(BuildingTypes, int) based on the old value and how many buildings of that type the city has.

Code:

int numBuildings = city->getNumActiveBuildings();
int oldHealth = numBuildings * getTechBuildingHealth(eIndex);
if (oldHealth > 0)
    city->changeBuildingGoodHealth(-oldHealth);
else if (oldHealth < 0)
    city->changeBuildingBadHealth(oldHealth);
int newHealth = numBuildings * getTechBuildingHealth(eIndex) + iChange;
if (newHealth > 0)
    city->changeBuildingGoodHealth(newHealth);
else if (newHealth < 0)
    city->changeBuildingBadHealth(-newHealth);

That's psuedocode obviously; it won't compile as-is. Use the code I posted previously to figure out the missing pieces. Unfortunately you're getting bit here because the SDK stores the total building good and bad health values in CvCity fields. You need to update these whenever anything that feeds them changes.

Afforess · Oct 17, 2009

EmperorFool said:
There are two fields that hold the total building good/bad health (and ones for happiness) in CvCity. These must be changed any time you change the values in CvTeam as well. Since these functions call AI_setAssignWorkDirty() themselves, you need to change CvTeam::setTechBuildingXXX().

1. Don't call that new AI_setAssignWorkDirtyInEveryPlayerCity...() function. This will be handled by the code below.

Okay...

2. Copy and modify the code in that function above to loop over every players' city and call changeBuildingGood/BadHealth(BuildingTypes, int) based on the old value and how many buildings of that type the city has.

The part that loops in AssignWorkDirtyInEveryPlayerCity...?

This is where you are losing me. What function should be looping and call changeBuildingGood/BadHealth...? A new one? The AI_setAssignWorkDirtyInEveryPlayerCity...()?

EmperorFool · Oct 17, 2009

Use the code in that new function that calls AI_setAssignWorkDirty() by copying it to both setXXX() functions that call it: setTechBuildingHappiness/Health(). They will each get a new loop that calculates the change for each city.

For example, if a city doesn't have the building being changed, you won't call changeBuildingGood/BadHealth() on it. If a city had 2 of those buildings, and you are changing the tech value for the building from +1 :health:

to +1

(a tech added +2 :yuck:

to the building), you need to call changeBuildingGoodHealth(-1) and changeBuildingBadHealth(+1).

Afforess · Oct 17, 2009

EmperorFool said:
Use the code in that new function that calls AI_setAssignWorkDirty() by copying it to both setXXX() functions that call it: setTechBuildingHappiness/Health(). They will each get a new loop that calculates the change for each city.

Let's see if I understand.

So I need to add that code that you posted to

Code:

AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(BuildingTypes eBuilding)

?

Or, do you mean to literally add the code to each set function?

EmperorFool · Oct 17, 2009

Code:

void setTechBuildingHealth(BuildingTypes eIndex, int iNewValue)
{
	if (m_piTechBuildingHealth[eIndex] != iNewValue)
	{
		int iOldValue = m_piTechBuildingHealth[eIndex];
		m_piTechBuildingHealth[eIndex] = iNewValue;
		
		int iI;
		int iLoopCity;
		CvCity* pLoopCity;
		
		for (iI = 0; iI < MAX_PLAYERS; iI++)
		{
			CvPlayer& kLoopPlayer = GET_PLAYER((PlayerTypes)iI);
			if (kLoopPlayer.isAlive() && kLoopPlayer.getTeam() == getID())
			{
				for (pLoopCity = kLoopPlayer.firstCity(&iLoopCity); pLoopCity != NULL; pLoopCity = kLoopPlayer.nextCity(&iLoop))
				{
					int iNumBuildings = pLoopCity->getNumActiveBuilding(eIndex)
					if (iNumBuildings > 0)
					{
						// Remove the old value
						if (iOldValue > 0)
						{
							pLoopCity->changeBuildingGoodHealth(-iOldValue * iNumBuildings);
						}
						else if (iOldValue < 0)
						{
							pLoopCity->changeBuildingBadHealth(iOldValue * iNumBuildings);
						}
						
						// Add the new value
						if (iNewValue > 0)
						{
							pLoopCity->changeBuildingGoodHealth(iNewValue * iNumBuildings);
						}
						else if (iNewValue < 0)
						{
							pLoopCity->changeBuildingBadHealth(-iNewValue * iNumBuildings);
						}
					}
				}
			}
		}
	}
}

Do the same thing for happiness except I think there's only one happiness function in CvCity: changeBuildingGoodHappiness(). Thus you'll need to muck with the values that get passed to that function, probably by using -iOldValue and removing the - from iNewValue.

xienwolf · Oct 17, 2009

Question: Did you test giving yourself the building in a city AFTER you gain the tech?

Afforess · Oct 17, 2009

xienwolf said:
Question: Did you test giving yourself the building in a city AFTER you gain the tech?

No Before.

And thanks for that code, Emperor Fool. I'll see how it goes.

EmperorFool · Oct 17, 2009

Yes I'm also thinking we need to add code to processBuilding() that calls changeBuildingGood/BadHealth/Happiness() in an ugly logic block.

Afforess · Oct 17, 2009

EmperorFool said:
Yes I'm also thinking we need to add code to processBuilding() that calls changeBuildingGood/BadHealth/Happiness() in an ugly logic block.

Apparently, not for Happiness, anyway. I haven't check healthiness code, but Happiness is now working!

EmperorFool · Oct 17, 2009

Did you test by adding the tech and then adding the building?

Afforess · Oct 17, 2009

Okay, I've done some more comprehensive testing. The SDK code seems to be doubling the effect of my XML. I set the XML to 100 health, the city received 200. Then, I set up another tech that would decrease the health, but an assert fired when I gave myself it. Badhealth > 0. So we are adding the bad health backwards, and doubling it somewhere. I'm going to look through the health code more extensively, and report my findings.

Afforess · Oct 17, 2009

EmperorFool said:
Did you test by adding the tech and then adding the building?

Yes, this works too.

EmperorFool · Oct 17, 2009

One thing to keep in mind is that in the XML, Info classes, and CvTeam arrays, :health:

is positive while :yuck:

is negative. Yet the totals set by changeBuildingGood/BadHealth() must remain positive (>= 0). This is why when the array value is < 0 its negative is passed to changeBuildingBadHealth() to "add" it to the bad health total.

There may be a problem with you storing these values in a single array in CvTeam instead of separate good and bad arrays.

Afforess · Oct 17, 2009

EmperorFool said:
One thing to keep in mind is that in the XML, Info classes, and CvTeam arrays, is positive while is negative. Yet the totals set by changeBuildingGood/BadHealth() must remain positive (>= 0). This is why when the array value is < 0 its negative is passed to changeBuildingBadHealth() to "add" it to the bad health total.

There may be a problem with you storing these values in a single array in CvTeam instead of separate good and bad arrays.

If I had to take a stab at it, it seems to me that the code is adding negative bad health (or good health) and then adding the good health, effectively doubling the result...

EmperorFool · Oct 17, 2009

Do the building changes show up in the building list of the city screen?

Troubles with Arrays and Arrays of Arrays...

The White Wizard

Deity

The White Wizard

Deity

The White Wizard

Deity

The White Wizard

Deity

The White Wizard

Deity

Deity

The White Wizard

Deity

The White Wizard

Deity

The White Wizard

The White Wizard

Deity

The White Wizard

Deity

Similar threads