Troubles with Arrays and Arrays of Arrays...

Afforess

The White Wizard
Joined
Jul 31, 2007
Messages
12,239
Location
Austin, Texas
I've been trying to add some new arrays to CvInfos, but have been having a more difficult time than usual, because the arrays are very different from what I'm used to.

See, I'm trying to clone and add several new arrays based on CvImprovementInfos::TechYieldchanges(); array. It appears to me that this array is really an array of an array. Regardless, I'm trying to move this array over to CvBuildingInfos, and add three similar arrays, TechHappinessChanges, TechHealthChanges, and TechCommerceChanges. I seemed to do fine just cloning TechYieldChanges, and then adapting it for TechCommerChanges. Where I really have problems is TechHappinessChanges and TechHealthChanges.

I'm fairly sure I'm doing this wrong, so that's why I'm asking for help.

Okay, here's a spot where I am confused on what to do:

This code is correct
Code:
int CvBuildingInfo::getTechCommerceChanges(int i, int j) const
{
    FAssertMsg(i < GC.getNumTechInfos(), "Index out of bounds");
    FAssertMsg(i > -1, "Index out of bounds");
    FAssertMsg(j < NUM_COMMERCE_TYPES, "Index out of bounds");
    FAssertMsg(j > -1, "Index out of bounds");
    return m_ppiTechCommerceChanges[i][j]; 
}

int* CvBuildingInfo::getTechCommerceChangesArray(int i)
{
    return m_ppiTechCommerceChanges[i];
}

This code is incorrect, but I'm not sure how to fix it. What do I use instead of NUM_YIELD_TYPES?
Code:
int CvBuildingInfo::getTechHealthChanges(int i, int j) const
{
    FAssertMsg(i < GC.getNumTechInfos(), "Index out of bounds");
    FAssertMsg(i > -1, "Index out of bounds");
//    FAssertMsg(j < NUM_YIELD_TYPES, "Index out of bounds");
//    FAssertMsg(j > -1, "Index out of bounds");
    return m_ppiTechHealthChanges[i][j]; 
}

int* CvBuildingInfo::getTechHealthChangesArray(int i)
{
    return m_ppiTechHealthChanges[i];
}

Personally, I'm thinking that the Commerce and Yield changes are array's of arrays, but the happiness and healthiness are not, and I should do those like I do every other array. Am I correct, or way off base?
 
You are correct, Commerces and Yields are arrays of arrays (there are multiple kinds of commerces and yields), but Health and Happiness you just want a simple array (there are TECHNICALLY two kinds of health (Health & Disease) and happiness (Happy & Angry), but in reality, they are just different ways of saying positive and negative)
 
You are correct, Commerces and Yields are arrays of arrays (there are multiple kinds of commerces and yields), but Health and Happiness you just want a simple array (there are TECHNICALLY two kinds of health (Health & Disease) and happiness (Happy & Angry), but in reality, they are just different ways of saying positive and negative)

Okay, thanks for confirming my suspicions. I'm going to plod forward and see where I get.
 
I have a quick question, in

Code:
bool CvBuildingInfo::read(CvXMLLoadUtility* pXML)
{

I'm not sure what to pass through for my Happiness and Healthness arrays...

Here's what I have now, but I'm sure their wrong. (See bolded sections)

Code:
	pXML->SetVariableListTagPair(&m_piTechHappinessChanges, "TechHappinessChanges", sizeof(GC.getNumTechInfos[B]((BuildingClassTypes)0)), GC.getNumBuildingClassInfos()[/B]);
	pXML->SetVariableListTagPair(&m_piTechHealthChanges, "TechHealthChanges", sizeof(GC.getNumTechInfos([B](BuildingClassTypes)0)), GC.getNumBuildingClassInfos()[/B]);
 
Okay, I believe I've got my TechCommerceChanges and TechYieldChanges 100% working in the SDK, but I'm still faltering with the TechHappinessChanges and TechHealthChanges...

I still have the above problems with those two, and more. I looked at CvTeam::ProcessTechs() and added two lines there for them:

Code:
			GET_PLAYER((PlayerTypes)iI).changeTechHealthChanges(GC.getTechInfo(eTech).getTechHealthChanges() * iChange);
			GET_PLAYER((PlayerTypes)iI).changeTechHappinessChanges(GC.getTechInfo(eTech).getTechHappinessChanges() * iChange);

But it appears I need to add those somewhere in CvPlayer... but I have no clue where I would do this.

Also, despite having my TechCommerceChanges and TechYieldChanges seem to compile fine, I also am having issues with the schema. Here's my schema change, but it gives errors every time I try to start the game.
Code:
	<ElementType name="TechCommerces" content="eltOnly">
		<element type="iCommerce" minOccurs="0" maxOccurs="*"/>
	</ElementType>
	<ElementType name="TechCommerceChange" content="eltOnly">
		<element type="PrereqTech"/>
		<element type="TechCommerces"/>
	</ElementType>
	<ElementType name="TechCommerceChanges" content="eltOnly">
		<element type="TechCommerceChange" minOccurs="0" maxOccurs="*"/>
	</ElementType>	
	<ElementType name="TechYields" content="eltOnly">
		<element type="iYield" minOccurs="0" maxOccurs="*"/>
	</ElementType>
	<ElementType name="TechYieldChange" content="eltOnly">
		<element type="PrereqTech"/>
		<element type="TechYields"/>
	</ElementType>
	<ElementType name="TechYieldChanges" content="eltOnly">
		<element type="TechYieldChange" minOccurs="0" maxOccurs="*"/>
	</ElementType>

Help would be appreciated.
 
There is an older post of basically the same mod
http://forums.civfanatics.com/showthread.php?t=256362&highlight=techcommerce
I tried to import that one into my mod, but had also problems with starting the game. I thought about replacing the TechCommerces and TechYields elements with the already implied CommerceChanges and YieldChanges because the same thing worked for me with the CommerceModifiers element for the BonusCommerceModifier mod. However, other things got in my way :).
 
Code:
pXML->SetVariableListTagPair(&m_piTechHappinessChanges, "TechHappinessChanges", sizeof(GC.getNumTechInfos((BuildingClassTypes)0)), GC.getNumBuildingClassInfos());

Notice how you changed only one case of BUILDING to TECH after your variable name:

sizeof(GC.getNumTechInfos((BuildingClassTypes)0)), GC.getNumBuildingClassInfos()

So you are fetching the Tech Information of a Buildingclass object. Bad juju. Then you are making this array size of BuildingClasses, instead of Techs. Bad juju in a tutu.

Code:
GET_PLAYER((PlayerTypes)iI).changeTechHappinessChanges(GC.getTechInfo(eTech).getTechHappinessChanges() * iChange);

Right at the start:

GET_PLAYER((PlayerTypes)iI)

You just started talking to CvPlayer. So it expects to find CvPlayer::changeTechHappinessChanges, which won't exist till you write it. Then you need to use a getTechHappiness function when the city is checking what the total happy level is. Look for Happiness in CvCity for that one, then add a call to check their owner's CvPlayer data for added Happiness from this function.


For your schema, make sure that iCommerce and iYield are defined somewhere in the schema as INT containers, and PrereqTech as a Text container. If those are defined, make sure none of the ones you posted here are double defined. If it isn't either of those two, then does the error give any specific details at all?
 
There is an older post of basically the same mod
http://forums.civfanatics.com/showthread.php?t=256362&highlight=techcommerce
I tried to import that one into my mod, but had also problems with starting the game. I thought about replacing the TechCommerces and TechYields elements with the already implied CommerceChanges and YieldChanges because the same thing worked for me with the CommerceModifiers element for the BonusCommerceModifier mod. However, other things got in my way :).

Oh man. I love to find out I'm reinventing the wheel. Well, I am so close I might as well finish though. But my Health and Happiness changes are different too, I suppose...
Code:
pXML->SetVariableListTagPair(&m_piTechHappinessChanges, "TechHappinessChanges", sizeof(GC.getNumTechInfos((BuildingClassTypes)0)), GC.getNumBuildingClassInfos());
Notice how you changed only one case of BUILDING to TECH after your variable name:

sizeof(GC.getNumTechInfos((BuildingClassTypes)0)), GC.getNumBuildingClassInfos()

So you are fetching the Tech Information of a Buildingclass object. Bad juju. Then you are making this array size of BuildingClasses, instead of Techs. Bad juju in a tutu.

So it should be

Code:
sizeof(GC.getNumTechInfos((TechTypes)0)), GC.getNumBuildingClassInfos()
right?
Code:
GET_PLAYER((PlayerTypes)iI).changeTechHappinessChanges(GC.getTechInfo(eTech).getTechHappinessChanges() * iChange);
Right at the start:

GET_PLAYER((PlayerTypes)iI)

You just started talking to CvPlayer. So it expects to find CvPlayer::changeTechHappinessChanges, which won't exist till you write it. Then you need to use a getTechHappiness function when the city is checking what the total happy level is. Look for Happiness in CvCity for that one, then add a call to check their owner's CvPlayer data for added Happiness from this function.

Okay, I'll see how that goes.

For your schema, make sure that iCommerce and iYield are defined somewhere in the schema as INT containers, and PrereqTech as a Text container. If those are defined, make sure none of the ones you posted here are double defined. If it isn't either of those two, then does the error give any specific details at all?

I figured out my schema issues on my own. I had accidentally deleted a </ElementType>.
 
No, you want

sizeof(GC.getNumTechInfos((TechTypes)0)), GC.getNumTechInfos().

That points you at the begining of the tech list and the end of it. Your repost still pointed at the end of BuildingInfos

Okay, Thanks. Your help, in combination with the TAB mod that SaibotLieh posted got me on the right direction there. I have my TechCommerceChanges and TechYieldChanges 100% working. Now, I need help with the Happiness and Healthiness Tech Arrays. I feel like I need to create another array in CvPlayer to get it to work right, but there are no array's in CvPlayer, which is leading me to doubt that conclusion...
 
You ought to be able to just leave it in the CvTeam datastream. You'll only use the information in a city, and he'll have to check his owner for the information anyway. So may as well check the owner's team instead of the owner himself.

And you shouldn't need to store it as an array (the health/happiness), but just as an integer which counts how many total bonus Happy/Health you have, and is increased when you gain a tech that increases it, decreased if you lose such a tech (and opposite if the tech had a negative mod for the field of course)
 
Okay. Well..., I'm still having issues with these two lines. I'm not even sure if they should be there, and if not, I don't know where else they go...

Code:
[plain]
CvTeam::ProcessTechs()
{
...			GET_PLAYER((PlayerTypes)iI).changeTechHealthChanges(GC.getBuildingInfo(eBuilding).getTechHealthChanges() * iChange);
			GET_PLAYER((PlayerTypes)iI).changeTechHappinessChanges(GC.getBuildingInfo(eBuilding).getTechHappinessChanges() * iChange);
...}
[/plain]

I'm getting compiler errors telling me that "eBuilding" is an undeclared identifier...
 
I read the first half, but that just ended up having me read things that were already solved, so I jumped to the end. :)

CvPlayer or CvTeam needs two int arrays each with GC.getNumBuildingInfos() elements to hold the happiness and health changes from techs. As xienwolf said, it makes sense to stick it on the team so you don't have to replicate the data for each player in the team. Anything that will be the same value for all members of a team should live in CvTeam.

I would recommend that you move the arrays from CvBuildingInfo to CvTechInfo. When I think of a typical use case, my mind more readily grasps that a single tech will provide happiness/health to various buildings rather than the other way around. This isn't mandatory, but it will make CvTeam::processTech() simpler.

Finally, do you want the tech to modify buildings or building classes. Say you make Biology provide +2 :health: to Grocer. Should it affect all Grocer UBs, or must the modder assign the bonus to any UBs they add?

In CvTeam::processTech(), you need to look up the CvTechInfo and then loop over its happiness/health changes for all buildings, adding them to the team's arrays.

Code:
CvTechInfo& kTech = GC.getTechInfo(eTech);
for (iI = 0; iI < GC.getNumBuildingInfos(); ++iI)
{
	changeBuildingTechHappiness((BuildingTypes)iI, kTech.getBuildingHappiness(iI) * iChange);
	changeBuildingTechHealth((BuildingTypes)iI, kTech.getBuildingHealth(iI) * iChange);
}

Actually, leaving the arrays in place doesn't make the code any more complicated.

Code:
for (iI = 0; iI < GC.getNumBuildingInfos(); ++iI)
{
	changeBuildingTechHappiness((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHappiness(eTech) * iChange);
	changeBuildingTechHealth((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHealth(eTech) * iChange);
}

Another suggestion is to change the arrays in CvTeam to remove the Tech part, i.e. changeBuildingHappiness(). These could then hold extra happiness/health for buildings from any source, whether that's techs, civics, whatever. Hmm, is that there already? I forget. Perhaps it's already on CvPlayer as BuildingExtraHappiness?
 
CvPlayer or CvTeam needs two int arrays each with GC.getNumBuildingInfos() elements to hold the happiness and health changes from techs. As xienwolf said, it makes sense to stick it on the team so you don't have to replicate the data for each player in the team. Anything that will be the same value for all members of a team should live in CvTeam.

So CvTeam needs an array?
Finally, do you want the tech to modify buildings or building classes. Say you make Biology provide +2 :health: to Grocer. Should it affect all Grocer UBs, or must the modder assign the bonus to any UBs they add?

The second way may be more pedantic, but I think it adds more flexibility, and flexibility is good. (As you always say.)
Actually, leaving the arrays in place doesn't make the code any more complicated.
Code:
for (iI = 0; iI < GC.getNumBuildingInfos(); ++iI)
{
    changeBuildingTechHappiness((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHappiness(eTech) * iChange);
    changeBuildingTechHealth((BuildingTypes)iI, GC.getBuildingInfo((BuildingTypes)iI).getTechHealth(eTech) * iChange);
}
Hmm. I added that, but I can't get it to compile, because the functions changeBuildingTechHappiness don't exist in CvTeam. That's because I haven't made the arrays, correct?

Another suggestion is to change the arrays in CvTeam to remove the Tech part, i.e. changeBuildingHappiness(). These could then hold extra happiness/health for buildings from any source, whether that's techs, civics, whatever. Hmm, is that there already? I forget. Perhaps it's already on CvPlayer as BuildingExtraHappiness?

I think this would be too confusing. Civics can already modify happiness and health fine, and so can most other things.
 
Ah, Good thing EF came by. I had lost track of the final goal (placing health/happy on buildings due to techs) and was starting to give bad advice (was advising on how to make a tech alone give health/happy). The CvInfos pieces I had advised properly on, but saying there is no need for an array in CvTech/CvPlayer was very wrong.

If there is already a CvPlayer::changeBuildingHealth (or similar, changeBuildingExtraHealth rings a bit of a bell for me), I agree that you ought to use it for tracking these changes.
 
Yes, going with buildings instead of their classes is more flexible. I was just pointing out the difference in case you hadn't considered it and went with buildings only because that's what you copied as an example.

AFAIK civics don't provide happiness or health to buildings yet. I see that CvPlayer has a processBuilding() function as well. There are two functions it calls that are semi-related to your changes.

1. changeBuildingHappiness(): This doesn't do what it's name suggests. It is a single happiness change for the player that is the sum of the happiness changes for all buildings processed. This would allow a building in one city to provide +2 :) for every city.

2. changeBuildingExtraHappiness(): This does what you're doing: provide happiness to any city that builds a particular building (+1 :) per Barracks). There is nothing like this for health, but duplicating this would be trivial. The XML tag applies to building classes, but processBuilding() looks up the UB to get to the building which the array uses as an index.

Now, should you put these arrays in CvTeam or CvPlayer? Several things factor in.

1. Do you insist on splitting the extra happiness based on its source? I don't think you understand my reasoning here because merging several sources into one array wouldn't be any more or less confusing. If <x> provides +2 :) for every Cothon, who cares if <x> is a tech or civic or building or specialist? Every Cothon gets +2 :), period. Keeping sources separate will require creating a new building-happiness array for each source of building-happiness.

2. Will you want to have sources (<x> above) that are not team-based? Only techs are team-based, so if you want to have a wonder that provides this or a civic, keep these arrays in CvPlayer. It's more flexible this way. If you go with having a separate array for each source of building-happiness, then put the tech-based one in CvTeam.

3. Do you want to use the existing building-extra-happiness arrays (I think this is a great idea!), keep the arrays in CvPlayer and duplicate the happiness one to make one for building-extra-health.

Yes, your compile errors are because those functions do not exist (no arrays). You've demonstrated a good aptitude for SDK coding so far, so I've been spelling things out less and less, leaving it up to you to fill in the supporting bits. :goodjob:
 
2. changeBuildingExtraHappiness(): This does what you're doing: provide happiness to any city that builds a particular building (+1 :) per Barracks). There is nothing like this for health, but duplicating this would be trivial. The XML tag applies to building classes, but processBuilding() looks up the UB to get to the building which the array uses as an index.

Okay, I;m going to look at this.

Now, should you put these arrays in CvTeam or CvPlayer? Several things factor in.

1. Do you insist on splitting the extra happiness based on its source? I don't think you understand my reasoning here because merging several sources into one array wouldn't be any more or less confusing. If <x> provides +2 :) for every Cothon, who cares if <x> is a tech or civic or building or specialist? Every Cothon gets +2 :), period. Keeping sources separate will require creating a new building-happiness array for each source of building-happiness.

For once, I'm going to go against your better judgment here, and insist that the array be for tech's only. (We'll see who ends up being wiser...)

3. Do you want to use the existing building-extra-happiness arrays (I think this is a great idea!), keep the arrays in CvPlayer and duplicate the happiness one to make one for building-extra-health.

This sounds reasonable to me.


You've demonstrated a good aptitude for SDK coding so far

Thanks. I've gotten a lot better on my own, but what I still can't do is more than what I can do...
 
The advantage which you gain by having an array which is JUST for the technologies is if you make a mouse-over help box which explains WHERE the bonuses come from. If you ever do that, having seperate arrays saves time. Otherwise, the disadvantage is you increase the size of a savegame considerably for not much of a good reason.
 
Thanks, xienwolf. I knew there had to be another good reason to keeping the arrays separate. You could still write code in CvGameTextMgr to calculate the :)/:health: from techs, but having them in separate arrays makes that unnecessary. Don't forget to add this part.

Using the existing building-extra-happiness array in CvPlayer means you won't have a separate array for techs, though. So you either use the existing CvPlayer array and copy it for health or you create two new arrays in CvTeam for techs. You cannot combine the two options into one.

Coding decisions tend to be about weighing advantages rather than having one clearly correct choice. Your decision to keep separate arrays for techs is a fine choice; it just means you need two new arrays in CvTeam.

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

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. :mischief:
 
The advantage which you gain by having an array which is JUST for the technologies is if you make a mouse-over help box which explains WHERE the bonuses come from. If you ever do that, having seperate arrays saves time. Otherwise, the disadvantage is you increase the size of a savegame considerably for not much of a good reason.

Good point. However, RoM saves usually top 2-3mb in later games anyway, so a few more kb isn't going to hurt anyone. Also, I'm not sure I'd ever use the extra functionality.

Thanks, xienwolf. I knew there had to be another good reason to keeping the arrays separate. You could still write code in CvGameTextMgr to calculate the :)/:health: from techs, but having them in separate arrays makes that unnecessary. Don't forget to add this part.

Using the existing building-extra-happiness array in CvPlayer means you won't have a separate array for techs, though. So you either use the existing CvPlayer array and copy it for health or you create two new arrays in CvTeam for techs. You cannot combine the two options into one.

Coding decisions tend to be about weighing advantages rather than having one clearly correct choice. Your decision to keep separate arrays for techs is a fine choice; it just means you need two new arrays in CvTeam.

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

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. :mischief:

I do have BULL in, so I guess I'll need to update that as well. I'm going to get as far as I can later on, and report my progress.
 
Top Bottom