Adding new Resource Yields to Future Techs

One issue is that you're calling CvReligionInfo::getReligionCommerce() and getHolyCityCommerce() in getReligionYieldsByReligion(). Is this intended? Are you using those functions because you haven't added the yield equivalents to CvReligionInfo yet?

Are you initializing the m_aiReligionYields array in CvCity::init()?

Are you calling updateReligionYields() everywhere that updateReligionCommerce() is called? You should probably create CvCity::updateReligionYield() that loops over all yields, calling updateReligionYield(eYield) for each. And you'll want CvPlayer::updateReligionYield() that loops over all the player's cities, calling updateReligionYield() on each.

You say it hangs? It doesn't crash to desktop and instead locks up your system?

BTW, since these functions all affect a single yield, I would drop the "-s" suffix on all four, e.g. getReligionYield().​
 
  1. I thought I could get away with using the exisiting getReligionCommerce() and getHolyCityCommerce() because I thought they wre just couting up the number of cities with the religion in question and checking whether or not the city was the Holy City, and so I could get away with using them too. I've decided I'm wrong on that...
  2. I have m_aiReligionYields listed, but like this:
    Code: 
    [B]CvCity::CvCity()[/B]
	m_aiTradeYield = new int[NUM_YIELD_TYPES];
	m_aiReligionYields = new int[NUM_YIELD_TYPES];
	m_aiCorporationYield = new int[NUM_YIELD_TYPES];

... etc...
[B]CvCity::~CvCity()[/B]
	SAFE_DELETE_ARRAY(m_aiTradeYield);
	SAFE_DELETE_ARRAY(m_aiReligionYields);
	SAFE_DELETE_ARRAY(m_aiCorporationYield);

... etc...
[B]void CvCity::reset(int iID, PlayerTypes eOwner, int iX, int iY, bool bConstructorCall)[/B]
		m_aiTradeYield[iI] = 0;
		m_aiReligionYields[iI] = 0;
		m_aiCorporationYield[iI] = 0;

...etc...
[B]void CvCity::read(FDataStreamBase* pStream)[/B]
	pStream->Read(NUM_YIELD_TYPES, m_aiTradeYield);
	pStream->Read(NUM_YIELD_TYPES, m_aiReligionYields);
	pStream->Read(NUM_YIELD_TYPES, m_aiCorporationYield);

... etc...
[B]void CvCity::write(FDataStreamBase* pStream)[/B]
	pStream->Write(NUM_YIELD_TYPES, m_aiTradeYield);
	pStream->Write(NUM_YIELD_TYPES, m_aiReligionYields);
	pStream->Write(NUM_YIELD_TYPES, m_aiCorporationYield);
    So, nothing in CvCity::init()...
  3. umm... no? I hadn't done a search for updateRelegionCommerce, I thought it was just used by getRelegionCommerce...
    And I haven't got anything in CvPlayer. It makes perfect sense now that you mention it of course, because that's where the game will count up how many cities have a given religion, not CvCity...
  4. Ya, the game just stops when I try to found a city. Music stops, GUI goes dead. The mouse still works, but that's it. I have to hit the "Windows" button on the keyboard to get to the desktop and the "Civ IV has encountered a fatal error, Full Dump/No Dump" dialogue box.
  5. Actually, I added the -s suffix deliberately. CvCity already has a variable called ReligionYield, under void CvCity:: processVoteSourceBonus(VoteSourceTypes eVoteSource, bool bActive), and I wanted to avoid confusion.

Thanks EF. I'm going to go trolling through CvPlayer now. Yeesh...
 
OK, added the following to CvPlayer.cpp, and am now being told that the function cannot take zero arguments. So, I know it's referring to the empty (), but I haven't a clue what to put in there, as both updateReligionCommerce & updateCorporation have empty ()...
Code: 
void CvPlayer::updateReligionYields()
{
	int iLoop;
	for (CvCity* pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
	{
		pLoopCity->updateReligionYields();
	}
}

EDIT (4:00 PM Sunday):

Found what I thought was an error in my code in CvCity.h, void updateReligionYields(YieldTypes eIndex);, changed it to void updateReligionYields();. That didn't work either, VS2008 gave me this error:
CvPlayer.obj : error LNK2019: unresolved external symbol "public: void __thiscall CvCity::updateReligionYields(void)" (?updateReligionYields@CvCity@@QAEXXZ) referenced in function "public: void __thiscall CvPlayer::updateReligionYields(void)" (?updateReligionYields@CvPlayer@@QAEXXZ)
Click to expand...
 
You need two updateReligionYields() functions: one taking zero parameters, and one taking the YieldTypes. You have the latter already; you need to add the zero-argument version that loops over all the yields, calling the other one for each.

Code: 
void CvCity::updateReligionYields()
{
    for (i = 0; i < NUM_YIELD_TYPES; i++)
    {
        updateReligionYields((YieldTypes)i);
    }
}

1. You should be able to safely use them for testing since there are more commerce types than yield types. The effect is that you'll gain +1:commerce: for having the religion and +5:commerce: for the holy city because :culture: and :commerce: are both the 3rd type in their respective enums.

Eventually you'll need to add your own getStateReligionYield() and getHolyCityYield() functions to CvReligionInfo and read their values from the XML. But as long as you just want a simple value to see that your code is working, I suggest that you hard-code values rather than call unrelated functions as above.

Code: 
if (isHasReligion(eReligion))
{
	iYield += 1;  // TODO: CvReligionInfo::getStateReligionYield(eYield)

	if (isHolyCity(eReligion))
	{
		iYield += 3;  // TODO: CvReligionInfo::getHolyCityYield(eYield)
	}
}

2. That looks fine. I thought CvCity::init() created the array, but I see from your code that it happens in the constructor--CvCity::CvCity().

3. CvPlayer needs a function to call when the player changes state religion. That function needs to loop over every city for the player, telling each to update themselves. CvPlayer calls its function from setStateReligionCount() and setLastStateReligion().

Also, CvGame::setHolyCity() needs to call CvCity::updateReligionYields() just like it calls updateReligionCommerce(). There are two places in that function.

4. That's a very strange crash. At first it sounds like an infinite loop, but when you go back to windows you say that it's doing the normal crash dump. It's odd that it doesn't exit the graphics environment first. Perhaps try your testing in non-full-screen mode? It won't fix it, but it will make dealing with crashes easier. ;)

5. The only thing related to ReligionYield I can find is VoteSourceInfo::getReligionYield(). Names in one class have no bearing on names in another class. Using the same name in two classes is a good thing if they are related.

For example, VoteSourceInfo and CvCity both have a function called getReligionCommerce(). They are slightly related, though the one in VSI applies to buildings while the one in CvCity applies to the city as a whole. However, they both relate to commerce types gained from having a religion.


What IDE/editor do you use to do your coding? I use Visual Studio 2005 (not 2008 only because the instructions I originally used were for 2005; I'll probably switch when I have some free time), and it has a very handy feature. Right-click on a function/variable name and select Find All References. It then shows every place in all files in the SDK that reference that function: the declaration, definition, and every place that calls it. This is how I am finding all of the above information.
 
Going to try to work through your suggestions, thanks EF, but I should clarify one thing quickly.

The goal of the ReligionYields code that I'm adding is to add Yield functionality to the Shrines only. So, if you have 5 cities with Buddhism, and it's your State Religion, and have the Holy City (say Rome), there would be no change from standard gameplay. But, if you built the Mahatma Bodhni (IIRC) in Rome, then instead of getting +5:gold:, you would instead get +5:commerce:, which will be split as normal depending on your Gold/Research/Culture ratios.

I hope that's clearer. Basically, as far as I can tell, in the finished product, I won't need any code checking what the state religion is, or whether or not I have the Holy City. The tag is tied exclusively to the Shrines in the XML, so I shouldn't have to do any separate checks for Holy City, and state religion has no impact on Tax income from the Shrines, so I don't need to check for that either.
 
Oh! This is entirely different from what I thought you were trying to do (hard to keep track of what everyone is doing in all these threads). Did you track through how the +1:gold:/city is applied to the existing shrines? Let's see...

First, whenever there's a building function I want to figure out, I start with CvCity::processBuilding(). This function is called whenever a building is added or removed to/from the city (technically whenever the count changes). And it looks like this isn't handled here. Nice.

Next step is looking in the XML for the tags that make this happen. There's <GlobalReligionCommerce> which is set to the religion of the shrine. This looks promising. In the C++ I see that CvGameTextMgr calls getGlobalReligionCommerce() and displays the message "TXT_KEY_BUILDING_PER_CITY_WITH" if it isn't NO_RELIGION. It displays the values from CvReligionInfo::getGlobalReligionCommerceArray() which in CIV4ReligionInfo.xml has 1:gold: for each religion. Bingo!

So you want to add this to each religion in CIV4ReligionInfo.xml:

Code: 
<GlobalReligionYields>
	<iGlobalReligionYield>0</iGlobalReligionYield>
	<iGlobalReligionYield>0</iGlobalReligionYield>
	<iGlobalReligionYield>1</iGlobalReligionYield>
</GlobalReligionYields>

and replicate the GlobalReligionCommerce functionality in CvReligionInfo for yields:

  • Add int* m_paiGlobalReligionYield and init/delete/read it appropriately
  • getGlobalReligionYield(int i)
  • getGlobalReligionYieldArray()
Now the tough part still remains: putting this into CvCity. CvCity::getBuildingCommerceByBuilding() is what handles the +1:gold: per city with the religion, but there is no CvCity::getBuildingYieldByBuilding(). Creating that function won't be too hard, but reworking CvCity to call it at the right times might be.

  • Whenever the religion spreads in CvCity::setHasReligion() or CvGame::??
  • Whenever a building is added/removed because that can change yields for other reasons
  • Basically, anywhere updateBuildingCommerce() is called
Regarding this last one, you need to be smart about it. For example, CvCity::setBuildingCommerceChange() calls updateBuildingCommerce(), but clearly you don't want to bother calling updateBuildingYield() here since it's about commerce. Yet look at setBuildingYieldChange(), it doesn't have a call to updateBuildingYield() because it doesn't exist. Instead, it calls changeBaseYieldRate() directly based on the number of active buildings. You should change this to a call to your new updateBuildingYield() function. There may be other similar cases.
 
OK, that all sounds reasonable, and we've gotten all the way to your last trio of bullet-points. I found a line of code that looks like it is specifically counting how many cities have the religion being checked, but it was written for CvCity::getBuildingCommerceByBuilding(), and so contains some variables that don't work for my purposes. But I don't know how to re-write it so it does.

Here's an UTD sampling:
Code: 
int CvCity::getReligionYields(YieldTypes eIndex) const
{
	FAssertMsg(eIndex >= 0, "eIndex expected to be >= 0");
	FAssertMsg(eIndex < NUM_YIELD_TYPES, "eIndex expected to be < NUM_YIELD_TYPES");
	return m_aiReligionYields[eIndex];
}

void CvCity::updateReligionYields(YieldTypes eYield)
{
	int iOldYield;
	int iNewYield;
	int iI;

	FAssertMsg(eYield >= 0, "eYield expected to be >= 0");
	FAssertMsg(eYield < NUM_YIELD_TYPES, "eYield expected to be < NUM_YIELD_TYPES");

	iOldYield = getReligionYields(eYield);

	iNewYield = 0;

	for (iI = 0; iI < GC.getNumReligionInfos(); iI++)
	{
		[B]iNewYield += (GC.getReligionInfo((ReligionTypes)(GC.getBuildingInfo(eBuilding).getGlobalReligionYields())).getGlobalReligionYields(eIndex) * GC.getGameINLINE().countReligionLevels((ReligionTypes)(GC.getBuildingInfo(eBuilding).getGlobalReligionYields()))) * getNumActiveBuilding(eBuilding);[/B]
	}

	if (iOldYield != iNewYield)
	{
		m_aiReligionYields[eYield] = iNewYield;
		FAssert(getReligionYields(eYield) >= 0);

		changeBaseYieldRate(eYield, (iNewYield - iOldYield));
	}
}

void CvCity::updateReligionYields()
{
	int iI;

	for (iI = 0; iI < NUM_YIELD_TYPES; iI++)
	{
		updateReligionYields((YieldTypes)iI);
	}
}
It's the eBuilding references... I don't know how I could re-word this...
 
updateReligionYields(YieldTypes) needs to loop over the buildings and call a new function getBuildingReligionYieldsByBuilding(YieldTypes, BuildingTypes) just as updateBuildingCommerce() does. Replace your bolded line with this:

Code: 
for (iJ = 0; iJ < GC.getNumBuildingInfos(); iJ++)
{
	iNewYield += getBuildingYieldsByBuilding(((CommerceTypes)iI), ((BuildingTypes)iJ));
}

Note that you're looping over all the religions in updateReligionYields(YieldTypes) whereas that's done by getBuildingCommerceByBuilding(). Note also that the latter function adds up all building-related commerce sources. You won't be able to get away with having getBuildingYieldsByBuilding() do just the religion-related stuff. That's why I say this is going to be much harder than it first appeared. The reason is that yields are handled in an entirely different way from commerce types. :mad:

Okay, new tack here. The other option is to just track as religions spread and adjust the base yield rate for the city appropriately. You'd need a function on CvCity that would get called by CvGame as religions spread.

Code: 
void CvCity::changeGlobalReligionCount(ReligionTypes eReligion, int iChange)
{
	for each building type
		if building's GlobalReligionCommerce == eReligion
			look up religion
			iTotal = multiply religion's global yields * num active building * iChange
			call changeBaseYieldRate(iTotal)
}

You'd either need to have CvGame call a function on CvPlayer which calls this function on each of that player's cities, or you can just look up the holy city for the religion and call it. This would limit the effect to shrines which can only be built in the holy city.
 
I could write the code for the psuedo-function in my previous post, but I'm not too keen on doing the rest of the work. It's still going to be some investigation and coding to get it plugged in as I stated. :( This first part is pretty easy and you can find ample examples already in the code you've posted. If you can't write this part, how will you get the harder coding done?

I'm not trying to be mean. I would like to help, but I would rather help you figure out some ideas than code it all myself.
 
OK, you're right. :blush: I get it, "Teach a man to fish, etc." I was just starting to feel a little overwhelmed. The logic I can see. It's finding code that looks like the logic...

Like I said before, I really like your proposed solution. The problem is, I don't know where to find where the game adds up all the cities that have a religion. I've realized, it can't be in CvCity, because that's only concerned with counting what's going on in each individual city. And it won't be in CvPlayer either, because counting up all the cities with a religion includes all foreign cities too. Then I thought, maybe CvTeam? and looking through it I found this:
Code: 
int CvTeam::getHasReligionCount(ReligionTypes eReligion) const
{
	int iCount;
	int iI;

	iCount = 0;

	for (iI = 0; iI < MAX_PLAYERS; iI++)
	{
		if (GET_PLAYER((PlayerTypes)iI).isAlive())
		{
			if (GET_PLAYER((PlayerTypes)iI).getTeam() == getID())
			{
				iCount += GET_PLAYER((PlayerTypes)iI).getHasReligionCount(eReligion);
			}
		}
	}

	return iCount;
}
Does that look to you like it's what counts how many cities in the world have a given religion? I'm not so sure, because it says GET_PLAYER. But I don't know enough to know what other file to look in...
 
That code will count how many cities the team has with the religion. You want to know how many cities the game has with the religion. For that we need CvGame.

You can see in your code above a function that reports how many cities that is:

Code: 
GC.getGameINLINE().countReligionLevels(eReligion)

However, you don't so much need to know the total as detect the cases where the total changes. There must be a function in CvGame that controls religion spread. It needs to tell the CvPlayer/CvTeam and the CvCity when that happens. I don't know which way that occurs.

Here's where a good IDE comes into play, and that's why I asked which IDE/editor you used. Whenever a city gets a new religion, some object calls CvCity::setHasReligion(). I would start there and see what that function does, but more importantly I would see who calls that function. You can do this easily in a good IDE. Without it, you're forced to do search-in-all-files for the function's name.
 
Ok, that's a starting point, thanks.

I'm using Visual Studio 2008. It has the same "Find all references", "Go to declaration", and "Go to definition" that you were referring to before. But I'm wondering if I'm using it wrong somehow (although I don't see how one could), because when I "Find all references" of say, getGlobalReligionCommerce, it doesn't find anything in CvPlayer...

But, I will go forward, with what you quoted in #252. Thanks EF!!
 
Looking at CvCity::setHasReligion(), you'll find this line:

Code: 
GET_PLAYER(getOwnerINLINE()).changeHasReligionCount(eIndex, ((isHasReligion(eIndex)) ? 1 : -1));

This is telling the city's owner (CvPlayer) to change the religion count. So clearly CvGame tells the CvCity, which tells the CvPlayer. Apparently the CvTeam doesn't keep a running total, so it doesn't get told.

I see a few places where setHasReligion() is called. I would suggest putting the code in CvCity (or calling a CvGame function) to call the holy city to update its yields, however there is at least one special case where this won't work.

When a city is acquired, a new CvCity is created and the old city's values are copied over. The problem is that the old city is not told to remove the religion. There's no need since the city is going to be dropped on the floor (deallocated in C++ terms). It is removed from the original CvPlayer owner, so there's no need to remove the religion.

You should check this code to see where it tells the previous owner to decrease their religion count. That would give you a clue how to handle this.

In any case, note that after the above line in setHasReligion(), it checks to see if it should broadcast the spread of the religion to the owner of the holy city. So this does seem like a good place to tell the holy city to update its yields.

draco963 said:
When I "Find all references" of say, getGlobalReligionCommerce, it doesn't find anything in CvPlayer.
Click to expand...

All I did was create a project and then add all .h and .cpp files to it. It should index everything once you've done that.
 
I think I may have a simple and elegant solution:

Your discoveries led me to getReligionCount, which seems to take care of, separately, counting up the number of cities with a religion. And, changeBaseYieldRate, which we encountered before, takes care of adding up Yields for a city, before modifiers. I recognize that shoe-horning the Shrine Yields in here may result in difficulties in getting them to display properly in Python, but I think I've got a way around that too. So:

Code: 
int CvCity::getReligionYields(YieldTypes eIndex) const
{
	FAssertMsg(eIndex >= 0, "eIndex expected to be >= 0");
	FAssertMsg(eIndex < NUM_YIELD_TYPES, "eIndex expected to be < NUM_YIELD_TYPES");
	return mai_ReligionYields[eIndex];
}

void CvCity::updateReligionYields(YieldTypes eYield, ReligionTypes eReligion)
{
	int iOldYield;
	int iNewYield;
	int iI;

	FAssertMsg(eYield >= 0, "eYield expected to be >= 0");
	FAssertMsg(eYield < NUM_YIELD_TYPES, "eYield expected to be < NUM_YIELD_TYPES");

	iOldYield = getReligionYields(eYield);

	iNewYield = 0;

	for (iI = 0; iI < GC.getNumReligionInfos(); iI++)
	{
		iNewYield += (pLoopCity->getReligionCount() * GC.getReligionInfo(eReligion).getGlobalReligionYields(eYield));
	}

	if (iOldYield != iNewYield)
	{
		m_aiReligionYields[eYield] = iNewYield;
		FAssert(getReligionYields(eYield) >= 0);

		changeBaseYieldRate(eYield, (iNewYield - iOldYield));
	}
}

void CvCity::updateReligionYields()
{
	int iI;

	for (iI = 0; iI < NUM_YIELD_TYPES; iI++)
	{
		updateReligionYields((YieldTypes)iI);
	}
}

So, mai_ReligionYields will hold the actual value of the Yields being produced to the Shrine which can then be passed to Python, and CvCity::updateReligionYields() is called by void CvCity::setHasReligion(ReligionTypes eIndex, bool bNewValue, bool bAnnounce, bool bArrows) to make sure the game runs the calculation for those Yields again each time getReligionCount changes. ANd with this, I don't need to go messing around anywhere else.

I don't think that I need to worry about conquering, because even though the old CvCity record is deleted and a new one created, with CvCity::updateReligionYields(YieldTypes eYield) written like it is, the game should immediately jump to the same value it had previously, because the total number of cities in the game with the religion won't have changed.

I'm testing this now, we'll see if it actually worked... :crazyeye:

EDIT: ... and not. VS2008 very much doesn't like how I wrote the above, I'm being told:
CvCity.cpp(8358) : error C2511: 'void CvCity::updateReligionYields(YieldTypes,ReligionTypes)' : overloaded member function not found in 'CvCity'
Click to expand...
. Unfortunately, I don't have a clue what that means...
 
If you track the current yield totals, a) you can expose them via Python which is good, b) you can add other ones later if you lie, and c) it makes recalculating the change as religion spreads easier. You'll have to do some testing to make sure that the function that counts the number of cities with the religion is okay to call during the switch of ownership. In other words, will it count both cities (old and new) when you call it?

One issue, this line won't fly:

Code: 
iNewYield += (pLoopCity->getReligionCount() * GC.getReligionInfo(eReligion).getGlobalReligionYields(eYield));

In updateReligionYields(), pLoopCity doesn't exist. But more importantly CvCity::getReligionCount() is probably not what you want. Your first clue is that it doesn't take a religion type, so what will it count? My guess without looking is that it returns the number of religions that the city has in it.

I posted the code to use via CvGame to count the number of cities a religion has a few posts back.
 
The compiler error message

Code: 
'void CvCity::updateReligionYields(YieldTypes,ReligionTypes)' : overloaded member function not found in 'CvCity'

means that your function definition in CvCity.ccp does not match any of the function declarations in CvCity.h. Because it says, "overloaded member function," it has found at least one declaration for updateReligionYields() that takes a different set of parameters. An overloaded function is a collection of similarly-named functions that each take a unique set of parameters.
 
Maybe these two items will clear things up for me a bit...

Could you translate this piece of code into plain English for me?
Code: 
int CvCity::getReligionCommerceByReligion(CommerceTypes eIndex, ReligionTypes eReligion) const
{
	int iCommerce;

	FAssertMsg(eIndex >= 0, "eIndex expected to be >= 0");
	FAssertMsg(eIndex < NUM_COMMERCE_TYPES, "eIndex expected to be < NUM_COMMERCE_TYPES");
	FAssertMsg(eReligion >= 0, "eReligion expected to be >= 0");
	FAssertMsg(eReligion < GC.getNumReligionInfos(), "GC.getNumReligionInfos expected to be >= 0");

	iCommerce = 0;

	if ((GET_PLAYER(getOwnerINLINE()).getStateReligion() == eReligion) || (GET_PLAYER(getOwnerINLINE()).getStateReligion() == NO_RELIGION))
	{
		if (isHasReligion(eReligion))
		{
			iCommerce += GC.getReligionInfo(eReligion).getStateReligionCommerce(eIndex);

			if (isHolyCity(eReligion))
			{
				iCommerce += GC.getReligionInfo(eReligion).getHolyCityCommerce(eIndex);
			}
		}
	}

	return iCommerce;
}

And I'm also having alot of trouble figuring out how how the game is passing the value of ReligionCommerce, read from the XML into CvGameInfos, into CvCity...

Thanks.
 
We request a specific commerce as linked to a specific religion (first line)

Ignore the assert stuff, it is to catch potential crashes if we asked for a religion/commerce that doesn't exist.

Start the count at 0.

Check if the owner is following this religion, or not following ANY religion at all

If he is, check if this city has the religion

If we have the religion, we get the default bonus of this religion in every city

Now check if we are the Holy city

If we are, add on a bonus for being the Holy City.

Now provide a response of this particular value (0, Basic, or Holy)



This function IS how the values get from CvInfos -- GC.getReligionInfo(eReligion) -- into CvCity. Anything which wants to know what commerces should exist due to religion will check this function (doing a loop over all religions and all commerces)
 
In psuedocode, this translates to

Code: 
set iCommerce to 0 to start

If the city's owner's state religion is eReligion or they have no state religion
    if the city has eReligion
        add eReligion's StateReligionCommerce to iCommerce
        if this is eReligion's holy city
            add eReligion's HolyCityCommerce to iCommerce
return the total iCommerce

in English, this says that the commerce is 0 if the city has a state religion that isn't eReligion, or 0/1/5 depending on whether the city has the religion and/or is the religion's holy city.
 
You must log in or register to reply here.

Similar threads

G
Tile Yields in Antiquity if you have the three basic techs and the brickyard, granary, and saw mill
Replies
1
Views
560
MutilationWave
MutilationWave
hdd
Resource Yields - need help
Replies
1
Views
942
Smayo
Smayo
planetmaker
Adding my planet Mod to Final Frontier Plus
4 5 6
Replies
110
Views
5K
planetmaker
planetmaker
A
Adding new locale to the game
Replies
2
Views
516
Acid.Crash
A
keldath
ai civic code review
Replies
0
Views
318
keldath
keldath
Back
Top Bottom