[mod] Religion and Revolution Extended (modmod of Religion and Revolution)

[colonialfan]

Hi guys,

I've been following this thread with some interest, especially around the topic of changing native civs. Indulge me for a moment while I make some observations and then some suggestions. I am by no means criticizing the idea to change civs, and my thoughts are just those, thoughts, so please feel free to do with them as you wish.

I assume the desire to replace the Comanche, Navajo and Crow is based on the cluster of 'somewhat similar' tribes located in the same area? If so, then I assume the new civs are to be created in mind for a geographically/historically accurate map?

With this being said, I think that area is probably a bit crowded and it would be good to spread the civs out a bit.

The major problem we on the RaR team encountered with additional civs was finding suitable leaderheads that were high quality enough for the game and at least historically plausible representations of each native civ. We also had to ensure the spread of civs in North/Central/South America was balanced enough so's not to negatively compromise the game.


I've looked at the suggestions of Haida, Ohlone and Timucua and I've taken the liberty of making some suggestions as none of the leaderheads for the Crow, Navajo or Comanche civs would fit the new suggested civs without extensive work done to alter their physical appearances.

Haida

Pro's
Great idea for a civ, and it would be placed in a largely uninhabited region of the map.

Con's
They have a very distinctive look. I found two possible leaderheads but I'm am not sure if they are static or not. Other than this there is nothing out there I could find that is even remotely close.




Ohlone

Pro's
There is a leadhead that could probably work for this tribe.
Placed in an uninhabited part of the map.

Con's
Rather a small tribe, not that well known in comparison to surrounding tribes.




Timucua

Pro's
Would be great to have a Florida tribe in the mix.
Area is currently empty on the map.

Con's
Again, only one leaderhead that would be anywhere near close to a physical representation, but it might need some work.




Might I suggest that if the one of these tribes is not an option, then perhaps the Cree might be considered using the leaderhead currently used by the Navajo. Obviously the background would need changing as would some of the clothing but it could work and the Cree would cover a large area not yet inhabited.



Again, let me stress I'm not trying to jump all over someone's project, or thrust my opinions on anyone. I think it's great that new civs are being considered, I am just putting out some thoughts for consideration.

 

[agnat86]

The solution is in the MODwiki.

Simply said you start Colonization normally (I recommend using a shortcut to start the mod directly) and then inside VC++ you select Debug->Attach to process and then you pick Colonization. Selecting start debugger will not work.

I tried to configure the project to start Colonization and debug that process, but I ended up with either "not working" or "froze system". For now (possibly forever) you select the process manually to attach to.

I tried it, and I got this message after the CTD:

"Unhandled exception at 0x04F64FF0 (CvGameCoreDLL.dll) in Colonization.exe: 0xC0000005: Access violation reading location 0x0000002A."

However, I don't know how to find the bug which caused this, as no location or cpp file is given.

According to the ModWiki, I should make a Debug-able DLL, and then attach the debugger, but I don't know how I can make that. I remember having made a debug-able DLL one time in the past, which then gave me exact locations of errors in the code, but I don't remember which settings I changed to be able to do that.

 

[raystuttgart]

In some cases (usually really easy to fix DLL-bugs), it is enough to simply have a debug-dll running and you do not need to attach the debugger to the colonization.exe.
In these cases the debug-dll is capable of directly pointing you to the line of code.

In more complicated cases you need to attach your debugger to the colonization.exe and use more advance techniques of debugging (break points, asserts, analyzing the stack, ...), since in these cases the debug-dll is not able to point you directly to the line of code.

Finding such bugs can be really tedious and the best weapon you have is excellent knowledge about the code you have written recently (meaning from a point of time where you can be quite sure that the bug did not exist).

Thus you should do extensive tests (e.g. Autoplay) and debugging regularly if you write a lot of code.

When I tried to find bugs, I usually attached the debugger to the exe and tried to find out what was going on.
(You should have a good savegame as a base for that.)

Edit:
When working on RaR I have spent about half of the time for quality checks, code improvements and searching for potential bugs (with Autoplay and debugging).
To my opinion that is absolutely necessary if you create a mod as big and complex as RaR.

 

[agnat86]

In some cases (usually really easy to fix DLL-bugs), it is enough to simply have a debug-dll running and you do not need to attach the debugger to the colonization.exe.
In these cases the debug-dll is capable of directly pointing you to the line of code.

I understand that, but the problem is that I don't know how to make a debug-dll. Can you tell me which settings I need to change in VC++ 2010?

Finding such bugs can be really tedious and the best weapon you have is excellent knowledge about the code you have written recently (meaning from a point of time where you can be quite sure that the bug did not exist).

You're right, that's why I document each and every change I make in DLL, now matter how insignificant. Since I got my last CTD, I've already made some changes to the code where it might be wrong, so maybe I've solved it already. Problem is, I have no modding experience whatsoever, and I'm still finding it very difficult to understand how the C++ language works, so I always have to guess about what's going wrong. Most of the time when writing code, I'm using trial and error until the code does what I want it to do.

Thus you should do extensive tests (e.g. Autoplay) and debugging regularly if you write a lot of code.

Autoplay would come in really handy here. How does it work? Is it a function I can find somewhere in the game?

When I tried to find bugs, I usually attached the debugger to the exe and tried to find out what was going on.
(You should have a good savegame as a base for that.)

Yes, I solved one in the past when I managed to make a debug-DLL (Sadly, I've forgotten how I did that). But that was a reproducable bug, the one I've encountered this time isn't (in fact, I had to reload the savegame 8 times before I finally encountered the bug again)

Edit:
When working on RaR I have spent about half of the time for quality checks, code improvements and searching for potential bugs (with Autoplay and debugging).
To my opinion that is absolutely necessary if you create a mod as big and complex as RaR.

Currently, I think about 80% of the time I spend modding consists of trying to eliminate bugs...

I just hope I will become a little more efficient once I get a little more experienced.

 

[raystuttgart]

I understand that, but the problem is that I don't know how to make a debug-dll. Can you tell me which settings I need to change in VC++ 2010?

Are you using the Workspace from my SVN ?
(see "Workspace Guide.txt")

Then it is really easy because everything is set up in the project already.

Simply switch your project configuration to "Debug" in Visual Studio IDE and compile.
It will then generate a debug-dll.

 

[Nightinggale]

According to the ModWiki, I should make a Debug-able DLL, and then attach the debugger, but I don't know how I can make that.

Assuming you did what the wiki said, then you have a menu in the top toolbar, which you can switch between Release and Debug.

I will soon (read: when I get rid of this headache) commit the source and a project file where stuff like that is set up correctly.

Using my makefile also allows a third option "Assert" where the code test itself while it's still optimized for speed, kind of like a mix of debug and release. Asserts are the FAssert() and FAssertMsg() functions spread out through the code. They contain something, which should be true and if it's false, then an error window will pop up telling where it went wrong. FAssertMsg() adds a test string to the failure.

FAssert(a < b);
FAssertMsg(a < b, "a is too big");
FAssert(a < b, CvString::format("a is %d, but should be below %d", a, b));

All of those will tell file and line in the code, which failed, but some provides more info than others. You really should add enough asserts everywhere as they really help to catch bugs. Also you can decide to go into the debugger once you encounter a failed assert, meaning they really help to find the right place to debug, kind of like automated breakpoints, which only activates in case of bugs.
For the record CvString::format() works kind of like printf() meaning you can add variables to the string. The main difference is that you can use it in function arguments like this.

 

[raystuttgart]

Currently, I think about 80% of the time I spend modding consists of trying to eliminate bugs...

Once a bug is known and can be reproduced, it is usually eliminated comparably fast if you do regular quality assurance and have some experience in programming.
Good quality assurance (searching for potential bugs and constant improvements) on the other hand is really time consuming.

 

[raystuttgart]

Autoplay would come in really handy here. How does it work? Is it a function I can find somewhere in the game?

For RaR see here.

 

[agnat86]

I assume the desire to replace the Comanche, Navajo and Crow is based on the cluster of 'somewhat similar' tribes located in the same area? If so, then I assume the new civs are to be created in mind for a geographically/historically accurate map?

Well, I would like it most to just keep them and only add a couple of civs, but that would be much more work. So changing them seems to be an easier solution.

But my objective here is to add natives from areas that aren't represented yet, not to remove them from areas that are crowded. If I can add these tribes and keep the Crow, Comanche and Navajo, that's all the better for me.

I've looked at the suggestions of Haida, Ohlone and Timucua and I've taken the liberty of making some suggestions as none of the leaderheads for the Crow, Navajo or Comanche civs would fit the new suggested civs without extensive work done to alter their physical appearances.

Haida

Pro's
Great idea for a civ, and it would be placed in a largely uninhabited region of the map.

Con's
They have a very distinctive look. I found two possible leaderheads but I'm am not sure if they are static or not. Other than this there is nothing out there I could find that is even remotely close.

View attachment 358563 View attachment 358564 View attachment 358565

I've downloaded the second leaderhead, and it's a static one.

The first one, though, is not static. It was part of the AoFD mod, as the Inuit leaderhead.

Do you maybe know if the Haida grew beards? As far as I know, native americans had little facial hair, but many northern tribes originated genetically from a different population influx into North America, and were closer related to Siberian tribes than to other native americans. So maybe beards would be acceptable for the Haida.

Ohlone

Pro's
There is a leadhead that could probably work for this tribe.
Placed in an uninhabited part of the map.

Con's
Rather a small tribe, not that well known in comparison to surrounding tribes.

View attachment 358566 View attachment 358567

That leaderhead looks really good!

Maybe you can upload them here?

About the choice for the Ohlone: when I looked for Californian tribes, I found long lists of tribes, but I couldn't really determine which tribes were the most important ones. My impression is that California was one of the most populous areas of the New World, but it was inhabitated by lots of small tribes, and no large ones.

The main reason I chose the Ohlone was because I found a long list of almost 100 village names.

But I would be okay with any nation for that region. Do you have any suggestions?

Timucua

Pro's
Would be great to have a Florida tribe in the mix.
Area is currently empty on the map.

Con's
Again, only one leaderhead that would be anywhere near close to a physical representation, but it might need some work.

View attachment 358568 View attachment 358569

I can have a look at it, if you can share that leaderhead with me.

The Timucua are the only tribe I already found a leaderhead for that might do:



The quality is good, and I think it can easily pass for a Timucua indian. But it looks very much like the Arawak and Carib leaderheads. The LH you suggest looks more unique.

Might I suggest that if the one of these tribes is not an option, then perhaps the Cree might be considered using the leaderhead currently used by the Navajo. Obviously the background would need changing as would some of the clothing but it could work and the Cree would cover a large area not yet inhabited.

View attachment 358570 View attachment 358571

The Cree would be a great addition. I have thought about them in the past, but they slipped from my mind again. But I think they would make a better addition than the Yupik (who are very similar to the Inuit).

Thanks for your input!

 

[agnat86]

Are you using the Workspace from my SVN ?
(see "Workspace Guide.txt")

Then it is really easy because everything is set up in the project already.

Simply switch your project configuration to "Debug" in Visual Studio IDE and compile.
It will then generate a debug-dll.

I was, but because it was some time ago that I set it up, and maybe inadvertently screwed up things, I've downloaded and set it up once again, and added my source files to it.

I finally succeeded in making a debug-DLL, but strangely, this time I can't compile a regular DLL.

Anyway, I can still make normal builds in the other location where I keep the mod, so I can now both build and debug. Thanks for the help!

 

[Nightinggale]

I've downloaded the second leaderhead, and it's a static one.

Wouldn't it be possible to use a static one at all? I mean it's not ideal, but if the alternative is to not use a tribe due to lack of graphics?

Do you maybe know if the Haida grew beards? As far as I know, native americans had little facial hair, but many northern tribes originated genetically from a different population influx into North America, and were closer related to Siberian tribes than to other native americans. So maybe beards would be acceptable for the Haida.

I really don't know. What I know of American natives is that they lack beard, but there could be exceptions. On the other hand if you go search for it now, you might end up with a bearded one, who turns out not to be a true blooded native. Maybe this is something we will never know for sure.

One interesting thing about American natives and beards is a Mayan tale. They carved a wall to tell that a god came from across the sea, were tall and had white skin. He returned to where he came from, but said he would return one day. The interesting thing is that he supposedly had a full beard.
However now that I search for it on google I find some guy called Dr. Van Stone claiming white was a holy colour like red and green and that they made drawings of those as well. Sounds reasonable, but that doesn't explain why the white man had a full beard, which were common in Europe, but nobody in America had that at that time. Remember this predates Columbus.
This likely ends up like the Haida beard meaning we will never really know for sure, specially since political views and personal careers makes some people state what they want to believe or want others to believe as fact rather than looking objectively at what we know. Actually that goes for more or less anything, specially if money is involved.

The main reason I chose the Ohlone was because I found a long list of almost 100 village names.

That would be a good reason to pick the Ohlone as long as there is no stronger argument to pick another tribe instead.

 

[Schmiddie]

Maybe we should optimize the natives already existing (traits etc.) before we are considering about new ones.

The biggest problems regarding new native tribes: Really good leaderheads and a LOT OF WORK fo new unit artstyles...

So maybe we can do other improvements first...

 

[raystuttgart]

I finally succeeded in making a debug-DLL, but strangely, this time I can't compile a regular DLL.

Well, I simply do not know what you are doing and where your problems might be in detail.
Generally it should be really easy to switch between the 2 configurations "Final_Release" and "Debug" to creat a normal DLL and a debug DLL.

But do not give up and try to figure out how to get your development environment working correctly.

 

[agnat86]

This is the error I keep getting:

"Assert Failed

File: CvPlayer.cpp
Line: 2411
Expression: checkPower(false)
Message: "

However, I have tried removing parts of the code I added, recompile the DLL, and try again, but I keep getting this error, usually directly at the end of the first turn.

Does any of you know what this means? Have I made mistakes in writing code that influences a unit's combat strength or something?

Even with all my lines fo code removed that do things, it still happens.

The line in CvPlayer.cpp it refers to is this :

Spoiler :

void CvPlayer::doTurn()
{
PROFILE_FUNC();

CvCity* pLoopCity;
int iLoop;

FAssertMsg(isAlive(), "isAlive is expected to be true");
FAssertMsg(!hasBusyUnit() || GC.getGameINLINE().isMPOption(MPOPTION_SIMULTANEOUS_TURNS) || GC.getGameINLINE().isSimultaneousTeamTurns(), "End of turn with busy units in a sequential-turn game");

gDLL->getEventReporterIFace()->beginPlayerTurn( GC.getGameINLINE().getGameTurn(), getID());

doEra();

doUpdateCacheOnTurn();

GC.getGameINLINE().verifyDeals();

AI_doTurnPre();

AI_assignWorkingPlots();

doGold();

doBells();

doCrosses();

// PatchMod: Achievements START
doAchievements(false);
// PatchMod: Achievements END

// TAC - LbD - Ray - START
doLbD();
// TAC - LbD - Ray - END


// R&R, ray, changes to Wild Animals
if (!GC.getGameINLINE().isBarbarianPlayer(getID()) && !isNative() && !isEurope() && isAlive())
{
//TAC Native Mercs
checkForNativeMercs();
//End TAC Native Mercs

// R&R, ray, Native Slave, START
checkForNativeSlaves();
// R&R, ray, Native Slave, END

// R&R, ray, African Slaves, START
checkForAfricanSlaves();
// R&R, ray, African Slaves, END

// R&R, ray, Prisons Crowded - START
checkForPrisonsCrowded();
// R&R, ray, Prisons Crowded - END

// R&R, ray, Revolutionary Noble - START
checkForRevolutionaryNoble();
// R&R, ray, Revolutionary Noble - END

// R&R, ray, Bishop - START
checkForBishop();
// R&R, ray, Bishop - END

// R&R, ray, Smuggling - START
checkForSmugglers();
// R&R, ray, Bargaining - END

// R&R, ray, Rangers - START
checkForRangers();
// R&R, ray, Rangers - END

// R&R, ray, Conquistadors - START
checkForConquistadors();
// R&R, ray, Conquistadors - END

// R&R, ray, Pirates - START
checkForPirates();
// R&R, ray, Pirates - END

// R&R, ray, European Peace, START
checkForEuropeanPeace();
// R&R, ray, European Peace, END

//TAC European Wars
checkForEuropeanWars();
//TAC European Wars

// R&R, ray, Continental Guard - START
checkForContinentalGuard();
// R&R, ray, Continental Guard - END

// R&R, ray, Mortar - START
checkForMortar();
// R&R, ray, Mortar - END

// TAC - AI Economy - Ray - START
if (!isHuman())
{
//koma13
CvPlayerAI& kPlayerAI = GET_PLAYER((PlayerTypes) getID());
kPlayerAI.AI_updateBestPortCities();

redistributeWood();
// R&R, ray, redistribute cannons and muskets
redistributeCannonsAndMuskets();
}
// TAC - AI Economy - Ray - END
}

// R&R, ray, Bargaining - START
decreaseTimeNoTrade();
// R&R, ray, Bargaining - END

// R&R Abandon City, ray START
for (pLoopCity = firstCity(&iLoop); pLoopCity != NULL; pLoopCity = nextCity(&iLoop))
{
if (pLoopCity->getPopulation() == 0 )
{
CvWString szBuffer = gDLL->getText("CITY_ABANDONED", pLoopCity->getNameKey());
gDLL->getInterfaceIFace()->addMessage(getID(), false, GC.getEVENT_MESSAGE_TIME(), szBuffer, "AS2D_CITYCAPTURED", MESSAGE_TYPE_MAJOR_EVENT, ARTFILEMGR.getInterfaceArtInfo("WORLDBUILDER_CITY_EDIT")->getPath(), (ColorTypes)GC.getInfoTypeForString("COLOR_RED"), pLoopCity->getX_INLINE(), pLoopCity->getY_INLINE(), true, true);
disband(pLoopCity);
}
else
{
pLoopCity->doTurn();
}
}
// R&R Abandon City, ray END

// R&R, Little Extra Native Income, ray START
if (isNative())
{
int iNumVillages = getNumCities();

int villagePerRoundIncomeMax = GC.getPER_ROUND_PER_VILLAGE_INCOME_MAX();
int villageIncomeLimit = GC.getPER_VILLAGE_FEATURE_GENERATION_LIMIT();

int featureGenerationLimit = iNumVillages * villageIncomeLimit;
int currentGold = getGold();

if (currentGold < featureGenerationLimit)
{
int featureGenerationMax = iNumVillages * villagePerRoundIncomeMax;
int nativeIncome = GC.getGameINLINE().getSorenRandNum(featureGenerationMax, "Small Native Income");
changeGold(nativeIncome);
}

}
// R&R, Little Extra Native Income, ray END

verifyCivics();

doPrices();

doEvents();

interceptEuropeUnits();

updateEconomyHistory(GC.getGameINLINE().getGameTurn(), getGold());
updateIndustryHistory(GC.getGameINLINE().getGameTurn(), calculateTotalYield(YIELD_HAMMERS));
updateAgricultureHistory(GC.getGameINLINE().getGameTurn(), calculateTotalYield(YIELD_FOOD));
updatePowerHistory(GC.getGameINLINE().getGameTurn(), getPower());
updateCultureHistory(GC.getGameINLINE().getGameTurn(), countTotalCulture());
expireMessages(); // turn log
m_aszTradeMessages.clear();

// TAC - Trade Messages - koma13 - START
m_aeTradeMessageTypes.clear();
m_aeTradeMessageYields.clear();
m_aiTradeMessageAmounts.clear();
m_aiTradeMessageCommissions.clear();
// TAC - Trade Messages - koma13 - END

gDLL->getInterfaceIFace()->setDirty(CityInfo_DIRTY_BIT, true);

AI_doTurnPost();

gDLL->getEventReporterIFace()->endPlayerTurn( GC.getGameINLINE().getGameTurn(), getID());

FAssert(checkPower(false));
FAssert(checkPopulation());
}

And the checkPower function (also in CvPlayer.spp) says this:

Spoiler :

bool CvPlayer::checkPower(bool bReset)
{
int iPower = 0;
int iAsset = 0;
std::map<int, int> mapAreaPower;
int iLoop;
for (CvUnit* pUnit = firstUnit(&iLoop); pUnit != NULL; pUnit = nextUnit(&iLoop))
{
int iUnitPower = pUnit->getPower();
iPower += iUnitPower;
CvArea* pArea = pUnit->area();
if (pArea != NULL)
{
mapAreaPower[pArea->getID()] += iUnitPower;
}
iAsset += pUnit->getAsset();
}
for (uint i = 0; i < m_aEuropeUnits.size(); ++i)
{
iPower += m_aEuropeUnits->getPower();
iAsset += m_aEuropeUnits->getAsset();
}
for (CvCity* pCity = firstCity(&iLoop); pCity != NULL; pCity = nextCity(&iLoop))
{
int iCityPower = 0;
int iCityAsset = 0;
for (int i = 0; i < pCity->getPopulation(); ++i)
{
iCityPower += pCity->getPopulationUnitByIndex(i)->getPower();
iCityAsset += pCity->getPopulationUnitByIndex(i)->getAsset();
}

for (int i = 0; i < GC.getNumBuildingInfos(); ++i)
{
BuildingTypes eBuilding = (BuildingTypes) i;
if (pCity->isHasBuilding(eBuilding))
{
iCityPower += GC.getBuildingInfo(eBuilding).getPowerValue();
iCityAsset += GC.getBuildingInfo(eBuilding).getAssetValue();
}
}

for (int i = 0; i < NUM_YIELD_TYPES; ++i)
{
YieldTypes eYield = (YieldTypes) i;
iCityPower += pCity->getYieldStored(eYield) * GC.getYieldInfo(eYield).getPowerValue();
iCityAsset += pCity->getYieldStored(eYield) * GC.getYieldInfo(eYield).getAssetValue();
}

iPower += iCityPower;
iAsset += iCityAsset;
mapAreaPower[pCity->area()->getID()] += iCityPower;
}

bool bCheck = true;
if (iPower != getPower())
{
if (bReset)
{
changePower(iPower - getPower());
}
bCheck = false;
}

if (iAsset != getAssets())
{
if (bReset)
{
changeAssets(iAsset - getAssets());
}
bCheck = false;
}

for (CvArea* pArea = GC.getMapINLINE().firstArea(&iLoop); pArea != NULL; pArea = GC.getMapINLINE().nextArea(&iLoop))
{
if (mapAreaPower[pArea->getID()] != pArea->getPower(getID()))
{
if (bReset)
{
pArea->changePower(getID(), mapAreaPower[pArea->getID()] - pArea->getPower(getID()));
}
bCheck = false;
}
}

return bCheck;
}

 

[Nightinggale]

checkPower(false) tells if the cached power of the current player is equal to the calculated power. In other words you have something incorrect in your cache. Somewhere you changed something, which affects power without updating the cache.

This check is done at the end of turn and it could be anything during the turn. Once you find it, you can modify the cache. If you don't know much much it's wrong (like all soldier's gained +50%, but don't know the number of soldiers), then you can recalculate the cache completely (the slowest option). This is done by the line

 checkPower(true);

The question remain what did you do to cause this? I can't answer that based on what you have written here, but look though Cv::checkPower() to see what is affecting the cache and see if you touch any of that at some point.

 

[raystuttgart]

This is the error I keep getting:

An Assert is not the same as a (critcal) bug.
It is an indication to a possible bug though.

 

[agnat86]

These are the lines of code I feel most uneasy about, and they influence the power of units. This code gives units a bonus for either defending or attacking a plot that is part of their owner's territory.

I added this in CvUnit.cpp under "maxCombatStr":

Spoiler :

if (pPlot->getOwner() == getOwnerINLINE() && pPlot->getOwner() != NO_PLAYER)
{
iExtraModifier = DomesticBonusModifier();
iModifier += iExtraModifier;
if (pCombatDetails != NULL)
{
pCombatDetails->iDomesticBonusModifier = iExtraModifier;
}
}

Spoiler :

if (pPlot->getOwner() == pAttacker->getOwner() && pPlot->getOwner() != NO_PLAYER)
{
iExtraModifier = -pAttacker->DomesticBonusModifier();
iTempModifier += iExtraModifier;
if (pCombatDetails != NULL)
{
pCombatDetails->iDomesticBonusModifier = iExtraModifier;
}
}

Could the problem be within these lines?

 

[raystuttgart]

Some comments about your style of coding:

1. Your naming of Methods is not optimal.

DomesticBonusModifier() should be called getDomesticBonusModifier()

2. Some of your code could be written more readable.

Instead of writing this

iExtraModifier = [COLOR="Red"]-[/COLOR]pAttacker->DomesticBonusModifier();
iTempModifier [COLOR="Red"]+=[/COLOR] iExtraModifier;

Why not write it like this:

iExtraModifier = pAttacker->DomesticBonusModifier();
iTempModifier [COLOR="Red"]-=[/COLOR] iExtraModifier;

3. What are you actually doing here ?

{
   pCombatDetails->iDomesticBonusModifier = iExtraModifier;
}

If you are setting a value to an object, why do you not use a set-Method ?

 

[agnat86]

Some comments about your style of coding:

1. Your naming of Methods is not optimal.

DomesticBonusModifier() should be called getDomesticBonusModifier()

I'll try to do that in the future, but in this case, there already is a method named like that, which is combined with another one ("getDomesticBonusPercent") to form "DomesticBonusModifier." This is because one of these methods is used by units, and another one by promotions. The reason I use this structure is because I copied all code for this attribute from the code for "hillsDefenseModifier," which also is a combination of an attribute used by units and one used by promotions.

2. Some of your code could be written more readable.

Instead of writing this

iExtraModifier = [COLOR="Red"]-[/COLOR]pAttacker->DomesticBonusModifier();
iTempModifier [COLOR="Red"]+=[/COLOR] iExtraModifier;

Why not write it like this:

iExtraModifier = pAttacker->DomesticBonusModifier();
iTempModifier [COLOR="Red"]-=[/COLOR] iExtraModifier;

I will do that. Thanks for the advice!

 

[agnat86]

3. What are you actually doing here ?

{
   pCombatDetails->iDomesticBonusModifier = iExtraModifier;
}

If you are setting a value to an object, why do you not use a set-Method ?

That's also a relic af my method of copying code already in place, and then changing the names of the objects. So I don't really know what I'm doing there, other then that I'm doing the same as other modders did when defining objects that have a similar function. How do you think I should change that function?