K-Mod: Far Beyond the Sword

[Ungomma]

Hello again, karadoc!

1. Running a lot slower is certainly bad news. I think it's highly likely that it is due to a combination of something in the K-Mod dll + the increased number of assets in RI. Maybe it is unit types, maybe improvement types or something else... it's really hard to say. I suggest that you compile a 'profiling' version of the dll; then use it load a save game which you know will be slow; then end turn; then quit and check the profile log to find out what the slowest functions are. It will only list functions which have "PROFILE_FUNC()" at the top of their code, but you can just put that line in to whichever functions you think are worth checking.

If you do that, and you find the source of the slowness, please let me know. Hopefully it's something that can be fixed easily without undoing any other changes.

Thanks for the advice. I have no real experience with C++ and profiling didn't even cross my mind. I ran the game with the profiling DLL and have easily located the most costly functions.

These were, in my case, CvUnitAI.AI_upgrade and CvPlayerAI.AI_doDiplo.

1. CvUnitAI.AI_upgrade had a cycle over getNumUnitInfos and, naturally, it ran extremely slow with RI assets. I've made a quick fix - changed it to cycle over getNumUnitClassInfos and it reduced the time this functions worked by 90%. I also moved canUpgrade check closer to the top to avoid the calculations of AI_unitValue for units you can't upgrade into anyway.

The function is still at the top of the list. Is it really necessary to try to upgrade every unit every turn?

2. In CvPlayerAI.AI_doDiplo the tech trading code is the culprit. RI has much more techs than regular CIV and loops over getNumTechInfos with AI_techValue being called for every tech are abundant in AI_doDiplo.

The worst part is that I play with Tech Trading turned off, so these calculations couldn't possible be of any use. I implemented a simple check for that option in places where AI_techValue is called.

2. You're right. They don't look fortified in K-Mod either, so it's almost certainly my fault. I'll try to fix that soon. (I don't yet know what's causing the problem. Maybe it's something to do with the stuff changed in updateCenterUnit - but maybe not.)
[edit]
Actually, I just did a quick test without any mods loaded. I started a new game, used the world builder to create a barb city with an archer in it where I had vision, and then pressed end turn. The barb archer did not look fortified. --- So. Apparently it's not my fault after all! The problem was there right from the start. Nevertheless, I'll see if I can fix it.
[/edit]

I am pretty sure that the AI's units are fortified in vanilla Civ, at least when I come close to their cities with my armies. In your mod they just stand there all the time. They still get their fortify modifier, but they don't look defensive.

I would prefer if all AI units on SLEEP had a fortified animation. Will make them more scary to approach.

Thank you!

 

[Ungomma]

Another problem, though, on a extra huge map with lots of players. Very slow.

At the top of the profiling list is CvPlayer::getBestRoute, which was called a monstrous 4016071 times in 1 turn. This is the very beginning of the game. I have no idea why this happens yet.

EDIT: the source of the problem appears to be AI_updateRouteToCity, or at least it takes a long (in mS) to run.

 

[noto2]

I see from your changelog that you lowered the collateral damage cap on siege units. While I empathize with the desire to reign them in, I think this is the wrong way to do it. Lowering the unit cap makes them less dangerous to large stacks, but siege units are supposed to be dangerous to large stacks - in fact, they're the only way to really counter them.

I think a better idea would be to lower the maximum amount of damage that siege units can inflict; this would preserve their effectiveness against stacks of doom, but it wouldn't let them weaken opponents to the point that your choice of other units is irrelevant. As an aside, this would mean seeing fewer siege units since you won't need as many to hit the damage cap.

Completely agree with this. Lowering the amount of collateral damage done and capping the damage to a lower amount would be the best way to nerf siege weapons. Right now Civ4 combat is quite boring because winning simply requires you to have 1) tons of siege and 2) any other unit to mop up. Reducing the power of siege (or even greatly increasing its cost) would make combat more interesting again.

 

[karadoc]

Hello again, karadoc!

Thanks for the advice. I have no real experience with C++ and profiling didn't even cross my mind. I ran the game with the profiling DLL and have easily located the most costly functions.

These were, in my case, CvUnitAI.AI_upgrade and CvPlayerAI.AI_doDiplo.

1. CvUnitAI.AI_upgrade had a cycle over getNumUnitInfos and, naturally, it ran extremely slow with RI assets. I've made a quick fix - changed it to cycle over getNumUnitClassInfos and it reduced the time this functions worked by 90%. I also moved canUpgrade check closer to the top to avoid the calculations of AI_unitValue for units you can't upgrade into anyway.

The function is still at the top of the list. Is it really necessary to try to upgrade every unit every turn?

Ok. Thanks for that, I'll make that change in K-Mod as well (although, for K-Mod itself I don't expect it to make much difference, because the number of unit types is very close to the number of unit class types.) As for whether it is necessary to check it every turn.. well, it isn't - but it is necessary to check it sometimes, and it's difficult to really say when it should be checked. We can't just check it when we get a new tech, because maybe the player doesn't have enough money on the turn they get the tech, and so on. -- Generally speaking, the AI does need to check everything every turn - or at least check to see if they need to check it! (somehow). ... Anyway, there are heaps of bits in the AI which are woefully inefficient, so I wouldn't be at all surprised if there were big gains to be made in the functions you mentioned. (I've just been rewriting the AI in bits and pieces whenever I happened to see something I disagreed with. I haven't really looked at AI_upgrade yet, except for fixing a grouping problem there.)

[edit]
I've just rewritten AI_upgrade. (It's much faster now, and also, it now chooses between upgrades by comparing the AI_unitValue rather than randomness.) -- Anyway, based on my tests, AI_unitValue is actually significantly faster than canUpgrade - so I suggest you switch it back to checking the unit value first. Or, try to work out how to make canUpgrade faster. It does do some obviously slow stuff, but I didn't see any obvious ways to speed it up without changing the game mechanics, and without adding new cache stuff.
[/edit]

2. In CvPlayerAI.AI_doDiplo the tech trading code is the culprit. RI has much more techs than regular CIV and loops over getNumTechInfos with AI_techValue being called for every tech are abundant in AI_doDiplo.

The worst part is that I play with Tech Trading turned off, so these calculations couldn't possible be of any use. I implemented a simple check for that option in places where AI_techValue is called.

From memory, I don't think AI_doDiplo actually calls AI_techValue, does it? AI_techValue is a pretty expensive function - I certainly hope the AI isn't calling it for every team, for every tech, on every turn... I thought AI_doDiplo only considers the cost of each tech and a few other things which are pretty fast. Never the less, I'm sure there are a few significant shortcuts which could be made to speed it up... for example skipping the whole thing if tech trading is disabled.

I am pretty sure that the AI's units are fortified in vanilla Civ, at least when I come close to their cities with my armies. In your mod they just stand there all the time. They still get their fortify modifier, but they don't look defensive.

When I said they were not fortified, what I meant was that the AI doesn't MISSION_FORTIFY. They just don't do that, at all. Instead, they just use MISSION_SKIP repeatedly, which gives all the same fortification bonuses and so on, but it doesn't trigger the fortify animation. (It's the same if human players do that - if you just skip turn instead of using fortify, you still get all the healing and bonuses and so on, but the unit will not 'look' fortified.)

The animation stuff isn't stored in save files, so when a save game is loaded, the game just makes some guesses about what's fortified and what's not, and so pretty much everything looks fortified... I can probably make some change to that as well, but it's really all just aesthetics.

Actually, I've just spent (ie. wasted) a heap of time trying to set the AI up in a way that allows it to use MISSION_FORTIFY without getting confused or missing turns and what-not. I think it'll look better in the next version. (Roughly speaking, they'll fortify city defenders and defenders on resource plots - but nothing else.)

Another problem, though, on a extra huge map with lots of players. Very slow.

At the top of the profiling list is CvPlayer::getBestRoute, which was called a monstrous 4016071 times in 1 turn. This is the very beginning of the game. I have no idea why this happens yet.

EDIT: the source of the problem appears to be AI_updateRouteToCity, or at least it takes a long (in mS) to run.

Ok. I'll look into that as well. (I don't really know what AI_updateRouteToCity is needed for. I haven't looked at it at all.)
I've glanced at CvPlayer::getBestRoute in the past, and I think the comments left by BBAI are probably right... ie. there could be some decent speed gains on large maps if the value was cached. It's a bit of a mess though, because it would still have to check that the cached value could actually be built in any given situation; and so it would still need to loop through all the builds to check that. (You can't just cache the build type, because there may be multiple builds which produce the same route, and maybe different builds can be used in different circumstances or something.) -- So... it's a bit tricky. It might be better to try to reduce the usage of the function rather than to speed it up directly.

Anyway, at least we know that there are some easy improvements to make to AI_upgrade. That's a decent start.
[edit]
In my opinion, that AI_upgrade function is junk anyway. It finds the best thing to upgrade to, and then just does it. I think that's stupid. Instead, there should be function which simply evaluates how useful it would be to upgrade - and then that value should be compared to the cost & value of upgrading different units before deciding what (if anything) to spend the upgrade money on. The current first in, first served upgrade AI is just bogus... ... but there are a lot of other bogus things in need of attention; so for now, I'll just do this speed thing and leave it alone. (Incidentally; why have the original devs left a comment saying that AI_upgrade is recursive? Do they not know what recursive means? AI_upgrade is does not call itself, and so it's not recursive...)
[/edit]


Actually, it just occurred to me that you said the K-Mod dll was slower than whatever you were using before... but the things you mentioned are not things K-Mod has changed stuff in. (at least, not much...) So I'm not sure why it was slower. Was there some BBAI change or something? Or is it something in K-Mod that I thought was minor slowing it down, (eg. in AI_upgrade, I put in something to help units stay grouped; but I wouldn't expect it to have a measurable effect on the speed.)

 

[Ungomma]

I've just rewritten AI_upgrade. (It's much faster now, and also, it now chooses between upgrades by comparing the AI_unitValue rather than randomness.) -- Anyway, based on my tests, AI_unitValue is actually significantly faster than canUpgrade - so I suggest you switch it back to checking the unit value first. Or, try to work out how to make canUpgrade faster. It does do some obviously slow stuff, but I didn't see any obvious ways to speed it up without changing the game mechanics, and without adding new cache stuff.

Excellent! Thank you! When do you plan to upload it to github?

From memory, I don't think AI_doDiplo actually calls AI_techValue, does it? AI_techValue is a pretty expensive function - I certainly hope the AI isn't calling it for every team, for every tech, on every turn... I thought AI_doDiplo only considers the cost of each tech and a few other things which are pretty fast. Never the less, I'm sure there are a few significant shortcuts which could be made to speed it up... for example skipping the whole thing if tech trading is disabled.

You have a good memory. It doesn't call AI_techValue indeed, it calls AI_techTrade and AI_techTradeVal. My mistake. I didn't read the code closely, I simply placed a GAMEOPTION_NO_TECH_TRADING wordaround.

It was pretty expensive though, even without AI_techValue.

Actually, I've just spent (ie. wasted) a heap of time trying to set the AI up in a way that allows it to use MISSION_FORTIFY without getting confused or missing turns and what-not. I think it'll look better in the next version. (Roughly speaking, they'll fortify city defenders and defenders on resource plots - but nothing else.)

Nice! Unfortified city defenders made me feel uneasy.

Ok. I'll look into that as well. (I don't really know what AI_updateRouteToCity is needed for. I haven't looked at it at all.)
I've glanced at CvPlayer::getBestRoute in the past, and I think the comments left by BBAI are probably right... ie. there could be some decent speed gains on large maps if the value was cached. It's a bit of a mess though, because it would still have to check that the cached value could actually be built in any given situation; and so it would still need to loop through all the builds to check that. (You can't just cache the build type, because there may be multiple builds which produce the same route, and maybe different builds can be used in different circumstances or something.) -- So... it's a bit tricky. It might be better to try to reduce the usage of the function rather than to speed it up directly.

Yes, that would be better. I have no idea what possible use could be of calling it 4016071 times per turn. Does every player call it for every city for every plot?

Unfortunately, I have not been able to discover from where exactly is getBestRoute called so much. I have said previously that AI_updateRouteToCity was suspect, but it has that getRouteFinder() and FAStar stuff I don't understand, so your help here will be greatly appreciated.

What's more important is that this issue is, apparently, map-specific. It occurs only on the RI giant world map. I haven't seen it happen on random maps.

The smaller RI World Map appears to not have this issue, but it crashes after the first turn. There's nothing in the logs - any hints on how to determine what caused the crash? I have tried to compile the debug DLL, but it says :debug information module size exceeded" and won't compile. I bet you know the way to solve this issue.

Actually, it just occurred to me that you said the K-Mod dll was slower than whatever you were using before... but the things you mentioned are not things K-Mod has changed stuff in. (at least, not much...) So I'm not sure why it was slower. Was there some BBAI change or something? Or is it something in K-Mod that I thought was minor slowing it down, (eg. in AI_upgrade, I put in something to help units stay grouped; but I wouldn't expect it to have a measurable effect on the speed.)

Yes, that is strange. But as I've just said, this seems to be map-specific.

Thank you!

EDIT: I am starting to suspect your pathfinder. Could your changes to path-related functions in CvGameCoreUtils affect the built-in RouteFinder?
Maybe you could make a KModRouteFinder to fix all this?
EDIT2: BTW, the size of the map is 210x90, there are 55 players on the map and 222 cities at the end of the first turn.

 

[JosEPh_II]

Don't you have to have a special made DLL to use more than 49AI (48 AI + Barb AI) in a game? That was the limit on vanilla BtS.

JosEPh

 

[karadoc]

None of my pathfinding changes affect anything the RouteFinder uses; and I don't think there would be much to gain from rewritting the RouteFinder. (The main reason it was helpful to rewrite the normal pathfinder was just so that I could use various shortcuts in special situations. But the RouteFinder doesn't really have situations like that anyway.)

Most of the calls to getBestRoute are probably for choosing plot improvements. The AI evaluates plot improvements for everything in range of all their cities on every turn. And when they do so, they consider every possible improvement they can build..... and consider the effect that the best route would have on the improvement. (For example, they have to consider the possibility that a mine might be better than a windmill if they build a railroad.) ... Anyway, that could potentially be an awful lot of calls to getBestRoute.

But is that really where the slowness is coming from? On the test I just did, getBestRoute was called 329380 times in one turn, but it only took 73 ms - which is less than 1% of the total turn time in this particular case. So it doesn't seem to be a big deal in K-Mod. Maybe it becomes much slower if there are lots of new improvements or routes or something. If your map is 4x bigger, with 3x as many possible improvements, then I guess that would explain why you are seeing 10x as many calls to getBestRoute... You could probably speed it up a lot by telling the AI not to worry about the route when choosing improvements. (It would be a minor weakening of the AI's ability to pick improvements, but it might make your game significantly faster.)

(By the way, in this same test game, AI_techTrade was called 3412 times, and took 27 ms... which again is pretty small, which is why I haven't been looking there to try to speed it up. Even if that time was reduced to 0ms it wouldn't be a noticeable improvement. (The total turn time on this test game is around 9000ms.)

As for the crash, I would say that the debug version of the DLL is by far the best thing for debugging crashes - essentially all you need to do is run with the debug DLL and make it crash and the bug will be found. (most of the time) --

I have tried to compile the debug DLL, but it says :debug information module size exceeded" and won't compile. I bet you know the way to solve this issue.

I've never seen that message. Are you saying that VC++ gives you that message? I just did a google search for it and found this: http://support.microsoft.com/kb/122539
Are you using an old version of VC++ or something? (I'm using Visual C++ 2008 Express, "Version 9.0.30729.1 SP", according to the about page. -- It is actually an 'old version', but not as old as the versions mentioned in that microsoft support article.)

If the crash has anything to do with K-Mod changes, then that's certainly something I'd like to know about.


There seems to be a disturbing trend of people reporting bugs and strange behaviour which I can't reproduce. So if you can find a crash bug, that would be helpful.

Which reminds me...

hmmm, the only time i can get python uninitialized is when i exit civ entirely. cleared kmod folder in my docs and installed v1.33.
again it's no biggie, doesn't deduct from my pleasure in k-mod.

however, should you have a spare moment i attach my debuglog, in the hope you can see something unusual. in this log, all i did was:
start civ, singleplayer/playnow/random stuff, settle persepolis, hit ctrl+shift+x and autoplay dialog appears. exit to main menu,multiplayer/LAN/newgame/random stuff, settle persepolis, make random moves, next turn,hit ctrl+shift+x and again autoplay dialog appears, hit cancel, exit to desktop.

satrapper, did you check your civ4 BtS version? It seems unlikely that you'd have a different version to me, but I just don't know why else my Python would be cleared and yours not cleared.

If anyone else feels like testing this, I'd be interested to hear what you see. (ie. turn on logging in the civ4 ini, then follow the satrapper's instructions and check the PythonDbg.log. In particular, check to see if it says "UnInit Python" somewhere in the middle. That's the key thing to look for.


[edit]
I almost forgot this:

When do you plan to upload it to github?

I usually don't upload the changes until the next full update. (ie. I push the changes to github just after I upload the whole thing to this site.) -- so... unless you're in a big hurry to see it, that's what I'll do again. (the new code isn't particularly fancy. I wouldn't consider it something worth looking forward to...)

 

[Leoreth]

Ok. Thanks for that, I'll make that change in K-Mod as well (although, for K-Mod itself I don't expect it to make much difference, because the number of unit types is very close to the number of unit class types.) As for whether it is necessary to check it every turn.. well, it isn't - but it is necessary to check it sometimes, and it's difficult to really say when it should be checked. We can't just check it when we get a new tech, because maybe the player doesn't have enough money on the turn they get the tech, and so on. -- Generally speaking, the AI does need to check everything every turn - or at least check to see if they need to check it! (somehow).

Wouldn't it make more sense if the AI decided only on certain occasions if it is worthwhile to update its units (after the relevant tech is discovered, when it feels threatened or is planning a war), then memorize the amount of gold needed and actively work to obtain it, and then update?

 

[Ungomma]

None of my pathfinding changes affect anything the RouteFinder uses; and I don't think there would be much to gain from rewritting the RouteFinder. (The main reason it was helpful to rewrite the normal pathfinder was just so that I could use various shortcuts in special situations. But the RouteFinder doesn't really have situations like that anyway.)

Most of the calls to getBestRoute are probably for choosing plot improvements. The AI evaluates plot improvements for everything in range of all their cities on every turn. And when they do so, they consider every possible improvement they can build..... and consider the effect that the best route would have on the improvement. (For example, they have to consider the possibility that a mine might be better than a windmill if they build a railroad.) ... Anyway, that could potentially be an awful lot of calls to getBestRoute.

Well, if profiling log tells the truth, almost all calls for getBestRoute are from RouteValid which, in turn, is called by the RouteFinder.

I've cached getBestRoute value and modified RouteValid to always return TRUE when the player has not discovered any routes yet. This helped somewhat but I still get 4153529 calls of RouteValid every turn. Each of those calls getBestRoute (4016065 calls). There used to be 6 calls for canBuild for every getBestRoute, but cache solved this.

I am still inclined to blame the RouteFinder for this issue. There is no possible reason to call RouteValid that many times. There are no roads on the map, so checking the 8 squares adjacent to the city square should be sufficient.

But is that really where the slowness is coming from? On the test I just did, getBestRoute was called 329380 times in one turn, but it only took 73 ms - which is less than 1% of the total turn time in this particular case. So it doesn't seem to be a big deal in K-Mod. Maybe it becomes much slower if there are lots of new improvements or routes or something. If your map is 4x bigger, with 3x as many possible improvements, then I guess that would explain why you are seeing 10x as many calls to getBestRoute... You could probably speed it up a lot by telling the AI not to worry about the route when choosing improvements. (It would be a minor weakening of the AI's ability to pick improvements, but it might make your game significantly faster.)

Why, yes, I am pretty sure. I see no reason for me to believe that my millions of getBestRoute calls come from improvement calculations. I mean, it does say "CvPlayerAI::AI_baseBonusVal::recalculate" in the parent column, but this seems to be a bug in the profiler. These calls come from RouteValid, as you can verify by placing PROFILE_FUNC() there are comparing the total time. But the number of calls already eliminates other possibilities.

Vanilla CIV has two route types, if I'm not mistaken, but RI has six. My 4016065 cached calls for getBestRoute() cost 220ms, RouteValid costs 841ms. Until I implemented the cache and other workarounds CvCityAI::AI_updateRouteToCity used to cost up to 7000ms every turn.

CvCityAI::AI_updateRouteToCity is still a problem - it now takes 1300ms out of 4900ms turn. Such easy operation is not worth that much time. It begs to be rewritten. If you run profiling on it, I am sure you'll see that it consumes a non-trivial amount of game time. I am, sadly, not competent enough (or at all) with C++ and can only make easy modifications.

So, I think there is much to gain, at least for those who play huge maps, from getting a good RouteFinder.

(By the way, in this same test game, AI_techTrade was called 3412 times, and took 27 ms... which again is pretty small, which is why I haven't been looking there to try to speed it up. Even if that time was reduced to 0ms it wouldn't be a noticeable improvement. (The total turn time on this test game is around 9000ms.)

Hmm.. Guess that depends of the number of TechInfos. I'll try to make my game playable without tech trading first. Maybe I'll come back to this later.

As for the crash, I would say that the debug version of the DLL is by far the best thing for debugging crashes - essentially all you need to do is run with the debug DLL and make it crash and the bug will be found. (most of the time) --

I've never seen that message. Are you saying that VC++ gives you that message? I just did a google search for it and found this: http://support.microsoft.com/kb/122539
Are you using an old version of VC++ or something? (I'm using Visual C++ 2008 Express, "Version 9.0.30729.1 SP", according to the about page. -- It is actually an 'old version', but not as old as the versions mentioned in that microsoft support article.)

If the crash has anything to do with K-Mod changes, then that's certainly something I'd like to know about.

I will try to compile the debug DLL. If the crash has something to with K-Mod, I'll be sure to inform you, although I doubt it.

[edit]
I almost forgot this:
I usually don't upload the changes until the next full update. (ie. I push the changes to github just after I upload the whole thing to this site.) -- so... unless you're in a big hurry to see it, that's what I'll do again. (the new code isn't particularly fancy. I wouldn't consider it something worth looking forward to...)

Well, I consider my DLL to be almost finished. My previous changes to AI_upgrade were not enough and it still took ~2600ms out of a 7000ms turn. I inserted a canTrain check there and it reduced the cost significantly, so AI_upgrade is no longer an issue.

As for the AI_upgrade function being recursive - it actually is. Or, more precisely, its child function, namely upgradeAvailable called from getUpgradeCity called from hasUpgrade called from canUpgrade. My understanding is that you made the whole upgrade thing faster and more streamlined, and I wanted to get a good version for my DLL. But I can wait, the canTrain workaround works around well.

Thank you for your assistance!

 

[sjodster]

In my opinion, that AI_upgrade function is junk anyway. It finds the best thing to upgrade to, and then just does it. I think that's stupid. Instead, there should be function which simply evaluates how useful it would be to upgrade - and then that value should be compared to the cost & value of upgrading different units before deciding what (if anything) to spend the upgrade money on. The current first in, first served upgrade AI is just bogus...

I agree completely.

I've just rewritten AI_upgrade. (It's much faster now, and also, it now chooses between upgrades by comparing the AI_unitValue rather than randomness.)

As always, I look forward to your work

(Incidentally; why have the original devs left a comment saying that AI_upgrade is recursive? Do they not know what recursive means? AI_upgrade is does not call itself, and so it's not recursive...)

As for the AI_upgrade function being recursive - it actually is. Or, more precisely, its child function, namely upgradeAvailable called from getUpgradeCity called from hasUpgrade called from canUpgrade.

That is correct, and the way upgradeAvailable was originally implemented made the computational cost increase EXPONENTIALLY for the number of unit infos. This was definitely a problem for Realism: Invictus and other big mods.

I ran the game with the profiling DLL and have easily located the most costly functions.

These were, in my case, CvUnitAI.AI_upgrade and CvPlayerAI.AI_doDiplo.

1. CvUnitAI.AI_upgrade had a cycle over getNumUnitInfos and, naturally, it ran extremely slow with RI assets. I've made a quick fix - changed it to cycle over getNumUnitClassInfos and it reduced the time this functions worked by 90%. I also moved canUpgrade check closer to the top to avoid the calculations of AI_unitValue for units you can't upgrade into anyway.

Ok. Thanks for that, I'll make that change in K-Mod as well (although, for K-Mod itself I don't expect it to make much difference, because the number of unit types is very close to the number of unit class types.)

[edit]Anyway, based on my tests, AI_unitValue is actually significantly faster than canUpgrade - so I suggest you switch it back to checking the unit value first. Or, try to work out how to make canUpgrade faster. It does do some obviously slow stuff, but I didn't see any obvious ways to speed it up without changing the game mechanics, and without adding new cache stuff.
[/edit]

Actually, it just occurred to me that you said the K-Mod dll was slower than whatever you were using before... but the things you mentioned are not things K-Mod has changed stuff in. (at least, not much...) So I'm not sure why it was slower.

Shortly after we moved Realism: Invictus to Beyond the Sword I brought over a modification to AI_Upgrade from the Sanguo mod (from which the CAR mod was derived). That modification caches all of the unit upgrade lines, so that instead of looping through every unit in the game to check for an upgrade, it only loops through the handful of unit classes that a particular unit could ever upgrade to. Doing this has a memory cost but saves a LOT of wasted cycles. The cache is constructed when Civ4 starts.

Ungomma, when you merged K-Mod in you must have overwritten these changes. The difference in speed is probably due to the difference that this modification made. However, if you missed this one I'm a little worried that you might have missed some others as well.

What's more important is that this issue is, apparently, map-specific. It occurs only on the RI giant world map. I haven't seen it happen on random maps.

Yep the world map is a beast. It stresses the system in every way conceivable, but for this reason it makes a very useful test case for profiling

-Josh

 

[Cole98]

How does the global warming get triggered? I have 6 Coal, 3 Oil and I've launched a bunch of nukes but it hasn't started!

 

[Ungomma]

Shortly after we moved Realism: Invictus to Beyond the Sword I brought over a modification to AI_Upgrade from the Sanguo mod (from which the CAR mod was derived). That modification caches all of the unit upgrade lines, so that instead of looping through every unit in the game to check for an upgrade, it only loops through the handful of unit classes that a particular unit could ever upgrade to. Doing this has a memory cost but saves a LOT of wasted cycles. The cache is constructed when Civ4 starts.

Ungomma, when you merged K-Mod in you must have overwritten these changes. The difference in speed is probably due to the difference that this modification made. However, if you missed this one I'm a little worried that you might have missed some others as well.

So that's why it was so slow! Oh, and it's more like ignored it than missed it.

My first attempt at merging was not very successful (too many errors in compiling), so the second time I decided to take the K-Mod DLL and merge the gameplay-related changes only. I assumed the K-Mod DLL to be superior speed-wise anyway. I've put the code you speak of back now. Should work well indeed.

Yep the world map is a beast. It stresses the system in every way conceivable, but for this reason it makes a very useful test case for profiling

I've managed to get reasonably good turn times on that map, so the game is playable in that sense. But, sadly, I encountered a CTD pretty early in the game. Meh.

 

[sjodster]

So that's why it was so slow! Oh, and it's more like ignored it than missed it.

Quite understandable, its usefulness wouldn't be obvious at first glance.

My first attempt at merging was not very successful (too many errors in compiling), so the second time I decided to take the K-Mod DLL and merge the gameplay-related changes only.

Yep I agree with using K-Mod as the "base" and merging the RI code into it. K-Mod has the greater number of edits and a lot of its minor edits are unmarked.

 

[Cruel]

@ Ungomma more information in your email.

 

[satrapper]

man, this thread is intimidating with all the heavies posting on it.

satrapper, did you check your civ4 BtS version? It seems unlikely that you'd have a different version to me, but I just don't know why else my Python would be cleared and yours not cleared.

yup, same version as you (3.1.9.0 (128100)). i tried other mods and python is not unloaded inbetween modes with them either; i've no idea what is default behaviour though.

i ran a debug dll to see if my limited skillset could catch anything unusual. autoplay threw up 2 assert failures but doubtful they're of any import as they appear under both sp and mp modes:

if i'm the only one exhibiting this behaviour, no point worrying too much about a minor issue like this. no doubt i've probably done something obscure and stupid in my setup

@cole98
global warming isn't triggered by nukes in k-mod, it's now got more to do with pollution/unhealthiness. check the environment tab in the finance advisor to know how likely it is to strike at any given time. or the changelog for more info on how its calculated.

 

[karadoc]

Maybe it's just me but if I had a function, say function A, and it did was call a different function, function B, then I would say function A is not recursive, regardless of what function B did - as long as it didn't call function A. .. Anyway, the bottom line is that upgradeAvailable is recursive, and potentially expensive (depending on how many unit types there are), and I'm not surprised the cache sped it up a heap.

[edit]
I've just looked briefly at the caching system used by RI for upgradeAvailable. I currently don't intend to copy that system, because it changes the game mechanics slightly. Here how:
in RI, upgradeAvailable simply checks a cached list of possible upgrades for each unit. The cache is set for each unit, and it does not depend on which civilization is doing the upgrading. However, in the current rules, there may be certain upgrade paths for some civilizations which are not available to other civilizations. For example, if A can upgrade to B, and B can upgrade to C; then A can upgrade to C. That's fine.. but what is B is a unique unit which is only available to a particular civ? In that case, other civs might not have any path to upgrade A -> C. Civ dependent rules like that are reflect in the current system, but not in the cache system used in RI.

This probably isn't a big deal. In fact, there might not be any real examples of this difference having any effect in the current xml... but never the less, I'd prefer not to change to rules for a small speed gain. (In RI, it's a bit different because it's actually a large speed gain... anyway, I'm going to see if a different way comes to mind, preferably without caching every path for every unit for every civ...)
[/edit]

If the original RI dll had a bunch of speed related changes, then we can't really assume that the K-Mod dll is faster. Apparently the RI efficiency changes target some different stuff to the K-Mod efficiency changes. (As I mentioned, I haven't really been worrying about some of the slow stuff related to having lots of assets mentioned here, because it isn't really slow in K-Mod anyway.)

... I'm a bit worried that my rewrite of AI_upgrade has generated a bit of unwarranted excitement. Really, it's not that good. If I end up implementing the evaluation type system that I mentioned earlier then AI_upgrade would be completely obsolete, so I didn't really want to speed a lot of time on it. It's just a bit neater, and a bit faster, and a bit smarter. Actually, I kind of expect that Ungomma's version would be faster than mine anyway based of what has been said here. -- So, maybe I can deflate the bubble of excitement by just posted the function here:

Spoiler :

void CvUnitAI::AI_upgrade()
{
	PROFILE_FUNC();

	FAssert(!isHuman());
	FAssert(AI_getUnitAIType() != NO_UNITAI);

	if (!isReadyForUpgrade())
		return;

	const CvPlayerAI& kPlayer = GET_PLAYER(getOwnerINLINE());
	const CvCivilizationInfo& kCivInfo = GC.getCivilizationInfo(kPlayer.getCivilizationType());
	UnitAITypes eUnitAI = AI_getUnitAIType();
	CvArea* pArea = area();

	int iBestValue = kPlayer.AI_unitValue(getUnitType(), eUnitAI, pArea) * 100;
	UnitTypes eBestUnit = NO_UNIT;

	// Note: the original code did two passes, presumably for speed reasons.
	// In the first pass, they checked only units which were flagged with the right unitAI.
	// Then, only if no such units were found, they checked all other units.
	//
	// I'm just jumping straight to the second (slower) pass, because most of the time no upgrades are available at all and so both passes would be used anyway.
	//
	// I've reversed the order of iteration because the stronger units are typically later in the list
	for (UnitClassTypes i = (UnitClassTypes)(GC.getNumUnitClassInfos()-1); i >= 0; i=(UnitClassTypes)(i-1))
	{
		UnitTypes eLoopUnit = (UnitTypes)kCivInfo.getCivilizationUnits(i);

		if (eLoopUnit != NO_UNIT)
		{
			int iValue = kPlayer.AI_unitValue(eLoopUnit, eUnitAI, pArea);
			// use a random factor. less than 100, so that the upgrade must be better than the current unit.
			iValue *= 80 + GC.getGameINLINE().getSorenRandNum(21, "AI Upgrade");

			// (believe it or not, AI_unitValue is faster than canUpgrade.)
			if (iValue > iBestValue && canUpgrade(eLoopUnit))
			{
				iBestValue = iValue;
				eBestUnit = eLoopUnit;
			}
		}
	}

	if (eBestUnit != NO_UNIT)
	{
		/* original bts code
		upgrade(eBestUnit);
		doDelayedDeath(); */

		// K-Mod. Ungroup the unit, so that we don't cause the whole group to miss their turn.
		CvUnit* pUpgradeUnit = upgrade(eBestUnit);
		doDelayedDeath();

		if (pUpgradeUnit != this)
		{
			CvSelectionGroup* pGroup = pUpgradeUnit->getGroup();
			if (pGroup->getHeadUnit() != pUpgradeUnit)
			{
				pUpgradeUnit->joinGroup(NULL);
				// indicate that the unit intends to rejoin the old group (although it might not actually do so...)
				pUpgradeUnit->getGroup()->AI_setMissionAI(MISSIONAI_GROUP, 0, pGroup->getHeadUnit());
			}
		}
	}
}

(By the way, in case anyone is interested, I've taken to putting the enum typecasts inside the head of the for loops so that then the body of the for loop doesn't have to have them anywhere. I figure that's a good way to prevent people (ie me) from accidentally casting to the wrong type later on. - Anyway, that's why the for looks a bit messy.)

@satrapper, that first assert failure probably isn't important, but the second one looks like something that should be fixed. What did you do to see that?

 

[satrapper]

just ran autoplay at start of game. either single or multiplayer(for me). happens everytime (well, the 3 or 4 times i tried it) with the debug dll

 

[karadoc]

I can't reproduce that problem either. I start a new game using the 'play now' button, and just click 'next' through all the settings, then I activate autoplay using ctrl+shift+x - and it just works. I've tried it a few times with both the most recent debug dll, and with the debug dll from v1.33. I've tried using an existing save game rather than a new game; and I've tried it with different numbers of autoplay turns. (sometimes 1 turn, or 10, or 40. I haven't tried more than 40, because autoplay is pretty slow with the debug dll.)

...

So.. yet another mystery. Which sucks, because that assert failure signals a problem which is likely to cause a crash at some point. (because the game will try to access the -1th element of an array, which obviously is invalid.)

 

[ripple01]

Hi karadoc,

First off, let me thank you for the work you've put into the Civ4 modding community. K-Mod is an amazing piece of work and I look forward to further developement.

I've been working on merging K-Mod 1.32 and RevolutionsDCM and I have one issue I'm running into that I'm hoping you might be able to give me a nudge in the right direction on. I've got all of the merged files to successfully compile except CvUnit.cpp. These are the compiler errors it is throwing up:

 "C:\Program Files\Microsoft Visual C++ Toolkit 2003\bin\cl.exe" /nologo /MD /O2 /Oy /Oi /G7 /DNDEBUG /DFINAL_RELEASE /Fp"Release\CvGameCoreDLL.pch" /GR /Gy /W3 /EHsc /Gd /Gm- /DWIN32 /D_WINDOWS /D_USRDLL /DCVGAMECOREDLL_EXPORTS /Yu"CvGameCoreDLL.h" /IBoost-1.32.0/include /IPython24/include /I"C:\Program Files\Microsoft Visual C++ Toolkit 2003/include" /I"C:\Program Files\WindowsSDK/Include" /I"C:\Program Files\WindowsSDK/Include/mfc" /I"C:\Program Files\Firaxis Games\Sid Meier's Civilization 4\Beyond the Sword\CvGameCoreDLL\Boost-1.32.0/include" /I"C:\Program Files\Firaxis Games\Sid Meier's Civilization 4\Beyond the Sword\CvGameCoreDLL\Python24/include" /FoRelease\CvUnit.obj /c CvUnit.cpp
1>CvUnit.cpp
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::basic_string<_Elem,_Traits,_Alloc> &,const _Elem *)' : could not deduce template argument for 'const std::basic_string<_Elem,_Traits,_Ax> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\string(148) : see declaration of 'std::operator`<''
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(85) : see reference to function template instantiation 'bool std::operator <<int,IDInfo>(const std::pair<_Ty1,_Ty2> &,const std::pair<_Ty1,_Ty2> &)' being compiled
1>        with
1>        [
1>            _Ty1=int,
1>            _Ty2=IDInfo
1>        ]
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\functional(128) : see reference to function template instantiation 'bool std::operator ><int,IDInfo>(const std::pair<_Ty1,_Ty2> &,const std::pair<_Ty1,_Ty2> &)' being compiled
1>        with
1>        [
1>            _Ty1=int,
1>            _Ty2=IDInfo
1>        ]
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\functional(127) : while compiling class-template member function 'bool std::greater<_Ty>::operator ()(const _Ty &,const _Ty &) const'
1>        with
1>        [
1>            _Ty=std::pair<int,IDInfo>
1>        ]
1>        CvUnit.cpp(13546) : see reference to class template instantiation 'std::greater<_Ty>' being compiled
1>        with
1>        [
1>            _Ty=std::pair<int,IDInfo>
1>        ]
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const _Elem *,const std::basic_string<_Elem,_Traits,_Alloc> &)' : could not deduce template argument for 'const T1 *' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\string(138) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::basic_string<_Elem,_Traits,_Alloc> &,const std::basic_string<_Elem,_Traits,_Alloc> &)' : could not deduce template argument for 'const std::basic_string<_Elem,_Traits,_Ax> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\string(128) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::_Tree<_Traits> &,const std::_Tree<_Traits> &)' : could not deduce template argument for 'const std::_Tree<_Traits> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\xtree(1170) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::list<_Ty,_Alloc> &,const std::list<_Ty,_Alloc> &)' : could not deduce template argument for 'const std::list<_Ty,_Ax> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\list(991) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::vector<_Ty,_Alloc> &,const std::vector<_Ty,_Alloc> &)' : could not deduce template argument for 'const std::vector<_Ty,_Ax> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\vector(915) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::reverse_iterator<_RanIt> &,const std::reverse_iterator<_RanIt> &)' : could not deduce template argument for 'const std::reverse_iterator<_RanIt> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\xutility(655) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2784: 'bool std::operator <(const std::pair<_Ty1,_Ty2> &,const std::pair<_Ty1,_Ty2> &)' : could not deduce template argument for 'const std::pair<_Ty1,_Ty2> &' from 'const IDInfo'
1>        C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(73) : see declaration of 'std::operator`<''
1>C:\Program Files\Microsoft Visual C++ Toolkit 2003\include\utility(77) : error C2676: binary '<' : 'const IDInfo' does not define this operator or a conversion to a type acceptable to the predefined operator
1>NMAKE : warning U4010: 'Release\CvUnit.obj' : build failed; /K specified, continuing ...

I'm having trouble making heads or tails of this; do you have any ideas of where to look? I fully intend to release this merge to the community as your mod and RevDCM are two of the most popular to base mods on.

Many thanks.

 

[karadoc]

There's a piece of K-Mod code in CvStruct.h which I think you are missing. It defines the '<' operator for the IDInfo type.

	// K-Mod
	bool operator< (const IDInfo& a) const
	{
		return eOwner < a.eOwner || (eOwner == a.eOwner && iID < a.iID);
	}
	// K-Mod end

(C++ error messages are notoriously cryptic when templates are involved.)