Troubles with Arrays and Arrays of Arrays...

[Afforess]

You need to add a call to CvTeam::getTechBuildingHappiness/Health(eBuilding) in CvCity::getBuildingHappiness(BuildingTypes eBuilding). Same for health.

Something like this?

    for (iI = 0; iI < GC.getNumTechInfos(); iI++)
    {
        iHappiness += getNumActiveBuilding((BuildingTypes)iI) * GC.getBuildingInfo((BuildingTypes) iI).getTechHappinessChanges(eTech);
    }

 

[EmperorFool]

No, you can use the arrays you'll be adding to CvTeam, exposed via CvTeam::getTechBuildingHappiness():

iHappiness += GET_TEAM(getTeam()).getTechBuildingHappiness(eBuilding);

You don't need to loop over the techs because the above function exposes the arrays that have all the techs the team has acquired already summed up.

 

[xienwolf]

Also, look closely at that loop you posted. It is a loop up to the number of technologies, but then you use that number to get a buildingtype. If it helps you to avoid such mistakes, don't use "iI", but rather use "iBuilding" or "iTech" instead, so you remember to cast to the right variable type.

 

[Afforess]

Maybe I'm just not understanding something, but what do I put here:

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)
	{
		m_paiTechExtraBuildingHealth[eIndex] += iChange;

		[B]update();[/B]
	}
}

There seems to be no CvTeam::UpdateHealth

Also, in CvCity,

iHappiness += GET_TEAM(getTeam()).getTechBuildingHappiness(eBuilding);

this sets off errors that eBuilding is an undeclared variable. Do I need to make a CvCity::getTechBuildingHappiness to get around that?

 

[EmperorFool]

There's nothing to update after you've set the value in the array. If you want you can use the change()/set()/get() combination of accessors to match the BTS codebase. change() calls set(), passing in get() + iChange. set() stores whatever value is passed in. get() returns that value, of course.

Where are you adding that last line of code? You should put it in CvCity::getBuildingHappiness(BuildingTypes eBuilding) which definitely declares eBuilding as its only parameter.

 

[Afforess]

There's nothing to update after you've set the value in the array. If you want you can use the change()/set()/get() combination of accessors to match the BTS codebase. change() calls set(), passing in get() + iChange. set() stores whatever value is passed in. get() returns that value, of course.

Where are you adding that last line of code? You should put it in CvCity::getBuildingHappiness(BuildingTypes eBuilding) which definitely declares eBuilding as its only parameter.

Okay, so I can just get rid of the Update line then?

And I way trying to add that to CvCity::HappyLevel(), that's what I was doing wrong.

Okay, that compiles fine. Now, for healthiness, do I use CvCity::getBuildingGoodHealth(BuildingTypes eBuilding) and CvCity::getBuildingBadHealth(BuildingTypes eBuilding)? Or do I need a different function?

 

[EmperorFool]

In CvCity when these things change it calls CvCity::AI_setAssignWorkDirty(). Perhaps the set() function in CvTeam should call this for all players' cities in the team. There might already be a function on CvPlayer that does this--look for all references to AI_setAssignWorkDirty().

It looks like those two functions are all you need to add code to for the health side of things.

 

[Afforess]

In CvCity when these things change it calls CvCity::AI_setAssignWorkDirty(). Perhaps the set() function in CvTeam should call this for all players' cities in the team. There might already be a function on CvPlayer that does this--look for all references to AI_setAssignWorkDirty().

There are only two references to AI_setAssignWorkDirty() in CvPlayer, and both relate to one'
s capital city.

 

[EmperorFool]

Here's what needs to happen. When a tech is acquired it calls changeTechBuildingHappiness/Health(). When that happens, every city for every player in the team that has at least one of those buildings needs to be told to reassign citizens via CvCity::AI_setAssignWorkDirty().

for all players
  if player is in team
    for all player's cities
      if city has building
        city.AI_setAssignWorkDirty()

You can either put the same code above into both functions-- and --or create a new function in CvTeam or CvPlayer that takes the building that has been changed and call that function from the two / functions.

There is code that already loops over all player cities in the SDK, probably search CvPlayer for pLoopCity. It involves having an int iterator plus the CvCity pointer IIRC.

 

[Afforess]

Here's what needs to happen. When a tech is acquired it calls changeTechBuildingHappiness/Health(). When that happens, every city for every player in the team that has at least one of those buildings needs to be told to reassign citizens via CvCity::AI_setAssignWorkDirty().

for all players
  if player is in team
    for all player's cities
      if city has building
        city.AI_setAssignWorkDirty()

You can either put the same code above into both functions-- and --or create a new function in CvTeam or CvPlayer that takes the building that has been changed and call that function from the two / functions.

There is code that already loops over all player cities in the SDK, probably search CvPlayer for pLoopCity. It involves having an int iterator plus the CvCity pointer IIRC.

Something like this:

    for (iI = 0; iI < MAX_PLAYERS; iI++)
                        {
                            if (GET_PLAYER((PlayerTypes)iI).isAlive())
                            {
                                if (GET_PLAYER((PlayerTypes)iI).getTeam() == getTeam())
                                {
                                    if (GC.getBuildingInfo(eIndex).isTeamShare() || (iI == getOwnerINLINE()))
                                    {
                                        for (pLoopCity = GET_PLAYER((PlayerTypes)iI).firstCity(&iLoop); pLoopCity != NULL; pLoopCity = GET_PLAYER((PlayerTypes)iI).nextCity(&iLoop))
                                        {
                                            pLoopCity->setPopulation(std::max(1, (pLoopCity->getPopulation() + iChangeNumRealBuilding * GC.getBuildingInfo(eIndex).getGlobalPopulationChange())));
                                            pLoopCity->AI_updateAssignWork();

 

[EmperorFool]

Something along those lines.

1. Store the CvPlayer into a reference and use it throughout -- CvPlayer& kPlayer = GET_PLAYER(...);
2. Remove check for isTeamShare(). Instead, check the city if it has the building.
3. Remove the call to setPopulation().

 

[Afforess]

Something along those lines.

1. Store the CvPlayer into a reference and use it throughout -- CvPlayer& kPlayer = GET_PLAYER(...);
2. Remove check for isTeamShare(). Instead, check the city if it has the building.
3. Remove the call to setPopulation().

Is this any better?

for (iI = 0; iI < MAX_PLAYERS; iI++)
                        {
                         CvPlayer& kPlayer = GET_PLAYER((PlayerTypes)iPlayer);
                            if (kPlayer.isAlive() && kPlayer.getTeam() == iTeam)
                            {
                                if (GET_PLAYER((PlayerTypes)iI).getTeam() == getTeam())
                                {
                                  if (GC.getBuildingInfo((BuildingTypes).getTechBuildingHappiness()))
                                    {
                                        for (pLoopCity = GET_PLAYER((PlayerTypes)iI).firstCity(&iLoop); pLoopCity != NULL; pLoopCity = GET_PLAYER((PlayerTypes)iI).nextCity(&iLoop))
                                        {
                                            pLoopCity->AI_updateAssignWork();

 

[EmperorFool]

More like this:

	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))
			{
				if (pLoopCity->getNumActiveBuilding(eBuilding) > 0)
				{
					pLoopCity->AI_setAssignWorkDirty();
				}
			}
		}
	}

This needs to go into both changeTechBuildingXXX(BuildingTypes eBuilding) functions. Actually, if you followed the change()/get()/set() pattern I lined out above, it should go in the set() functions, probably guarded by a test that checks that the value is being set to a different value:

if (m_paiTechBuildingHappiness[eBuilding] != iValue)
{
    m_paiTechBuildingHappiness[eBuilding] = iValue;
    
    ... looping code above ...
}

Even better, put the code at the top of this post into its own function called CvTeam::AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(BuildingTypes eBuilding).

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(&iLoop))
			{
				if (pLoopCity->getNumActiveBuilding(eBuilding) > 0)
				{
					pLoopCity->AI_setAssignWorkDirty();
				}
			}
		}
	}
}

void CvTeam::setTechBuildingHappiness(BuildingTypes eBuilding, int iValue)
{
	if (m_paiTechBuildingHappiness[eBuilding] != iValue)
	{
		m_paiTechBuildingHappiness[eBuilding] = iValue;
		[B]AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(eBuilding);[/B]
	}
}

void CvTeam::setTechBuildingHealth(BuildingTypes eBuilding, int iValue)
{
	if (m_paiTechBuildingHealth[eBuilding] != iValue)
	{
		m_paiTechBuildingHealth[eBuilding] = iValue;
		[B]AI_setAssignWorkDirtyInEveryPlayerCityWithActiveBuilding(eBuilding);[/B]
	}
}

 

[Afforess]

Okay, I got that set up, with the new Set functions and the CvTeam::AI_setAssignWorkDirtyInEveryPlayerCityWith ActiveBuilding(BuildingTypes eBuilding).

Where do I need to change my other functions to call the set functions? As it is now, they don't seem to be used at all. I'm a little confused on how to move forward.

 

[EmperorFool]

The pattern for get()/set()/change() looks like this:

int getFoo()
{
    return m_iFoo;
}

void setFoo(int iNewValue)
{
    if (m_iFoo != iNewValue)
    {
        m_iFoo = iNewValue;

        ... anything that should happen when foo changes ...
    }
}

void changeFoo(int iValue)
{
    setFoo(getFoo() + iValue);
}

CvTeam:rocessTechnology() should call the changeTechBuildingHappiness/Health() functions which call their respective set() functions.

 

[Afforess]

The end of the Change bit should be like this?

setTechExtraBuildingHappiness(GC.getNumTechInfos().getTechHappinessChanges()) += iChange;

Because that gives me errors...

 

[EmperorFool]

setTechExtraBuildingHappiness() takes a BuildingTypes and an int value. setTechExtraBuildingHappiness() takes the same parameters except the int value should be the final value (original plus change). Just pass in eBuilding and call set() with eBuilding and get() + iChange.

setTechExtraBuildingHappiness(eBuilding, getTechExtraBuildingHappiness(eBuilding) + iChange);

I recommend writing documentation comments for these functions to help you understand a) what they do and b) why they take the parameters they take.

/* Returns the happiness added to a single building type by all acquired techs.
 *
 * eBuilding - the building type to look up
 */
int getTechBuildingHappiness(BuildingTypes eBuilding)
{ ... }

/* 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 setTechBuildingHappiness(BuildingTypes eBuilding, int iNewValue)
{ ... }

/* 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.
 *
 * eBuilding - the building type to change
 * iChange - the additional happiness to add to the existing value
 */
void setTechBuildingHappiness(BuildingTypes eBuilding, int iChange)
{ ... }

Here you can see that eBuilding is used for the same thing (looking up in the array) with all three functions. And you can see the relationship between the value returned by get() and those passed to set() and change().

 

[Afforess]

Okay, thanks. I will add that documentation, so I remember this in the future.

My next question:
Is there anything else left to do (Other than CvGameTextMgr, I can handle that [I hope].)?

Edit:
Spoke too soon. I'm getting "eBuilding" as an undeclared variable. So I need to change the "CvTeam::changeTechExtraBuildingHappiness(BuildingTypes eIndex, int iChange)" to
"CvTeam::changeTechExtraBuildingHappiness(BuildingTypes eIndex, int iChange, eBuilding)" to declare it, or is there something else I'm missing?

 

[EmperorFool]

Ah, I used eBuilding where you use eIndex. I typically only use eIndex for CommerceTypes and YieldsTypes since that's what the Firaxis code does. Sometimes it will also use eIndex for a BuildingTypes or PlayerTypes, but that's just overly confusing.

That's like using iI and iJ as loop counters which of course I'd never do myself.

 

[Afforess]

Ah, I used eBuilding where you use eIndex. I typically only use eIndex for CommerceTypes and YieldsTypes since that's what the Firaxis code does. Sometimes it will also use eIndex for a BuildingTypes or PlayerTypes, but that's just overly confusing.

Ah, I see. For the sake of not wanting to do a clean rebuild to change the headers, I'll use eIndex.

That's like using iI and iJ as loop counters which of course I'd never do myself.

What do you have against i,j & k? They all have a special place in every programmers heart.